Confused over instance doubles, keyword args, with, and Ruby 3.2 previews

[philnash]

Subject of the issue

I ran a bunch of specs on my project through GitHub actions and a bunch failed recently against Ruby head. I'm not sure whether it's the fault of Ruby or RSpec or my own misunderstanding, so I thought I would report it here first.

The issue came down to expectations for instance doubles for objects that take keyword arguments and how to test them. Please see the steps to reproduce below for details.

Your environment

• Ruby version: 3.2.0-preview2
• rspec-mocks version: 3.12.0

Steps to reproduce

# frozen_string_literal: true

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"

  gem "rspec", "3.12.0"
end

puts "Ruby version is: #{RUBY_VERSION}"
require 'rspec/autorun'

class TestObject
  def kw_args_method(a:, b:); end
end

RSpec.describe TestObject do
  it "expects to receive keyword args" do
    dbl = instance_double(TestObject)
    expect(dbl).to receive(:kw_args_method).with(a: 1, b: 2)
    dbl.kw_args_method(a: 1, b: 2)
  end

  it "expects to receive keyword args with a hash" do
    dbl = instance_double(TestObject)
    expect(dbl).to receive(:kw_args_method).with({a: 1, b: 2})
    dbl.kw_args_method(a: 1, b: 2)
  end
end

Expected behavior

I expected the first test to pass, since the expectation is that the method is called with two keyword arguments and then the method is called with two keyword arguments.

I'd expect the second test to fail because the expectation is that the method is called with a hash, but the method is called with keyword arguments.

Actual behavior

The first test fails and the second passes.

Failures:

  1) TestObject expects to receive keyword args
     Failure/Error: dbl.kw_args_method(a: 1, b: 2)
     
       #<InstanceDouble(TestObject) (anonymous)> received :kw_args_method with unexpected arguments
         expected: ({:a=>1, :b=>2}) (keyword arguments)
              got: ({:a=>1, :b=>2}) (options hash)
       Diff:
       
     # test_spec.rb:28:in `block (2 levels) in <main>'

However, just to throw some extra information in:

• On Ruby 3.1 with RSpec 3.12.0 both tests pass.
• On Ruby 3.2-preview2 with RSpec 3.10.0 and RSpec Mocks 3.10.3, both tests pass.

Is this something that should be raised with Ruby or is it something that RSpec needs to handle for Ruby 3.2?

[JonRowe]

It sounds like Ruby 3.2 has changed something that has broken our detection between keywords and hashes again, wether that fix needs to go into Ruby or us depends on if it was a deliberate change on their side or an accidental one.

[eregon]

It's likely due to this change, search for ruby2_keywords in https://github.com/ruby/ruby/blob/master/NEWS.md, and it is deliberate, so probably it needs some extra ruby2_keywords in RSpec.

[ojab]

diff --git a/lib/rspec/mocks/verifying_proxy.rb b/lib/rspec/mocks/verifying_proxy.rb
index b39871c8..1d8d207d 100644
--- a/lib/rspec/mocks/verifying_proxy.rb
+++ b/lib/rspec/mocks/verifying_proxy.rb
@@ -160,6 +160,7 @@ module RSpec
         validate_arguments!(args)
         super
       end
+      ruby2_keywords :proxy_method_invoked if respond_to?(:ruby2_keywords, true)
 
       def validate_arguments!(actual_args)
         @method_reference.with_signature do |signature|

fixes

class Xxx
  def self.x(extra:)
  end
end


RSpec.describe 'ruby-3.2.0' do
  specify do
    allow(Xxx).to receive(:x).with(extra: {})

    Xxx.x(extra: {})
  end

  specify do
    expect(Xxx).to receive(:x).with(extra: {})

    Xxx.x(extra: {})
  end
end

for me (and CI for some small internal project)

[ojab]

With the change above testsuite passes except reentrant mutext inside the Fiber spec, see ruby/ruby#6680 (comment)

[pirj]

Would you like to send a PR, @ojab ?

[ojab]

@pirj sure, I just want to get some clarification on ruby/ruby#6680 (comment) first.
Or I could make a PR with the fix above and adding ruby-3.2 to CI + skipping reentrant mutex specs for rubies >=3.2 for the time being.

[eregon]

As I replied there, I think the best is to adjust that spec: ruby/ruby#6680 (comment)

[pirj]

We have a ruby-head job, will that suffice, @ojab ?

[ojab]

@pirj yep, ruby-head is fine too.
rspec/rspec-support#553
#1502 (draft until rspec-support PR is merged because CI would be broken anyway)

[adsteel]

I found another place we need to instrument a method via ruby2_keywords. #1508

I almost wonder if we should just audit all methods that have a *args in the signature without a **kwargs and blindly instrument with ruby2_keywords, otherwise there might be a longer tail of bug reports trickling in.

[eregon]

I almost wonder if we should just audit all methods that have a *args in the signature without a **kwargs and blindly instrument with ruby2_keywords, otherwise there might be a longer tail of bug reports trickling in.

I did that for some gems, yes it's a good idea to audit all *args (or *<name> of course) methods which use those arguments to call another method and either:

• They don't accept keyword arguments and are unlikely to ever will, nothing to do
• They might and then adding ruby2_keywords is necessary

Indeed it's not semantically harmful to add ruby2_keywords to more places than needed, although it can have a little bit of a performance impact and it may confuse people reading the code if no keywords should ever come there.
At least I wouldn't do it if there is no delegation, i.e., if *args is not used for calling some other method later.