From ac7232424685509cbd529f510d7a2169998c1bfc Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Sat, 18 Nov 2023 11:17:56 +0000
Subject: [PATCH] Improve sequence failure message
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

I think there's more we could do in this area, but this implements
@dchelimsky's suggestion which is definitely an improvement.

If an invocation was unexpected because, although there is an
expectation that expectation does not match due to an ordering
constraint, then append "invoked out of order" to the failure message.

e.g.

    unexpected invocation: #<Mock:mock>.foo() invoked out of order

Closes #60 (only 11 years late!😅)
---
 lib/mocha/expectation.rb               | 10 ++++++++--
 lib/mocha/expectation_list.rb          | 10 +++++++---
 lib/mocha/mock.rb                      |  8 ++++++--
 test/acceptance/sequence_block_test.rb |  2 ++
 test/acceptance/sequence_test.rb       |  2 ++
 5 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/lib/mocha/expectation.rb b/lib/mocha/expectation.rb
index 240747c76..5e6f95d4f 100644
--- a/lib/mocha/expectation.rb
+++ b/lib/mocha/expectation.rb
@@ -632,14 +632,20 @@ def in_correct_order?
       @ordering_constraints.all?(&:allows_invocation_now?)
     end
 
+    # @private
+    def ordering_constraints_not_allowing_invocation_now
+      @ordering_constraints.reject(&:allows_invocation_now?)
+    end
+
     # @private
     def matches_method?(method_name)
       @method_matcher.match?(method_name)
     end
 
     # @private
-    def match?(invocation)
-      @method_matcher.match?(invocation.method_name) && @parameters_matcher.match?(invocation.arguments) && @block_matcher.match?(invocation.block) && in_correct_order?
+    def match?(invocation, ignoring_order: false)
+      order_independent_match = @method_matcher.match?(invocation.method_name) && @parameters_matcher.match?(invocation.arguments) && @block_matcher.match?(invocation.block)
+      ignoring_order ? order_independent_match : order_independent_match && in_correct_order?
     end
 
     # @private
diff --git a/lib/mocha/expectation_list.rb b/lib/mocha/expectation_list.rb
index f5366ec17..74c720da4 100644
--- a/lib/mocha/expectation_list.rb
+++ b/lib/mocha/expectation_list.rb
@@ -17,7 +17,11 @@ def matches_method?(method_name)
       @expectations.any? { |expectation| expectation.matches_method?(method_name) }
     end
 
-    def match(invocation)
+    def match(invocation, ignoring_order: false)
+      matching_expectations(invocation, ignoring_order: ignoring_order).first
+    end
+
+    def match_but_out_of_order(invocation)
       matching_expectations(invocation).first
     end
 
@@ -51,8 +55,8 @@ def +(other)
 
     private
 
-    def matching_expectations(invocation)
-      @expectations.select { |e| e.match?(invocation) }
+    def matching_expectations(invocation, ignoring_order: false)
+      @expectations.select { |e| e.match?(invocation, ignoring_order: ignoring_order) }
     end
   end
 end
diff --git a/lib/mocha/mock.rb b/lib/mocha/mock.rb
index 397d602d6..6b2b9737d 100644
--- a/lib/mocha/mock.rb
+++ b/lib/mocha/mock.rb
@@ -323,7 +323,7 @@ def handle_method_call(symbol, arguments, block)
       invocation = Invocation.new(self, symbol, arguments, block)
       if (matching_expectation_allowing_invocation = all_expectations.match_allowing_invocation(invocation))
         matching_expectation_allowing_invocation.invoke(invocation)
-      elsif (matching_expectation = all_expectations.match(invocation)) || (!matching_expectation && !@everything_stubbed)
+      elsif (matching_expectation = all_expectations.match(invocation, ignoring_order: true)) || (!matching_expectation && !@everything_stubbed)
         raise_unexpected_invocation_error(invocation, matching_expectation)
       end
     end
@@ -373,7 +373,11 @@ def raise_unexpected_invocation_error(invocation, matching_expectation)
       if @unexpected_invocation.nil?
         @unexpected_invocation = invocation
         matching_expectation.invoke(invocation) if matching_expectation
-        message = "#{@unexpected_invocation.call_description}\n#{@mockery.mocha_inspect}"
+        call_description = @unexpected_invocation.call_description
+        if matching_expectation && !matching_expectation.in_correct_order?
+          call_description += ' invoked out of order'
+        end
+        message = "#{call_description}\n#{@mockery.mocha_inspect}"
       else
         message = @unexpected_invocation.short_call_description
       end
diff --git a/test/acceptance/sequence_block_test.rb b/test/acceptance/sequence_block_test.rb
index 468191424..86513cc9f 100644
--- a/test/acceptance/sequence_block_test.rb
+++ b/test/acceptance/sequence_block_test.rb
@@ -24,6 +24,7 @@ def test_should_constrain_invocations_to_occur_in_expected_order
       mock.first
     end
     assert_failed(test_result)
+    assert_match 'second() invoked out of order', test_result.failures.first.message
   end
 
   def test_should_allow_invocations_in_sequence
@@ -87,6 +88,7 @@ def test_should_constrain_invocations_to_occur_in_expected_order_even_if_expecte
       partial_mock_one.first
     end
     assert_failed(test_result)
+    assert_match 'second() invoked out of order', test_result.failures.first.message
   end
 
   def test_should_allow_invocations_in_sequence_even_if_expected_on_partial_mocks
diff --git a/test/acceptance/sequence_test.rb b/test/acceptance/sequence_test.rb
index 85ec8cbc7..52bcd6145 100644
--- a/test/acceptance/sequence_test.rb
+++ b/test/acceptance/sequence_test.rb
@@ -23,6 +23,7 @@ def test_should_constrain_invocations_to_occur_in_expected_order
       mock.first
     end
     assert_failed(test_result)
+    assert_match 'second() invoked out of order', test_result.failures.first.message
   end
 
   def test_should_allow_invocations_in_sequence
@@ -82,6 +83,7 @@ def test_should_constrain_invocations_to_occur_in_expected_order_even_if_expecte
       partial_mock_one.first
     end
     assert_failed(test_result)
+    assert_match 'second() invoked out of order', test_result.failures.first.message
   end
 
   def test_should_allow_invocations_in_sequence_even_if_expected_on_partial_mocks