Merge pull request #1440 from henrahmagix/fix-compound-redefined-actu…

…al-diff-on-main

Fix diff for compound when transforming actual

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)
 

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

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

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

lib/rspec/matchers/expecteds_for_multiple_diffs.rb → 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,33 +19,33 @@ 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<Any>] 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
       # Returns message with diff(s) appended for provided differ
       # 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"

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

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

spec/rspec/matchers/expecteds_for_multiple_diffs_spec.rb → 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,26 +75,28 @@ 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
         end
 
         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"]
           EOS
         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"]