Skip to content

Commit

Permalink
Fix never expectation deprecation warning logic
Browse files Browse the repository at this point in the history
In #681 I added logic to display a deprecation warning about a change I
planned to make in #679. However, it turns out the logic in both was
faulty as demonstrated in this example [1] where the deprecation
warning was incorrectly displayed for the 2nd call to `Foo.create_if`:

    class Foo
      def self.create_if(condition)
        new if condition
      end
    end

    test "it creates only if condition is true" do
      Foo.expects(:new).never
      Foo.create_if(false)

      Foo.expects(:new).once
      Foo.create_if(true)
    end

This commit adds a new acceptance test to cover an example where the
logic was incorrect and updates the logic in `Mock#handle_method_call`
to fix the logic so this new test passes and none of the existing tests
fail. Unfortunately the implementation is now rather complicated and
hard to follow, but I think it will do for now. I have a couple of ideas
for ways to simplify it, but I don't have time to work on that now.

I plan to update the equivalent logic in #679 to match the new logic
implemented here.

[1]: #679 (comment)
  • Loading branch information
floehopper committed Nov 27, 2024
1 parent 862237d commit 01874f1
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 12 deletions.
30 changes: 18 additions & 12 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -318,25 +318,31 @@ def method_missing(symbol, *arguments, &block) # rubocop:disable Style/MethodMis
ruby2_keywords(:method_missing)

# @private
def handle_method_call(symbol, arguments, block)
def handle_method_call(symbol, arguments, block) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
check_expiry
check_responder_responds_to(symbol)
invocation = Invocation.new(self, symbol, arguments, block)

matching_expectations = all_expectations.matching_expectations(invocation)
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
matching_expectation_allowing_invocation.invoke(invocation)
else
matching_expectation_ignoring_order = all_expectations.match(invocation, ignoring_order: true)
if matching_expectation_ignoring_order || (!matching_expectation_ignoring_order && !@everything_stubbed)
raise_unexpected_invocation_error(invocation, matching_expectation_ignoring_order)
index = 0
never_allowed_expectation = nil
while index < matching_expectations.length
matching_expectation = matching_expectations[index]
if matching_expectation.invocations_never_allowed?
never_allowed_expectation = matching_expectation
elsif matching_expectation.invocations_allowed?
if never_allowed_expectation
invocation_not_allowed_warning(invocation, never_allowed_expectation)
end
return matching_expectation.invoke(invocation)
end
index += 1
end

matching_expectation_ignoring_order = all_expectations.match(invocation, ignoring_order: true)
if matching_expectation_ignoring_order || (!matching_expectation_ignoring_order && !@everything_stubbed) # rubocop:disable Style/GuardClause
raise_unexpected_invocation_error(invocation, matching_expectation_ignoring_order)
end
end

Expand Down
14 changes: 14 additions & 0 deletions test/acceptance/mocked_methods_dispatch_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,18 @@ def test_should_display_deprecation_warning_if_invocation_matches_expectation_wi
]
assert_equal expected.join(' '), message
end

def test_should_not_display_deprecation_warning_if_invocation_matches_expectation_allowing_invocation_before_matching_expectation_with_never_cardinality
test_result = run_as_test do
mock = mock('mock')
mock.expects(:method).never
mock.expects(:method).once
Mocha::Deprecation.messages = []
DeprecationDisabler.disable_deprecations do
mock.method
end
end
assert_passed(test_result)
assert Mocha::Deprecation.messages.empty?
end
end

0 comments on commit 01874f1

Please sign in to comment.