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

Already on GitHub? Sign in to your account

"a string".should_not respond_to(:reverse, :some_other_method) does not fail #26

Closed
myronmarston opened this Issue Oct 5, 2010 · 7 comments

Comments

Projects
None yet
2 participants
Owner

myronmarston commented Oct 5, 2010

I just noticed that this doesn't fail:

"a string".should_not respond_to(:reverse, :some_other_method)

It passes because "a string" does not respond_to :some_other_method. I guess this is an issue of semantics--does the matcher mean it does not respond to any of the given methods, or to all of the given methods?

I personally think the current behavior is confusing. As I see it, this multi-method form is basically a replacement for:

"a string".should_not respond_to(:reverse)
"a string".should_not respond_to(:some_other_method)

...except that this would fail, while the multi-method form passes, as it currently works.

I'm happy to fix this, but wanted to ask about the intended behavior first.

Owner

dchelimsky commented Oct 6, 2010

Agree that "a string".should_not respond_to(:reverse, :some_other_method) should fail if it responds to either method.

Owner

myronmarston commented Oct 7, 2010

I've got a fix for this in my issue 26 branch. Besides the fix for this bug, I also improved the handling of the argument arity checking to handle splat args and I added a cuke for the matcher.

Owner

dchelimsky commented Oct 7, 2010

Fix respond_to matcher.

  • Object.new.should_not respond_to(:object_id, :some_undefined_method)
    should fail (but passed before).
  • Refactored and DRYed up the matcher a bit.
  • Added argument arity info to failure_message_for_should_not.
  • Added test coverage for more should_not cases.
  • Closed by 64f9e9a.
Owner

dchelimsky commented Oct 7, 2010

Really nice patch, Myron. Thanks!

Owner

myronmarston commented Oct 7, 2010

Sure. Are you going to merge the other two commits?

Owner

dchelimsky commented Oct 7, 2010

I squashed them all into one.

Owner

myronmarston commented Oct 7, 2010

Ah, somehow I missed that. Thanks.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment