14 June 2014

Given..When..Then.. in JUnit

Warning: In this blog I fret over minutiae.

Background
A popular way of writing tests (or testable specifications) in BDD (or ATDD) is the Given..When..Then.. format, as seen in Cucumber, JBehave, etc.
The given..when..then.. format is a sequence of English statements e.g. 
    Given a user called Smith exists
    And a user called Jones exists
    When I go to the list users page
    Then Jones should appear before Smith in the user list
There can be multiple of each type of clause, using ‘and’.
    Given...
    And...
    When...
    And...
    Then...
    And...
Each clause gets implemented using regular code.  ‘Given' clauses set up the data ready for the test.  ‘When' clauses run code under test.  ‘Then' clauses assert things about the results of the code under test.


Given..When..Then.. in JUnit
At last year’s Devoxx UK I saw a talk on BDD.  BDD is of course so much more than the Given..When..Then.. style of tests, but they looked like something I could try straight away.  But, I didn’t want to commit to a whole new set of tools (and introducing new tools at work takes more paperwork than would be worthwhile for a whim).

So, I looked for easy ways to do the Given..When..Then.. style of tests in plain old JUnit.  My first attempt was just a matter of:
  1. extracting chunks of the test into methods named given… when…. then…,
  2. changing local variables that were used by more than one chunk into fields,

e.g. (This is a simple example, but) I would turn a test like:

@Test
public void testAdd() {
 Set<String> set = MySet.of("value1", "value2", "value3");
 set.add("value3");
 assertTrue(set.contains("value1"));
 assertTrue(set.contains("value2"));
 assertTrue(set.contains("value3"));
 assertTrue(set.contains("value4"));
 assertThat(set.size(), is(4));
}

into something like:

@Test
public void testAdd() {
 givenIHaveASetContaining("value1", "value2", "value3");
 whenIAddAValue("value4");
 thenTheSetShouldContain("value1", "value2", "value3", "value4");
 andTheSizeOfTheSetShouldBe(4);
}
private void givenIHaveASetContaining(String… values) {
 set = MySet.of(values);
}
private void whenIAddAValue(String value) {
 set.add(value);
}
private void thenTheSetShouldContain(String… values) {
 for(String value : values ) {
  assertTrue(set.contains(value));
 }
}
private void andTheSizeOfTheSetShouldBe(int size) {
 assertThat( set.size(), is(size) );
}

That’s certainly longer, but is it better?  Try looking at just the @Test method.  It is more readable, and this style stays readable even for more complicated tests with lots of mocks and ‘verify's and ‘ArgumentMatcher’s (I use mockito).  That is better, though I’ll improve on it further down.

When I was first getting used to this, I would write tests the same way I used to, and then use the IDE’s ‘extract method’ refactoring to pull out the givens, whens, and thens.  I quickly found that these given/when/then methods were very reusable when I was writing related tests.  For example a test for the addAll method could use all of the same given and then methods.

Once I got used to it, it became easier to start writing the test in the given..when..then.. form before I implemented the given..when..then.. methods.  I was thinking about what was being tested, instead of how to test it.  This turned out to make it easier for me to reason about what path through code the test was exercising, making it easier for me to write tests that covered all branches of the code.

The first and easiest thing to fix is that the test with the example above, is that the name is breaking the very important rule to describe behaviour not implementation.  So, that’s an easy change:
@Test
public void testAdd()
can become
@Test
public void shouldBeAbleToAddOneItemToASet()

That was the first thing to fix, so there must be a second:
The given/when/then methods were very reusable, but consider this test:

@Test
public void removingLastItemLeavesSetEmpty {
 Set<String> set = MySet.of("value1");//givenIHaveASetContaining("value1");
 set.remove("value1");//whenIRemoveTheValue("value1");
 assertThat(set.size(), is(0));
}

The assertion here was previously extracted as andTheSizeOfTheSetShouldBe.  Which is weird given there’s no then… call before it in this test.  So, either I need to have two methods doing the same thing: one with a then… prefix, one with the and… prefix, or I need to stop using the and… prefix entirely.  I didn’t like that, so I came up with another way.

It turns out that the compiler will let you have more than one label with the same name in a method (only an issue if you have multiple and… clauses).  My IDE complains about multiple of the same label, but I never use labels in other situations, so I turned off that check.

@Test
public void shouldBeAbleToAddOneItemToASet() {
 given: iHaveASetContaining("value1", "value2", "value3");
 when: iAddAValue("value4");
 then: theSetShouldContain("value1", "value2", "value3", "value4");
 and: theSizeOfTheSetShouldBe(4);
}

@Test
public void removingLastItemLeavesSetEmpty {
 given: iHaveASetContaining("value1");
 when: iRemoveTheValue("value1");
 then: theSizeOfTheSetShouldBe(4);
}

So I am able to have a then clause shown with a then: prefix or with an and: prefix.  This makes it easier to read.

Guidelines
Most of these are standard advice, some may be my own invention.
  1. Give tests sentence like names that describe the behaviour of the code under test, and not just test + name of method under test.
  2. Your when clause should not be named after the method under test either.  There are many ways a method could be renamed without changing its behaviour.  It is the behaviour that you are interested in testing.
  3. The sequence for the clauses of your test should be ‘given' ‘when' ‘then'.  If you have them out of order, then either
    • you have a mislabeled clause: I have seen tests that had a clause mislabeled as a 'when’ instead of ‘given’ (because it was calling part of the code under test).  If it is setting things up for the code being tested, then it should be a ‘given’ clause.
    • or you are testing two things instead of one thing: in which case split your test into separate tests that test only one thing each.
  4. The ‘then' clauses need to include the word ‘should’.  Then clauses are not necessarily describing what does happen, they are describing what is supposed to happen.
        then:userNameIs("fred"); //bad
        then:userNameWillBe("fred"); //bad
        then:userNameShouldBe("fred"); //good
    
  5. try to keep a consistent and appropriate level of abstraction for the clauses of your tests.
    • you can have multiple statements in one given/when/then method, to hide the fiddly details of one step behind a clause that describes things at a higher level.
  6. Given clauses are optional (e.g. if there’s nothing to set up for the code under test)
  7. When clauses are necessary (otherwise you’re not running the code under test)
  8. Then clauses are necessary (otherwise you’re not checking the results of running the code under test)
  9. If some prepopulated or default data is very relevant to your test, then consider having a given clause that asserts the data matches the expected value.
  10. I have a trick for dealing with exceptions that I’ll address in another post.

Summary
There is a lot more to BDD than given..when..then.. style tests, and I’d certainly recommend learning about BDD, but that isn’t the point of this post.  The given..when..then.. style does work at the unit test level, and you can try it without switching to new tools.  If you decide you don’t like it you can just inline the methods again.  
Given..when..then.. turns tests into documents that describe code behaviour in a way that’s easy to read.  It makes it easier to reuse test code.  It makes it easier to cover more of the code under test.  It makes it easier to maintain tests (as the purpose of tests are easier to understand later on).

Please try it.  This style will bend your brain to start with, but you will end up at a place where testing becomes easier (still work, but easier).