Like many people at AO, I have come to enjoy testing code in Jay Fields’ one assertion per test style.
This style is great for testing because it clearly establishes intent for each test. This point is invaluable when another developer either inherits your code, or you revisit it at some point in the future. The main problem with this style is the need to create stubs for all the calls in the describe’s before block, or resort to using stub_everything for all your mock objects. These are both fine solutions for simple code, but as soon as the logic encounters a condition you may be setting yourself up for potential testing bugs, and difficulty when you add new logic.
One way to avoid this problem is to leverage the power of rspec, and use nested describe blocks for each condition statement. This helps limit the before block for each describe to only define the stubs for that condition block. This is helpful because it prevents the tester from accidentally defining a stubbed expectation for something that should not happen. It also creates a nice placeholder for adding logic into the code in the future.
The following example shows how nesting rspec describes works. It is important to note that the branch that contains the most logic should be encapsulated in the sub-describe.
Example Source Code:
1 2 3 4 5 6 7 8 9 10 11 12 13 |
class Example def run(director, runner) runner.prepare if director.go? runner.run if runner.complete? director.stop end runner.stop end runner.finish end end |
Example Test Code with nesting describes:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 |
describe Example do before do @target = Example.new end describe '#run' do before do @director = mock @runner = mock @runner.stubs(:prepare) @director.stubs(:go?).returns false @runner.stubs(:finish) end it "should prepare the runner" do @runner.expects(:prepare) @target.run @director, @runner end it "should check the director" do @director.expects(:go?).returns false @target.run @director, @runner end describe 'director returns true for go' do before do @director.stubs(:go?).returns true # redefine @runner.stubs(:run) @runner.stubs(:complete?).returns false @runner.stubs(:stop) end it "should run the runner" do @runner.expects(:run) @target.run @director, @runner end it "should check if the runner is complete" do @runner.expects(:complete?).returns false @target.run @director, @runner end describe 'runner returns true for complete' do before do @runner.stubs(:complete?).returns true # redefine end it "should stop the director" do @director.expects(:stop) @target.run @director, @runner end end it "should stop the runner" do @runner.expects(:stop) @target.run @director, @runner end end it "should finish the runner" do @runner.expects(:finish) @target.run @director, @runner end end end |
I realize the test code gets a bit long, but it is very easy to understand and extend. Also, nesting on conditional breaks creates an easy framework to follow when writing test code.
As someone who’s gone down this path, I would recommend against this.
What happened with our specs was they become too deep and unwieldy. Primarily because it became difficult for our developers to decipher all the setup code prior to the spec/assertion itself.
We take the stance of shallow nesting only allowing 2 levels deep (parent and children, but no grandchildren). We only describe the Class/Module as the parent and a method as a child, and even then we barely put anything is the parent’s setup/before.
If there happens to be a lot of duplicated preconditions, we take that as a code smell to clean up the code under test. Or we’ll deal with un-DRY preconditions (wet specs?).
Hrm, bummer. Somehow the paragraph formatting got lost.
Zach,
Thank you for the comment. You bring up an interesting point about problems that happen when you begin to nest too deep. This is always an interesting line to walk. Certainly, when the nesting gets too deep it becomes time to refactor. Using nested describes helps to point out that smell. However, I disagree that you should limit yourself to only a single describe for each method and stay away from adding stubs in the before. When you nest the describes like I talked about above you are able to leverage the framework to keep your before blocks thin and isolate your code under test into smaller compartments. If you look closely at the example each conditional block becomes a nested describe, and its before only defines what happens inside that condition. When you stick to these rules it becomes easier to read the code because you know what to expect.
That being said, this is a pattern that worked for me and my pair. I would suggest trying it again. It really works nicely when you have blocks in your code like yields or transactions.
This technique can be very useful whenever you stub out methods in a before block. I am currently using mocha for mocking, and as far as I can tell once you stub out a method in mocha you can no longer do things like check how many times a method has been called (using .never, .once, .times, etc.)
In those situations where I want to verify the number of times a method is being called, I will write a test for it, and then in a nested describe stub out those methods for further testing.
I’m well aware how the nesting describes are being used. That’s exactly how we started down the path, and for us it was also primarily for Mocha stub/mock setup code. I should also mention we’re strong proponents for the “1 assertion per test” philosophy too
Using your example, for your single spec “should stop the director” (line 49), this is all the setup code in order:
@target = Example.new # from line 3
@director = mock # starting from line 8
@runner = mock
@runner.stubs(:prepare)
@director.stubs(:go?).returns false
@runner.stubs(:finish)
@director.stubs(:go?).returns true # from line 28
@runner.stubs(:run)
@runner.stubs(:complete?).returns false
@runner.stubs(:stop)
@runner.stubs(:complete?).returns true # from line 46
(I’ve also uploaded a gist http://gist.github.com/119950 in case the formatting goes awry)
I think the key difference is that for us tracking the path of nesting in order to mentally consolidate the setup code is too much work. In other words, we’re DRYing up the spec at the cost of spec readability. The assertion itself may be more readable, but the context of the assertion is scattered. For us, that cost was too high.
Also for us this wasn’t a light decision we made with sample code, but instead a substantial codebase written from scratch that we went back and re-organized all the specs (also not a trivial amount of work).
If this works for you and your pair, that’s awesome. For us the cost-to-benefit ratio was too costly for us to continue.