From ca5bf20edde5c88f6b3e2672767893e050d274b1 Mon Sep 17 00:00:00 2001
From: James Mead <james@floehopper.org>
Date: Sat, 9 Nov 2024 10:50:07 +0000
Subject: [PATCH] Fail fast if invocation matches never expectation

Previously when an invocation matched an expectation which did not allow
invocations (i.e. `Expectation#never` had been called on it), but the
invocation also matched another expectation which did allow invocations,
then the test would not fail with an unexpected invocation error.

This was happening because neither the `if` condition was `true`,
because the "never" expectation was not returned by
`ExpectationList#match_allowing_invocation`, but the other expectation
allowing expectations was returned. Thus `Expectation#invoke` was called
on the latter and `Mock#raise_unexpected_invocation_error` was not
called.

This behaviour was confusing and had led to a number of issues being
raised over the years: #44, #131, #490 & most recently #678. Previously
I had thought this was a symptom of the JMock v1 dispatch behaviour
(which _might_ still be true) and thus would be best addressed by
adopting the JMock v2 dispatch behaviour (#173). However, having
considered this specific scenario involving a "never" expectation, I've
decided to try to fix it with the changes in this commit.

Now a test like this will fail with an unexpected invocation error:

    mock = mock('mock')
    mock.stubs(:method)
    mock.expects(:method).never
    mock.method

    unexpected invocation: #<Mock:mock>.method()
    unsatisfied expectations:
    - expected never, invoked once: #<Mock:mock>.method(any_parameters)
    satisfied expectations:
    - allowed any number of times, invoked never: #<Mock:mock>.method(any_parameters)

Closes #678. Also addresses #490, #131 & #44.
---
 lib/mocha/mock.rb                               |  3 ++-
 test/acceptance/mocked_methods_dispatch_test.rb | 17 +++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/mocha/mock.rb b/lib/mocha/mock.rb
index 858e69628..f349297a7 100644
--- a/lib/mocha/mock.rb
+++ b/lib/mocha/mock.rb
@@ -323,9 +323,10 @@ def handle_method_call(symbol, arguments, block)
       invocation = Invocation.new(self, symbol, arguments, block)
 
       matching_expectation_allowing_invocation = all_expectations.match_allowing_invocation(invocation)
+      matching_expectation_never_allowing_invocation = all_expectations.match_never_allowing_invocation(invocation)
       matching_expectation_ignoring_order = all_expectations.match(invocation, ignoring_order: true)
 
-      if matching_expectation_allowing_invocation
+      if matching_expectation_allowing_invocation && !matching_expectation_never_allowing_invocation
         matching_expectation_allowing_invocation.invoke(invocation)
       elsif matching_expectation_ignoring_order || (!matching_expectation_ignoring_order && !@everything_stubbed)
         raise_unexpected_invocation_error(invocation, matching_expectation_ignoring_order)
diff --git a/test/acceptance/mocked_methods_dispatch_test.rb b/test/acceptance/mocked_methods_dispatch_test.rb
index 9b53aa992..5789dc422 100644
--- a/test/acceptance/mocked_methods_dispatch_test.rb
+++ b/test/acceptance/mocked_methods_dispatch_test.rb
@@ -72,4 +72,21 @@ def test_should_find_latest_expectation_with_range_of_expected_invocation_count_
     end
     assert_passed(test_result)
   end
+
+  def test_should_fail_fast_if_invocation_matches_expectation_with_never_cardinality
+    test_result = run_as_test do
+      mock = mock('mock')
+      mock.stubs(:method)
+      mock.expects(:method).never
+      mock.method
+    end
+    assert_failed(test_result)
+    assert_equal [
+      'unexpected invocation: #<Mock:mock>.method()',
+      'unsatisfied expectations:',
+      '- expected never, invoked once: #<Mock:mock>.method(any_parameters)',
+      'satisfied expectations:',
+      '- allowed any number of times, invoked never: #<Mock:mock>.method(any_parameters)'
+    ], test_result.failure_message_lines
+  end
 end