[crackerjackmack]

Some general advice I've given multiple junior developers over the years, you probably aren't a junior but most likely applicable to the advice you are seeking. These were passed down to me by other developers. Other HN folk will have links to literature but hopefully my advice will give you a precursor.

* testing - write your functions small enough to be readable, but not so small their abstractions are meaningless (because you have to test them all)

* testing - don't reach into your code's modules and mock. Instead use dependency injection with non-testing defaults

* code review - It shouldn't be personal, if it is are you reading it wrong or are they attacking you personally?

* code review - when referencing style complaints ask for reference material. Don't get caught in cyclic-pedantic style war between lead devs.

* code - your code should be environment agnostic, if you have environment/context specific things to do, pass along a environment/configuration dict or make a global config singleton. As long as your code depends on that you can write code more discretely.

* code - personal preference but try to not nest your loops too deeply, when you can use itertools.

* code - if you can help it, try not to mutate dicts/objects in place while in a loop. Makes testing a difficult.

* code - exit early if possible, test for failures instead of nesting your entire function inside a single `if`. Helps identify the bad inputs faster as well.

Above all, remember code isn't perfect. It's a tool to get to an end goal. If you aren't solving for the end goal you aren't solving the right problem. At the end of the day, you are employed to build a product and that product needs to perform it's job. (that isn't a pass to write super shitty code)

edit: formatting

[ahussain]

I quite like this short piece as a way to explain why things like exiting early and not nesting loops are important: https://medium.com/@matryer/line-of-sight-in-code-186dd7cdea...

[loup-vaillant]

> write your functions small enough to be readable, but not so small their abstractions are meaningless

More precisely, you want your functions to be deep. The interface to implementation ratio must be as low as is reasonable: you want to hide implementation behind interfaces that are much smaller than them.

This goes for functions (something with 5 arguments that takes 2 lines is too shallow, something with 2 arguments that spans 20 lines is deep), classes (something that is mostly getters and setters is too shalow, something with 3 methods full of business logic is deep), or anything else.

This is not just for testing, this is to make sure you can understand the program at all. Deep functions (and deep classes, and deep modules…), are also about good old decoupling: the less you have to understand about a piece of code to be able to use it, the better.

[keshab]

I couldn't agree more with you about nesting loops. It seems clever at the moment when you're writing but when you have to come back after a while or worse, another developer has to, it becomes a nightmare.

I would also go a bit further and put nesting if statements. Sometimes it's really required but other times nesting can be avoided. I try to avoid nesting as much as possible.

[mikekchar]

Quick tip: extract the contents of loops out into functions. That allows you to test the function outside of the context of the loop. It makes it easy to test boundary conditions and other exceptions. Often it is unnecessary to actually test the loop.

This is also true of branches. If you have an if statement (or other branch), consider extracting the contents of each half out. It allows you to test each half independently. If you have nested branches, you need to have 2^n tests (where n is the number of nested branches). If you extract the contents, then you need 2n tests.

This is one of the reason for unit rather than integration tests: you can dramatically reduce the number of tests while still getting full test coverage. Of course the downside is decomposing the structure of the code more than you might be used to. It's always a judgement call.

[fraudsyndrome]

> testing - don't reach into your code's modules and mock. Instead use dependency injection with non-testing defaults

Could you please go into more depth with this?

[crackerjackmack]

In an example. NoopTelemetry would be some type of empty class not dependent on mock (I've used a meta class a singleton in this case, but whichever, could be a module just the same). To test, you'd pass in a mock object to telemetry and check that a both timer_start and timer_stop are both called with the correct function name.

In your main or context, you setup your application with the needed pieces.

  def main():
     context = {'telemetry': StatsClient(....)}
     start(context)

  def start(context):
      algo_5(1, 4, context['telemetry'])

  def algo_5(param1, param2, telemetry=NoopTelemetry):
     telemetry.timer_start('algo_5')
     ret = param1 / param2 ** param2 # whatever
     telemetry.timer_stop('algo_5)
     return ret
  
  def test_start():
    context = {'telemetry': MagicMock()}
    start(context)
    # more testing.

  def test_algo5_math():
    ret = algo5(4, 5)
    assert 78  # maybe?

  def test_algo5_telemetry():
    mm = MagicMock()
    algo5(1, 1, mm)
    assert mm.timer_start.called_with_args(['algo5'])
    assert mm.timer_stop.called_with_args(['algo5'])

[loup-vaillant]

Mocking everything is the unwise conventional wisdom. It complicates the code base for no discernible benefit. In practice, mocks are only useful for external dependencies, and computationally heavy internal dependencies. Possibly not even the latter.

Why external dependencies are a good thing to mock is not just related to testing, it's related to code rot: external dependencies tend to change under your feet, so isolating them from your code base is a good thing, especially if you want to minimise vendor lock in. And for tests, it helps you simplify the test environment.

Internal dependencies aren't like that: they only change if you change them, and they're already part of your environment. Mocking them just complicates your code without simplifying your tests.

---

Now there is this idea that modules should be tested "in isolation". Imagine you have three classes, A, B and C, such that A uses B, and B uses C (and C is self contained). Without mocking, testing A will automatically involve B's and C's code as well. Mocking lets you test A alone, without the distraction of B and C.

This is mostly bullshit. If A depends on B all the time, there's no point into making B a parameter (dependency injection). It's simpler to just hard code the dependency. As for the tests, who cares about isolation? If A is only used when it depends on B, there's no point in testing it under other conditions. More generally, it is okay to pull in the transitive closure of dependencies when you test a component.

Of course, you should have tested each dependency (C, then B) before you test your module (A). That way you can mostly ensure the absence of bugs in the dependencies. For instance, if the tests for C and B succeed, and the test for A fails, the bug is probably in A. The order of the tests also matters for the test suite: if the tests for the dependencies are ran first, the first failed test should point to the failed module (instead of a user of the failed module).

Now if your program is highly stateful, you might have to mock. Testing a highly stateful program without mocking requires setting lots of bits of states, and making sure all the pieces of runtime state interact with each other as they are supposed to. But then the problem isn't the lack of mocking, it's the fact your program is highly stateful in the first place. Make your program less stateful, first, then it will no longer need mocking.

---

There is this mistaken notion that we should mock because if we don't, we're no longer making unit tests, we're making integration tests. The answer to that is simple: unit tests aren't the holy grail they are touted to be. Don't force them. If the dependencies are well tested (which they should be), then you can trust them like you would the standard library.

Besides, the integration tests will catch more bugs for less effort than pure unit tests (with mocking) would have done. Don't waste your time with mocking, just go catch the bugs.