From 5e6a07b2710dac76e9346def561ca0d44765bf86 Mon Sep 17 00:00:00 2001 From: James Mead Date: Mon, 22 Jul 2024 12:30:08 +0100 Subject: [PATCH] Require exact match for top-level Hash parameter In this commit [1] which fixed #647 and which was released in v2.3.0, a top-level `Hash` argument passed into `Expectation#with` was implicitly converted into a `HasEntries` matcher in order to handle "nested" matchers used within the keys & values of the `Hash`. This _new_ behaviour makes particular sense for keyword arguments where the requirement to wrap such a `Hash` in a call to `#has_entries` was a bit surprising as described in #647. However, that change inadvertently broke basic `Hash` parameter matching as described in #657, because the `HasEntries` matcher only needs to match each of the entries specified; it will still match if there are *extra* entries in the `Hash` it is matching against. In order to fix this without breaking the "nested" matching behaviour added in [1], I've added an optional `exact` keyword argument to the `HasEntries` constructor and set this to `true` when it's called from `PositionalOrKeywordHash#matches?`. I haven't bothered to document the new `exact` argument, because currently it's only used internally. I did consider introducing a new matcher more like `RSpec::Matchers#match` matcher [2], but I decided that wouldn't provide as elegant a solution to #647 as the change in this commit. I can imagine introducing something like `#has_exact_entries` to the public API, but it would probably make sense to add something similar for exact matching of array items at the same time, so I'm going to leave that for now. I've added a few acceptance tests in this commit which would have caught the problem that I inadvertently introduced in [1] and a few that cover the "nested" matching behaviour that was introduced in that commit (previously I had only added a unit test). [1]: https://github.com/freerange/mocha/commit/f94e250414335d822ec8d0a1331cb25cf90bf837 [2]: https://www.rubydoc.info/github/rspec/rspec-expectations/RSpec/Matchers#match-instance_method --- lib/mocha/parameter_matchers/has_entries.rb | 7 ++- .../positional_or_keyword_hash.rb | 3 +- test/acceptance/parameter_matcher_test.rb | 45 +++++++++++++++++++ .../positional_or_keyword_hash_test.rb | 16 +++++++ 4 files changed, 68 insertions(+), 3 deletions(-) diff --git a/lib/mocha/parameter_matchers/has_entries.rb b/lib/mocha/parameter_matchers/has_entries.rb index 1b6d18d81..2b90f34b8 100644 --- a/lib/mocha/parameter_matchers/has_entries.rb +++ b/lib/mocha/parameter_matchers/has_entries.rb @@ -30,20 +30,23 @@ def has_entries(entries) # rubocop:disable Naming/PredicateName # Parameter matcher which matches when actual parameter contains all expected +Hash+ entries. class HasEntries < Base # @private - def initialize(entries) + def initialize(entries, exact: false) @entries = entries + @exact = exact end # @private def matches?(available_parameters) parameter = available_parameters.shift + return false if @exact && @entries.length != parameter.length + has_entry_matchers = @entries.map { |key, value| HasEntry.new(key, value) } AllOf.new(*has_entry_matchers).matches?([parameter]) end # @private def mocha_inspect - "has_entries(#{@entries.mocha_inspect})" + @exact ? @entries.mocha_inspect : "has_entries(#{@entries.mocha_inspect})" end end end diff --git a/lib/mocha/parameter_matchers/positional_or_keyword_hash.rb b/lib/mocha/parameter_matchers/positional_or_keyword_hash.rb index d9b77c428..341d314b8 100644 --- a/lib/mocha/parameter_matchers/positional_or_keyword_hash.rb +++ b/lib/mocha/parameter_matchers/positional_or_keyword_hash.rb @@ -1,6 +1,7 @@ require 'mocha/configuration' require 'mocha/deprecation' require 'mocha/parameter_matchers/base' +require 'mocha/parameter_matchers/has_entries' module Mocha module ParameterMatchers @@ -14,7 +15,7 @@ def initialize(value, expectation) def matches?(available_parameters) parameter, is_last_parameter = extract_parameter(available_parameters) - return false unless HasEntries.new(@value).matches?([parameter]) + 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? diff --git a/test/acceptance/parameter_matcher_test.rb b/test/acceptance/parameter_matcher_test.rb index d982539fa..49cb58226 100644 --- a/test/acceptance/parameter_matcher_test.rb +++ b/test/acceptance/parameter_matcher_test.rb @@ -11,6 +11,24 @@ def teardown teardown_acceptance_test end + def test_should_match_hash_parameter_which_is_exactly_the_same + test_result = run_as_test do + mock = mock() + mock.expects(:method).with(key_1: 'value_1') + mock.method(key_1: 'value_1') + end + assert_passed(test_result) + end + + def test_should_not_match_hash_parameter_which_is_not_exactly_the_same + test_result = run_as_test do + mock = mock() + mock.expects(:method).with(key_1: 'value_1') + mock.method(key_1: 'value_1', key_2: 'value_2') + end + assert_failed(test_result) + end + def test_should_match_hash_parameter_with_specified_key test_result = run_as_test do mock = mock() @@ -137,6 +155,33 @@ def test_should_not_match_hash_parameter_with_specified_entries_using_nested_mat assert_failed(test_result) end + def test_should_match_hash_parameter_that_is_exactly_a_key_that_is_a_string_with_a_value_that_is_an_integer + test_result = run_as_test do + mock = mock() + mock.expects(:method).with(is_a(String) => is_a(Integer)) + mock.method('key_1' => 123) + end + assert_passed(test_result) + end + + def test_should_not_match_hash_parameter_that_is_exactly_a_key_that_is_a_string_with_a_value_that_is_an_integer_because_value_not_integer + test_result = run_as_test do + mock = mock() + mock.expects(:method).with(is_a(String) => is_a(Integer)) + mock.method('key_1' => '123') + end + assert_failed(test_result) + end + + def test_should_not_match_hash_parameter_that_is_exactly_a_key_that_is_a_string_with_a_value_that_is_an_integer_because_of_extra_entry + test_result = run_as_test do + mock = mock() + mock.expects(:method).with(is_a(String) => is_a(Integer)) + mock.method('key_1' => 123, 'key_2' => 'doesntmatter') + end + assert_failed(test_result) + end + def test_should_match_parameter_that_matches_any_value test_result = run_as_test do mock = mock() diff --git a/test/unit/parameter_matchers/positional_or_keyword_hash_test.rb b/test/unit/parameter_matchers/positional_or_keyword_hash_test.rb index 2db66c758..591a01b9e 100644 --- a/test/unit/parameter_matchers/positional_or_keyword_hash_test.rb +++ b/test/unit/parameter_matchers/positional_or_keyword_hash_test.rb @@ -34,16 +34,32 @@ def test_should_match_hash_arg_with_hash_arg assert matcher.matches?([{ key_1: 1, key_2: 2 }]) end + def test_should_not_match_hash_arg_with_different_hash_arg + hash = { key_1: 1 } + matcher = build_matcher(hash) + assert !matcher.matches?([{ key_1: 1, key_2: 2 }]) + end + def test_should_match_keyword_args_with_keyword_args matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })) # rubocop:disable Style/BracesAroundHashParameters assert matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })]) # rubocop:disable Style/BracesAroundHashParameters end + def test_should_not_match_keyword_args_with_different_keyword_args + matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1 })) # rubocop:disable Style/BracesAroundHashParameters + assert !matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 })]) # rubocop:disable Style/BracesAroundHashParameters + end + def test_should_match_keyword_args_with_matchers_using_keyword_args matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: is_a(String), key_2: is_a(Integer) })) # rubocop:disable Style/BracesAroundHashParameters assert matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 'foo', key_2: 2 })]) # rubocop:disable Style/BracesAroundHashParameters end + def test_should_not_match_keyword_args_with_matchers_using_keyword_args_when_not_all_entries_are_matched + matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: is_a(String) })) # rubocop:disable Style/BracesAroundHashParameters + assert !matcher.matches?([Hash.ruby2_keywords_hash({ key_1: 'foo', key_2: 2 })]) # rubocop:disable Style/BracesAroundHashParameters + end + def test_should_match_hash_arg_with_keyword_args_but_display_deprecation_warning_if_appropriate expectation = Mocha::Expectation.new(self, :foo); execution_point = ExecutionPoint.current matcher = build_matcher(Hash.ruby2_keywords_hash({ key_1: 1, key_2: 2 }), expectation) # rubocop:disable Style/BracesAroundHashParameters