3.10.3 breaks hash argument matching in Ruby 3+

[alextwoods]

Subject of the issue

With rspec-mocks 3.10.3 we started seeing test failures in our CI: Previously passing run - fails now with rspec mocks 3.10.3 for ruby 3+. (Note, passes with rspec-mocks 3.10.2).

I believe the change in behavior is related to #1394.

Note: This issue is similar to #1457 but applies to a slightly different case

Your environment

• Ruby version: 3.0.2
• rspec-mocks version: 3.10.3

Steps to reproduce

class TestObject
  def initialize(object)
    @object = object
  end

  def foo(arg, options={})
    @object.foo(arg, options)
  end
end

describe 'rspec-mocks 3.10.3' do
  # This code passes in Ruby 2.7, but fails on Ruby 3.
  # Note: this test passes on Ruby 3 w/ rspec-mocks 3.10.2
  it 'breaks in ruby 3.0' do
    object = double('object')
    t = TestObject.new(object)

    expect(object).to receive(:foo).with('arg', opt: 'value')
    t.foo('arg', opt: 'value')
  end

  # This is the issue from: #1457 
  # this code passes in ruby 3
  it 'breaks in ruby 2.7' do
    obj = Object.new
    expect(obj).to receive(:call).with(:foo, **{})
    obj.call(:foo, **{})
  end
end

The above tests can be fixed by either:

1. Changing the spec to: with('arg', {opt: 'value'})
2. Or changing the code to use the double splat: @object.foo(arg, **options)

Expected behavior

Tests that pass in 3.10.2 should pass in 3.10.3.

Actual behavior

Some tests fail.

[pirj]

Thanks for reporting.

Do you want to give this patch a go to see if it resolves the problem?

[alextwoods]

The patch does not fix the issue, but it does at least provide better failure output, eg:

  expected: ("source", opt_name: "opt-value")
       got: ("source", {:opt_name=>"opt-value"})

[JonRowe]

What this issue is reporting is actually the correct behaviour. On Ruby 3:

expect(object).to receive(:foo).with('arg', opt: 'value')

This is setting an expectation of calling foo('arg', opt: 'value') with opt being a keyword argument. This is why passing in a proper hash is not matching, prior to 3.10.3 this was a bug and hashes would allowed to be silently be passed when keywords were required etc, and using your own snippet to demonstrate that hashes and keywords are not compatible:

class TestObject
  def initialize(object)
    @object = object
  end

  def foo(arg, options={})
    @object.foo(arg, options)
  end
end

class RealObject
  def foo(arg, opt:)
  end
end

TestObject.new(RealObject.new).foo('arg', opt: 'a')
# wrong number of arguments (given 2, expected 1; required keyword: opt) (ArgumentError)

In the naive sense, e.g. without verifying doubles, there is simply no way to do any other behaviour, we must fail a spec for not matching keywords vs hashes, as the default behaviour on Ruby3 matches this, so your example is unsolvable.

Now I expect prehaps in reality you are mocking a collaborator which has a defined behaviour, and indeed with instance doubles we could solve that scenario, where we have a real class we can see if the real object expects keywords or a hash, but we'd have to extend this existing behaviour in that scenario.

[alextwoods]

I agree this is generally correct Ruby 3 behavior - a proper hash is not the same as keyword args. My issue is that a patch version change introduced behavior that makes matchers less permissive and breaks existing, working code.

In your RealObject example above, if the RealObject#foo takes a hash, rather than keyword args, then the code is valid and works as expected, eg:

class RealObject
  def foo(arg, options={})
  end
end

TestObject.new(RealObject.new).foo('arg', opt: 'a') # RealObject#foo will be called with option={opt: 'a'}

[JonRowe]

The problem is a generic double has no real implementation, so we should assume Ruby 3 semantics on Ruby 3.

Previously keyword arguments were broken, and this change fixed them. That it unforuntately makes some previous specs invalid (which were incorrect on Ruby 3) is sad but there is no way for a generic double to fix it. I'd happily review a PR to make instance verifying doubles work (.e.g. instance_double("ClassName")).

The simple work around is to set with('arg', {opt: 'a'})

[JonRowe]

The TL;DR is we cannot make expect(double()).to receive(:foo).with('arg', opt: :a) work with both keywords and a hash correctly, so we must pick the Ruby 3 semantics as correct.

[gingerlime]

We're seeing a similar issue(?) after upgrading rspec-rails, which updated rspec-mocks as a dependency...

     Failure/Error: update_async(user.id, properties)

       ClassName received :method with unexpected arguments
         expected: (5, {:subscription_started_date=>1646810075})
              got: (5, {:subscription_started_date=>1646810075})

[raldred]

We have this issue during upgrade to ruby 3, the diff is misleading.
It seems the diff can't distinguish between a hash being passed or keyword args.

In our case the hash being passed needed double splatting.

#<Double "check"> received :create with unexpected arguments
         expected: ("1111111-123123-123123", {:reports=>[{:name=>"identity"}, {:name=>"document"}], :tags=>["USA"], :type=>"express"})
              got: ("1111111-123123-123123", {:reports=>[{:name=>"identity"}, {:name=>"document"}], :tags=>["USA"], :type=>"express"})

Fix:

@api.check.create(applicant_id, **create_check_params(country_code: country_code))

[pirj]

Do you mind sharing your spec and the subject under test?

…

On 25 Aug 2022, at 14:06, Rob Aldred ***@***.***> wrote:  We have this issue during upgrade to ruby 3, the diff is misleading. #<Double "check"> received :create with unexpected arguments expected: ("1111111-123123-123123", {:reports=>[{:name=>"identity"}, {:name=>"document"}], :tags=>["USA"], :type=>"express"}) got: ("1111111-123123-123123", {:reports=>[{:name=>"identity"}, {:name=>"document"}], :tags=>["USA"], :type=>"express"}) Fixed by changing the code calling this method to double splat the hash being passed @api.check.create(applicant_id, **create_check_params(country_code: country_code)) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.