Skip to content

Commit

Permalink
Include the original test name in expired stub error messages
Browse files Browse the repository at this point in the history
It is currently very hard to track down such leaked stubs, when
they happen, as they might be created in one test and cause an
exception dozens of tests later.

By including the name of the test that invalidated them, it makes
the debugging process a bit easier.

Note, I made `setup` also accept the current test instance purely
for consistency with `teardown` but it's not strictly necessary.
  • Loading branch information
byroot authored and floehopper committed Apr 10, 2024
1 parent 85848fb commit 9d2b88e
Show file tree
Hide file tree
Showing 9 changed files with 77 additions and 13 deletions.
10 changes: 8 additions & 2 deletions lib/mocha/hooks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,14 @@ def mocha_verify(assertion_counter = nil)
# Resets Mocha after a test (only for use by authors of test libraries).
#
# This method should be called after each individual test has finished (including after any "teardown" code).
def mocha_teardown
Mockery.teardown
def mocha_teardown(origin = mocha_test_name)
Mockery.teardown(origin)
end

# Returns a string representing the unit test name, to be included in some Mocha
# to help track down potential bugs.
def mocha_test_name
nil
end
end
end
15 changes: 15 additions & 0 deletions lib/mocha/integration/minitest/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,21 @@ def after_teardown
super
mocha_teardown
end

# @private
def mocha_test_name
if respond_to?(:name)
test_name = name
elsif respond_to?(:__name__) # Older minitest
test_name = __name__
end

if test_name
"#{self.class.name}##{test_name}"
else
self.class.name
end
end
end
end
end
Expand Down
5 changes: 5 additions & 0 deletions lib/mocha/integration/test_unit/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ def self.included(mod)

private

# @private
def mocha_test_name
name
end

# @private
def handle_mocha_expectation_error(exception)
return false unless exception.is_a?(Mocha::ExpectationError)
Expand Down
8 changes: 5 additions & 3 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,8 @@ def __verified__?(assertion_counter = nil)
end

# @private
def __expire__
@expired = true
def __expire__(origin)
@expired = origin || true
end

# @private
Expand Down Expand Up @@ -393,8 +393,10 @@ def check_responder_responds_to(symbol)
def check_expiry
return unless @expired

origin = @expired == true ? 'one test' : @expired

sentences = [
"#{mocha_inspect} was instantiated in one test but it is receiving invocations within another test.",
"#{mocha_inspect} was instantiated in #{origin} but it is receiving invocations within another test.",
'This can lead to unintended interactions between tests and hence unexpected test failures.',
'Ensure that every test correctly cleans up any state that it introduces.'
]
Expand Down
8 changes: 4 additions & 4 deletions lib/mocha/mockery.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def verify(*args)
instance.verify(*args)
end

def teardown
instance.teardown
def teardown(origin = nil)
instance.teardown(origin)
ensure
@instances.pop
end
Expand Down Expand Up @@ -91,9 +91,9 @@ def verify(assertion_counter = nil)
end
end

def teardown
def teardown(origin = nil)
stubba.unstub_all
mocks.each(&:__expire__)
mocks.each { |m| m.__expire__(origin) }
reset
end

Expand Down
2 changes: 1 addition & 1 deletion test/acceptance/mock_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def test_should_raise_stubbing_error_if_mock_receives_invocations_in_another_tes
reuse_mock_test_result = run_as_test do
Foo.logger.expects(:log).with('Foo was here')
e = assert_raises(Mocha::StubbingError) { Foo.new.use_the_mock }
assert e.message.include?('#<Mock:Logger> was instantiated in one test but it is receiving invocations within another test.')
assert e.message.include?('#<Mock:Logger> was instantiated in FakeTest#test_me but it is receiving invocations within another test.')
assert e.message.include?('This can lead to unintended interactions between tests and hence unexpected test failures.')
assert e.message.include?('Ensure that every test correctly cleans up any state that it introduces.')
end
Expand Down
32 changes: 32 additions & 0 deletions test/integration/shared_tests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -172,5 +172,37 @@ def test_real_object_expectation_does_not_leak_into_subsequent_test
assert_equal execution_point, ExecutionPoint.new(exception.backtrace)
assert_match(/undefined method `foo'/, exception.message)
end

def test_leaky_mock
origin = nil
leaky_mock = nil
execution_point = nil
test_result = run_as_tests(
test_1: lambda {
origin = mocha_test_name
leaky_mock ||= begin
bad_mock = mock(:leaky)
bad_mock.expects(:call)
bad_mock
end

leaky_mock.call
},
test_2: lambda {
leaky_mock ||= begin
bad_mock = mock(:leaky)
bad_mock.expects(:call)
bad_mock
end

execution_point = ExecutionPoint.current; leaky_mock.call
}
)
assert_errored(test_result)
exception = test_result.errors.first.exception
assert_equal execution_point, ExecutionPoint.new(exception.backtrace)
expected = /#<Mock:leaky> was instantiated in #{Regexp.escape(origin)} but it is receiving invocations within another test/
assert_match(expected, exception.message)
end
end
# rubocop:enable Metrics/ModuleLength
8 changes: 6 additions & 2 deletions test/test_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ def run_as_test(&block)
run_as_tests(test_me: block)
end

# rubocop:disable Metrics/AbcSize
# rubocop:disable Metrics/AbcSize, Metrics/MethodLength
def run_as_tests(methods = {})
base_class = Mocha::TestCase
test_class = Class.new(base_class) do
def self.name;
'FakeTest'
end

include Assertions

methods.each do |(method_name, proc)|
Expand Down Expand Up @@ -45,7 +49,7 @@ def run_as_tests(methods = {})

test_result
end
# rubocop:enable Metrics/AbcSize
# rubocop:enable Metrics/AbcSize, Metrics/MethodLength

def assert_passed(test_result)
flunk "Test errored unexpectedly with message: #{test_result.errors.map(&:exception)}" if test_result.error_count > 0
Expand Down
2 changes: 1 addition & 1 deletion test/unit/hooks_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class << self
class FakeMockery
def verify(*args); end

def teardown
def teardown(_origin = nil)
raise 'exception within Mockery#teardown'
end
end
Expand Down

0 comments on commit 9d2b88e

Please sign in to comment.