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

Doublespeak: Define #respond_to_missing? on ObjectDouble #1038

Merged
merged 2 commits into from
Jul 30, 2017

Conversation

mcmire
Copy link
Collaborator

@mcmire mcmire commented Jul 25, 2017

The delegate_method matcher uses Doublespeak internally to do its job.
The delegate object is completely stubbed using an ObjectDouble,
available from Doublespeak, which is shoulda-matchers's own test double
library. ObjectDouble is a class that responds to every
method by implementing method_missing.

Say you are using Ruby 2.4 and you are testing a class that uses the
Forwardable module to do some delegation, and you are using
delegate_method in your test. When you run your test you will a
warning that looks something like:

Courier#deliver at ~/.rvm/rubies/ruby-2.4.0/lib/ruby/2.4.0/forwardable.rb:156 forwarding to private method #deliver

Why is this happening? When the code in your class gets exercised,
Forwardable will delegate the delegate method in question to
ObjectDouble, and the method will get intercepted by its
method_missing method. The fact that this is actually a private method
is what Forwardable is complaining about.

To fix this, all we need to do is add respond_to_missing? in addition
to method_missing?. This is explained here:

https://bugs.ruby-lang.org/issues/13326


This fixes #1006.

The `delegate_method` matcher uses Doublespeak internally to do its job.
The delegate object is completely stubbed using an ObjectDouble,
available from Doublespeak, which is shoulda-matchers's own test double
library. ObjectDouble is a class that responds to every
method by implementing `method_missing`.

Say you are using Ruby 2.4 and you are testing a class that uses the
Forwardable module to do some delegation, and you are using
`delegate_method` in your test. When you run your test you will a
warning that looks something like:

    Courier#deliver at ~/.rvm/rubies/ruby-2.4.0/lib/ruby/2.4.0/forwardable.rb:156 forwarding to private method #deliver

Why is this happening? When the code in your class gets exercised,
Forwardable will delegate the delegate method in question to
ObjectDouble, and the method will get intercepted by its
`method_missing` method. The fact that this is actually a private method
is what Forwardable is complaining about.

To fix this, all we need to do is add `respond_to_missing?` in addition
to `method_missing?`. This is explained here:

    https://bugs.ruby-lang.org/issues/13326
@@ -18,6 +18,10 @@ def respond_to?(name, include_private = nil)
true
end

def respond_to_missing?(name, include_all)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused method argument - name. If it's necessary, use _ or _name as an argument name to indicate that it won't be used. You can also write as respond_to_missing?() if you want the method to accept any arguments but don't care about them.
Unused method argument - include_all. If it's necessary, use _ or _include_all as an argument name to indicate that it won't be used. You can also write as respond_to_missing?(
) if you want the method to accept any arguments but don't care about them.

@mcmire
Copy link
Collaborator Author

mcmire commented Jul 30, 2017

@guialbuk Pinging you on this. Wanted to make sure you saw this and had a chance to review it. The build fails right now because this is dependent on #1037.

@mcmire
Copy link
Collaborator Author

mcmire commented Jul 30, 2017

Oh whoops! Looks like you saw it, never mind :)

@mcmire mcmire merged commit e58e689 into rails-5 Jul 30, 2017
@mcmire mcmire deleted the fix-deprecation-warning-using-delegate-method branch July 30, 2017 05:44
@guialbuk guialbuk added this to the v4.0 milestone Jul 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants