As a big fan of Test Driven Design and Development, I believe creating good tests is one of the single most important things we can do as Java Developers. We write tests for a number of reasons:
To shape the design of our system. We know what the input and output should be, but what objects do we need to create to do this? What “shape” should the code take? Writing tests allows us to understand what our code should be created.
To ensure initial and ongoing correctness. It is important our application behaves as expected and is consistently accurate. Tests should do their best to ensure this is the case
Documentation. Tests are the documentation of the system, saying what it should do and how it should do it.
With this in mind, what exactly does a "Good Test" look like?
Naming Your Test
The name of the test is crucially important, particularly from a documentation standpoint. We should be able to read the test names aloud like a set of requirements. There is in fact a great IntelliJ plugin called Enso which will turn your test names into sentences which appear next to each class so you can see exactly what you’re doing.
Never start a test name with “test”. This is a hangover from the early days of JUnit when it was needed to execute. Your Test class is going to be in the Test folder, in a class which has the word Test at the end. It will have an @Test annotation on it. We know it’s a test.
You should also avoid starting with “should” or “will”. These are noise words. You’ve written a test for this functionality, we know it should/will work (or, if it doesn’t, we know we need to fix it).
Treat the test name as a requirement. Here're some examples
addingNumbersWillSumValuesTogether()
explodesOnNegativeID()
notifiesListenersOnUpdates()
Don’t be afraid to be expressive. If your test name needs to be really long then go for it if it’s clear what’s going on.
Test Code
Your test will be split into 3 sections: setup, action, assertion.
Setup
The setup code for your test should only be relevant to the values being asserted in your test. If you have extraneous setup code it will become unclear what is and is not relevant to the test.
This can be achieved in multiple ways:
Move generic setup to a specific setup method using the @Before annotation.
Move repeated setup code into helper methods
User a Maker to create complex test objects and only set the relevant values in your test.
Let me reiterate; the setup portion of each test should only have code relevant to the values being asserted at the end of it.
Bad Example:
@Test
public void returnsBooksWherePartialTitleMatchesInAnyCast(){
Bookstore bookstore = new Bookstore();
Book harryPotterOne = new Book("Harry Potter and The Philosopher Stone");
bookstore.add(harryPotterOne);
bookstore.add(new Book("Guardians of the Galaxy"));
Book harryPotterTwo = new Book("The Truth about HARRY POTTER");
bookstore.add(harryPotterTwo);
List<Book> results = bookstore.findByTitle("RY pot");
assertThat(results.size(), is(2));
assertThat(results, containsInAnyOrder(harryPotterOne, harryPotterTwo));
}
The initialization of bookstore takes place in the test, as does the creation of the books. This muddies the test so it's not clear what's going on.
Better Example:
private Bookstore bookstore = new Bookstore();
private Book aHarryPotterBook = new Book("Harry Potter and The Philosopher Stone");
private Book anotherHarryPotterBook = new Book("The Truth about HARRY POTTER");
private Book aBook = new Book("Guardians of the Galaxy");
@Test
public void returnsBooksWherePartialTitleMatchesInAnyCast(){
bookstore.add(aHarryPotterBook);
bookstore.add(aBook);
bookstore.add(anotherHarryPotterBook);
List<Book> results = bookstore.findByTitle("RY pot");
assertThat(results.size(), is(2));
assertThat(results, containsInAnyOrder(aHarryPotterBook, anotherHarryPotterBook));
}
Initialization takes place in the fields so it's crystal clear what's happening in the test.
Action
The doing bit! Preferably keep it to one line, the isolated action that you are going to test. Sometimes you’re specifically testing what the output is if something is called multiple times, or what the result of a call is after certain prior actions, so this isn’t a hard and fast rule. When reading the test the user should be quickly and easily be able to say “with these values set to this, if I perform this action/these actions, then this is the expected result”. In the example above, this is the bookstore.findByTitle() method.
Assertion
Use Hamcrest. Hamcrest is a wonderful library that gives us a fluent API for writing tests in. Instead of code like this:
assertEquals(results.size(), 2);
assertTrue(results.contains(aHarryPotterBook))
assertTrue(results.contains(anotherHarryPotterBook))
We can clear, easy to read code like this:
assertThat(results.size(), is(2));
assertThat(results, containsInAnyOrder(aHarryPotterBook, anotherHarryPotterBook));
These are fairly trivial examples; Hamcrest has a lot of great methods to make writing complex tests easy and allows you to create your own matchers (coming in an article soon!)
Again, ideally, we want to have a solitary assertion. This makes it clear what we are testing and shows that our code doesn’t have side effects. Like everything in this article this is not a hard rule as there are cases when it will be necessary, but if you have a test like this:
assertThat(orderBook.bids.size(), is(4));
assertThat(orderBook.asks.size(), is(3));
assertThat(orderBook.bids.get(0).price, is(5200));
assertThat(orderBook.asks.get(2).price, is(10000000));
assertThat(orderBook.asks.get(2).isBuy, is(false));
It becomes much harder to understand where a test has failed or which assertion is important.
It is possible to write custom matchers in Hamcrest which can provide an elegant solution to complex assertions. If you need to run assertions in a loop, or you have a large number of fields to assert on, a custom matcher could be the way to go. It merits an entire article of it's own, which I'll write soon!
One of the most important parts of a test is that when it fails it should be obvious to a 5-year-old what went wrong and where. Fail messages must not be cryptic. Ways around this:
If doing any kind of object comparison ensure the object has a decent toString() message. There’s nothing worse than <MyObject@142131> not matching.
Better yet, use a custom matcher for your object. This way you can specify exactly which field has failed to match so it’s totally clear.
Ensure it’s clear why the values you're comparing against are why they are. For example, if you’re comparing a field value to the number 3000, why is it 3000? This should be painfully clear. Obviously don’t use a magic number and ensure the variable is well named to show how its value is derived.
All of these should be taken with a healthy dose of common sense. There are no hard and fast rules!
Sam Atkinson
