Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail fast if invocation matches never expectation #679

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/mocha/cardinality.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def invocations_allowed?
@invocations.size < maximum
end

def invocations_never_allowed?
maximum.zero?
end

def satisfied?
@invocations.size >= required
end
Expand Down
5 changes: 5 additions & 0 deletions lib/mocha/expectation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,11 @@ def invocations_allowed?
@cardinality.invocations_allowed?
end

# @private
def invocations_never_allowed?
@cardinality.invocations_never_allowed?
end

# @private
def satisfied?
@cardinality.satisfied?
Expand Down
6 changes: 4 additions & 2 deletions lib/mocha/expectation_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def match_allowing_invocation(invocation)
matching_expectations(invocation).detect(&:invocations_allowed?)
end

def match_never_allowing_invocation(invocation)
matching_expectations(invocation).detect(&:invocations_never_allowed?)
end

def verified?(assertion_counter = nil)
@expectations.all? { |expectation| expectation.verified?(assertion_counter) }
end
Expand All @@ -49,8 +53,6 @@ def +(other)
self.class.new(to_a + other.to_a)
end

private

def matching_expectations(invocation, ignoring_order: false)
@expectations.select { |e| e.match?(invocation, ignoring_order: ignoring_order) }
end
Expand Down
14 changes: 11 additions & 3 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,18 @@ def handle_method_call(symbol, arguments, block)
check_expiry
check_responder_responds_to(symbol)
invocation = Invocation.new(self, symbol, arguments, block)
if (matching_expectation_allowing_invocation = all_expectations.match_allowing_invocation(invocation))

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 && !matching_expectation_never_allowing_invocation
matching_expectation_allowing_invocation.invoke(invocation)
elsif (matching_expectation = all_expectations.match(invocation, ignoring_order: true)) || (!matching_expectation && !@everything_stubbed)
raise_unexpected_invocation_error(invocation, matching_expectation)
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)
end
end
end

Expand Down
17 changes: 17 additions & 0 deletions test/acceptance/mocked_methods_dispatch_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 11 additions & 0 deletions test/acceptance/parameter_matcher_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -373,4 +373,15 @@ def test_should_not_match_parameters_when_values_do_not_add_up_to_ten
end
assert_failed(test_result)
end

def test_should_only_call_matcher_block_once
test_result = run_as_test do
number_of_invocations = 0
mock = mock()
mock.stubs(:method).with { number_of_invocations += 1; true }
mock.method
assert_equal 1, number_of_invocations
end
assert_passed(test_result)
end
end
7 changes: 7 additions & 0 deletions test/unit/cardinality_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ def test_should_allow_invocations_if_invocation_count_has_not_yet_reached_maximu
assert !cardinality.invocations_allowed?
end

def test_should_never_allow_invocations
cardinality = Cardinality.new.exactly(0)
assert cardinality.invocations_never_allowed?
cardinality << new_invocation
assert cardinality.invocations_never_allowed?
end

def test_should_be_satisfied_if_invocations_so_far_have_reached_required_threshold
cardinality = Cardinality.new(2, 3)
assert !cardinality.satisfied?
Expand Down
11 changes: 11 additions & 0 deletions test/unit/expectation_list_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,17 @@ def test_should_find_most_recent_matching_expectation_allowing_invocation
assert_same expectation1, expectation_list.match_allowing_invocation(Invocation.new(:irrelevant, :my_method))
end

def test_should_find_matching_expectation_never_allowing_invocation
expectation_list = ExpectationList.new
expectation1 = Expectation.new(nil, :my_method)
expectation2 = Expectation.new(nil, :my_method)
define_instance_method(expectation1, :invocations_never_allowed?) { true }
define_instance_method(expectation2, :invocations_never_allowed?) { false }
expectation_list.add(expectation1)
expectation_list.add(expectation2)
assert_same expectation1, expectation_list.match_never_allowing_invocation(Invocation.new(:irrelevant, :my_method))
end

def test_should_combine_two_expectation_lists_into_one
expectation_list1 = ExpectationList.new
expectation_list2 = ExpectationList.new
Expand Down
7 changes: 7 additions & 0 deletions test/unit/expectation_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ def test_should_allow_invocations_until_expected_invocation_count_is_a_range_fro
assert !expectation.invocations_allowed?
end

def test_should_never_allow_invocations
expectation = new_expectation.never
assert expectation.invocations_never_allowed?
invoke(expectation)
assert expectation.invocations_never_allowed?
end

def test_should_store_provided_backtrace
backtrace = Object.new
assert_equal backtrace, Expectation.new(nil, :expected_method, backtrace).backtrace
Expand Down