From 228acc9e67932d12a0b57f92d0eeeee9d1407f57 Mon Sep 17 00:00:00 2001 From: James Mead Date: Wed, 24 Jul 2024 10:17:04 +0100 Subject: [PATCH] Fix exact match on ActionController::Parameters This was broken in this previous commit [1] which was part of #660 and which was released in v2.4.3. Previously the only requirement we put on `Hash`-like arguments was that they respond to `#keys` & `#[]` methods. However, the change in [1] we introduced an implicit assumption that they also respond to a `#length` method which is caused the problem described here [2]. However, `ActionController::Parameters` [3] is commonly used in Rails apps and it does *not* respond to `#length`. So it seems better to add a guard condition to ensure the object responds to `#keys` and use that to determine the length. Note that the `HasEntry`, `HasKey` and `HasKeys` matchers already work this way and so this feels like it makes `HasEntries` more consistent. [1]: https://github.com/freerange/mocha/commit/5e6a07b2710dac76e9346def561ca0d44765bf86 [2]: https://github.com/freerange/mocha/issues/662#issuecomment-2246105530 [3]: https://api.rubyonrails.org/classes/ActionController/Parameters.html --- lib/mocha/parameter_matchers/has_entries.rb | 3 ++- test/acceptance/parameter_matcher_test.rb | 23 +++++++++++++++++++ .../parameter_matchers/has_entries_test.rb | 12 ++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/lib/mocha/parameter_matchers/has_entries.rb b/lib/mocha/parameter_matchers/has_entries.rb index 51bb160b3..917ea7122 100644 --- a/lib/mocha/parameter_matchers/has_entries.rb +++ b/lib/mocha/parameter_matchers/has_entries.rb @@ -39,7 +39,8 @@ def initialize(entries, exact: false) def matches?(available_parameters) parameter = available_parameters.shift return false unless parameter - return false if @exact && @entries.length != parameter.length + return false unless parameter.respond_to?(:keys) + return false if @exact && @entries.length != parameter.keys.length has_entry_matchers = @entries.map { |key, value| HasEntry.new(key, value) } AllOf.new(*has_entry_matchers).matches?([parameter]) diff --git a/test/acceptance/parameter_matcher_test.rb b/test/acceptance/parameter_matcher_test.rb index 89b936d61..88530151e 100644 --- a/test/acceptance/parameter_matcher_test.rb +++ b/test/acceptance/parameter_matcher_test.rb @@ -1,5 +1,7 @@ require File.expand_path('../acceptance_test_helper', __FILE__) +require 'hashlike' + class ParameterMatcherTest < Mocha::TestCase include AcceptanceTest @@ -20,6 +22,16 @@ def test_should_match_hash_parameter_which_is_exactly_the_same assert_passed(test_result) end + def test_should_match_hash_parameter_when_method_invoked_with_hashlike_object_with_no_length_method + test_result = run_as_test do + mock = mock() + hash = { key_1: 'value_1' } + mock.expects(:method).with(hash) + mock.method(Hashlike.new(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() @@ -38,6 +50,17 @@ def test_should_not_match_hash_parameter_when_method_invoked_with_no_parameters assert_failed(test_result) end + def test_should_not_match_hash_parameter_when_method_invoked_with_empty_hashlike_object_with_no_length_method + test_result = run_as_test do + mock = mock() + hash = { key_1: 'value_1' } + mock.expects(:method).with(hash) + hashlike = Hashlike.new({}) + mock.method(hashlike) + end + assert_failed(test_result) + end + def test_should_match_hash_parameter_with_specified_key test_result = run_as_test do mock = mock() diff --git a/test/unit/parameter_matchers/has_entries_test.rb b/test/unit/parameter_matchers/has_entries_test.rb index b65eb0c5f..e462517e0 100644 --- a/test/unit/parameter_matchers/has_entries_test.rb +++ b/test/unit/parameter_matchers/has_entries_test.rb @@ -50,6 +50,18 @@ def test_should_not_match_hashlike_object assert !matcher.matches?([hashlike]) end + def test_should_match_hashlike_object_with_no_length_method_when_exact_match_required + matcher = HasEntries.new({ key_1: 'value_1' }, exact: true) + hashlike = Hashlike.new(key_1: 'value_1') + assert matcher.matches?([hashlike]) + end + + def test_should_not_match_hashlike_object_with_no_length_method_when_exact_match_required + matcher = HasEntries.new({ key_1: 'value_1' }, exact: true) + hashlike = Hashlike.new(key_1: 'value_1', key_2: 'value_2') + assert !matcher.matches?([hashlike]) + end + def test_should_describe_matcher matcher = has_entries(key_1: 'value_1', key_2: 'value_2') description = matcher.mocha_inspect