JUnit Best Practices, Part 2 -- Keep 'em Separated

A common practice for those new to JUnit is to try and cover more ground with fewer tests. As long as you're writing your testAdd() unit test, you may as well invoke add() as many different ways as possible and see what breaks, right?

Wrong. Not only should a unit test focus on one specific unit, it should focus on one specific behavior of that unit. Testing multiple behaviors at once only muddies the water. Consider our hypothetical testAdd() method:



Now suppose the assertion fails. Which of the two add() invocations returned the wrong value? You won't know until you start digging. The purpose of unit testing is to take the guesswork out of error location, and this technique defeats that purpose.

One exception to the rule here is that sometimes you may need to invoke your method under test multiple times in order put things in a particular state before performing the actual test of your method. In such cases, consider using a mock object to prepare the test state in a controlled fashion. The net result is the same -- the only variable in your test is the one behavior that you're testing.

Testing many different behaviors in a single test method is a common practice because it's so easy to do, but this is a clear case of being penny-wise and pound-foolish. Take the time to separate each behavior into its own unit test.

JUnit Best Practices, Part 1 -- Don't Decide Anything

Creating JUnit tests can be something of a craft when you first start writing them. In our JUnit Best Practices articles, we hope to shed some light on what makes a good JUnit test, and why.

The purpose of a JUnit test is to set up a controlled environment in which the unit under test can be run. To that end, each JUnit test should be as logically simple as possible, preferably with no decisions at all.

Consider this JUnit test for a sample method called countDuplicates():



If the getTestInput() returns null, then countDuplicates() is never invoked. If you're looking at your testing status summary, this test will be marked as a success despite the fact that it didn't test anything! Of course, every rule has an exception, so if you absolutely must have decision logic in your test case, be extra diligent to make sure that your test fails if it doesn't execute as expected.

Adding decisions to your test methods only adds another variable to the test, which is exactly what you should avoid when unit testing. Keeping your unit tests decision-free will increase their effectiveness, clarity, and maintainability.

Unit Testing + Code Coverage = Bad For Business?

Measuring code coverage is a natural complement to unit testing. It provides a clear picture of what parts of your code remain untested, and helps you focus additional tests on the uncovered code. Even with just a few unit tests you can quickly cover the basic functionality of several methods in your class under test. Unfortunately, a common mistake is to measure coverage without actually verifying that the executed code is correct, and this is exactly what happens with many code coverage tools.

Consider this example:



This is a simple class that converts an array of integers into the corresponding array of Strings. Note that there is an intentional bug in getIntegerName() -- case 1 of the switch is missing a break statement and causes the method to return the wrong value.

We've created a couple of JUnit tests to start with.



We'll use Coverlipse, a popular open-source coverage tool for Eclipse, to help us measure statement coverage for these unit test. Here are the Coverlipse results.



Both JUnit tests ran successfully and Coverlipse reports 100% statement coverage for the Sample class. Looking at the detailed coverage markers in the Eclipse editor, Coverlipse reports that all statements have been covered. It looks like the perfect testing summary to show to the boss, but a deeper analysis reveals some serious problems with these tests.

Most serious, obviously, is that we did not find the bug in getIntegerName(). In this case, our test assertions missed checking for the appropriate "one" value, but it could just as easily have been the case that the method under test masked the incorrect value returned by getIntegerName(). The real reason that the bug was not found is that no assertions were made about the subordinate method.

The createIntegerNameList() method is invoked by our tests. That's the unit under test, and the only method for which we should be making assertions. The getIntegerName() method was obviously executed, but because we didn't verify its behavior, we missed an obvious problem. Until we've verified the subordinate method directly, we should consider it untested.

Even when this coverage technique is successful in uncovering bugs, it's an inefficient way to conduct testing. Imagine trying to thread a needle from ten feet away. Eventually you'll be able to do it, but how many attempts will fail first? Similarly, testing a subordinate method via its invoker is coarse work at best and impossible at worst. In our example it's easy to see how we can drive down into the subordinate method, but as a method's logic increases, this task becomes increasingly difficult. If you're examining the logic of the invoking method to determine how to increase the coverage of the subordinate method, why not just examine (and test) the logic of the subordinate method directly? Don't waste time with test that might give you a minimal boost in coverage. Test the underlying method directly.

Now let's look at the same test suite using a technique that reports path coverage for unit tests in isolation.



Notice that no coverage has been reported for getIntegerName(), which is what we expect since we haven't created any unit tests for it. Until we examine and test each of the paths identified in this method, our testing effort remains incomplete.

Unit tests are so named for a reason -- concentrate on coverage only for the unit under test and ignore all others.