8

I read this article from the google testing blog. Basically, the idea expressed in the article is that it's a good practice to avoid writing "change-detector tests". This code is given as an example to avoid 

// Production code:
def process(w: Work)
  firstPart.process(w)
  secondPart.process(w)

// Test code:
part1 = mock(FirstPart)
part2 = mock(SecondPart)
w = Work()
Processor(part1, part2).process(w)
verify_in_order
  was_called part1.process(w)
  was_called part2.process(w)

To be honest, I don't really understand why this is a bad test. My approach would be to unit test part1.process, part2.process and then to do exactly this test. Then, with time, if the behavior of this method is changed in a way that the methods are not called as expected (let's say we add a condition depending on the first method call), the test will fail and I will have to make sure that the change of behavior is expected and desired.

What am I getting wrong here?

Improve this question
0

3 Answers 3

Reset to default
8

The code snippet you have shown is an excellent unit test, provided that the unit under test is an abstract part combinator, and does not have a more concrete responsibility within the problem domain of the application.

For example, assume that the actual point of this piece of code is to calculate an interest rate. As an implementation detail, this calculation was split into two pieces. A test that is not about interest rates is then a useless test. The shown test would pass whether or not the calculation is correct. It does not ensure that the system behaves as expected, instead it asserts that the system is implemented in a particular manner. In general, such tests are not a good idea. They are fragile, and will break whenever the system is refactored. The blog post is correct in asserting that such tests can cause more work than they are worth.

However, the code is presented without context, and it may well be the entire point of that code to sequentially combine such processors. Such code might be part of an abstract rule engine library, or more generally as part of a Composite Pattern. It is then absolutely correct to test that the two processors are invoked, and are invoked in the correct order. I have written such code and have written such tests without a bad conscience.

It is completely reasonable to use mock objects for this test, since the concrete behaviour of the processor parts is outside of the scope of the unit being tested. According to the book Quality Code by Stephen Vance: “A unit test is a test that verifies the value added by the code under test. Any use of independently testable collaborators is simply a matter of convenience” (Vance 2013, p. 26). Here, the value added by the code is combining two processors into one. It is then sufficient to test what this code does (chaining two sub-processors), and not necessary to also test what these collaborators do (e.g. that the sub-processors perform a correct interest rate calculation). Such tests are also useful, but would be integration tests.

So in either case, the question to be asked is: What is the point of this code, why does it exist? What does this abstraction represent? What interface does it offer to callers? What behaviour is documented? Then test its behaviour, and not the mechanism through which it is implemented.

Improve this answer
7

I'm not surprised you are confused, the Blog is as clear as mud as to the real problem. To be considered a useful and meaningful unit test, the test code should be testing the objective not the implementation.

In the example, the test code verifies that the Mock objects have been called and not that they have functioned as expected and that the objective has been achieved.

Assume that the purpose of process is to turn "w" into "w12", the requirement is for partOne to add the "1" and partTwo to add the "2". The test doesn't actually prove that test objective is achieved.

Improve this answer
1
  • 6
    But the real purpose of the code under test is to delegate to other units to get the desired behaviour. The contract for firstPart.process and secondPart.process specify the behaviour that each of them must implement, so those behaviours should be tested as part of the implementation of those objects. Don't forget we should also have integration/functional tests to test that the units all behave nicely together to achieve the expected result... unit testing should test what a single unit does and only what that unit does. Commented Mar 21, 2017 at 0:24
1

The order of calls is an implementation detail. If they absolutely must happen in that order for every caller, for all time, that sounds like your abstraction is broken.

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.