Reputation: 395
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
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
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
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
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