Skip to content

Commit

Permalink
Merge pull request #1394 from mame/with-for-ruby3
Browse files Browse the repository at this point in the history 
Make `with` support Ruby 3 keywords
  • Loading branch information
@JonRowe
JonRowe committed Feb 18, 2021
2 parents 7013c5f + 865ea3a commit fefabc4
Show file tree
Hide file tree
Showing 6 changed files with 115 additions and 44 deletions.
20 changes: 17 additions & 3 deletions lib/rspec/mocks/argument_list_matcher.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,30 @@ def initialize(*expected_args)
@expected_args = expected_args
ensure_expected_args_valid!
end
ruby2_keywords :initialize if Module.private_method_defined?(:ruby2_keywords)

# @api public
# @param [Array] args
# @param [Array] actual_args
#
# Matches each element in the `expected_args` against the element in the same
# position of the arguments passed to `new`.
#
# @see #initialize
def args_match?(*args)
Support::FuzzyMatcher.values_match?(resolve_expected_args_based_on(args), args)
def args_match?(*actual_args)
expected_args = resolve_expected_args_based_on(actual_args)

return false if expected_args.size != actual_args.size

if RUBY_VERSION >= "3"
# if both arguments end with Hashes, and if one is a keyword hash and the other is not, they don't match
if Hash === expected_args.last && Hash === actual_args.last
if !Hash.ruby2_keywords_hash?(actual_args.last) && Hash.ruby2_keywords_hash?(expected_args.last)
return false
end
end
end

Support::FuzzyMatcher.values_match?(expected_args, actual_args)
end

# @private
Expand Down
1 change: 1 addition & 0 deletions lib/rspec/mocks/matchers/receive.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def setup_any_instance_allowance(subject, &block)
@recorded_customizations << ExpectationCustomization.new(method, args, block)
self
end
ruby2_keywords(method) if Module.private_method_defined?(:ruby2_keywords)
end

private
Expand Down
1 change: 1 addition & 0 deletions lib/rspec/mocks/message_expectation.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ def with(*args, &block)
@argument_list_matcher = ArgumentListMatcher.new(*args)
self
end
ruby2_keywords(:with) if Module.private_method_defined?(:ruby2_keywords)

# Expect messages to be received in a specific order.
#
Expand Down
20 changes: 15 additions & 5 deletions spec/rspec/mocks/argument_matchers_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,16 +381,26 @@ def ==(other)
a_double.random_call(:a => "a", :b => "b")
end

it "matches against a hash submitted by reference and received by value" do
it "matches against a hash submitted as keyword arguments a and received as a positional argument (in both Ruby 2 and Ruby 3)" do
opts = {:a => "a", :b => "b"}
expect(a_double).to receive(:random_call).with(opts)
a_double.random_call(:a => "a", :b => "b")
end

it "matches against a hash submitted by value and received by reference" do
opts = {:a => "a", :b => "b"}
expect(a_double).to receive(:random_call).with(:a => "a", :b => "b")
a_double.random_call(opts)
if RUBY_VERSION >= "3"
it "fails to matches against a hash submitted as a positional argument and received as keyword arguments in Ruby 3.0 or later", :reset => true do
opts = {:a => "a", :b => "b"}
expect(a_double).to receive(:random_call).with(:a => "a", :b => "b")
expect do
a_double.random_call(opts)
end.to fail_with(/expected: \(\{(:a=>\"a\", :b=>\"b\"|:b=>\"b\", :a=>\"a\")\}\)/)

This comment has been minimized.

Sign in to view

Copy link
@simi

simi Jan 31, 2022

Error message reported here is slightly confusing.

       expected RSpec::Mocks::MockExpectationError with "...", got #<RSpec::Mocks::MockExpectationError: #<Double (anonymous)> received :random_call with unexpected arguments
         expected: ({:a=>"a", :b=>"b"})                                                                                                                                                       
              got: ({:a=>"a", :b=>"b"})

We found out this in bundler spec suite rubygems/rubygems#5322 (comment).

@JonRowe @mame Would it be possible to fail with more explanatory message mentioning different kind of argument was expected? It is impossible to find out with current message and it seems like rspec bug instead. I can try to contribute as well, but I'm not sure how to report this kind of difference.

This comment has been minimized.

Sign in to view

Copy link
@simi

simi Jan 31, 2022

One quick idea would be to raise ArgumentError and explain hash argument was passed, but keyword argument was expected (or opposite).

This comment has been minimized.

Sign in to view

Copy link
@JonRowe

JonRowe Feb 1, 2022

Author Member

The code that does this is above in this commit (def args_match?(*actual_args)) the improvement I'd support here is the diff output adding on something like (keyword hash) when a hash is a keyword hash but would need to be implemented in the differ when its printing hashes to strings https://github.com/rspec/rspec-support/blob/main/lib/rspec/support/differ.rb#L192

This comment has been minimized.

Sign in to view

Copy link
@simi

simi Feb 1, 2022

OK, I'll take a look @JonRowe. Thanks for the recommendation.

This comment has been minimized.

Sign in to view

Copy link
@simi

simi Feb 2, 2022

@mame Do you have still the code you mentioned at #1394 (comment) around?

This comment has been minimized.

Sign in to view

Copy link
@mame

mame Feb 2, 2022

Contributor

As far as I recall, this is my proof-of-concept patch: https://gist.github.com/mame/130a97f917300aa404b7dbc422263fd2

Maybe we need to modify some expectations of tests too.

This comment has been minimized.

Sign in to view

Copy link
@mame

mame Feb 2, 2022

Contributor

Note that it was just a PoC. I have no strong preference about the style. I think it would be good enough to add something like (keyword hash) as @JonRowe said. Test modification will be needed anyway.
end
else
it "matches against a hash submitted as a positional argument and received as keyword arguments in Ruby 2.7 or before" do
opts = {:a => "a", :b => "b"}
expect(a_double).to receive(:random_call).with(:a => "a", :b => "b")
a_double.random_call(opts)
end
end

it "fails for a hash w/ wrong values", :reset => true do
Expand Down
13 changes: 9 additions & 4 deletions spec/rspec/mocks/partial_double_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,15 @@ def call(name)
expect(object.foobar(:key => "value")).to equal(1)
end

it "can accept an inner hash as a message argument" do
hash = {:a => {:key => "value"}}
expect(object).to receive(:foobar).with(:key => "value").and_return(1)
expect(object.foobar(hash[:a])).to equal(1)
if RSpec::Support::RubyFeatures.required_kw_args_supported?
# Use eval to avoid syntax error on 1.8 and 1.9
binding.eval(<<-CODE, __FILE__, __LINE__)
it "can accept an inner hash as a message argument" do
hash = {:a => {:key => "value"}}
expect(object).to receive(:foobar).with(:key => "value").and_return(1)
expect(object.foobar(**hash[:a])).to equal(1)
end
CODE
end

it "can create a positive message expectation" do
Expand Down
104 changes: 72 additions & 32 deletions spec/rspec/mocks/should_syntax_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,39 +463,79 @@ def use_rspec_mocks
after(:all) { RSpec::Mocks.configuration.syntax = orig_syntax }
before { RSpec::Mocks.configuration.reset_syntaxes_to_default }

let(:expected_arguments) {
[
/Using.*without explicitly enabling/,
if RSpec::Support::RubyFeatures.required_kw_args_supported?
let(:expected_arguments) {
[
/Using.*without explicitly enabling/,
]
}
let(:expected_keywords) {
{:replacement => "the new `:expect` syntax or explicitly enable `:should`"}
]
}

it "it warns about should once, regardless of how many times it is called" do
expect(RSpec).to receive(:deprecate).with(*expected_arguments)
o = Object.new
o2 = Object.new
o.should_receive(:bees)
o2.should_receive(:bees)

o.bees
o2.bees
end

it "warns about should not once, regardless of how many times it is called" do
expect(RSpec).to receive(:deprecate).with(*expected_arguments)
o = Object.new
o2 = Object.new
o.should_not_receive(:bees)
o2.should_not_receive(:bees)
end

it "warns about stubbing once, regardless of how many times it is called" do
expect(RSpec).to receive(:deprecate).with(*expected_arguments)
o = Object.new
o2 = Object.new

o.stub(:faces)
o2.stub(:faces)
}
it "it warns about should once, regardless of how many times it is called" do
# Use eval to avoid syntax error on 1.8 and 1.9
eval("expect(RSpec).to receive(:deprecate).with(*expected_arguments, **expected_keywords)")
o = Object.new
o2 = Object.new
o.should_receive(:bees)
o2.should_receive(:bees)

o.bees
o2.bees
end

it "warns about should not once, regardless of how many times it is called" do
# Use eval to avoid syntax error on 1.8 and 1.9
eval("expect(RSpec).to receive(:deprecate).with(*expected_arguments, **expected_keywords)")
o = Object.new
o2 = Object.new
o.should_not_receive(:bees)
o2.should_not_receive(:bees)
end

it "warns about stubbing once, regardless of how many times it is called" do
# Use eval to avoid syntax error on 1.8 and 1.9
eval("expect(RSpec).to receive(:deprecate).with(*expected_arguments, **expected_keywords)")
o = Object.new
o2 = Object.new

o.stub(:faces)
o2.stub(:faces)
end
else
let(:expected_arguments) {
[
/Using.*without explicitly enabling/,
{:replacement => "the new `:expect` syntax or explicitly enable `:should`"}
]
}
it "it warns about should once, regardless of how many times it is called" do
expect(RSpec).to receive(:deprecate).with(*expected_arguments)
o = Object.new
o2 = Object.new
o.should_receive(:bees)
o2.should_receive(:bees)

o.bees
o2.bees
end

it "warns about should not once, regardless of how many times it is called" do
expect(RSpec).to receive(:deprecate).with(*expected_arguments)
o = Object.new
o2 = Object.new
o.should_not_receive(:bees)
o2.should_not_receive(:bees)
end

it "warns about stubbing once, regardless of how many times it is called" do
expect(RSpec).to receive(:deprecate).with(*expected_arguments)
o = Object.new
o2 = Object.new

o.stub(:faces)
o2.stub(:faces)
end
end

it "warns about unstubbing once, regardless of how many times it is called" do
Expand Down

0 comments on commit fefabc4

Please sign in to comment.