Testing on the Toilet: What Makes a Good Test?
Tuesday, March 18, 2014
by Erik Kuefler
This article was adapted from a Google Testing on the Toilet (TotT) episode. You can download a printer-friendly version of this TotT episode and post it in your office.
Unit tests are important tools for verifying that our code is correct. But writing good tests is about much more than just verifying correctness — a good unit test should exhibit several other properties in order to be readable and maintainable.
One property of a good test is clarity. Clarity means that a test should serve as readable documentation for humans, describing the code being tested in terms of its public APIs. Tests shouldn't refer directly to implementation details. The names of a class's tests should say everything the class does, and the tests themselves should serve as examples for how to use the class.
Two more important properties are completeness and conciseness. A test is complete when its body contains all of the information you need to understand it, and concise when it doesn't contain any other distracting information. This test fails on both counts:
Lots of distracting information is being passed to the constructor, and the important parts are hidden off in a helper method. The test can be made more complete by clarifying the purpose of the helper method, and more concise by using another helper to hide the irrelevant details of constructing the calculator:
One final property of a good test is resilience. Once written, a resilient test doesn't have to change unless the purpose or behavior of the class being tested changes. Adding new behavior should only require adding new tests, not changing old ones. The original test above isn't resilient since you'll have to update it (and probably dozens of other tests!) whenever you add a new irrelevant constructor parameter. Moving these details into the helper method solved this problem.
This article was adapted from a Google Testing on the Toilet (TotT) episode. You can download a printer-friendly version of this TotT episode and post it in your office.
Unit tests are important tools for verifying that our code is correct. But writing good tests is about much more than just verifying correctness — a good unit test should exhibit several other properties in order to be readable and maintainable.
One property of a good test is clarity. Clarity means that a test should serve as readable documentation for humans, describing the code being tested in terms of its public APIs. Tests shouldn't refer directly to implementation details. The names of a class's tests should say everything the class does, and the tests themselves should serve as examples for how to use the class.
Two more important properties are completeness and conciseness. A test is complete when its body contains all of the information you need to understand it, and concise when it doesn't contain any other distracting information. This test fails on both counts:
@Test public void shouldPerformAddition() { Calculator calculator = new Calculator(new RoundingStrategy(), "unused", ENABLE_COSIN_FEATURE, 0.01, calculusEngine, false); int result = calculator.doComputation(makeTestComputation()); assertEquals(5, result); // Where did this number come from? }
Lots of distracting information is being passed to the constructor, and the important parts are hidden off in a helper method. The test can be made more complete by clarifying the purpose of the helper method, and more concise by using another helper to hide the irrelevant details of constructing the calculator:
@Test public void shouldPerformAddition() { Calculator calculator = newCalculator(); int result = calculator.doComputation(makeAdditionComputation(2, 3)); assertEquals(5, result); }
One final property of a good test is resilience. Once written, a resilient test doesn't have to change unless the purpose or behavior of the class being tested changes. Adding new behavior should only require adding new tests, not changing old ones. The original test above isn't resilient since you'll have to update it (and probably dozens of other tests!) whenever you add a new irrelevant constructor parameter. Moving these details into the helper method solved this problem.
I agree with your recommending clarity of code.
ReplyDeleteHowever example suggests a problem elsewhere (the constructor of the Calculator class). Your proposal will necessitate helpers like newCalculatorWithCosine() newCalculatorWithoutCosine() ad infitum for every permutation of the constructor parameters.
In fact should those not be helpers (factories) in the Calculator itself? But we digress from your main point....
I've seen one method that looked ok, which was to have a CalculatorBuilder class, that then has fluent style methods that you can chain like .WithCosine() and then a final .Build() method that constructs your instance. Not actually got around to using it in practise yet unfortunately... need to write more tests :)
ReplyDeleteIt's a good point that you should definitely be paying attention to the clarity of your constructors - if there are a lot of places in your code base where you have to invoke a clunky six-argument constructor, something is probably wrong! The builder pattern that Sam mentions (http://www.javacodegeeks.com/2013/01/the-builder-pattern-in-practice.html) is a very good way to make instances easier to construct, and will clean up your tests too.
ReplyDeleteHowever, there are a lot of situations where a many-argument constructor is only really called once during your application's setup. This is especially common if you're doing dependency injection, manually or via a framework like Guice. In these situations there often isn't much point in creating builders for your production code since you don't ever reuse them, but it can definitely still be useful to define builders for tests. Builders used in this way are essentially a generalization of helper methods that allow you to specify an arbitrary number of named parameters.
There is often a trade off between readability and maintainability. Modularizing your test cases reduces it's readability while improves maintainability. But I agree that the test cases can be served as documentation of the system too.
ReplyDeleteI am going to suggest something for the hidden constructor variants here, and call it table driven testing. It looks like this test-code uncovered 2 test-cases, one for ENABLE_COSIN_FEATURE and another for (I assume) DISABLE_COSIN_FEATURE, where either the result or the inputs can be partitioned as the tester sees fit into a table of csv data:
ReplyDeleteENABLE_COSIN_FEATURE, 2,3,5
DISABLE_COSIN_FEATURE,2,3,5
I have a co-worker who does this kind of breakdown in their test-code regularly, it removes hard-coded stuff from the test-code, and results in hundreds of tiny custom csv files, but it rocks - only because its easy to test new corner cases without making a code-change. The downside is longer time to write the test, and ability to get carried away with the possibilities.
We put lot more effort in breaking dependencies but on the other hand we introduce constructor dependencies and the above example constructor dependencies is horrible . Here I see more of coding style than TDDing itself. If one does not follow the good design patterns and practises and write would end up in writing test what has been shown by Erik.
ReplyDelete