CODEWITHSUNDEEP
ruby-on-railstestingrspec
ivan
ivan

Reputation: 6322

Rspec: how to expect mock to receive a method in an `each` loop

I need to test something like this (simplified):

class Event
  def self.charge_unpaid_events
    unpaid_events.each do |event|
      event.charge
    end
  end
  ...
end

I'm using FactoryGirl and have a test like this:

event = FactoryGirl.create(:event)
event.should receive(:charge)
Event.charge_unpaid_events

This fails. I've run into this before, and my understanding is that within the context of the each loop, the block parameter event is a different object than my mock object event. I've gotten around this in the past by ensuring there's only one event instance and using Event.any_instance.should receive..., but that won't work in the context I'm trying to test right now (for other reasons I won't go into).

Is there a smarter way around this limitation?

Upvotes: 3

Views: 2056

Answers (1)

Aaron K
Aaron K

Reputation: 6961

It's hard to tell from that small code snippet but I'm going to guess that unpaid_events is a scope or similar query. Additionally, Event is some sort of DB backed "traditional" Rails-y model.

With those assumptions and given the way the method is currently written, I would normally say this should be an acceptance spec. With that type of spec, I would not mock anything. However, it sounds like charge may have some far reaching and undesired side effects.

If that is truly the case, then without changing your code, you would need to stub Event.unpaid_events to return a collection of mocks. Generally, stubbing the object under test is a code smell and points to an object that is doing too much.

Here's what such a spec may look like:

it "charges all unpaid events" do
  mock_events = [ double(Event, charge: true), double(Event, charge: true) ]
  allow(Event).to receive(:unpaid_events).and_return(mock_events)

  Event.charge_unpaid_events

  mock_events.all?{ |mock| expect(mock).to have_received(:charge) }
end

The first refactor I would probably make is to inject the events that should be charged, with a default of the scope. I probably would at the same time only charge those events that were unpaid:

class Event
  def self.charge_unpaid_events(events = unpaid_events)
    events.select(&:unpaid?)
          .each(&:charge)
  end
end

it "charges all the unpaid events" do
  paid_events = [ double(Event, unpaid: false), double(Event, unpaid: false) ]
  unpaid_events = [
    double(Event, unpaid: true, charge: true),
    double(Event, unpaid: true, charge: true)
  ]
  mock_events = paid_events + unpaid_events

  Event.charge_unpaid_events mock_events

  unpaid_events.all?{ |event| expect(event).to have_received(:charge) }
end

it "does not charge paid events" do
  # Normally testing 'not' conditions is more work than necessary, however for a
  # simple method such as this; plus the fact that charging things that are
  # `paid` can have more drastic consequences; I would argue this deserves a spec
  paid_events = [ double(Event, unpaid: false), double(Event, unpaid: false) ]
  unpaid_events = [
    double(Event, unpaid: true, charge: true),
    double(Event, unpaid: true, charge: true)
  ]
  mock_events = paid_events + unpaid_events

  Event.charge_unpaid_events mock_events

  paid_events.all?{ |event| expect(event).not_to have_received(:charge) }
end

At this point I would call into question the job of Event to both be responsible for managing what gets charged and how something should be charged. I would probably investigate moving those to external service classes to keep charging responsibilities out of a simple event object.

Upvotes: 2

Related Questions