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

Allow keyword arguments to match an expectation expecting *only* positional arguments #732

Merged
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
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
50 changes: 35 additions & 15 deletions lib/mocha/parameter_matchers/positional_or_keyword_hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,33 +12,53 @@ module ParameterMatchers
class PositionalOrKeywordHash
include Base

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

def matches?(available_parameters)
parameter, is_last_parameter = extract_parameter(available_parameters)
def matches?(actual_values)
actual_value, is_last_actual_value = extract_actual_value(actual_values)

return false unless HasEntries.new(@value, exact: true).matches?([parameter])

if is_last_parameter && !same_type_of_hash?(parameter, @value)
return false if Mocha.configuration.strict_keyword_argument_matching?

deprecation_warning(parameter, @value) if Mocha::RUBY_V27_PLUS
if !matches_entries_exactly?(actual_value)
false
elsif is_last_actual_value
matches_last_actual_value?(actual_value)
else
true
end

true
end

def mocha_inspect
@value.mocha_inspect
@expected_value.mocha_inspect
end

private

def extract_parameter(available_parameters)
[available_parameters.shift, available_parameters.empty?]
def matches_entries_exactly?(actual_value)
HasEntries.new(@expected_value, exact: true).matches?([actual_value])
end

def matches_last_actual_value?(actual_value)
if same_type_of_hash?(actual_value, @expected_value)
true
elsif last_expected_value_is_positional_hash? # rubocop:disable Lint/DuplicateBranch
true
elsif Mocha.configuration.strict_keyword_argument_matching?
false
else
deprecation_warning(actual_value, @expected_value) if Mocha::RUBY_V27_PLUS
true
end
end

def last_expected_value_is_positional_hash?
@last_expected_value && !ruby2_keywords_hash?(@expected_value)
end

def extract_actual_value(actual_values)
[actual_values.shift, actual_values.empty?]
end

def same_type_of_hash?(actual, expected)
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
50 changes: 26 additions & 24 deletions test/acceptance/loose_keyword_argument_matching_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,13 @@ def test_should_match_hash_parameter_with_keyword_args
mock.method({ key: 42 })
end
end
if Mocha::RUBY_V27_PLUS
assert_deprecation_warning_location(test_result, execution_point)
assert_deprecation_warning(
test_result, 'expected keyword arguments (key: 42), but received positional hash ({key: 42})'
)
end
assert_passed(test_result)
return unless Mocha::RUBY_V27_PLUS

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

def test_should_match_hash_parameter_with_splatted_keyword_args
Expand All @@ -46,13 +46,13 @@ def test_should_match_hash_parameter_with_splatted_keyword_args
mock.method({ key: 42 })
end
end
if Mocha::RUBY_V27_PLUS
assert_deprecation_warning_location(test_result, execution_point)
assert_deprecation_warning(
test_result, 'expected keyword arguments (key: 42), but received positional hash ({key: 42})'
)
end
assert_passed(test_result)
return unless Mocha::RUBY_V27_PLUS

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

def test_should_match_positional_and_keyword_args_with_last_positional_hash
Expand All @@ -64,13 +64,10 @@ def test_should_match_positional_and_keyword_args_with_last_positional_hash
mock.method(1, key: 42)
end
end
if 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)'
)
end
assert_passed(test_result)
return unless Mocha::RUBY_V27_PLUS

assert_no_deprecation_warning(test_result)
end

def test_should_match_last_positional_hash_with_keyword_args
Expand All @@ -82,13 +79,13 @@ def test_should_match_last_positional_hash_with_keyword_args
mock.method(1, { key: 42 })
end
end
if Mocha::RUBY_V27_PLUS
assert_deprecation_warning_location(test_result, execution_point)
assert_deprecation_warning(
test_result, 'expected keyword arguments (key: 42), but received positional hash ({key: 42})'
)
end
assert_passed(test_result)
return unless Mocha::RUBY_V27_PLUS

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

private
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
10 changes: 5 additions & 5 deletions test/acceptance/strict_keyword_argument_matching_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def teardown
teardown_acceptance_test
end

def test_should_not_match_hash_parameter_with_keyword_args_when_strict_keyword_matching_is_enabled
def test_should_not_match_hash_parameter_with_keyword_args
test_result = run_as_test do
mock = mock()
mock.expects(:method).with(key: 42)
Expand All @@ -27,7 +27,7 @@ def test_should_not_match_hash_parameter_with_keyword_args_when_strict_keyword_m
assert_failed(test_result)
end

def test_should_not_match_hash_parameter_with_splatted_keyword_args_when_strict_keyword_matching_is_enabled
def test_should_not_match_hash_parameter_with_splatted_keyword_args
test_result = run_as_test do
mock = mock()
kwargs = { key: 42 }
Expand All @@ -37,16 +37,16 @@ def test_should_not_match_hash_parameter_with_splatted_keyword_args_when_strict_
assert_failed(test_result)
end

def test_should_not_match_positional_and_keyword_args_with_last_positional_hash_when_strict_keyword_args_is_enabled
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_when_strict_keyword_args_is_enabled
def test_should_not_match_last_positional_hash_with_keyword_args
test_result = run_as_test do
mock = mock()
mock.expects(:method).with(1, key: 42)
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 @@ -9,23 +9,19 @@
require 'mocha/ruby_version'
require 'mocha/configuration'

class StrictPositionalOrKeywordHashTest < Mocha::TestCase
include Mocha::ParameterMatchers
if Mocha::RUBY_V27_PLUS
class StrictPositionalOrKeywordHashTest < Mocha::TestCase
include Mocha::ParameterMatchers

def setup
return unless Mocha::RUBY_V27_PLUS

@original = Mocha.configuration.strict_keyword_argument_matching?
Mocha.configure { |c| c.strict_keyword_argument_matching = true }
end

def teardown
return unless Mocha::RUBY_V27_PLUS
def setup
@original = Mocha.configuration.strict_keyword_argument_matching?
Mocha.configure { |c| c.strict_keyword_argument_matching = true }
end

Mocha.configure { |c| c.strict_keyword_argument_matching = @original } if Mocha::RUBY_V27_PLUS
end
def teardown
Mocha.configure { |c| c.strict_keyword_argument_matching = @original }
end

if Mocha::RUBY_V27_PLUS
def test_should_match_non_last_hash_arg_with_hash_arg
hash = { key_1: 1, key_2: 2 }
matcher = build_matcher(hash)
Expand All @@ -49,21 +45,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
end

private
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

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
end