daveharris
daveharris

Reputation: 395

rspec-mocks' doubles are designed to only last for one example

I've got a question about how to share rspec-mocks' double between examples. I'm writing a new rails app with rspec-mocks 3.1.3. I'm used to using the old (< 2.14 and and trying to update my knowledge if current rspec usage.

I have a model method:

def self.from_strava(activity_id, race_id, user)
  @client ||= Strava::Api::V3::Client.new(access_token: 'abc123')

  activity = @client.retrieve_an_activity(activity_id)

  result_details = {race_id: race_id, user: user}
  result_details[:duration] = activity['moving_time']
  result_details[:date] = Date.parse(activity['start_date'])
  result_details[:comment] = activity['description']
  result_details[:strava_url] = "http://www.strava.com/activities/#{activity_id}"


  Result.create!(result_details)
end

And here is the spec:

describe ".from_strava" do
  let(:user) { FactoryGirl.build(:user) }
  let(:client) { double(:client) }
  let(:json_response) { JSON.parse(File.read('spec/support/strava_response.json')) }

  before(:each) do
    allow(Strava::Api::V3::Client).to receive(:new) { client }
    allow(client).to receive(:retrieve_an_activity) { json_response }
    allow(Result).to receive(:create!)
  end

  it "sets the duration" do
    expect(Result).to receive(:create!).with(hash_including(duration: 3635))
    Result.from_strava('123', 456, user)
  end

  it "sets the date" do
    expect(Result).to receive(:create!).with(hash_including(date: Date.parse("2014-11-14")))
    Result.from_strava('123', 456, user)
  end
end

When I run a single test on it's own it's fine, but when I run the whole describe ".from_strava" block it fails with the message

Double :client was originally created in one example but has leaked into another example and can no longer be used. rspec-mocks' doubles are designed to only last for one example, and you need to create a new one in each example you wish to use it for.

I understand what it's saying, but surely this is an appropriate use of a double being used in 2 examples. After all, the client double isn't important to the example, it's just a way for me to load the canned response. I guess I could use WebMock but that seems very low-level and doesn't translate well to the actual code written. We should only be asserting one thing per example after all.

I had thought about replacing the client double with a call to

allow(Strava::Api::V3::Client).to receive_message_chain(:new, :retrieve_an_activity) { json_response }

but that doesn't seem to be the right approach either, given that the documentation states that receive_message_chain should be a code smell.

So if I shouldn't use receive_message_chain, shared client double and also follow the standard DRY principle then how should I fix this?

I would love some feedback on this.

Thanks, Dave

Upvotes: 19

Views: 13450

Answers (4)

Marcin Kołodziej
Marcin Kołodziej

Reputation: 5313

Caching clients for external components can often be really desired (keeping alive connections/any SSL setup that you might need, etc.) and removing that for the sake of fixing an issue with tests is not a desirable solution.

In order to fix your test (without refactoring your code), you can do the following to clear the instance variable after each of your tests:

after { Result.instance_variable_set("@client", nil) }

While admittedly, this is not the cleanest solution, it seems to be the simplest and achieves both, lets you have a clear setup with no state shared in between tests, and keep your client cached in "normal" operation mode.

Upvotes: 21

David Hempy
David Hempy

Reputation: 6287

TLDR: Add after { order.vendor_service = nil } to balance the before block. Or read on...

I ran into this, and it was not obvious where it was coming from. In order_spec.rb model tests, I had this:

  describe 'order history' do
    before do
      service = double('VendorAPI')
      allow(service).to receive(:order_count).and_return(5)
      order.vendor_service = service
    end   

    # tests here  ..
  end

And in my Order model:

  def too_many_orders?
    @@vendor_service ||= VendorAPI.new(key: 'abc', account: '123')
    return @@vendor_service.order_count > 10
  end

This worked fine when I only ran rspec on order_spec.rb

I was mocking something completely different in order_controller_spec.rb a little differently, using allow_any_instance_of() instead of double and allow:

  allow_any_instance_of(Order).to receive(:too_many_orders?).and_return(true)

This, too, tested out fine.

The confounding trouble is that when I ran the full suite of tests, I got the OP's error on the controller mock -- the one using allow_any_instance. This was very hard to track down, as the problem (or at least my solution) lay in the model tests where I use double/allow.

To fix this, I added an after block clearing the class variable @@vendor_service, balancing the before block's action:

  describe 'order history' do
    before do
      service = double('VendorAPI')
      allow(service).to receive(:order_count).and_return(5)
      order.vendor_service = service
    end   

    after do
      order.vendor_service = nil 
    end

    # tests here  ..
  end

This forced the ||= VendorAPI.new() to use the real new function in later unrelated tests, not the mock object.

Upvotes: 2

Nathan Wallace
Nathan Wallace

Reputation: 2264

I had the same use case in an app of mine, and we solved it by extracting the cacheing into a private method and then stubbing that method to return the double (instead of stubbing the new method directly).

For example, in the class under test:

def self.from_strava(activity_id, race_id, user)
  activity = strava_client.retrieve_an_activity(activity_id)
  ...
end

private
def self.strava_client
  @client ||= Strava::Api::V3::Client.new(access_token: 'abc123')
end

And in the spec:

let(:client) { double(:client) }

before { allow(described_class).to receive(:strava_client).and_return(client) }

...

Upvotes: 3

joelparkerhenderson
joelparkerhenderson

Reputation: 35483

surely this is an appropriate use of a double being used in 2 examples.

No, it's not. :) You're trying to use a class variable; do not do that because the variable doesn't span examples. The solution is to set the client each time i.e. in each example.

Bad:

@client ||= Strava::Api::V3::Client.new(access_token: 'abc123')

Good:

@client = Strava::Api::V3::Client.new(access_token: 'abc123')

Upvotes: 8

Related Questions