Skip to content

Commit

Permalink
Fix regression in Mock#responds_like behaviour
Browse files Browse the repository at this point in the history
The regression was inadvertently introduced in this commit [1] and
indirectly led to this issue [2].

I've attempted to thoroughly characterise the pre-regression behaviour
of Mock#responds_like in a new acceptance test, RespondsLikeTest, in
order to reduce the chances of any such regressions occurring in the
future. I've also added another acceptance test, ArrayFlattenTest, which
reproduced the issue in [2] until the regression was fixed.

Investigating this has made me realise that the behaviour of
Mock#responds_like in combination with protected & private methods on
the responder was not specified in the documentation. I've attempted to
reverse engineer the documentation so that it more fully describes the
pre-regression behaviour.

Note that the CI build for Ruby v1.9 is using an old version of Minitest
which doesn't work with the RespondsLikeTest and since support for Ruby
v1.9 was dropped in Mocha v2, I don't think it's worth the effort to
sort this out so I've just disabled the test for Ruby v1.9.

[1]: 365a734
[2]: #580
  • Loading branch information
floehopper committed Nov 7, 2022
1 parent 113b148 commit ba4d619
Show file tree
Hide file tree
Showing 3 changed files with 451 additions and 8 deletions.
16 changes: 8 additions & 8 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,11 @@ def unstub(*method_names)
end
end

# Constrains the {Mock} instance so that it can only expect or stub methods to which +responder+ responds. The constraint is only applied at method invocation time.
# Constrains the {Mock} instance so that it can only expect or stub methods to which +responder+ responds publicly. The constraint is only applied at method invocation time.
#
# A +NoMethodError+ will be raised if the +responder+ does not +#respond_to?+ a method invocation (even if the method has been expected or stubbed).
# A +NoMethodError+ will be raised if the +responder+ does not publicly +#respond_to?+ the invoked method (even if the method has been expected or stubbed).
#
# The {Mock} instance will delegate its +#respond_to?+ method to the +responder+.
# The {Mock} instance will delegate its +#respond_to?+ method to the +responder+. However, the +include_all+ parameter is not passed through, so only public methods on the +responder+ will be considered.
#
# Note that the methods on +responder+ are never actually invoked.
#
Expand Down Expand Up @@ -237,11 +237,11 @@ def responds_like(responder)
self
end

# Constrains the {Mock} instance so that it can only expect or stub methods to which an instance of the +responder_class+ responds. The constraint is only applied at method invocation time. Note that the responder instance is instantiated using +Class#allocate+.
# Constrains the {Mock} instance so that it can only expect or stub methods to which an instance of the +responder_class+ responds publicly. The constraint is only applied at method invocation time. Note that the responder instance is instantiated using +Class#allocate+.
#
# A +NoMethodError+ will be raised if the responder instance does not +#respond_to?+ a method invocation (even if the method has been expected or stubbed).
# A +NoMethodError+ will be raised if the responder instance does not publicly +#respond_to?+ the invoked method (even if the method has been expected or stubbed).
#
# The {Mock} instance will delegate its +#respond_to?+ method to the responder instance.
# The {Mock} instance will delegate its +#respond_to?+ method to the responder instance. However, the +include_all+ parameter is not passed through, so only public methods on the +responder+ will be considered.
#
# Note that the methods on the responder instance are never actually invoked.
#
Expand Down Expand Up @@ -329,9 +329,9 @@ def handle_method_call(symbol, arguments, block)
end

# @private
def respond_to_missing?(symbol, include_all)
def respond_to_missing?(symbol, _include_all)
if @responder
@responder.respond_to?(symbol, include_all)
@responder.respond_to?(symbol)
else
@everything_stubbed || all_expectations.matches_method?(symbol)
end
Expand Down
28 changes: 28 additions & 0 deletions test/acceptance/array_flatten_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
require File.expand_path('../acceptance_test_helper', __FILE__)

class ArrayFlattenTest < Mocha::TestCase
include AcceptanceTest

def setup
setup_acceptance_test
end

def teardown
teardown_acceptance_test
end

def test_flattens_array_containing_mock_which_responds_like_active_record_model
model = Class.new do
# Ref: https://github.com/rails/rails/blob/7db044f38594eb43e1d241cc82025155666cc6f1/activerecord/lib/active_record/core.rb#L734-L745
def to_ary
nil
end
private :to_ary
end.new
test_result = run_as_test do
m = mock.responds_like(model)
assert_equal [m], [m].flatten
end
assert_passed(test_result)
end
end

0 comments on commit ba4d619

Please sign in to comment.