The problem with loops in tests

I have used loops in tests many times. Sometimes for iterating through an array of inputs, sometimes an array of tuples with an input and the expected output. It is a very convenient and dry way to code a test when you want to test against a lot of cases. But there is a problem with this approach.

Let's start with an example. It is in Ruby but it should not be hard to transpose to another language. We will write a test for a blank? method which returns true when the input is considered blank.

it 'returns true when input is blank' do
  [ '', '   ', [], nil ].each do |input|
    assert blank?(input)
  end
end

I am sure you know what problem I am talking about. When testing this and one of the input fails, the summary will give you the line on which the error is. But because this is a loop, the line is the same for all cases. Therefore you don't know which one failed.

If you practice TDD by the book. Then you would not experience it immediately because you would add inputs one by one, watch it fail and make it pass. But it still can show up on a refactoring.

Now there are many approaches to this. The most obvious if you really want to stick to a loop is to use the report message. All assert* methods have a last argument which lets you write a custom message.

it 'returns true when input is blank' do
  [ '', '   ', [], nil ].each do |input|
    assert(blank?(input), "The input `#{input.inspect}` is not blank." )
  end
end

Another option is to simply write each line and not use a loop.

it 'returns true when input is blank' do
  assert blank?('')
  assert blank?('   ')
  assert blank?([])
  assert blank?(nil)
end

Here the repetition is not too bad. I would keep it at one line each. If there are multiple things to test, then I would create a helper method.

it 'is a successful request' do
  assert_existing_page '/'
  assert_existing_page '/contact'
  assert_existing_page '/user/1'
end

def assert_existing_page path
  assert_equal 200, last_response.status
  assert_equal 'text/html', last_response.headers['Content-Type']
  assert_match 'cool', last_response.body
end

Depending on the case, if it is meaningful to you, you might sometimes separate the cases in their own test. It often make sense. It is more verbose, but put your intent into words. The fact that it is repetitive does not mean you have to put them in the same test, in a loop.

A good example is the case for the string with white spaces. It is debatable if you consider this blank or not. It depends what you use this method for. By separating it, you make a clear statement that you want this to be a feature.

it 'considers white space characters to be blank' do
  assert blank?('   ')
  assert blank?("\n\t")
  assert blank?(" \n \t ")
end

Conclusion

My advice would be to use the method that makes sense to you (and your team) for the specific case. Think about your future self going back to the code. Each test is different, don't think you always have to do things the same way. And certainly don't think you have to do things in the supposedly "right way".

Tags:     test tdd bdd