diff --git a/Changelog.md b/Changelog.md index 60ce3150e..caef450ba 100644 --- a/Changelog.md +++ b/Changelog.md @@ -6,6 +6,11 @@ Enhancements: * Update `eq` and `eql` matchers to better highlight difference in string encoding. (Alan Foster, #1425) +Bug Fixes: + +* Fix the diff for redefined `actual` and reassigned `@actual` in compound + expectations failure messages. (Phil Pirozhkov, #1440) + ### 3.12.3 / 2023-04-20 [Full Changelog](http://github.com/rspec/rspec-expectations/compare/v3.12.2...v3.12.3) diff --git a/lib/rspec/expectations/fail_with.rb b/lib/rspec/expectations/fail_with.rb index 212e71c42..17b66b3d6 100644 --- a/lib/rspec/expectations/fail_with.rb +++ b/lib/rspec/expectations/fail_with.rb @@ -30,7 +30,7 @@ def fail_with(message, expected=nil, actual=nil) "appropriate failure_message[_when_negated] method to return a string?" end - message = ::RSpec::Matchers::ExpectedsForMultipleDiffs.from(expected).message_with_diff(message, differ, actual) + message = ::RSpec::Matchers::MultiMatcherDiff.from(expected, actual).message_with_diff(message, differ) RSpec::Support.notify_failure(RSpec::Expectations::ExpectationNotMetError.new message) end diff --git a/lib/rspec/matchers.rb b/lib/rspec/matchers.rb index 6a853ddf1..f0e6cf8ac 100644 --- a/lib/rspec/matchers.rb +++ b/lib/rspec/matchers.rb @@ -10,7 +10,7 @@ dsl matcher_delegator aliased_matcher - expecteds_for_multiple_diffs + multi_matcher_diff ].each { |file| RSpec::Support.require_rspec_matchers(file) } # RSpec's top level namespace. All of rspec-expectations is contained diff --git a/lib/rspec/matchers/built_in/compound.rb b/lib/rspec/matchers/built_in/compound.rb index 74f8ccd2e..0d8090c68 100644 --- a/lib/rspec/matchers/built_in/compound.rb +++ b/lib/rspec/matchers/built_in/compound.rb @@ -51,10 +51,10 @@ def diffable? end # @api private - # @return [RSpec::Matchers::ExpectedsForMultipleDiffs] + # @return [RSpec::Matchers::MultiMatcherDiff] def expected return nil unless evaluator - ::RSpec::Matchers::ExpectedsForMultipleDiffs.for_many_matchers(diffable_matcher_list) + ::RSpec::Matchers::MultiMatcherDiff.for_many_matchers(diffable_matcher_list) end protected diff --git a/lib/rspec/matchers/expecteds_for_multiple_diffs.rb b/lib/rspec/matchers/multi_matcher_diff.rb similarity index 71% rename from lib/rspec/matchers/expecteds_for_multiple_diffs.rb rename to lib/rspec/matchers/multi_matcher_diff.rb index b286bb5ae..cd2264e23 100644 --- a/lib/rspec/matchers/expecteds_for_multiple_diffs.rb +++ b/lib/rspec/matchers/multi_matcher_diff.rb @@ -1,9 +1,9 @@ module RSpec module Matchers # @api private - # Handles list of expected values when there is a need to render - # multiple diffs. Also can handle one value. - class ExpectedsForMultipleDiffs + # Handles list of expected and actual value pairs when there is a need + # to render multiple diffs. Also can handle one pair. + class MultiMatcherDiff # @private # Default diff label when there is only one matcher in diff # output @@ -19,22 +19,23 @@ def initialize(expected_list) # @api private # Wraps provided expected value in instance of - # ExpectedForMultipleDiffs. If provided value is already an - # ExpectedForMultipleDiffs then it just returns it. + # MultiMatcherDiff. If provided value is already an + # MultiMatcherDiff then it just returns it. # @param [Any] expected value to be wrapped - # @return [RSpec::Matchers::ExpectedsForMultipleDiffs] - def self.from(expected) + # @param [Any] actual value + # @return [RSpec::Matchers::MultiMatcherDiff] + def self.from(expected, actual) return expected if self === expected - new([[expected, DEFAULT_DIFF_LABEL]]) + new([[expected, DEFAULT_DIFF_LABEL, actual]]) end # @api private # Wraps provided matcher list in instance of - # ExpectedForMultipleDiffs. + # MultiMatcherDiff. # @param [Array] matchers list of matchers to wrap - # @return [RSpec::Matchers::ExpectedsForMultipleDiffs] + # @return [RSpec::Matchers::MultiMatcherDiff] def self.for_many_matchers(matchers) - new(matchers.map { |m| [m.expected, diff_label_for(m)] }) + new(matchers.map { |m| [m.expected, diff_label_for(m), m.actual] }) end # @api private @@ -42,10 +43,9 @@ def self.for_many_matchers(matchers) # factory and actual value if there are any # @param [String] message original failure message # @param [Proc] differ - # @param [Any] actual value # @return [String] - def message_with_diff(message, differ, actual) - diff = diffs(differ, actual) + def message_with_diff(message, differ) + diff = diffs(differ) message = "#{message}\n#{diff}" unless diff.empty? message end @@ -65,8 +65,8 @@ def truncated(description) end end - def diffs(differ, actual) - @expected_list.map do |(expected, diff_label)| + def diffs(differ) + @expected_list.map do |(expected, diff_label, actual)| diff = differ.diff(actual, expected) next if diff.strip.empty? if diff == "\e[0m\n\e[0m" diff --git a/spec/rspec/matchers/built_in/compound_spec.rb b/spec/rspec/matchers/built_in/compound_spec.rb index 82a8ead7b..2fc72fcf4 100644 --- a/spec/rspec/matchers/built_in/compound_spec.rb +++ b/spec/rspec/matchers/built_in/compound_spec.rb @@ -543,6 +543,98 @@ def expect_block expect(error.message).to include(expected_failure) end end + + context 'when matcher transforms the actual' do + context 'when the matcher redefines `actual`' do + matcher :eq_downcase do |expected| + match do |actual| + @matcher_internal_actual = actual.downcase + values_match? expected, @matcher_internal_actual + end + + def actual + @matcher_internal_actual + end + + diffable + end + + it 'shows the redefined value in diff' do + expected_failure = + dedent(<<-EOS) + | expected "HELLO\\nWORLD" to eq downcase "bonjour\\nmonde" + | + |...and: + | + | expected "HELLO\\nWORLD" to eq downcase "hola\\nmon" + |Diff for (eq downcase "bonjour\\nmonde"): + |@@ -1,3 +1,3 @@ + |-bonjour + |-monde + |+hello + |+world + | + |Diff for (eq downcase "hola\\nmon"): + |@@ -1,3 +1,3 @@ + |-hola + |-mon + |+hello + |+world + EOS + + expect { + expect( + "HELLO\nWORLD" + ).to eq_downcase("bonjour\nmonde").and eq_downcase("hola\nmon") + }.to fail do |error| + expect(error.message).to include(expected_failure) + end + end + end + + context 'when the matcher reassigns `@actual`' do + matcher :eq_downcase do |expected| + match do |actual| + @actual = actual.downcase + values_match? expected, @actual + end + + diffable + end + + it 'shows the reassigned value in diff' do + expected_failure = + dedent(<<-EOS) + | expected "hello\\nworld" to eq downcase "bonjour\\nmonde" + | + |...and: + | + | expected "hello\\nworld" to eq downcase "hola\\nmon" + |Diff for (eq downcase "bonjour\\nmonde"): + |@@ -1,3 +1,3 @@ + |-bonjour + |-monde + |+hello + |+world + | + |Diff for (eq downcase "hola\\nmon"): + |@@ -1,3 +1,3 @@ + |-hola + |-mon + |+hello + |+world + EOS + + expect { + expect( + "HELLO\nWORLD" + ).to eq_downcase("bonjour\nmonde").and eq_downcase("hola\nmon") + }.to fail do |error| + expect(error.message).to include(expected_failure) + end + end + end + end end context "when both matchers are not diffable" do diff --git a/spec/rspec/matchers/built_in/output_spec.rb b/spec/rspec/matchers/built_in/output_spec.rb index 07f9e8979..7e90b83db 100644 --- a/spec/rspec/matchers/built_in/output_spec.rb +++ b/spec/rspec/matchers/built_in/output_spec.rb @@ -194,5 +194,25 @@ def print_to_stream(msg) expect(output("foo").description).to eq('output "foo" to some stream') end end + + RSpec.describe "can capture stdin and stderr" do + it "prints diff for both when both fail" do + expect { + expect { + print "foo"; $stderr.print("bar") + }.to output(/baz/).to_stdout.and output(/qux/).to_stderr + }.to fail_including( + 'expected block to output /baz/ to stdout, but output "foo"', + '...and:', + 'expected block to output /qux/ to stderr, but output "bar"', + 'Diff for (output /baz/ to stdout):', + '-/baz/', + '+"foo"', + 'Diff for (output /qux/ to stderr):', + '-/qux/', + '+"bar"' + ) + end + end end end diff --git a/spec/rspec/matchers/expecteds_for_multiple_diffs_spec.rb b/spec/rspec/matchers/multi_matcher_diff_spec.rb similarity index 85% rename from spec/rspec/matchers/expecteds_for_multiple_diffs_spec.rb rename to spec/rspec/matchers/multi_matcher_diff_spec.rb index 222feaf6c..16c1c4576 100644 --- a/spec/rspec/matchers/expecteds_for_multiple_diffs_spec.rb +++ b/spec/rspec/matchers/multi_matcher_diff_spec.rb @@ -1,9 +1,9 @@ module RSpec module Matchers - RSpec.describe ExpectedsForMultipleDiffs do + RSpec.describe MultiMatcherDiff do before do - stub_const("::RSpec::Matchers::ExpectedsForMultipleDiffs::DESCRIPTION_MAX_LENGTH", 30) + stub_const("::RSpec::Matchers::MultiMatcherDiff::DESCRIPTION_MAX_LENGTH", 30) end class FakeDiffer @@ -17,11 +17,12 @@ def self.diff(actual, expected) let(:message) { "a message" } let(:actual) { "actual value" } - let(:wrapped_value) { described_class.from("expected value") } + let(:wrapped_value) { described_class.from("expected value", actual) } def create_matcher(stubs) instance_double(BuiltIn::BaseMatcher, stubs.merge( :matches? => true, + :actual => actual, :failure_message => "" )) end @@ -35,12 +36,12 @@ def create_matcher(stubs) let(:matcher_with_long_description) { create_matcher(:description => long_description, :expected => "expected value") } describe ".from" do - it "wraps provided value in ExpectedsForMultipleDiffs" do + it "wraps provided value in MultiMatcherDiff" do expect(wrapped_value).to be_a(described_class) end it "returns original value if it was already wrapped" do - expect(described_class.from(wrapped_value)).to be(wrapped_value) + expect(described_class.from(wrapped_value, actual)).to be(wrapped_value) end end @@ -49,7 +50,7 @@ def create_matcher(stubs) it "has a diff for all matchers with their description" do expect(wrapped_value.message_with_diff( - message, differ, actual + message, differ )).to eq(dedent <<-EOS) |a message |Diff for (matcher 1 description):["actual value", "expected 1"] @@ -63,7 +64,7 @@ def create_matcher(stubs) it "returns a message warning if the diff is empty" do allow(FakeDiffer).to receive(:diff) { "\e[0m\n\e[0m" } expect(wrapped_value.message_with_diff( - message, differ, actual + message, differ )).to eq(dedent <<-EOS) |a message |Diff: @@ -74,7 +75,7 @@ def create_matcher(stubs) it "returns just provided message if diff is just whitespace" do allow(FakeDiffer).to receive(:diff) { " \n \t" } expect(wrapped_value.message_with_diff( - message, differ, actual + message, differ )).to eq(dedent <<-EOS) |a message EOS @@ -82,7 +83,7 @@ def create_matcher(stubs) it "returns regular message with diff when single expected" do expect(wrapped_value.message_with_diff( - message, differ, actual + message, differ )).to eq(dedent <<-EOS) |a message |Diff:["actual value", "expected value"] @@ -90,10 +91,12 @@ def create_matcher(stubs) end it "returns message with diff and matcher description when single expected with matcher" do - wrapped_value = described_class.for_many_matchers([include("expected value")]) + matcher = include("expected value") + matcher.matches?(actual) + wrapped_value = described_class.for_many_matchers([matcher]) expect(wrapped_value.message_with_diff( - message, differ, actual + message, differ )).to eq(dedent <<-EOS) |a message |Diff for (include "expected value"):["actual value", ["expected value"]] @@ -104,7 +107,7 @@ def create_matcher(stubs) wrapped_value = described_class.for_many_matchers([matcher_with_long_description]) expect(wrapped_value.message_with_diff( - message, differ, actual + message, differ )).to eq(dedent <<-EOS) |a message |Diff for (#{truncated_description}):["actual value", "expected value"]