Skip to content

Commit

Permalink
Improve rendering of keyword arguments
Browse files Browse the repository at this point in the history
Using the `=>` for them was a bit confusing.

The string representation of the keyword hash `{a: true, b: 'c'}` is
now 'a: true, b: "c"' instead of ':a => true, :b => "c"'.

One reason (perhaps the only reason?) we didn't make this change when we
introduced keyword argument matching was the theoretical edge case where
a keyword-style hash could have a non-symbol key.

However, the changes in this commit handle that scenario while still
rendering a less-confusing string representation of a keyword hash in
the most common scenario where the keys are all symbols.
  • Loading branch information
byroot authored and floehopper committed Jun 16, 2024
1 parent 19a3d21 commit 4ea9e38
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 19 deletions.
15 changes: 13 additions & 2 deletions lib/mocha/inspect.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,19 @@ def mocha_inspect(wrapped = true)

module HashMethods
def mocha_inspect
unwrapped = collect { |key, value| "#{key.mocha_inspect} => #{value.mocha_inspect}" }.join(', ')
Hash.ruby2_keywords_hash?(self) ? unwrapped : "{#{unwrapped}}"
if Hash.ruby2_keywords_hash?(self)
collect do |key, value|
case key
when Symbol
"#{key}: #{value.mocha_inspect}"
else
"#{key.mocha_inspect} => #{value.mocha_inspect}"
end
end.join(', ')
else
unwrapped = collect { |key, value| "#{key.mocha_inspect} => #{value.mocha_inspect}" }.join(', ')
"{#{unwrapped}}"
end
end
end

Expand Down
8 changes: 4 additions & 4 deletions test/acceptance/keyword_argument_matching_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ def test_should_match_hash_parameter_with_keyword_args
end
if Mocha::RUBY_V27_PLUS
location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (:key => 42)"
assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (key: 42)"
assert_includes Mocha::Deprecation.messages.last, 'but received positional hash ({:key => 42})'
end
end
Expand Down Expand Up @@ -55,7 +55,7 @@ def test_should_match_hash_parameter_with_splatted_keyword_args
end
if Mocha::RUBY_V27_PLUS
location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (:key => 42)"
assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (key: 42)"
assert_includes Mocha::Deprecation.messages.last, 'but received positional hash ({:key => 42})'
end
end
Expand Down Expand Up @@ -104,7 +104,7 @@ def test_should_match_positional_and_keyword_args_with_last_positional_hash
if Mocha::RUBY_V27_PLUS
location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected positional hash ({:key => 42})"
assert_includes Mocha::Deprecation.messages.last, 'but received keyword arguments (:key => 42)'
assert_includes Mocha::Deprecation.messages.last, 'but received keyword arguments (key: 42)'
end
end
assert_passed(test_result)
Expand Down Expand Up @@ -133,7 +133,7 @@ def test_should_match_last_positional_hash_with_keyword_args
end
if Mocha::RUBY_V27_PLUS
location = "#{execution_point.file_name}:#{execution_point.line_number}:in `block in #{test_name}'"
assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (:key => 42)"
assert_includes Mocha::Deprecation.messages.last, "Expectation defined at #{location} expected keyword arguments (key: 42)"
assert_includes Mocha::Deprecation.messages.last, 'but received positional hash ({:key => 42})'
end
end
Expand Down
16 changes: 8 additions & 8 deletions test/acceptance/positional_and_keyword_has_inspect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ def test_unexpected_keyword_arguments_in_invocation_and_expectation_print_correc
end
if Mocha::RUBY_V27_PLUS
assert_equal [
'unexpected invocation: #<Mock:mock>.method(:key => 42)',
'unexpected invocation: #<Mock:mock>.method(key: 42)',
'unsatisfied expectations:',
'- expected exactly once, invoked never: #<Mock:mock>.method(:foo => 42)'
'- expected exactly once, invoked never: #<Mock:mock>.method(foo: 42)'
], test_result.failure_message_lines
else
assert_equal [
Expand Down Expand Up @@ -64,9 +64,9 @@ def test_unexpected_keyword_arguments_with_other_positionals_in_invocation_and_e
end
if Mocha::RUBY_V27_PLUS
assert_equal [
'unexpected invocation: #<Mock:mock>.method(1, :key => 42)',
'unexpected invocation: #<Mock:mock>.method(1, key: 42)',
'unsatisfied expectations:',
'- expected exactly once, invoked never: #<Mock:mock>.method(1, :foo => 42)'
'- expected exactly once, invoked never: #<Mock:mock>.method(1, foo: 42)'
], test_result.failure_message_lines
else
assert_equal [
Expand Down Expand Up @@ -102,9 +102,9 @@ def test_unexpected_keyword_arguments_in_invocation_and_expectation_print_correc
end
end
assert_equal [
'unexpected invocation: #<Mock:mock>.method(:key => 42)',
'unexpected invocation: #<Mock:mock>.method(key: 42)',
'unsatisfied expectations:',
'- expected exactly once, invoked never: #<Mock:mock>.method(:foo => 42)'
'- expected exactly once, invoked never: #<Mock:mock>.method(foo: 42)'
], test_result.failure_message_lines
end

Expand Down Expand Up @@ -132,9 +132,9 @@ def test_unexpected_keyword_arguments_with_other_positionals_in_invocation_and_e
end
end
assert_equal [
'unexpected invocation: #<Mock:mock>.method(1, :key => 42)',
'unexpected invocation: #<Mock:mock>.method(1, key: 42)',
'unsatisfied expectations:',
'- expected exactly once, invoked never: #<Mock:mock>.method(1, :foo => 42)'
'- expected exactly once, invoked never: #<Mock:mock>.method(1, foo: 42)'
], test_result.failure_message_lines
end
end
Expand Down
9 changes: 7 additions & 2 deletions test/unit/hash_inspect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,14 @@ def test_should_return_string_representation_of_hash
end

if Mocha::RUBY_V27_PLUS
def test_should_return_unwrapped_hash_when_keyword_hash
def test_should_return_unwrapped_keyword_style_hash_when_keyword_hash
hash = Hash.ruby2_keywords_hash(a: true, b: false)
assert_equal ':a => true, :b => false', hash.mocha_inspect
assert_equal 'a: true, b: false', hash.mocha_inspect
end

def test_should_return_unwrapped_hash_when_keyword_hash_keys_are_not_symbols
hash = Hash.ruby2_keywords_hash('a' => true, 'b' => false)
assert_equal '"a" => true, "b" => false', hash.mocha_inspect
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_should_match_hash_arg_with_keyword_args_but_display_deprecation_warning

message = Mocha::Deprecation.messages.last
location = "#{execution_point.file_name}:#{execution_point.line_number}:in `new'"
assert_includes message, "Expectation defined at #{location} expected keyword arguments (:key_1 => 1, :key_2 => 2)"
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=.'
Expand All @@ -67,7 +67,7 @@ def test_should_match_keyword_args_with_hash_arg_but_display_deprecation_warning
message = Mocha::Deprecation.messages.last
location = "#{execution_point.file_name}:#{execution_point.line_number}:in `new'"
assert_includes message, "Expectation defined at #{location} expected positional hash ({:key_1 => 1, :key_2 => 2})"
assert_includes message, 'but received keyword arguments (:key_1 => 1, :key_2 => 2)'
assert_includes message, 'but received keyword arguments (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
Expand Down Expand Up @@ -124,7 +124,7 @@ def test_should_display_deprecation_warning_even_if_parent_expectation_is_nil

message = Mocha::Deprecation.messages.last
assert_includes message, 'Expectation expected positional hash ({:key_1 => 1, :key_2 => 2})'
assert_includes message, 'but received keyword arguments (:key_1 => 1, :key_2 => 2)'
assert_includes message, 'but received keyword arguments (key_1: 1, key_2: 2)'
end
end
end

0 comments on commit 4ea9e38

Please sign in to comment.