Rails / Rspec - writing spec for delegate method (allow_nil option)

keruilin picture keruilin · Feb 3, 2012 · Viewed 7.6k times · Source

Given the code below:

(1) How would you go about writing a spec to test the :allow_nil => false option?

(2) Is it even worth writing a spec to test?

class Event < ActiveRecord::Base
  belongs_to :league

  delegate :name, :to => :league, :prefix => true, :allow_nil => false
end

describe Event do

  context 'when delegating methods to league object' do
    it { should respond_to(:league_name) }
  end

end

It would actually be nice if you could extend shoulda to do:

it { should delegate(:name).to(:league).with_options(:prefix => true, :allow_nil => false) }

Answer

Matt picture Matt · Feb 26, 2012

According to the documentation for the delegate rails module:

If the delegate object is nil an exception is raised, and that happens no matter whether nil responds to the delegated method. You can get a nil instead with the :allow_nil option.

I would create an Event object event with a nil league, or set event.league = nil, then try to call event.name, and check that it raises an exception, since that is what is supposed to happen when allow_nil is false (which is also the default). I know rspec has this idiom for exception testing:

lambda{dangerous_operation}.should raise_exception(optional_exception_class)

I'm not sure if shoulda has this construct, though there are some articles, kinda old, about how to get this behavior in shoulda.

I think this is worth testing if it is behavior that users of this class can expect or assume will happen - which I think is probably true in this case. I wouldn't extend shoulda to test "should delegate", because that seems more implementation-dependent: you're really saying your Event should raise an exception if you try to call #name when it has a nil league. It's really unimportant to users of Event how you are making that happen. I would even go so far, if you wish to assert and make a note of this behavior, to test that Event#name has the same semantics as League#name, without mentioning anything about delegate, since this is a behavior-centric approach.

Build your tests based on how your code behaves, not on how it's built - testing this way is better documentation for those who stumble into your tests, as they're more interested in the question "why is my Event throwing?" or "what can cause Event to throw?" than "is this Event delegating?".

You can highlight this sort of situation by imagining what failures might happen if you change your code in a way that users of Event shouldn't care about. If they don't care about it, the test shouldn't break when you change it. What if you want to, for example, handle the delegation yourself, by writing a #name function that first logs or increments a counter and then delegates to league? by testing the exception-raising behavior, you are protected from this change, but by testing if Event is a delegate, you will break that test when you make this change - and so your test wasn't looking at what is really important about the call to #name.

Anyway, that's all just soapbox talk. tl;dr: test it if it's behavior someone might rely upon. Anything that's not tested is Shroedinger's cat: broken and not broken at the same time. Truthfully, much of the time this can be OK: It's a matter of taste whether you want to say something rigorous and definitive about how the system should be behaving, or just let it be "unspecified behavior".