rspec-mocks: receive_message_chain ... exactly() || twice() || () produces undefined method

I’m testing a piece of code in a rails controller like:

things.each do |thing|
  current_user.all_the_things.create(thing)
end

render json: {success: true}

I have a mock like:

allow_any_instance_of(User).to receive_message_chain('all_the_things.create').
  exactly(2).times.and_return(true)
# ... more setup
expect(response).to be_ok

When the spec is run, an exception is thrown: undefined method 'exactly' for for #RSpec::Mocks::Matchers::ReceiveMessageChain:0x0000000afb3d08>

The same happens if exactly is replaced with twice or anything else defined in the documentation

I’m happy to provide a more concrete test case (and pull request), but before I get too deep into it I want to confirm that the behavior I’m expecting is rational.

According to the message chain docs, the receive_message_chain method should be a replacement for the receive message. My expectation is that the receive_message_chain message should be allowed to have the ‘receive count’ messages chained to them in the same way that receive does.

All that said, this whole approach feels a bit like a code smell, but I’m not confident of another approach. Law of Demeter would suggest not chaining current_user.all_the_things to create. However, it’s good practice in rails to create relationships belonging to the user by sending messages to related objects through the original object (current_user in this case).

TL;DR: is the receive_message_chain behavior described above a bug or a feature?

About this issue

Original URL
State: closed
Created 9 years ago
Comments: 19 (17 by maintainers)

Commits related to this issue

doc update for `receive_message_chain` and `receive` parity * summarizes conversation from rspec/rspec-mocks#921 * adds warning — committed to brycemcd/rspec-mocks by brycemcd 9 years ago

Most upvoted comments

things.each do |thing|
  att = AllTheThing.new(thing)
  att.user = current_user
  att.save
end

On its own, this code is fine. However, once you’re at a point where you have a need to substitute a test double for it, you run into the complexity of the interaction here. This bit of code has to use 3 method calls (new, user= and save) when it could be one, simpler interface:

class AllTheThing
  def self.update_user(thing, user)
    att = AllTheThing.new(thing)
    att.user = user
    att.save
  end
end

things.each do |thing|
  AllTheThing.update_user(thing, current_user)
end

Now you’ve created a very simple seam (AllTheThing.update_user) which can be very simply mocked w/o the need to resort to receive_message_chain. IMO, this makes the code better: you’ve reified a common use case into a first-class method that directly supports that use case with a simpler interface.

We’re hesitant to extend the feature set of receive_message_chain because there’s good design pressure that comes from a complex interaction in your implementation code forcing a complex use of your mocking tool. You can, of course, mock this just fine w/o receive_message_chain:

alt = instance_double(AllTheThing)
expect(alt).to receive(:user=).with(current_user)
expect(alt).to receive(:save)
allow(AllTheThing).to receive(:new).with(thing).and_return(alt)

…but that rightfully makes you uncomfortable. It’s a complex interaction that you are encoding directly into your tests. Mocking tools should exert design pressure towards simpler interfaces rather than simply encoding complex interactions in your tests. Using receive_message_chain hides the complexity of the interaction (which can be OK in some cases, particularly when getting existing legacy code under test), but isn’t generally a good idea. Instead of looking to solve the complexity of mocking this interaction with an increased feature set from rspec-mocks, consider if there’s a way you can improve the implementation by simplifying the interaction. Usually there is.

Anyhow, I’m personally not opposed to extending the feature set of receive_message_chain, provided that:

The implementation is very simple – none of the core team members actively use the feature and we don’t want to be stuck supporting something complex we don’t use.
There are no semantic ambiguities in the API
Someone else does the work and submits the PR (we have other things we care more about that we are working).

Does that help explain our position (or at least my position – @JonRowe may feel different)?

myronmarston on Apr 10, 2015

So how should I write expectation for scopes? Does the example below make any sense? I have the idea to add new spy for each chained scope and than expectation on it

let(:relation) { spy 'relation' }

before do 
    expect(Model).to receive(:with_color).with('red') { relation }
   expect(relation).to receive(:with_shape).with(:triangle) { [instance_double('model')] }
end

it 'does cool things' do
   expect(subject).to eq ...
end

I’ve tried to connect scopes together (like scope :with_color_and_shape, ->(color,share) { ... }) so I could just do expect(Model).to receive(:with_color_and_shape).with(color, shape) {} but I ended up with huge scope taking many arguments and calling bunch of smaller one which was hard to manage.

Bartuz on Dec 4, 2015

@myronmarston do you consider using receive_message_chain to stub chained scopes on model as code-smell?

Bartuz on Dec 4, 2015