Test patterns: One test per path
I’ve seen many test suites which were much less thought through and much less well factored than the code they tested. But that’s not right: Tests drive the code, so tests are at least as important as code. You should care about the quality of your tests just as much as you do the quality of your code, and refactor your tests just as vigorously as you do your code. Here, then, are some thoughts on one aspect of good test design: the One test per path pattern.
Thorough testing means exercising every execution path. You do that by varying the input — test objects and/or method parameters. Let’s use as an example a very common type of method, a query for domain objects. For a specific example, suppose our program plays or manages a game which has Clues, which can be unsolved or solved, and our first task is to list the unsolved clues. Here’s our first unit test:
describe Clue do describe '#all_unsolved' do it 'lists unsolved clues' do Clue.make :solved => false unsolved_clues = Clue.all_unsolved unsolved_clues.length.should == 1 unsolved_clues[0].solved.should == false end end end
(This discussion has nothing to do with any particular programming language; I just happen to be using Ruby and RSpec these days.) We’re using the strategy of creating test objects as we need them, so the only Clue in the system is the one we insert in the first line. So far so good.
Now to test the ‘unsolved’ part of the spec. Simple: just add another Clue, this time a solved one.
describe Clue do describe '#all_unsolved' do it 'lists unsolved clues, but not solved clues' do Clue.make :solved => false Clue.make :solved => true unsolved_clues = Clue.all_unsolved unsolved_clues.length.should == 1 unsolved_clues[0].solved.should == false end end end
That single test exercises both the path that does include an object in the list and the path which doesn’t. But it’s not as clear as it could be. The reader has to read the spec and both object creations to puzzle out the roles of the two objects and why the expected result is what it is, and that ‘but’ is awkward. Instead, break the test in two:
describe Clue do describe '#all_unsolved' do it 'lists unsolved clues' do Clue.make :solved => false unsolved_clues = Clue.all_unsolved unsolved_clues.length.should == 1 unsolved_clues[0].solved.should == false end it 'ignores solved clues' do Clue.make :solved => true Clue.all_unsolved.length.should == 0 end end end
Now each test says exactly one thing, both in text and in code. And, although this example uses a boolean attribute, so there are only two execution paths, you can see how much more scalable this approach is if there are more paths: just keep adding tests. Because each test tests something different, it will be easy to write a clear specification (no more ands or buts), and it will be as easy to write the next test as it was the previous. (Of course, take every opportunity to extract methods to eliminate duplication among tests.)
To be clear, you needn’t test every possible value of a test object’s attributes, or of a method’s parameters, only those that are meaningfully different to the method’s specification. The key to understanding which is which is to get the text specification right. If a Clue had more than two states on the road to being solved, for example :unsolved, :awaiting_judgement and :solved, a good way to write the specification of all_unsolved would be
it 'lists unsolved clues' it 'ignores clues that are not unsolved'
It doesn’t matter to the specification whether you test the second spec with a Clue that is :awaiting_judgement or :solved.
This is a lot like the classic case of testing a function that takes a numeric parameter: to cover all outcomes, you need to test values at each end of each of the parameter’s meaningfully different ranges. The examples above differ only in that the parameter isn’t ordered (or not in a way that the specification cares about), so its position in the range isn’t an issue.
One more thing: You may have noticed that I didn’t write any tests with a system that contained no Clues at the start of the test, or with a system that had two unsolved Clues. It’s true that there’s an implicit branch in whatever construct you’d use to iterate over the Clues that return from your database query or what have you. It’s also true that when writing your first test you might want to write a test that returns an empty array; maybe it’s enough the first time around to get the call chain to execute without error without actually getting in to how to create a test object or read it back out and assert its state. But you’ll write that one-Clue test eventually, and my experience has been that if iteration works for one object it’ll work for zero and two. There’s no point in retesting your language’s iteration construct. Delete those other tests and save the space and time.
Leave a Reply