json_api_client icon indicating copy to clipboard operation
json_api_client copied to clipboard

Support for RSpec's `instance_double`

Open delner opened this issue 8 years ago • 5 comments

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.

delner avatar Jul 13 '16 14:07 delner

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.

chingor13 avatar Oct 03 '16 17:10 chingor13

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

gaorlov avatar Mar 20 '19 15:03 gaorlov

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.

delner avatar Mar 20 '19 23:03 delner

@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

senid231 avatar Mar 21 '19 12:03 senid231

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...

delner avatar Mar 21 '19 21:03 delner