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

Allow to define custom differ object in matchers #1096

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions lib/rspec/expectations/fail_with.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,23 @@ def differ

# Raises an RSpec::Expectations::ExpectationNotMetError with message.
# @param [String] message
# @param [Object] expected
# @param [Object] actual
# @param [Object] matcher
#
# Adds a diff to the failure message when `expected` and `actual` are
# Adds a diff to the failure message when `matcher.expected` and `matcher.actual` are
# both present.
def fail_with(message, expected=nil, actual=nil)
def fail_with(message, matcher=nil)
Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid you can't do this, it changes a public api.

unless message
raise ArgumentError, "Failure message is nil. Does your matcher define the " \
"appropriate failure_message[_when_negated] method to return a string?"
end

message = ::RSpec::Matchers::ExpectedsForMultipleDiffs.from(expected).message_with_diff(message, differ, actual)
message = if !matcher.nil?
Copy link
Member

Choose a reason for hiding this comment

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

As you can see above we already inject the differ, it just can't come from a param here, or at least, it'd have to be an extra param.

differ_in_use = matcher.respond_to?(:differ) ? matcher.differ : differ

::RSpec::Matchers::ExpectedsForMultipleDiffs.from(matcher.expected).message_with_diff(message, differ_in_use, matcher.actual)
else
::RSpec::Matchers::ExpectedsForMultipleDiffs.from(nil).message_with_diff(message, differ, nil)
Copy link
Member

Choose a reason for hiding this comment

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

I'm yeah nahing this, expected and actual need to be passed in, differ could be an optional extra, this would eliminate the need for this if.

end

RSpec::Support.notify_failure(RSpec::Expectations::ExpectationNotMetError.new message)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rspec/expectations/handler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def self.handle_failure(matcher, message, failure_message_method)
message ||= matcher.__send__(failure_message_method)

if matcher.respond_to?(:diffable?) && matcher.diffable?
::RSpec::Expectations.fail_with message, matcher.expected, matcher.actual
::RSpec::Expectations.fail_with message, matcher
else
::RSpec::Expectations.fail_with message
end
Expand Down
4 changes: 3 additions & 1 deletion lib/rspec/matchers/built_in/operators.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ def get(klass, operator)

register Enumerable, '=~', BuiltIn::ContainExactly

attr_reader :actual, :expected

def initialize(actual)
@actual = actual
end
Expand Down Expand Up @@ -68,7 +70,7 @@ def self.use_custom_matcher_or_delegate(operator)

# @private
def fail_with_message(message)
RSpec::Expectations.fail_with(message, @expected, @actual)
RSpec::Expectations.fail_with(message, self)
end

# @api private
Expand Down
7 changes: 7 additions & 0 deletions lib/rspec/matchers/matcher_protocol.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,13 @@ class MatcherProtocol
# and that the values returned by these can be usefully diffed, which can
# be included in the output.

# @!method differ
# @return [Object] An `RSpec::Support::Differ` compatible object.
# Provides a custom differ object to calculate printable difference
# between `actual` and `expected` attributes. The result will be
# included in the output. `diffable?` should be defined as well to make
# it work.

# @!method actual
# @return [String, Object] If an object (rather than a string) is provided,
# RSpec will use the `pp` library to convert it to multi-line output in
Expand Down
39 changes: 31 additions & 8 deletions spec/rspec/expectations/fail_with_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

RSpec.describe RSpec::Expectations, "#fail_with" do
let(:differ) { double("differ") }
let(:matcher_class) { Struct.new(:expected, :actual)}

before(:example) do
allow(RSpec::Matchers.configuration).to receive_messages(:color? => false)
Expand All @@ -12,15 +13,15 @@
expect(differ).to receive(:diff).and_return("diff text")

expect {
RSpec::Expectations.fail_with "message", "abc", "def"
RSpec::Expectations.fail_with "message", matcher_class.new("abc", "def")
}.to fail_with("message\nDiff:diff text")
end

it "does not include the diff if expected and actual are not diffable" do
expect(differ).to receive(:diff).and_return("")

expect {
RSpec::Expectations.fail_with "message", "abc", "def"
RSpec::Expectations.fail_with "message", matcher_class.new("abc", "def")
}.to fail_with("message")
end

Expand All @@ -34,6 +35,8 @@
end

RSpec.describe RSpec::Expectations, "#fail_with with matchers" do
let(:matcher_class) { Struct.new(:expected, :actual)}

before do
allow(RSpec::Matchers.configuration).to receive_messages(:color? => false)
end
Expand All @@ -45,29 +48,49 @@
expected_diff = dedent(<<-EOS)
|
|@@ -1,2 +1,2 @@
|-["poo", "car"]
|+[(a string matching /foo/), (a string matching /bar/)]
|-[(a string matching /foo/), (a string matching /bar/)]
|+["poo", "car"]
Copy link
Member

Choose a reason for hiding this comment

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

To be backwards compatible, existing specs shouldn't change...

|
EOS

expect {
RSpec::Expectations.fail_with "message", actual, expected
RSpec::Expectations.fail_with "message", matcher_class.new(expected, actual)
}.to fail_with("message\nDiff:#{expected_diff}")
end

context "when matcher provides custom differ" do
let(:custom_differ) { double("custom differ") }
let(:custom_diff) { "custom diff" }
let(:matcher_class) { Struct.new(:expected, :actual, :differ) }

before(:example) do
allow(custom_differ).to receive(:diff).and_return(custom_diff)
end

it "uses custom differ instead of default" do
expect(RSpec::Expectations).not_to receive(:differ)

expect {
RSpec::Expectations.fail_with "message", matcher_class.new("abc", "def", custom_differ)
}.to fail_with("message\nDiff:#{custom_diff}")
end
end
end

RSpec.describe RSpec::Expectations, "#fail_with with --color" do
let(:matcher_class) { Struct.new(:expected, :actual)}

before do
allow(RSpec::Matchers.configuration).to receive_messages(:color? => true)
end

it "tells the differ to use color" do
expected = "foo bar baz\n"
actual = "foo bang baz\n"
expected = "foo bang baz\n"
actual = "foo bar baz\n"
Copy link
Member

Choose a reason for hiding this comment

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

To prove your new implementation works, these should have been left alone.

expected_diff = "\e[0m\n\e[0m\e[34m@@ -1,2 +1,2 @@\n\e[0m\e[31m-foo bang baz\n\e[0m\e[32m+foo bar baz\n\e[0m"

expect {
RSpec::Expectations.fail_with "message", actual, expected
RSpec::Expectations.fail_with "message", matcher_class.new(expected, actual)
}.to fail_with("message\nDiff:#{expected_diff}")
end
end
4 changes: 2 additions & 2 deletions spec/rspec/expectations/handler_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ module Expectations
)
actual = Object.new

expect(::RSpec::Expectations).to receive(:fail_with).with("message", 1, 2)
expect(::RSpec::Expectations).to receive(:fail_with).with("message", matcher)

RSpec::Expectations::PositiveExpectationHandler.handle_matcher(actual, matcher)
end
Expand Down Expand Up @@ -187,7 +187,7 @@ module Expectations
)
actual = Object.new

expect(::RSpec::Expectations).to receive(:fail_with).with("message", 1, 2)
expect(::RSpec::Expectations).to receive(:fail_with).with("message", matcher)

RSpec::Expectations::NegativeExpectationHandler.handle_matcher(actual, matcher)
end
Expand Down
70 changes: 60 additions & 10 deletions spec/rspec/matchers/built_in/operators_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ def method_missing(name, *args, &block)

it "fails when target.==(actual) returns false" do
subject = "apple"
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected: "orange"\n got: "apple" (using ==)], "orange", "apple")
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected: "orange"\n got: "apple" (using ==)],
an_object_having_attributes(:expected => "orange", :actual => "apple").and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
subject.should == "orange"
end

Expand Down Expand Up @@ -87,7 +92,12 @@ def method_missing(name, *args, &block)

it "fails when target.==(actual) returns false" do
subject = "apple"
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected not: == "apple"\n got: "apple"], "apple", "apple")
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected not: == "apple"\n got: "apple"],
an_object_having_attributes(:expected => "apple", :actual => "apple").and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
subject.should_not == "apple"
end
end
Expand All @@ -102,7 +112,12 @@ def method_missing(name, *args, &block)
it "fails when target.===(actual) returns false" do
subject = "apple".dup
expect(subject).to receive(:===).with("orange").and_return(false)
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected: "orange"\n got: "apple" (using ===)], "orange", "apple")
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected: "orange"\n got: "apple" (using ===)],
an_object_having_attributes(:expected => "orange", :actual => "apple").and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
subject.should === "orange"
end
end
Expand All @@ -117,7 +132,12 @@ def method_missing(name, *args, &block)
it "fails when target.===(actual) returns false" do
subject = "apple".dup
expect(subject).to receive(:===).with("apple").and_return(true)
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected not: === "apple"\n got: "apple"], "apple", "apple")
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected not: === "apple"\n got: "apple"],
an_object_having_attributes(:expected => "apple", :actual => "apple").and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
subject.should_not === "apple"
end
end
Expand All @@ -132,7 +152,12 @@ def method_missing(name, *args, &block)
it "fails when target.=~(actual) returns false" do
subject = "fu".dup
expect(subject).to receive(:=~).with(/oo/).and_return(false)
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected: /oo/\n got: "fu" (using =~)], /oo/, "fu")
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected: /oo/\n got: "fu" (using =~)],
an_object_having_attributes(:expected => /oo/, :actual => "fu").and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
subject.should =~ /oo/
end
end
Expand All @@ -147,7 +172,12 @@ def method_missing(name, *args, &block)
it "fails when target.=~(actual) returns false" do
subject = "foo".dup
expect(subject).to receive(:=~).with(/oo/).and_return(true)
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected not: =~ /oo/\n got: "foo"], /oo/, "foo")
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected not: =~ /oo/\n got: "foo"],
an_object_having_attributes(:expected => /oo/, :actual => "foo").and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
subject.should_not =~ /oo/
end
end
Expand All @@ -158,7 +188,12 @@ def method_missing(name, *args, &block)
end

it "fails if > fails" do
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected: > 5\n got: 4], 5, 4)
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected: > 5\n got: 4],
an_object_having_attributes(:expected => 5, :actual => 4).and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
4.should > 5
end
end
Expand All @@ -173,7 +208,12 @@ def method_missing(name, *args, &block)
end

it "fails if > fails" do
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected: >= 5\n got: 4], 5, 4)
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected: >= 5\n got: 4],
an_object_having_attributes(:expected => 5, :actual => 4).and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
4.should >= 5
end
end
Expand All @@ -184,7 +224,12 @@ def method_missing(name, *args, &block)
end

it "fails if > fails" do
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected: < 3\n got: 4], 3, 4)
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected: < 3\n got: 4],
an_object_having_attributes(:expected => 3, :actual => 4).and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
4.should < 3
end
end
Expand All @@ -199,7 +244,12 @@ def method_missing(name, *args, &block)
end

it "fails if > fails" do
expect(RSpec::Expectations).to receive(:fail_with).with(%[expected: <= 3\n got: 4], 3, 4)
expect(RSpec::Expectations).to receive(:fail_with).with(
%[expected: <= 3\n got: 4],
an_object_having_attributes(:expected => 3, :actual => 4).and(
be_a(RSpec::Matchers::BuiltIn::OperatorMatcher)
)
)
4.should <= 3
end
end
Expand Down