Skip to content

Commit

Permalink
WIP: Kwargs always match last positional hash
Browse files Browse the repository at this point in the history
TODO:
* Try to simpify logic in `PositionalOrKeywordHash#matches?`
* Maybe use keyword arguments for `PositionalOrKeywordHash` constructor
* Check whether all relevant scenarios are covered by tests
* Check whether this has any effect on #594 (I don't think it does)

* Fixes #593
* Supersedes #605
  • Loading branch information
floehopper committed Jan 4, 2025
1 parent 63d3854 commit f4dfc58
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 23 deletions.
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,11 +9,11 @@ module ParameterMatchers
# @private
module InstanceMethods
# @private
def to_matcher(expectation: nil, top_level: false)
def to_matcher(expectation: nil, top_level: false, last: false)
if Base === self
self
elsif Hash === self && top_level
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self, expectation)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(self, expectation, last)
else
Mocha::ParameterMatchers::Equals.new(self)
end
Expand Down
5 changes: 4 additions & 1 deletion lib/mocha/parameter_matchers/positional_or_keyword_hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,10 @@ module ParameterMatchers
class PositionalOrKeywordHash
include Base

def initialize(expected_value, expectation)
def initialize(expected_value, expectation, last_expected_value)
@expected_value = expected_value
@expectation = expectation
@last_expected_value = last_expected_value
end

def matches?(actual_values)
Expand All @@ -23,6 +24,8 @@ def matches?(actual_values)
return false unless HasEntries.new(@expected_value, exact: true).matches?([actual_value])

if is_last_actual_value && !same_type_of_hash?(actual_value, @expected_value)
return true if @last_expected_value && !ruby2_keywords_hash?(@expected_value)

return false if Mocha.configuration.strict_keyword_argument_matching?

deprecation_warning(actual_value, @expected_value) if Mocha::RUBY_V27_PLUS
Expand Down
8 changes: 7 additions & 1 deletion lib/mocha/parameters_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,13 @@ def mocha_inspect
end

def matchers
@expected_parameters.map { |p| p.to_matcher(expectation: @expectation, top_level: true) }
@expected_parameters.map.with_index do |parameter, index|
parameter.to_matcher(
expectation: @expectation,
top_level: true,
last: index == @expected_parameters.length - 1
)
end
end
end
end
10 changes: 6 additions & 4 deletions test/acceptance/loose_keyword_argument_matching_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,7 @@ def test_should_match_positional_and_keyword_args_with_last_positional_hash
assert_passed(test_result)
return unless Mocha::RUBY_V27_PLUS

assert_deprecation_warning_location(test_result, execution_point)
assert_deprecation_warning(
test_result, 'expected positional hash ({key: 42}), but received keyword arguments (key: 42)'
)
assert_no_deprecation_warning(test_result)
end

def test_should_match_last_positional_hash_with_keyword_args
Expand All @@ -97,6 +94,11 @@ def assert_deprecation_warning(test_result, expected_warning)
assert_includes test_result.last_deprecation_warning, expected_warning
end

def assert_no_deprecation_warning(test_result)
warning = test_result.last_deprecation_warning
assert_nil warning, "Unexpected deprecation warning: #{warning}"
end

def assert_deprecation_warning_location(test_result, execution_point)
expected_location = "Expectation defined at #{execution_point.location}"
assert_deprecation_warning test_result, expected_location
Expand Down
3 changes: 2 additions & 1 deletion test/acceptance/strict_keyword_argument_matching_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ def test_should_not_match_hash_parameter_with_splatted_keyword_args
assert_failed(test_result)
end

# TODO: Maybe move to KeywordArgumentMatchingTest?
def test_should_not_match_positional_and_keyword_args_with_last_positional_hash
test_result = run_as_test do
mock = mock()
mock.expects(:method).with(1, { key: 42 })
mock.method(1, key: 42)
end
assert_failed(test_result)
assert_passed(test_result)
end

def test_should_not_match_last_positional_hash_with_keyword_args
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ def teardown
Mocha.configure { |c| c.strict_keyword_argument_matching = @original }
end

def test_should_match_hash_arg_with_keyword_args_but_display_deprecation_warning_if_appropriate
def test_should_match_hash_arg_with_keyword_args_but_display_deprecation_warning_if_not_last_matcher
expectation = Mocha::Expectation.new(self, :foo)
matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 }), expectation)
last_matcher = false
matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 }), expectation, last_matcher)
capture_deprecation_warnings do
assert matcher.matches?([{ key_1: 1, key_2: 2 }])
end
Expand All @@ -43,9 +44,27 @@ def test_should_match_hash_arg_with_keyword_args_but_display_deprecation_warning
assert_includes message, 'See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.'
end

def test_should_match_keyword_args_with_hash_arg_but_display_deprecation_warning_if_appropriate
def test_should_match_hash_arg_with_keyword_args_but_display_deprecation_warning_if_last_matcher
expectation = Mocha::Expectation.new(self, :foo)
matcher = build_matcher({ key_1: 1, key_2: 2 }, expectation)
last_matcher = true
matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 }), expectation, last_matcher)
capture_deprecation_warnings do
assert matcher.matches?([{ key_1: 1, key_2: 2 }])
end
return unless Mocha::RUBY_V27_PLUS

message = last_deprecation_warning
location = expectation.definition_location
assert_includes message, "Expectation defined at #{location} expected keyword arguments (key_1: 1, key_2: 2)"
assert_includes message, 'but received positional hash ({key_1: 1, key_2: 2})'
assert_includes message, 'These will stop matching when strict keyword argument matching is enabled.'
assert_includes message, 'See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.'
end

def test_should_match_keyword_args_with_hash_arg_but_display_deprecation_warning_if_not_last_matcher
expectation = Mocha::Expectation.new(self, :foo)
last_matcher = false
matcher = build_matcher({ key_1: 1, key_2: 2 }, expectation, last_matcher)
capture_deprecation_warnings do
assert matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })])
end
Expand All @@ -59,6 +78,18 @@ def test_should_match_keyword_args_with_hash_arg_but_display_deprecation_warning
assert_includes message, 'See the documentation for Mocha::Configuration#strict_keyword_argument_matching=.'
end

def test_should_match_keyword_args_with_hash_arg_but_not_display_deprecation_warning_if_last_matcher
expectation = Mocha::Expectation.new(self, :foo)
last_matcher = true
matcher = build_matcher({ key_1: 1, key_2: 2 }, expectation, last_matcher)
capture_deprecation_warnings do
assert matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })])
end
return unless Mocha::RUBY_V27_PLUS

assert_nil last_deprecation_warning
end

def test_should_display_deprecation_warning_even_if_parent_expectation_is_nil
expectation = nil
matcher = build_matcher({ key_1: 1, key_2: 2 }, expectation)
Expand All @@ -74,7 +105,7 @@ def test_should_display_deprecation_warning_even_if_parent_expectation_is_nil

private

def build_matcher(hash, expectation = nil)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(hash, expectation)
def build_matcher(hash, expectation = nil, last = nil)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(hash, expectation, last)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_should_not_match_keyword_args_with_matchers_using_keyword_args_when_not

private

def build_matcher(hash, expectation = nil)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(hash, expectation)
def build_matcher(hash, expectation = nil, last = nil)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(hash, expectation, last)
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -49,21 +49,36 @@ def test_should_match_keyword_args_with_keyword_args
assert matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })])
end

def test_should_not_match_hash_arg_with_keyword_args
matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 }))
def test_should_not_match_hash_arg_with_keyword_args_if_not_last_matcher
last_matcher = false
matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 }), nil, last_matcher)
assert !matcher.matches?([{ key_1: 1, key_2: 2 }])
end

def test_should_not_match_keyword_args_with_hash_arg
def test_should_not_match_hash_arg_with_keyword_args_if_last_matcher
last_matcher = true
matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 }), nil, last_matcher)
assert !matcher.matches?([{ key_1: 1, key_2: 2 }])
end

def test_should_not_match_keyword_args_with_hash_arg_if_not_last_matcher
hash = { key_1: 1, key_2: 2 }
matcher = build_matcher(hash)
last_matcher = false
matcher = build_matcher(hash, nil, last_matcher)
assert !matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })])
end

def test_should_match_keyword_args_with_hash_arg_if_last_matcher
hash = { key_1: 1, key_2: 2 }
last_matcher = true
matcher = build_matcher(hash, nil, last_matcher)
assert matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })])
end
end

private

def build_matcher(hash, expectation = nil)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(hash, expectation)
def build_matcher(hash, expectation = nil, last = nil)
Mocha::ParameterMatchers::PositionalOrKeywordHash.new(hash, expectation, last)
end
end

0 comments on commit f4dfc58

Please sign in to comment.