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

it { should include foo } fails or provides unexpected output if foo.respond_to?(:description) #260

Closed
gregates opened this issue May 31, 2013 · 2 comments · Fixed by #264

Comments

@gregates
Copy link

This is probably a pretty weird edge case, but here goes:

I have a kind of wrapper object that behaves like this (the details have been abstracted away for hopefully clearer presentation):

class Handler

  def initialize(feature)
    @feature = feature
  end

  def description
    @feature.description
  end
end

Now, it's normally the case that the kind of object passed into @feature responds to :description. But I had a test that looked like this:

describe "::all" do
  subject { Handler.all }

  it { should include Handler.new(Object.new) }
end

This passed when I was using v2.6, but after upgrading to 2.13 it started failing with a NoMethodError: undefined method 'description'. I was surprised by this, because instantiating a Handler should not call the description method. When I tried stubbing the Object.new with a description method that said "This shouldn't be here", I got a passing test that looked like:

FeatureHandler
  ::all
    should include This shouldn't be here

So it appears to me that what's going wrong is that the to_word method in rspec-expectations/lib/rspec/matchers/pretty.rb is using respond_to?(:description) not just to check that the argument responds to :description, but also (implicitly) as a duck-type check -- it's expecting that if the object does respond to :description, we'll have an object of type RSpec::Core::Example or some other rspec object. But obviously that assumption fails in my weird edge case!

I thought about submitting a patch along with this issue report but I'm not really sure what the correct fix is. More explicit type-checking? Differently named methods so that there's even less likelihood of unintended collisions?

@JonRowe
Copy link
Member

JonRowe commented Jun 4, 2013

Can you try 2.14, this has already been worked around.

@JonRowe
Copy link
Member

JonRowe commented Jun 4, 2013

My bad, I was thinking of rspec/rspec-mocks#288 carry on :)

alindeman added a commit that referenced this issue Jun 5, 2013
alindeman added a commit that referenced this issue Jun 5, 2013
* `#description` is only used for matchers.

[Fixes #260]

Signed-off-by: Andy Lindeman <[email protected]>
Signed-off-by: Sam Phippen <[email protected]>
alindeman added a commit that referenced this issue Jun 5, 2013
alindeman added a commit that referenced this issue Jun 5, 2013
* `#description` is only used for matchers.

[Fixes #260]

Signed-off-by: Andy Lindeman <[email protected]>
Signed-off-by: Sam Phippen <[email protected]>
alindeman added a commit that referenced this issue Jun 5, 2013
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this issue Oct 6, 2021
* `#description` is only used for matchers.

[Fixes rspec/rspec-expectations#260]

Signed-off-by: Andy Lindeman <[email protected]>
Signed-off-by: Sam Phippen <[email protected]>

---
This commit was imported from rspec/rspec-expectations@5412202.
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this issue Oct 6, 2021
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this issue Oct 6, 2021
* `#description` is only used for matchers.

[Fixes rspec/rspec-expectations#260]

Signed-off-by: Andy Lindeman <[email protected]>
Signed-off-by: Sam Phippen <[email protected]>

---
This commit was imported from rspec/rspec-expectations@5d541bb.
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this issue Oct 6, 2021
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

Successfully merging a pull request may close this issue.

2 participants