The Top 5 Unit Testing Sins
Here are my top five sins when writing unit tests. All of these tests are in Java and based on a TicTacToe test template. All of these test cases were written by me.
In reverse order:
5. Adding tests for the sake of adding tests
Every test that you write will be executed many times. Each test will take a small amount of time during every build / CI process that runs. Therefore, the last thing you want to have is a lot of tests that don’t add any value to the developers that are maintaining your code.
@Test public void constructorTest() {
// Act
TicTacToe actual = new TicTacToe();
// Assert
Assert.assertTrue("Constructor didn't produce correct object", actual instanceof TicTacToe);
}In this example, we are testing that the constructor produces an instance of TicTacToe. However, is this really testing our code? Or are we just testing Java itself? Can you think of an instance where this would fail based on your coding error? No. This means the test case is simply wasting time (or your peers’ time) so it doesn’t need to be there.
4. Testing too many things
We know that a good unit test checks one thing only. This means that when the test case fails it is obvious to the developer what has caused the failure. When you start testing multiple cases with one test it gets harder for the developer to understand how they broke the code.
@Test public void playerOneWinning() {
// Arrange
boolean playerOneWon = true; TicTacToe ticTacToe = new TicTacToe();
// Act
if (ticTacToe.checkTicTacToePosition(new int[]) != 1) {
playerOneWon = false;
}
if (ticTacToe.checkTicTacToePosition(new int[]) != 1) {
playerOneWon = false;
}
if (ticTacToe.checkTicTacToePosition(new int[]) != 1) {
playerOneWon = false;
}
if (ticTacToe.checkTicTacToePosition(new int[]) != 1) {
playerOneWon = false;
}
// Assert
Assert.assertTrue("Player One didn't win", playerOneWon);
}Of course this tests valuable parts of the logic, e.g. checking that player one wins vertically, horizontally and diagonally. However, when this test case fails, how can you tell which path caused the error? Without further debugging, you can’t. If it was split out into three test cases, then it would give more information to the developer using it. The quicker any developers working on your project can isolate the cause of their regressions, the less time they waste debugging code.
3. Not actually testing anything
Test cases must test something (the clue is in the name). By checking that the code performs as expected, the tests are going to give the developer confidence that their changes are not introducing any regressions.
@Test public void errorCondition() {
// Arrange
TicTacToe ticTacToe = new TicTacToe();
int[] input = new int[];
// Act
try catch (Exception e) {
// Don't worry the exception is expected
}
}Here, we are executing one of the error paths with a valid input for that error. However, we don’t check that the code behaves correctly. There is no assertion that the exception occurs. Even worse, we are swallowing the exception in the test case. This means that if a different, unexpected exception, e.g. a runtime exception, occurs, then we aren’t going to notice this with the unit test. This test is giving the developer a false sense of security for their code changes.
2. “The test case no longer passes; I will delete it”
Test cases don’t add any value when they are written. They add the most value when they fail. They are alerting the developer to a change in behavior. When this happens, you almost certainly want to update the test case so that it passes with the new behavior.
By simply deleting a test case when it fails, you are reducing the effectiveness of your test suite. Also, the fact that you have put some effort into changing the behavior of your code means that you should ensure that this behavior doesn’t regress in the future.
1. “My test is super efficient… but nobody can understand it”
As I have said throughout this post, tests are most valuable to developers when they fail. As soon as they fail, the developer will look at the test to understand why. This means they first need to understand what the test is checking and how it’s working.
@Test public void oneLineTest() {
Assert.assertTrue((new TicTacToe().checkTicTacToePosition(new int[])) == 1 );
}Here we have a test case in a single line. It is much less verbose than the tests we typically see. This makes it much harder to understand; there is no separation between the setup of the test and the execution of the test. It will be almost impossible to use a debugger in an IDE with this test as everything happens on a single line. So there we have it: the top five sins when writing unit tests.
