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

Fix strict kwargs matching when method doesn't accept kwargs #605

Draft
wants to merge 2 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
2 changes: 1 addition & 1 deletion lib/mocha/class_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def initialize(klass)

def mocha(instantiate: true)
if instantiate
@mocha ||= Mocha::Mockery.instance.mock_impersonating_any_instance_of(@stubba_object)
@mocha ||= Mocha::Mockery.instance.mock_impersonating_any_instance_of(@stubba_object).responds_like_instance_of(@stubba_object)
else
defined?(@mocha) ? @mocha : nil
end
Expand Down
2 changes: 1 addition & 1 deletion lib/mocha/expectation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ def matches_method?(method_name)

# @private
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)
order_independent_match = @method_matcher.match?(invocation.method_name) && @parameters_matcher.match?(invocation) && @block_matcher.match?(invocation.block)
ignoring_order ? order_independent_match : order_independent_match && in_correct_order?
end

Expand Down
9 changes: 8 additions & 1 deletion lib/mocha/invocation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ module Mocha
class Invocation
attr_reader :method_name, :block

def initialize(mock, method_name, arguments = [], block = nil)
def initialize(mock, method_name, arguments = [], block = nil, responder = nil)
@mock = mock
@method_name = method_name
@arguments = arguments
@block = block
@responder = responder
@yields = []
@result = nil
end
Expand Down Expand Up @@ -65,6 +66,12 @@ def full_description
"\n - #{call_description} #{result_description}"
end

def method_accepts_keyword_arguments?
return true unless @responder

@responder.method(@method_name).parameters.any? { |k, _v| %i[keyreq key keyrest].include?(k) }
end

private

def argument_description
Expand Down
14 changes: 9 additions & 5 deletions lib/mocha/mock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
require 'mocha/parameters_matcher'
require 'mocha/argument_iterator'
require 'mocha/expectation_error_factory'
require 'mocha/deprecation'
require 'mocha/ruby_version'

module Mocha
# Traditional mock object.
Expand Down Expand Up @@ -324,7 +324,7 @@ def method_missing(symbol, *arguments, &block)
def handle_method_call(symbol, arguments, block)
check_expiry
check_responder_responds_to(symbol)
invocation = Invocation.new(self, symbol, arguments, block)
invocation = Invocation.new(self, symbol, arguments, block, @responder)

matching_expectations = all_expectations.matching_expectations(invocation)

Expand Down Expand Up @@ -402,9 +402,13 @@ def raise_unexpected_invocation_error(invocation, matching_expectation)
end

def check_responder_responds_to(symbol)
if @responder && [email protected]_to?(symbol) # rubocop:disable Style/GuardClause
raise NoMethodError, "undefined method `#{symbol}' for #{mocha_inspect} which responds like #{@responder.mocha_inspect}"
end
return unless @responder

legacy_behaviour_for_array_flatten = !RUBY_V23_PLUS && [email protected]_to?(symbol) && (symbol == :to_ary)

return if @responder.respond_to?(symbol, true) && !legacy_behaviour_for_array_flatten

raise NoMethodError, "undefined method `#{symbol}' for #{mocha_inspect} which responds like #{@responder.mocha_inspect}"
end

def check_expiry
Expand Down
2 changes: 1 addition & 1 deletion lib/mocha/object_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ module ObjectMethods
# @private
def mocha(instantiate: true)
if instantiate
@mocha ||= Mocha::Mockery.instance.mock_impersonating(self)
@mocha ||= Mocha::Mockery.instance.mock_impersonating(self).responds_like(self)
else
defined?(@mocha) ? @mocha : nil
end
Expand Down
4 changes: 2 additions & 2 deletions lib/mocha/parameter_matchers/instance_methods.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ module ParameterMatchers
# @private
module InstanceMethods
# @private
def to_matcher(expectation: nil, top_level: false)
def to_matcher(expectation: nil, top_level: false, method_accepts_keyword_arguments: true)
if Base === self
self
elsif Hash === self && top_level
elsif Hash === self && (top_level || method_accepts_keyword_arguments)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self, expectation)
else
Mocha::ParameterMatchers::Equals.new(self)
Expand Down
15 changes: 7 additions & 8 deletions lib/mocha/parameters_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,15 @@ def initialize(expected_parameters = [ParameterMatchers::AnyParameters.new], exp
@matching_block = matching_block
end

def match?(actual_parameters = [])
def match?(invocation)
actual_parameters = invocation.arguments || []
if @matching_block
@matching_block.call(*actual_parameters)
else
parameters_match?(actual_parameters)
matchers(invocation).all? { |matcher| matcher.matches?(actual_parameters) } && actual_parameters.empty?
end
end

def parameters_match?(actual_parameters)
matchers.all? { |matcher| matcher.matches?(actual_parameters) } && actual_parameters.empty?
end

def mocha_inspect
if @matching_block
'(arguments_accepted_by_custom_matching_block)'
Expand All @@ -33,8 +30,10 @@ def mocha_inspect
end
end

def matchers
@expected_parameters.map { |p| p.to_matcher(expectation: @expectation, top_level: true) }
def matchers(invocation = nil)
@expected_parameters.map do |p|
p.to_matcher(expectation: @expectation, top_level: true, method_accepts_keyword_arguments: invocation ? invocation.method_accepts_keyword_arguments? : true)
end
end
end
end
1 change: 1 addition & 0 deletions lib/mocha/ruby_version.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

module Mocha
RUBY_V23_PLUS = Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('2.3')
RUBY_V27_PLUS = Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('2.7')
RUBY_V30_PLUS = Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('3.0')
RUBY_V34_PLUS = Gem::Version.new(RUBY_VERSION.dup) >= Gem::Version.new('3.4')
Expand Down
16 changes: 8 additions & 8 deletions test/acceptance/responds_like_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ def foo; end
assert_passed(test_result)
end

def test_mock_which_responds_like_object_with_protected_method_raises_no_method_error_when_method_is_not_stubbed
def test_mock_which_responds_like_object_with_protected_method_raises_unexpected_invocation_exception_when_method_is_not_stubbed
object = Class.new do
def foo; end
protected :foo
end.new
test_result = run_as_test do
m = mock.responds_like(object)
assert_raises(NoMethodError) { m.foo } # vs Minitest::Assertion for public method
assert_raises(Minitest::Assertion) { m.foo }
end
assert_passed(test_result)
end
Expand Down Expand Up @@ -170,15 +170,15 @@ def foo; end
assert_passed(test_result)
end

def test_mock_which_responds_like_object_with_protected_method_raises_no_method_error_when_method_is_stubbed
def test_mock_which_responds_like_object_with_protected_method_does_not_raise_exception_when_method_is_stubbed
object = Class.new do
def foo; end
protected :foo
end.new
test_result = run_as_test do
m = mock.responds_like(object)
m.stubs(:foo)
assert_raises(NoMethodError) { m.foo } # vs no exception for public method
assert_nil m.foo
end
assert_passed(test_result)
end
Expand All @@ -198,14 +198,14 @@ def foo; end
assert_passed(test_result)
end

def test_mock_which_responds_like_object_with_private_method_raises_no_method_error_when_method_is_not_stubbed
def test_mock_which_responds_like_object_with_private_method_raises_unexpected_invocation_exception_when_method_is_not_stubbed
object = Class.new do
def foo; end
private :foo
end.new
test_result = run_as_test do
m = mock.responds_like(object)
assert_raises(NoMethodError) { m.foo } # vs Minitest::Assertion for public method
assert_raises(Minitest::Assertion) { m.foo }
end
assert_passed(test_result)
end
Expand Down Expand Up @@ -236,15 +236,15 @@ def foo; end
assert_passed(test_result)
end

def test_mock_which_responds_like_object_with_private_method_raises_no_method_error_when_method_is_stubbed
def test_mock_which_responds_like_object_with_private_method_does_not_raise_exception_when_method_is_stubbed
object = Class.new do
def foo; end
private :foo
end.new
test_result = run_as_test do
m = mock.responds_like(object)
m.stubs(:foo)
assert_raises(NoMethodError) { m.foo } # vs no exception for public method
assert_nil m.foo
end
assert_passed(test_result)
end
Expand Down
37 changes: 22 additions & 15 deletions test/unit/parameters_matcher_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,91 +2,92 @@

require File.expand_path('../../test_helper', __FILE__)
require 'mocha/parameters_matcher'
require 'mocha/invocation'

class ParametersMatcherTest < Mocha::TestCase
include Mocha

def test_should_match_any_actual_parameters_if_no_expected_parameters_specified
parameters_matcher = ParametersMatcher.new
assert parameters_matcher.match?([1, 2, 3])
assert parameters_matcher.match?(invocation_with_arguments([1, 2, 3]))
end

def test_should_match_if_actual_parameters_are_same_as_expected_parameters
parameters_matcher = ParametersMatcher.new([4, 5, 6])
assert parameters_matcher.match?([4, 5, 6])
assert parameters_matcher.match?(invocation_with_arguments([4, 5, 6]))
end

def test_should_not_match_if_actual_parameters_are_different_from_expected_parameters
parameters_matcher = ParametersMatcher.new([4, 5, 6])
assert !parameters_matcher.match?([1, 2, 3])
assert !parameters_matcher.match?(invocation_with_arguments([1, 2, 3]))
end

def test_should_not_match_if_there_are_less_actual_parameters_than_expected_parameters
parameters_matcher = ParametersMatcher.new([4, 5, 6])
assert !parameters_matcher.match?([4, 5])
assert !parameters_matcher.match?(invocation_with_arguments([4, 5]))
end

def test_should_not_match_if_there_are_more_actual_parameters_than_expected_parameters
parameters_matcher = ParametersMatcher.new([4, 5])
assert !parameters_matcher.match?([4, 5, 6])
assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 6]))
end

def test_should_not_match_if_not_all_required_parameters_are_supplied
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert !parameters_matcher.match?([4])
assert !parameters_matcher.match?(invocation_with_arguments([4]))
end

def test_should_match_if_all_required_parameters_match_and_no_optional_parameters_are_supplied
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert parameters_matcher.match?([4, 5])
assert parameters_matcher.match?(invocation_with_arguments([4, 5]))
end

def test_should_match_if_all_required_and_optional_parameters_match_and_some_optional_parameters_are_supplied
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert parameters_matcher.match?([4, 5, 6])
assert parameters_matcher.match?(invocation_with_arguments([4, 5, 6]))
end

def test_should_match_if_all_required_and_optional_parameters_match_and_all_optional_parameters_are_supplied
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert parameters_matcher.match?([4, 5, 6, 7])
assert parameters_matcher.match?(invocation_with_arguments([4, 5, 6, 7]))
end

def test_should_not_match_if_all_required_and_optional_parameters_match_but_too_many_optional_parameters_are_supplied
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert !parameters_matcher.match?([4, 5, 6, 7, 8])
assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 6, 7, 8]))
end

def test_should_not_match_if_all_required_parameters_match_but_some_optional_parameters_do_not_match
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert !parameters_matcher.match?([4, 5, 6, 0])
assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 6, 0]))
end

def test_should_not_match_if_some_required_parameters_do_not_match_although_all_optional_parameters_do_match
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert !parameters_matcher.match?([4, 0, 6])
assert !parameters_matcher.match?(invocation_with_arguments([4, 0, 6]))
end

def test_should_not_match_if_all_required_parameters_match_but_no_optional_parameters_match
optionals = ParameterMatchers::Optionally.new(6, 7)
parameters_matcher = ParametersMatcher.new([4, 5, optionals])
assert !parameters_matcher.match?([4, 5, 0, 0])
assert !parameters_matcher.match?(invocation_with_arguments([4, 5, 0, 0]))
end

def test_should_match_if_actual_parameters_satisfy_matching_block
parameters_matcher = ParametersMatcher.new { |x, y| x + y == 3 }
assert parameters_matcher.match?([1, 2])
assert parameters_matcher.match?(invocation_with_arguments([1, 2]))
end

def test_should_not_match_if_actual_parameters_do_not_satisfy_matching_block
parameters_matcher = ParametersMatcher.new { |x, y| x + y == 3 }
assert !parameters_matcher.match?([2, 3])
assert !parameters_matcher.match?(invocation_with_arguments([2, 3]))
end

def test_should_remove_outer_array_braces
Expand All @@ -110,4 +111,10 @@ def test_should_indicate_that_matcher_logic_is_defined_by_custom_block
parameters_matcher = ParametersMatcher.new { true }
assert_equal '(arguments_accepted_by_custom_matching_block)', parameters_matcher.mocha_inspect
end

private

def invocation_with_arguments(arguments)
Invocation.new(nil, nil, arguments, nil, nil)
end
end