Let’s Add Cruft To Our Tests

TL;DR

Stop with the given/when/then comments already! It’s a test smell, I call Herp Derp.

Once Upon a Time

Once upon a time, when people were patronising junior level developers who didn’t really have a grasp of development or testing, it was decided to drill the 1-2-3 of testing into them. We’ve all been guilty of it:

  1. Prepare
  2. Act
  3. Assert

When the BDD movement came along, trying to make software engineers think like users and think about the outcome of their software, not the mechanics of writing a unit test, it was more fashionable to rephrase this with

  1. Given
  2. When
  3. Then

On the whole I think this is a good idea!

I don’t think teaching people to learn something by rote without it becoming meaningful is a good idea, less so if they’re taught to paste this crap into their code everywhere but we’ll come to that in a moment.

The Given/When/Then Comments

I’m not a historian of this stuff, but I have a feeling that the Mockito docs have a role to play in people using a lot of this:

@Test
public void someTest() {
  //given

  //when

  //then
}

I’ve seen/heard of coding standards where the test code MUST, I repeat MUST contain the above comments. I’ve seen the above comments, with or without correct sentence case, often without the normal whitespace you’d expect between a // and its comment.

It’s spread through the development world like a virus.

STOP IT!

But We Need to Explicitly Think Given/When/Then

And you may also need to consider moving to a proper Gherkin based BDD framework. If you want to express some of the given/when/then, though, look at this article. The author of this shows ways to add the meaningful benefit of them to tests. If you are going to write a comment, so let the comment finish the damned sentence!

When I see // given I immediately think given WHAT!? and if the answer is read the next bit of code to be honest, I could have done that anyway. I know that tests are in the order given/when/then or prepare/act/assert and if they’re written properly, I should be able to fathom them out.

But there IS benefit in expressing the given/when/then.

But Not in a Tagging Comment

A comment which decorates the code without adding helpfulness is a waste of space… But it could be worse. Look at what this blogger wrote about tagging the given/when/then. It was posted in January 2015, so it can’t have been an April Fool’s Day prank… He was proposing to use Java code label constructs to tag the code.

Let’s look at that again. He proposed to use a redundant don’t use this feature of Java in order to add virtually meaningless and valueless annotation to the text of the code, with no benefit to the runtime or test reporting, or execution of the test.

Seriously!?

Why Annotate?

Note: I’m still in favour of getting software engineers to think about the given/when/then of their test in order to understand the spec of the software and construct meaningful tests to help construct it.

So the purpose of annotation is for the author of the test to express their intent. How do we consume that? We mainly consume that at test reporting time.

In short, I don’t want comments, though I’ll accept them as a compliment to other good things we do, so long as they say something interesting. I want to see the test described in the test runner’s output.

This leads us to three very good solutions:

  • Make stuff self annotating
  • Use a test runner which breaks up the stages of the test into identifiable steps
  • Name tests appropriately

Self Annotating

Compare:

// then the resulting list is empty
assertEquals(0, resultList.size());

The above appears to explain the then of a test… but why on earth does that need explaining in natural language. Can’t we choose a better assertion and perhaps drop the unnecessary variable?

assertThat(database.fetchAll())
  .isEmpty();

The fetch all on the database returns something that’s empty… that’s what the code SHOUTs.

The Test Runner Solution

While Cucumber is not the best test runner for unit testing, frequently the need for showing one’s given/when/then working to someone else is at a level of testing where you can and should use Gherkin as the script and Cucumber as the runner, and then you’re done!

If you wanted a pure-Java solution to this, then the SpectrumBDD runner, which I’ve worked on, and its counterparts, as discussed here, are a good option. If you suspected me of using this article to push Spectrum, then stop right away. Even I have never actually used Spectrum in a production context. I think it’s an interesting opportunity to do some thing that are hard with other techniques, but in the end it’s not quite made it into my personal toolbox.

The Test Name Solution

If you can’t output the step by step given/when/then of your test, why not use the name of the test to express it.

In a previous project, people used to do something like this:

// Test case: given an empty database, when we search for the user, then none is found
@Test
public void searchEmpty() {
}

Guys, you wrote an inert comment RIGHT NEXT DOOR to the place where you could have used that knowledge to make a better test name.

I’ve worked with the standard of test names being either:

given<something>_when<something>_then<something>

or maybe

when<something>_given<something>_then<something>

or even

<when>_<given>_<then>

The order of given/when being sometimes easier to reverse in the name of a test, and the choice of whether to include the keywords given/when/then or just imply them being left to the author. Similarly, if any of the three doesn’t have any detail to add, for example thenTheAnswerIsCorrect then it can be omitted.

Some examples:

  • fetchUsers_withEmptyDatabase_isEmptyList
  • givenNoUsers_whenFetchAll_thenIsEmpty
  • fetchUsers_withEmptyDatabase

Conclusion

If you have reached the bottom of this article and still think that adding repetitive //given comments to your tests breaks them up neatly so that it improves the test, then you’ve learned nothing.

Adding boilerplate that people don’t read, because it contains nothing of any value, is NOT helpful.

Find a way to think of the meaning of your tests and bake it into the names, the test execution, and rich comments, or rich assertions, so that you add value!

2 comments

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s