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

Improve failure message for invocations received in the wrong order #633

Merged
merged 1 commit into from
Nov 18, 2023
Merged
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
Improve sequence failure message
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!😅)
floehopper committed Nov 18, 2023
commit ac7232424685509cbd529f510d7a2169998c1bfc
10 changes: 8 additions & 2 deletions lib/mocha/expectation.rb
Original file line number Diff line number Diff line change
@@ -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
10 changes: 7 additions & 3 deletions lib/mocha/expectation_list.rb
Original file line number Diff line number Diff line change
@@ -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
8 changes: 6 additions & 2 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions test/acceptance/sequence_block_test.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions test/acceptance/sequence_test.rb
Original file line number Diff line number Diff line change
@@ -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