Writing Good Unit Tests plays an important, proven role in improving and maintaining code quality and productivity. Still, quite many developers haven't embraced it. Based on experience (have tried it and failed) or haven't even considered trying it in the first place. Probably had their reasons, in any case I find it very interesting that developers who have successfully practiced unit-testing stick to it and rarely go back. This should at least cast a question on those reasons and make one think about the cause for the initial disbelief and failed attempts. My intention is to collect a few practices that address common pitfalls and show some of the actual value added by unit tests.
Test the testable only
Test testable code only. What code is testable? Potentially everything inside the system's boundaries. Outside that there are the external dependencies, which generally don't need unit testing. Identifying the system boundaries and separating it well from external dependencies is a key element. This separation also tends to lead to better overall design. The ratio of testable code depends on the concrete piece of software: it can be low in some cases.
Example code
The example code we aim to test implements a Queue.
public class Queue<T> {
private LinkedList<T> queue;
public Queue() {
queue = new LinkedList<T>();
}
public void add(T obj) {
if(obj == null)
throw new NullPointerException();
queue.add(obj);
}
public T poll() {
T first = queue.getFirst();
queue.removeFirst();
return first;
}
public int size() {
return queue.size();
}
}
A bad testcase
"Public methods of all public classes should be tested." I read this in a best practices list. I think guidelines like this are confusing and don't focus on the issue at all. Let's take a look at a naive attempt to follow such an advice.
public class QueueBadTest {
@Test
public void constructor() {
Queue<Integer> queue = new Queue<Integer>();
Assert.assertEquals(0, queue.size());
}
@Test
public void add() {
Integer i = 5;
Queue<Integer> queue = new Queue<Integer>();
queue.add(i);
Assert.assertEquals(1, queue.size());
Assert.assertEquals(i, queue.poll());
}
@Test
public void poll() {
Integer i = 5;
Queue<Integer> queue = new Queue<Integer>();
queue.add(i);
Assert.assertEquals(i, queue.poll());
Assert.assertEquals(0, queue.size());
}
@Test
public void size() {
Integer i = 5;
Queue<Integer> queue = new Queue<Integer>();
queue.add(i);
Assert.assertEquals(1, queue.size());
queue.poll();
Assert.assertEquals(0, queue.size());
}
}
This testing style fails for a number of reasons. It is obviously impossible to test methods (sometimes even classes) in isolation. The constructor test depends on the correctness of size(), all other tests depend on the constructor and a few other methods too. The test for poll() doesn't test polling from an empty queue for instance. poll2()? Or making the existing test method more busy? Circular test dependencies can potentially make the whole testcase worthless anyway.
Unfortunately there are many tools that generate such testcases and advocate this style. It parrots the class interface, doesn't achieve it's goal and rightfully gives the impression of more work without much added value. We're looking for something more useful.
Identify the unit
A unit must be identified first. A unit test is not called a class test, there is a reason for that. Anything can be considered as a Unit as long as it remains in the system's boundary. Trying to write isolated class tests may not be practical in some scenarios. Fine-grained units are to be preferred of course, so the test points to the issues more accurately.
Concentrate on requirements
When the unit is identified we should ask what behaviour we expect from it? Returning to the Queue example, here is a possible requirement list for our Queue:
- queue is empty when created
- accepts non-null elements for adding
- elements can be polled (if not empty)
- values and order of elements must not change (when adding - polling)
- can tell the number of contained elements
Requirement-driven testcase
An expressive, explicit testcase can be built based on this requirement list.
public class QueueBetterTest {
@Test
public void emptyOnCreation() {
Queue<Integer> queue = new Queue<Integer>();
Assert.assertEquals(queue.size(), 0);
}
@Test(expected = NoSuchElementException.class)
public void emptyPollShouldFail() {
Queue<Integer> queue = new Queue<Integer>();
queue.poll();
}
@Test(expected = NullPointerException.class)
public void addingNullElementShouldFail() {
Queue<Integer> queue = new Queue<Integer>();
queue.add(null);
}
@Test
public void keepsValuesAndOrderOfElements() {
Integer first = 1;
Integer second = 2;
Integer third = 3;
Queue<Integer> queue = new Queue<Integer>();
queue.add(first);
queue.add(second);
queue.add(third);
Assert.assertEquals(queue.poll(), first);
Assert.assertEquals(queue.poll(), second);
Assert.assertEquals(queue.poll(), third);
}
@Test
public void sizeMaintained() {
Queue<Integer> queue = new Queue<Integer>();
queue.add(1);
Assert.assertEquals(1, queue.size());
queue.poll();
Assert.assertEquals(0, queue.size());
}
}
The value
Capturing the requirements in code and having them checked automatically saves a lot of development effort continously and provides a lot of confidence and safety for making changes.
Notice that test code uses the class in a similar way as client code: this provides documentation and early feedback on the design. If writing a test is difficult, the cause may be a design flaw. Test code is ugly because of interface flaws? Client code won't look any prettier. Class depends on too many other classes? Maybe they could be decoupled some more. No sense for writing test for "dumb" classes without logic? Testing their overly complex controller classes is difficult? These are all most useful hints to drive design.
