Skip to content

Commit

Permalink
Fail fast if invocation matches never expectation
Browse files Browse the repository at this point in the history
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.

After #681 a deprecation warning was displayed in this scenario, but now
an unexpected invocation error is reported.

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.

Also the new behaviour will bring the behaviour into line with what is
already the case for mocks where `Mock#stub_everything` has been called
as per this acceptance test [1] that was introduced in this commit [2].

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.

[1]: https://github.com/freerange/mocha/blob/f2fa99197f35e2d2ce9554db63f6964057e29ce0/test/acceptance/expected_invocation_count_test.rb#L202-L214
[2]: d358377
  • Loading branch information
floehopper committed Nov 24, 2024
1 parent 862237d commit 15985bc
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 21 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ Maybe, but probably not. Partial mocking changes the state of objects in the `Ob

Stubs and expectations are basically the same thing. A stub is just an expectation of zero or more invocations. The `Expectation#stubs` method is syntactic sugar to make the intent of the test more explicit.

When a method is invoked on a mock object, the mock object searches through its expectations from newest to oldest to find one that matches the invocation. After the invocation, the matching expectation might stop matching further invocations.
When a method is invoked on a mock object, the mock object searches through its expectations from newest to oldest to find one that matches the invocation. After the invocation, the matching expectation might stop matching further invocations. If the expectation that matches the invocation has a cardinality of "never", then an unexpected invocation error is reported.

See the [documentation](https://mocha.jamesmead.org/Mocha/Mock.html) for `Mocha::Mock` for further details.

Expand Down
8 changes: 4 additions & 4 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ module Mocha
# while an +expects(:foo).at_least_once+ expectation will always be matched
# against invocations.
#
# However, note that if the expectation that matches the invocation has a
# cardinality of "never", then an unexpected invocation error is reported.
#
# This scheme allows you to:
#
# - Set up default stubs in your the +setup+ method of your test class and
Expand Down Expand Up @@ -327,10 +330,7 @@ def handle_method_call(symbol, arguments, block)
matching_expectation_allowing_invocation = matching_expectations.detect(&:invocations_allowed?)
matching_expectation_never_allowing_invocation = matching_expectations.detect(&:invocations_never_allowed?)

if matching_expectation_allowing_invocation
if matching_expectation_never_allowing_invocation
invocation_not_allowed_warning(invocation, matching_expectation_never_allowing_invocation)
end
if matching_expectation_allowing_invocation && !matching_expectation_never_allowing_invocation
matching_expectation_allowing_invocation.invoke(invocation)
else
matching_expectation_ignoring_order = all_expectations.match(invocation, ignoring_order: true)
Expand Down
27 changes: 11 additions & 16 deletions test/acceptance/mocked_methods_dispatch_test.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
require File.expand_path('../acceptance_test_helper', __FILE__)
require 'deprecation_disabler'
require 'execution_point'

class MockedMethodDispatchTest < Mocha::TestCase
include AcceptanceTest
Expand Down Expand Up @@ -75,23 +73,20 @@ def test_should_find_latest_expectation_with_range_of_expected_invocation_count_
assert_passed(test_result)
end

def test_should_display_deprecation_warning_if_invocation_matches_expectation_with_never_cardinality
execution_point = nil
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; execution_point = ExecutionPoint.current
DeprecationDisabler.disable_deprecations do
mock.method
end
mock.expects(:method).never
mock.method
end
assert_passed(test_result)
message = Mocha::Deprecation.messages.last
location = execution_point.location
expected = [
"The expectation defined at #{location} does not allow invocations, but #<Mock:mock>.method() was invoked.",
'This invocation will cause the test to fail fast in a future version of Mocha.'
]
assert_equal expected.join(' '), message
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

0 comments on commit 15985bc

Please sign in to comment.