Skip to content

Commit

Permalink
Fix exact match on ActionController::Parameters
Browse files Browse the repository at this point in the history
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]: 5e6a07b
[2]: #662 (comment)
[3]: https://api.rubyonrails.org/classes/ActionController/Parameters.html
  • Loading branch information
floehopper committed Jul 24, 2024
1 parent 0e2b73c commit 228acc9
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/mocha/parameter_matchers/has_entries.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
23 changes: 23 additions & 0 deletions test/acceptance/parameter_matcher_test.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require File.expand_path('../acceptance_test_helper', __FILE__)

require 'hashlike'

class ParameterMatcherTest < Mocha::TestCase
include AcceptanceTest

Expand All @@ -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()
Expand All @@ -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()
Expand Down
12 changes: 12 additions & 0 deletions test/unit/parameter_matchers/has_entries_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 228acc9

Please sign in to comment.