Multiple should statements in one rspec it clause - bad idea?

Question

Here's my rspec test:

it "can release an idea" do
  james.claim(si_title)
  james.release(si_title)
  james.ideas.size.should eq 0
  si_title.status.should eq "available"
end

Are the two should lines at the end a really bad idea? I read somewhere that you should only test for one thing per it block, but it seems silly to make a whole test just to make sure that the title status changes (the same function does both things in my code).

Answer 1

My interpretation of this isn't so much that there should be precisely one assertion / call to should per spec, but that there should only be one bit of behaviour tested per spec, so for example

it 'should do foo and bar' do
  subject.do_foo.should be_true
  subject.do_bar.should be_true
end

is bad - you're speccing 2 different behaviours at the same time.

on the other hand if your 2 assertions are just verifying different aspects of one thing then I'm ok with that, for example

it 'should return a prime integer' do
  result = subject.do_x
  result.should be_a(Integer)
  result.foo.should be_prime
end

For me it wouldn't make too much sense to have one spec that checks that it returns an integer and a separate one that it returns a prime.

Of course in this case, the be_prime matcher could easily do both of these checks - perhaps a good rule of thumb is that multiple assertions are ok if you could sensibly reduce them to 1 with a custom matcher (whether actually doing this is actually worthwhile probably depends on your situation)

In your particular case, it could be argued that there are 2 behaviours at play - one is changing the status and other is mutating the ideas collection. I would reword your specs to say what the release method should do -

it 'should change the status to available'
it 'should remove the idea from the claimants ideas'

At the moment those things always happen simultaneously but I would argue they are separate behaviours - you could easily imagine a system where multiple people can claim/release an idea and the status only change when the last person releases the idea.

Answer 2

I have the same problem... It ought to be one should per it (my boss says) by it makes testing time consuming and, as you said, silly. Tests require good sense and flexibility or they might end up enslaving you. Anyway, I agree with your test.

Answer 3

My policy is to always consider multiple assertions to be a sign of a potential problem and worth a second thought, but not necessarily wrong. Probably 1/3 of the specs I write end up having multiple assertions in them for one reason or another.

One of the problems with having multiple assertions is that when one fails, you can't see if the other one passed or not. This can sometimes be worked around by building an array of results and asserting the value of the array.

In the case you are asking about, I don't think that multiple assertions are a problem, but I do see something else that seems an issue to me. It looks like your spec may be overly coupled.

I think that you're trying to assert the behavior of whatever kind of a thing james is, but the test is also depending on behavior of si_title in order to tell us what was done to it (by way of its eventual #status value). What I would usually do instead is make si_title a test-double and use #should_receive to directly specify the messages that it should expect.