8

If I have a complex unit tested function:

def do_everything():
    # turn twizzles
    # push buttons
    # move mountain

And I re-factor it into some smaller units:

def do_everything():
    turn_twizzles()
    push_buttons()
    move_mountain()

def turn_twizzles():
    # turn twizzles

def push_buttons():
    # push buttons

def move_mountain():
    # move mountain

Am I wasting my time writing extra unit tests for those smaller units?

15

I assume you already have unit tests covering the behavior of do_everything()? If you have broken turn_twizzles() etc. out as private methods, then you have not changed any external behavior so you don't need to change any tests.

If however turn_twizzles() is made public, then you have introduced a new functionality (as observed from outside the class) so it would be valuable to test this.

  • 2
    Not sure why this answer has been downvoted. It is short, to the point and 100% spot-on in its advice. – David Arno Nov 17 '17 at 17:12
  • This is a very clear answer, thank you. I think the issue (credit to @Samuel for mentioning it fist) is the issue of weather or not the public interface has changed. – Adam Terrey Nov 17 '17 at 22:44
  • Downvote because this is not complete. Even if the new functions were not part of a public API, writing tests can be very helpful. Say a test for do_everything() fails: where is the error? If you have tests for all three sub-functions it will be way easier to find. That's a benefit of writing the tests and it should be mentioned here. – marstato Nov 18 '17 at 13:25
  • 1
    @marstato: It is generally considered a bad idea to unittest private methods, since it couples the tests to implementation details. – JacquesB Nov 18 '17 at 15:14
  • @JaxquesB That's not generally a bad thing. You could say that anything more precise than a usual integration test is tied to implementation detail. But you write those tests anyways because they help you track bugs, not because they help you prove correct functionality. The access modifier is always subjective. A private keyword in a programming language is just one of many ways of restricting access to a piece of code. – marstato Nov 19 '17 at 1:41
12

It depends. To be more precise, it depends on

  • the complexity of the original function (if it was very complex, testing each parts on its own will pay off)
  • the complexity of the smaller functions (if they are complex parts on its own, testing them individually will lead to more fine-grained tests and more precise root-cause detection in case of a defect)
  • the existing unit tests (if they already produce sufficient coverage for all those "parts", then it is probably less worth to write individual tests)
  • if you want to keep those smaller functions "implementation details" of the original functions, or not (for the former, writing unit tests for the smaller functions would become counter-productive for this goal).

Especially when those "smaller functions" are not that trivial as in your example, but will have a more or less complex list of input parameters, it can become really hard to produce enough unit tests for your original function to guarantee the smaller functions are tested with all "interesting" input combinations. That would be a clear sign to write specific unit tests for the smaller functions as well.

So, there is no clear "yes" or "no" to this question, it is a trade-off you have to decide per case .

6

If turn_twizzles, push_buttons, and move_mountain are public and called by other code, then I think it's important to refactor your tests to test these functions individually.

Unfortunately after your refactor you have a problem: to unit test do_everything you need to be able to mock turn_twizzles, push_buttons, and move_mountain. Writing tests for do_everything without mocking the dependencies will be an integration test--not necessarily a bad thing depending on your test plan, but there won't be much benefit because you're already testing the three smaller functions individually. This may be the right time for you to redesign this component and collaborate with other objects to do all the work of do_everything.

If turn_twizzles, push_buttons, and move_mountain are not called externally, they should be marked private, and I wouldn't recommend testing them separately from do_everything. This is because from an outside-looking-in perspective, do_everything would be the smallest unit (because the others are inaccessible). Also see this answer about breaking up methods using private methods.

  • 4
    I have downvoted this as, "to unit test do_everything you need to be able to mock turn_twizzles, push_buttons, and move_mountain..." depends on a nonsense definition of "unit test". – David Arno Nov 17 '17 at 17:14
  • 1
    @David to be fair, Samuel does acknowledge that in the answer. Sort of. – GnP Nov 17 '17 at 22:34
4

No. The extra unit tests are more precise. If move_mountain fails, then a single test would fail that says very specifically what went wrong.

That precision cuts down on debugging time, which is valuable. Also, since the test is more focused, it should be faster to run than testing the same functionality via the full function, providing quicker feedback, which is valuable.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.