Skip to content

Commit

Permalink
refactor matchers to use BaseMatcher's initialize and matches? methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@dchelimsky
dchelimsky committed Jun 20, 2012
1 parent 6a8c932 commit 4024344
Show file tree
Hide file tree
Showing 11 changed files with 52 additions and 81 deletions.
9 changes: 9 additions & 0 deletions lib/rspec/matchers/built_in/base_matcher.rb
View file
Expand Up @@ -16,6 +16,15 @@ module BaseMatcher

attr_reader :actual, :expected, :rescued_exception

def initialize(expected = nil)
@expected = expected
end

def matches?(actual)
@actual = actual
match(expected, actual)
end

def match_unless_raises(*exceptions)
exceptions.unshift Exception if exceptions.empty?
begin
Expand Down
32 changes: 14 additions & 18 deletions lib/rspec/matchers/built_in/be.rb
View file
Expand Up @@ -6,44 +6,44 @@ module BuiltIn
class BeTrue
include BaseMatcher

def matches?(actual)
@actual = actual
def match(_, actual)
!!actual
end

def failure_message_for_should
"expected: true value\n got: #{@actual.inspect}"
"expected: true value\n got: #{actual.inspect}"
end

def failure_message_for_should_not
"expected: non-true value\n got: #{@actual.inspect}"
"expected: non-true value\n got: #{actual.inspect}"
end
end

class BeFalse
include BaseMatcher

def matches?(actual)
!(@actual = actual)
def match(_, actual)
!actual
end

def failure_message_for_should
"expected: false value\n got: #{@actual.inspect}"
"expected: false value\n got: #{actual.inspect}"
end

def failure_message_for_should_not
"expected: non-false value\n got: #{@actual.inspect}"
"expected: non-false value\n got: #{actual.inspect}"
end
end

class BeNil
include BaseMatcher

def matches?(actual)
(@actual = actual).nil?
def match(_, actual)
actual.nil?
end

def failure_message_for_should
"expected: nil\n got: #{@actual.inspect}"
"expected: nil\n got: #{actual.inspect}"
end

def failure_message_for_should_not
Expand All @@ -52,14 +52,14 @@ def failure_message_for_should_not
end

class Be
include RSpec::Matchers::Pretty
include BaseMatcher

def initialize(*args, &block)
@args = args
end

def matches?(actual)
!!(@actual = actual)
def match(_, actual)
!!actual
end

def failure_message_for_should
Expand All @@ -70,10 +70,6 @@ def failure_message_for_should_not
"expected #{@actual.inspect} to evaluate to false"
end

def description
"be"
end

[:==, :<, :<=, :>=, :>, :===].each do |operator|
define_method operator do |operand|
BeComparedTo.new(operand, operator)
Expand Down
9 changes: 2 additions & 7 deletions lib/rspec/matchers/built_in/be_instance_of.rb
View file
Expand Up @@ -4,13 +4,8 @@ module BuiltIn
class BeAnInstanceOf
include BaseMatcher

def initialize(expected)
@expected = expected
end

def matches?(actual)
@actual = actual
@actual.instance_of? @expected
def match(expected, actual)
actual.instance_of? expected
end
end
end
Expand Down
8 changes: 2 additions & 6 deletions lib/rspec/matchers/built_in/be_kind_of.rb
View file
Expand Up @@ -4,12 +4,8 @@ module BuiltIn
class BeAKindOf
include BaseMatcher

def initialize(expected)
@expected = expected
end

def matches?(actual)
(@actual = actual).kind_of?(@expected)
def match(expected, actual)
actual.kind_of? expected
end
end
end
Expand Down
2 changes: 0 additions & 2 deletions lib/rspec/matchers/built_in/be_within.rb
View file
Expand Up @@ -2,8 +2,6 @@ module RSpec
module Matchers
module BuiltIn
class BeWithin
include BaseMatcher

def initialize(delta)
@delta = delta
end
Expand Down
4 changes: 2 additions & 2 deletions lib/rspec/matchers/built_in/cover.rb
View file
Expand Up @@ -10,12 +10,12 @@ def initialize(*expected)

def matches?(range)
@actual = range
@expected.all? {|e| range.cover?(e)}
@expected.all? { |e| range.cover?(e) }
end

def does_not_match?(range)
@actual = range
expected.none? {|e| range.cover?(e)}
expected.none? { |e| range.cover?(e) }
end
end
end
Expand Down
12 changes: 4 additions & 8 deletions lib/rspec/matchers/built_in/eq.rb
View file
Expand Up @@ -4,20 +4,16 @@ module BuiltIn
class Eq
include BaseMatcher

def initialize(expected)
@expected = expected
end

def matches?(actual)
(@actual = actual) == @expected
def match(expected, actual)
expected == actual
end

def failure_message_for_should
"\nexpected: #{@expected.inspect}\n got: #{@actual.inspect}\n\n(compared using ==)\n"
"\nexpected: #{expected.inspect}\n got: #{actual.inspect}\n\n(compared using ==)\n"
end

def failure_message_for_should_not
"\nexpected: value != #{@expected.inspect}\n got: #{@actual.inspect}\n\n(compared using ==)\n"
"\nexpected: value != #{expected.inspect}\n got: #{actual.inspect}\n\n(compared using ==)\n"
end

def diffable?; true; end
Expand Down
12 changes: 4 additions & 8 deletions lib/rspec/matchers/built_in/eql.rb
View file
Expand Up @@ -4,20 +4,16 @@ module BuiltIn
class Eql
include BaseMatcher

def initialize(expected)
@expected = expected
end

def matches?(actual)
(@actual = actual).eql?(@expected)
def match(expected, actual)
actual.eql? expected
end

def failure_message_for_should
"\nexpected: #{@expected.inspect}\n got: #{@actual.inspect}\n\n(compared using eql?)\n"
"\nexpected: #{expected.inspect}\n got: #{actual.inspect}\n\n(compared using eql?)\n"
end

def failure_message_for_should_not
"\nexpected: value != #{@expected.inspect}\n got: #{@actual.inspect}\n\n(compared using eql?)\n"
"\nexpected: value != #{expected.inspect}\n got: #{actual.inspect}\n\n(compared using eql?)\n"
end

def diffable?
Expand Down
17 changes: 6 additions & 11 deletions lib/rspec/matchers/built_in/equal.rb
View file
Expand Up @@ -4,20 +4,15 @@ module BuiltIn
class Equal
include BaseMatcher

def initialize(expected)
@expected = expected
end

def matches?(actual)
@actual = actual
@actual.equal? @expected
def match(expected, actual)
actual.equal? expected
end

def failure_message_for_should
return <<-MESSAGE
expected #{inspect_object(@expected)}
got #{inspect_object(@actual)}
expected #{inspect_object(expected)}
got #{inspect_object(actual)}
Compared using equal?, which compares object identity,
but expected and actual are not the same object. Use
Expand All @@ -30,8 +25,8 @@ def failure_message_for_should
def failure_message_for_should_not
return <<-MESSAGE
expected not #{inspect_object(@actual)}
got #{inspect_object(@expected)}
expected not #{inspect_object(actual)}
got #{inspect_object(expected)}
Compared using equal?, which compares object identity.
Expand Down
9 changes: 2 additions & 7 deletions lib/rspec/matchers/built_in/match.rb
View file
Expand Up @@ -4,13 +4,8 @@ module BuiltIn
class Match
include BaseMatcher

def initialize(expected)
@expected = expected
end

def matches?(actual)
@actual = actual
@actual.match @expected
def match(expected, actual)
actual.match expected
end
end
end
Expand Down
19 changes: 7 additions & 12 deletions lib/rspec/matchers/built_in/match_array.rb
View file
Expand Up @@ -2,22 +2,17 @@ module RSpec
module Matchers
module BuiltIn
class MatchArray
include RSpec::Matchers::Pretty
include BaseMatcher

def initialize(expected)
@expected = expected
end

def matches?(actual)
@actual = actual
@extra_items = difference_between_arrays(@actual, @expected)
@missing_items = difference_between_arrays(@expected, @actual)
def match(expected, actual)
@extra_items = difference_between_arrays(actual, expected)
@missing_items = difference_between_arrays(expected, actual)
@extra_items.empty? & @missing_items.empty?
end

def failure_message_for_should
message = "expected collection contained: #{safe_sort(@expected).inspect}\n"
message += "actual collection contained: #{safe_sort(@actual).inspect}\n"
message = "expected collection contained: #{safe_sort(expected).inspect}\n"
message += "actual collection contained: #{safe_sort(actual).inspect}\n"
message += "the missing elements were: #{safe_sort(@missing_items).inspect}\n" unless @missing_items.empty?
message += "the extra elements were: #{safe_sort(@extra_items).inspect}\n" unless @extra_items.empty?
message
Expand All @@ -28,7 +23,7 @@ def failure_message_for_should_not
end

def description
"contain exactly #{_pretty_print(@expected)}"
"contain exactly #{_pretty_print(expected)}"
end

private
Expand Down

2 comments on commit 4024344

@myronmarston
Copy link
Member

Choose a reason for hiding this comment

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

@dchelimsky -- This breaks lots of specs in rspec-mocks:

http://travis-ci.org/#!/rspec/rspec-mocks/builds/1692434

The rspec-mocks specs all passed for me locally before I pushed, so I was surprised to see the build fail. Then I git pulled the other rspec repos (including this one) and was able to reproduce the spec failures. Reverting this commit locally fixed the rspec-mocks failures.

I can look into fixing things in rspec-mocks, but it seems like this refactoring might not be as innocuous as originally thought, given that it broke another project. Could it break end users as well?

@dchelimsky
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing this out. Fixed by a3e2839. The problem was that the eq matcher needs to call actual == expected (not expected == actual) to support actuals that define their own equality (like the HashIncludingMatcher in rspec-mocks).

Please sign in to comment.