Adjust evaluation order of change matcher expressions

Handles reference types better and matches RSpec more closely.

CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 ### Changed
 - Forward example procsy `to_s` to underlying example. [#70](https://gitlab.com/arctic-fox/spectator/-/issues/70)
+- Adjust evaluation order of `change` matcher expressions.
 
 ## [0.10.5] - 2022-01-27
 ### Fixed

spec/rspec/core/explicit_subject_spec.cr

@@ -34,16 +34,14 @@ Spectator.describe "Explicit Subject" do
 
       subject { @@element_list.pop }
 
-      skip "is memoized across calls (i.e. the block is invoked once)",
+      it "is memoized across calls (i.e. the block is invoked once)" do
-        reason: "RSpec calls the \"actual\" block after the \"change block\"." do
         expect do
           3.times { subject }
         end.to change { @@element_list }.from([1, 2, 3]).to([1, 2])
         expect(subject).to eq(3)
       end
 
-      skip "is not memoized across examples",
+      it "is not memoized across examples" do
-        reason: "RSpec calls the \"actual\" block after the \"change block\"." do
         expect { subject }.to change { @@element_list }.from([1, 2]).to([1])
         expect(subject).to eq(2)
       end

src/spectator/matchers/change_exact_matcher.cr

@@ -27,26 +27,32 @@ module Spectator::Matchers
 
     # Actually performs the test against the expression.
     def match(actual : Expression(T)) : MatchData forall T
-      before, after = change(actual)
+      before = expression.value
+      before_inspect = before.inspect
+
       if expected_before == before
-        if before == after
+        actual.value # Trigger block that might cause a change.
-          FailedMatchData.new(match_data_description(actual), "#{actual.label} did not change #{expression.label}",
+        after = expression.value
-            before: before.inspect,
+        after_inspect = after.inspect
-            after: after.inspect
+
-          )
+        if expected_after == after
-        elsif expected_after == after
           SuccessfulMatchData.new(match_data_description(actual))
+        elsif before == after
+          FailedMatchData.new(match_data_description(actual), "#{actual.label} did not change #{expression.label}",
+            before: before_inspect,
+            after: after_inspect
+          )
         else
           FailedMatchData.new(match_data_description(actual), "#{actual.label} did not change #{expression.label} to #{expected_after.inspect}",
-            before: before.inspect,
+            before: before_inspect,
-            after: after.inspect,
+            after: after_inspect,
             expected: expected_after.inspect
           )
         end
       else
         FailedMatchData.new(match_data_description(actual), "#{expression.label} was not initially #{expected_before.inspect}",
           expected: expected_before.inspect,
-          actual: before.inspect,
+          actual: before_inspect,
         )
       end
     end
@@ -54,12 +60,18 @@ module Spectator::Matchers
     # Performs the test against the expression, but inverted.
     # A successful match with `#match` should normally fail for this method, and vice-versa.
     def negated_match(actual : Expression(T)) : MatchData forall T
-      before, after = change(actual)
+      before = expression.value
+      before_inspect = before.inspect
+
       if expected_before == before
+        actual.value # Trigger block that might cause a change.
+        after = expression.value
+        after_inspect = after.inspect
+
         if expected_after == after
           FailedMatchData.new(match_data_description(actual), "#{actual.label} changed #{expression.label} from #{expected_before.inspect} to #{expected_after.inspect}",
-            before: before.inspect,
+            before: before_inspect,
-            after: after.inspect
+            after: after_inspect
           )
         else
           SuccessfulMatchData.new(match_data_description(actual))
@@ -67,18 +79,9 @@ module Spectator::Matchers
       else
         FailedMatchData.new(match_data_description(actual), "#{expression.label} was not initially #{expected_before.inspect}",
           expected: expected_before.inspect,
-          actual: before.inspect,
+          actual: before_inspect,
         )
       end
     end
-
-    # Performs the change and reports the before and after values.
-    private def change(actual)
-      before = expression.value # Retrieve the expression's initial value.
-      actual.value              # Invoke action that might change the expression's value.
-      after = expression.value  # Retrieve the expression's value again.
-
-      {before, after}
-    end
   end
 end

src/spectator/matchers/change_from_matcher.cr

@@ -25,16 +25,24 @@ module Spectator::Matchers
 
     # Actually performs the test against the expression.
     def match(actual : Expression(T)) : MatchData forall T
-      before, after = change(actual)
+      before = expression.value
+      before_inspect = before.inspect
+
       if expected != before
-        FailedMatchData.new(match_data_description(actual), "#{expression.label} was not initially #{expected}",
+        return FailedMatchData.new(match_data_description(actual), "#{expression.label} was not initially #{expected}",
           expected: expected.inspect,
-          actual: before.inspect,
+          actual: before_inspect,
         )
-      elsif before == after
+      end
+
+      actual.value # Trigger block that might change the expression.
+      after = expression.value
+      after_inspect = after.inspect
+
+      if expected == after
         FailedMatchData.new(match_data_description(actual), "#{actual.label} did not change #{expression.label} from #{expected}",
-          before: before.inspect,
+          before: before_inspect,
-          after: after.inspect,
+          after: after_inspect,
           expected: "Not #{expected.inspect}"
         )
       else
@@ -45,18 +53,26 @@ module Spectator::Matchers
     # Performs the test against the expression, but inverted.
     # A successful match with `#match` should normally fail for this method, and vice-versa.
     def negated_match(actual : Expression(T)) : MatchData forall T
-      before, after = change(actual)
+      before = expression.value
+      before_inspect = before.inspect
+
       if expected != before
-        FailedMatchData.new(match_data_description(actual), "#{expression.label} was not initially #{expected}",
+        return FailedMatchData.new(match_data_description(actual), "#{expression.label} was not initially #{expected}",
           expected: expected.inspect,
-          actual: before.inspect
+          actual: before_inspect
         )
-      elsif before == after
+      end
+
+      actual.value # Trigger block that might change the expression.
+      after = expression.value
+      after_inspect = after.inspect
+
+      if expected == after
         SuccessfulMatchData.new(match_data_description(actual))
       else
         FailedMatchData.new(match_data_description(actual), "#{actual.label} changed #{expression.label} from #{expected}",
-          before: before.inspect,
+          before: before_inspect,
-          after: after.inspect,
+          after: after_inspect,
           expected: expected.inspect
         )
       end
@@ -71,14 +87,5 @@ module Spectator::Matchers
     def by(amount)
       ChangeExactMatcher.new(@expression, @expected, @expected + value)
     end
-
-    # Performs the change and reports the before and after values.
-    private def change(actual)
-      before = expression.value # Retrieve the expression's initial value.
-      actual.value              # Invoke action that might change the expression's value.
-      after = expression.value  # Retrieve the expression's value again.
-
-      {before, after}
-    end
   end
 end

src/spectator/matchers/change_to_matcher.cr

@@ -25,19 +25,26 @@ module Spectator::Matchers
 
     # Actually performs the test against the expression.
     def match(actual : Expression(T)) : MatchData forall T
-      before, after = change(actual)
+      before = expression.value
-      if before == after
+      before_inspect = before.inspect
-        FailedMatchData.new(match_data_description(actual), "#{actual.label} did not change #{expression.label}",
+
-          before: before.inspect,
+      if expected == before
-          after: after.inspect,
+        return FailedMatchData.new(match_data_description(actual), "#{expression.label} was already #{expected}",
-          expected: expected.inspect
+          before: before_inspect,
+          expected: "Not #{expected.inspect}"
         )
-      elsif expected == after
+      end
+
+      actual.value # Trigger block that could change the expression.
+      after = expression.value
+      after_inspect = after.inspect
+
+      if expected == after
         SuccessfulMatchData.new(match_data_description(actual))
       else
         FailedMatchData.new(match_data_description(actual), "#{actual.label} did not change #{expression.label} to #{expected}",
-          before: before.inspect,
+          before: before_inspect,
-          after: after.inspect,
+          after: after_inspect,
           expected: expected.inspect
         )
       end
@@ -65,14 +72,5 @@ module Spectator::Matchers
     def by(amount)
       ChangeExactMatcher.new(@expression, @expected - amount, @expected)
     end
-
-    # Performs the change and reports the before and after values.
-    private def change(actual)
-      before = expression.value # Retrieve the expression's initial value.
-      actual.value              # Invoke action that might change the expression's value.
-      after = expression.value  # Retrieve the expression's value again.
-
-      {before, after}
-    end
   end
 end