Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for RSpec's instance_double #197

Open
delner opened this issue Jul 13, 2016 · 5 comments
Open

Support for RSpec's instance_double #197

delner opened this issue Jul 13, 2016 · 5 comments

Comments

@delner
Copy link

delner commented Jul 13, 2016

As a developer using the JsonApiClient gem to implement a client, I'd like to test code that uses this client.

Take for example this client resource:

module MyApi
  class Widget < JsonApiClient::Resource
    property :serial_number, type: :string
  end
end

If I write an RSpec test that uses a double to stand in for a Widget object obtained from this API:

let(:widget) { double(MyApi::Widget) }
let(:serial_number) { "A12345" }
it do
  expect(widget).to receive(:serial_number).and_return(serial_number)
  expect(widget.serial_number).to eq(serial_number)
end

...the test passes. If instead I use an instance_double, because I want to verify I'm not mocking a property that doesn't exist:

let(:widget) { instance_double(MyApi::Widget) }
let(:serial_number) { "A12345" }
it do
  expect(widget).to receive(:serial_number).and_return(serial_number)
  expect(widget.serial_number).to eq(serial_number)
end

...then the same test fails, despite the Widget class having a serial_number property defined, which can be accessed like an instance method.

Failure/Error: expect(widget).to receive(:serial_number).and_return(serial_number)
       the MyApi::Widget class does not implement the instance method: serial_number

Looking at the JsonApiClient::Resource object, it appears the likely explanation comes from the use of method_missing to implement these properties. I would suggest either adding a respond_to? or other mechanism that allows instance_double to be used, or refactoring out the use of method_missing (the latter of which is probably a tall order.)

I think there's a lot of value in adding support for instance_double for developers who want to use JsonApiClient. As developers write more unit tests against clients that use JsonApiClient, they will rely heavily on mocks and stubs to prevent actual API calls from being made in their tests. And as highly encouraged by the RSpec folks, using instance_double over double will prevent those tests from errantly stubbing properties that don't exist on rapidly changing APIs, that otherwise could result in false negative tests.

@chingor13
Copy link
Collaborator

I'm sorry, I don't have much experience working with RSpec but would appreciate a PR if you think it's valuable. I also wouldn't be opposed to having a spec suite in parallel to the minitest one if this is something that's easy to break.

@gaorlov
Copy link
Collaborator

gaorlov commented Mar 20, 2019

Is this still an issue, @delner? Would you be willing to work with us to PR it into the gem?

@delner
Copy link
Author

delner commented Mar 20, 2019

I haven't used this in some time, but I think method_missing in https://github.com/JsonApiClient/json_api_client/blob/master/lib/json_api_client/resource.rb#L551 as a means to access dynamic methods is a big part of this. If you query the Resource class with respond_to? it will respond false for any of these dynamic methods, which is clearly not the case. This is one of the reasons use of method_missing is generally not favored.

Although we'd have to verify it with RSpec, you might be able to remedy this by adding an override of respond_to? to Response that returns true if method_missing would've done something other than raise an error, false otherwise. Whether this will work will depend on how RSpec implements verifying doubles and the mechanism by which it verifies.

@senid231
Copy link
Member

senid231 commented Mar 21, 2019

@delner did you check it on master
I think instance_double should work now because we add getter and setter methods for defined properties

https://github.com/JsonApiClient/json_api_client/blob/master/lib/json_api_client/resource.rb#L259

@delner
Copy link
Author

delner commented Mar 21, 2019

I have not; I don't think I've checked it since 2016, so there's a fair chance that things might behave a bit differently...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants