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

Question

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) }

Marked

You may want to look into this - someone else did the work for you. gist.github.com/txus/807456 – Antipyretic

Answer 1

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".

Uella

You'll have to test the exception is a RuntimeError otherwise, simply not delegating will raise a method not found. – Cost

Answer 2

Shoulda matchers now check allow nil option.

class Account
  delegate :name, to: :league, allow_nil: true
end

# RSpec
describe Account do
  it { should delegate_method(:name).to(:league).allow_nil }
end

Kornher

Answer 3

So a couple of things here on whether or not to test this:

1) I don't think there is anything wrong with spec'ing out this behavior. If you have users who need to learn your software/library, its often very helpful to ensure that all your methods that are part of your public facing contract are spec'd. If you don't want to make this part of this model's api, I might recommend doing the delegation manually as to not expose more methods to the outside world than you need to.

2) Specs of this sort help to ensure that the contract that you have with the object your delegating to remains enforced. This is particularly helpful if you are using stubs and mocks in your tests, since they are often implementing the same contract, so at least you are aware when this contract changes.

In terms of how you test the allow_nil portion of it, I would agree with Matt, the best idea is to ensure that league is nil, then attempt to call name on league. This you could test to ensure that nil is returned.

Hope this helps.

Seibert

Answer 4

I would have tested the delegation, as effecitvely we have created is a contract between two classes

describe '#league_name' do
  let(:event) { create(:event) }

  after { event.league_name }

  it "delegates to league's name with prefix" do
     expect(event.league).to receive(:name)
  end
end

Absolutely