Keep your tests clean, by refactoring

There is a lot of documentation regarding TDD and the benefits of developing software using this methodology, here is one example. In short, TDD involves a 3 step cycle, commonly referred to as the “red-green-refactor” loop.

It involves writing a failing test, then writing the minimum code required to make the test pass, and finally refactoring the code until it is considered “clean” meaning readable and maintainable.

The tests are used to ensure correctness of the software, and also to make sure that future changes to the code do not “break” existing functionality. But another very important purpose of the tests is that they should describe the expected behaviour of the application.

In order to achieve this, during the refactoring phase, you need to pay attention to the tests also, ensuring that they clearly and concisely describe exactly what the intended functionality should be.

In this article I will discuss some of the common problems that I have come across when test code is not refactored appropriately. The examples I use are in Scala with Specs2, but the concepts are applicable in any language and any testing framework.

Test Structure

Tests should have 3 steps:

1 - Setup the current state of the system.

2 - Perform some action.

3 - Verify the new state of the system.

This structure is commonly referred to by the names “Arrange, Act, Assert (AAA)” or “Given, When, Then (GWT)”

These 3 steps should be easily distinguishable and separated. It is a good idea to try and keep each step in one line of code. Sometimes the setup section is too complex for one line, but in general this should be a goal. It is preferable that all tests should have only 1 occurrence of each step. If you find there are more than 1, particularly assertions, it is usually a sign that the test is doing too much, and should probably be separated into multiple tests.

Test Clarity

The purpose of the test should be clearly defined in the test description. For example "save data with correct values", "perform expected action" these descriptions are too vague. While you are in the process of developing, descriptions like these could make sense, but if someone else reads the code, or even yourself in a few weeks time, there is not enough information to let the reader know exactly what the expected behaviour should be.

Variable naming is extremely important to help the reader understand the intention of the test. For example lets say we have the following data class

case class Video(id: Int, name: String, genre: Genre, rating: Int)

In the test we have

val video1 = Video(randomInt, randomStr, randomGenre, 1)
val video2 = Video(randomInt, randomStr, randomGenre, 3)
val video3 = Video(randomInt, randomStr, randomGenre, 5)

It is clear that we have 3 videos, each with different ratings, but the names don’t tell us that information, it therefore requires extra thought process to understand and remember these details. It would be better if that information was included in the names.

val terribleVideo = Video(randomInt, randomStr, randomGenre, 1)
val averageVideo  = Video(randomInt, randomStr, randomGenre, 3)
val greatVideo    = Video(randomInt, randomStr, randomGenre, 5)

Now the names are clear, but we can still do better. The id, name and genre are not relevant for this test, so it is additional “noise” which can be removed.

val terribleVideo = aVideo.withRating(1)
val averageVideo  = aVideo.withRating(3)
val greatVideo    = aVideo.withRating(5) 

Uninformative Assertion messages

The assertion section of the test should actually be refactored while the test is still failing.

The first step in the process is to use custom matchers to improve readability. For example if we have the following assertion

videoService.getById(1).name must be_===("Lord of the Rings")

we could write a custom matcher as follows

def haveName(name: String) = be_===(name) ^^ ((_: Video).name)

then our previous assertion could be written more clearly as

videoService.getById(1) must haveName("Lord of the Rings")

The next step is to make sure that the failure message tells us exactly what the problem is. The test fails with the message

'Harry Potter and the Sorcerer's Stone' is not equal to 'Lord of the Rings'

But without looking at the code, we cannot tell why the test failed. Adding descriptive messages to the matchers will help to solve this

def haveName(name: String) =
  be_===(name) ^^ ((_: Video).name aka "Video Name")

The test now clearly tells us

Video Name 'Harry Potter and the Sorcerer's Stone'
is not equal to
'Lord of the Rings'

When doing assertions that compare entire objects, it is also necessary to use custom matchers with descriptive error messages. The “built-in” object matchers print out the whole object, for example

val expectedVideo = Video(2, "Lord of the Rings", Fantasy, 3)
videoService.getById(1) must be_===(expectedVideo)

This fails with

'Video(1,Harry Potter and the Sorcerer's Stone,Fantasy,3)'
is not equal to
'Video(2,Lord of the Rings,Fantasy,3)'

For complex objects with many fields, it soon becomes very difficult to figure out which are the fields that are causing the failure. One way to solve this problem is to compose many custom matchers, where each custom matcher has a descriptive message, for example

def beMatchingVideo(video: Video) =
  haveId(video.id) and
  haveName(video.name) and
  haveGenre(video.genre) and
  haveRating(video.rating)

Code Duplication

It is quite common that each test is very similar to the previous one, with some minor adjustment to the setup and expectation. Often this leads to “copy-and-pasting” the previous test, and making the relevant changes in the new test.

It is very important to remove this duplication as soon as possible. As the test suite grows, more and more duplication gets added, eventually leading to overly complex setup of tests which are difficult to maintain and reason about.

The most common way to solve this problem is to extract duplicate code into functions, and use parameters for the parts that are different for each test. It goes without saying, that the extracted functions should have clear names, that describe what the functions do. Names like setup and initialize are too vague, and should include more information regarding exactly what the setup process does, for example setupExistingVideos(terribleVideo, greatVideo).

Conclusion

Your tests are an extremely important part of the code base. One of the roles of the test suite is to describe the application behaviour. In order to do this properly, the tests need to be clearly written and highly readable.

The only way to achieve this, is to take care to refactor the tests while performing the refactoring step in the TDD loop. Tests should be well structured according to the AAA or GWT patterns. They must clearly describe the requirements by using concise descriptions, informative variable and function names, and custom matchers. Matchers should include descriptions to make failure messages clear. Duplicate setup and assertion code should be extracted into functions.