Browse files

Provide a clear failure message when using `{}.should =~ {}`.

The match_array matcher (delegated to by `=~` for enumerables) is
not meant to be used for hashes, but the `should =~` syntax doesn't
make that obvious. Previously, you would get a failure like:

     Failure/Error: actual.should =~ actual
       expected: {:foo=>"bar"}
            got: {:foo=>"bar"} (using =~)

...which is pretty confusing. Our `match_array` matcher already
includes handling for invalid arguments (such as hashes) to return
a clear failure message, but it wasn't being used for an expression
like `{}.should =~ {}` because it was only registered as an operator
matcher for `Array`. This changes it so that it is registered as an
operator matcher for any Enumerable. This improves the failure message
for enumerable types like hash and set, but on its own, it could cause
breakage for things like 1.8 strings that are Enumerable but also
define a reasonable `=~`.  The fix here changes the operator matcher
delegation logic so that it only delegates to the registered matcher
if the object has the generic Kernel implementation of the operator.
If it has a more specific implementation, we assume the user actually
wants to match using the given operator itself.

Fixes #191.
  • Loading branch information...
1 parent abefb76 commit c7a91af3aa0aa6f1197a36d4cdcd4efdfe757bad @myronmarston myronmarston committed Dec 6, 2012
Showing with 33 additions and 2 deletions.
  1. +1 −1 lib/rspec/matchers.rb
  2. +17 −1 lib/rspec/matchers/operator_matcher.rb
  3. +15 −0 spec/rspec/matchers/match_array_spec.rb
View
2 lib/rspec/matchers.rb
@@ -684,6 +684,6 @@ def match_array(array)
BuiltIn::MatchArray.new(array)
end
- OperatorMatcher.register(Array, '=~', BuiltIn::MatchArray)
+ OperatorMatcher.register(Enumerable, '=~', BuiltIn::MatchArray)
end
end
View
18 lib/rspec/matchers/operator_matcher.rb
@@ -31,7 +31,7 @@ def initialize(actual)
def self.use_custom_matcher_or_delegate(operator)
define_method(operator) do |expected|
- if matcher = OperatorMatcher.get(@actual.class, operator)
+ if uses_generic_implementation_of?(operator) && matcher = OperatorMatcher.get(@actual.class, operator)
@actual.send(::RSpec::Matchers.last_should, matcher.new(expected))
else
eval_match(@actual, operator, expected)
@@ -62,6 +62,22 @@ def description
private
+ if Method.method_defined?(:owner) # 1.8.6 lacks Method#owner :-(
+ def uses_generic_implementation_of?(op)
+ @actual.method(op).owner == ::Kernel
+ end
+ else
+ def uses_generic_implementation_of?(op)
+ # This is a bit of a hack, but:
+ #
+ # {}.method(:=~).to_s # => "#<Method: Hash(Kernel)#=~>"
+ #
+ # In the absence of Method#owner, this is the best we
+ # can do to see if the method comes from Kernel.
+ @actual.method(op).to_s.include?('(Kernel)')
+ end
+ end
+
def eval_match(actual, operator, expected)
::RSpec::Matchers.last_matcher = self
@operator, @expected = operator, expected
View
15 spec/rspec/matchers/match_array_spec.rb
@@ -113,6 +113,17 @@ def ==(other)
MESSAGE
end
+ context "when the array defines a `=~` method" do
+ it 'delegates to that method rather than using the match_array matcher' do
+ array = []
+ def array.=~(other)
+ other == :foo
+ end
+
+ array.should =~ :foo
+ expect { array.should =~ :bar }.to fail_with(/expected: :bar/)
+ end
+ end
end
describe "should_not =~ [:with, :multiple, :args]" do
@@ -135,4 +146,8 @@ def ==(other)
it "fails with a string and the expected error message is given" do
expect { "I like turtles".should match_array([1,2,3]) }.to fail_with(/expected an array/)
end
+
+ it 'fails with a clear message when given a hash using the `should =~` syntax' do
+ expect { {}.should =~ {} }.to fail_with(/expected an array/)
+ end
end

0 comments on commit c7a91af

Please sign in to comment.