Version 3.11.2 issue with kwargs

[vibro]

Version 3.11.2 issue with kwargs

Today our CI started failing with the new version of rspec-mocks released. It seems that mocking the kwargs no longer has the same behavior

Your environment

• Ruby version: 2.7.4
• rspec-mocks version: 3.11.2

Steps to reproduce

I've provided a sample that is close to the behavior in our tests:

# 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 "net-ssh", "6.1.0"
  gem "rspec-mocks", "3.11.2"
  gem "rspec", "3.11.0" 
end

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

RSpec.describe "Test" do
  let(:kwargs) { Hash.new }
  let(:user) { 'party_parrot' }
  let(:host) { 'host.com' }

  before(:each) do
    allow(Net::SSH)
      .to receive(:start)
            .with(host, user, **kwargs)
  end

  it do
    Net::SSH.start(host, user, **kwargs)
    expect(Net::SSH).to have_received(:start).with(host, user, **kwargs)
  end
end

Expected behavior

Previously, this test ran ok.

Test
 INFO  [84734] [17WK] === FINISHED EXAMPLE "Test is expected to have received start(\"host.com\", \"party_parrot\") 1 time" ./sample.rb:35
  is expected to have received start("host.com", "party_parrot") 1 time

Actual behavior

Now, we get a mocking error:

  1) Test 
     Failure/Error: Net::SSH.start(host, user, **kwargs)
     
       Net::SSH received :start with unexpected arguments
         expected: ("host.com", "party_parrot", {})
              got: ("host.com", "party_parrot")
       Diff:
       @@ -1 +1 @@
       -["host.com", "party_parrot", {}]
       +["host.com", "party_parrot"]
       
        Please stub a default value first if message might be received with other args as well. 
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-support-3.11.1/lib/rspec/support.rb:102:in `block in <module:Support>'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-support-3.11.1/lib/rspec/support.rb:111:in `notify_failure'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/error_generator.rb:327:in `notify'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/error_generator.rb:311:in `__raise'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/error_generator.rb:60:in `raise_missing_default_stub_error'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/proxy.rb:242:in `raise_missing_default_stub_error'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/proxy.rb:226:in `message_received'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/proxy.rb:366:in `message_received'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/method_double.rb:80:in `proxy_method_invoked'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/verifying_proxy.rb:161:in `proxy_method_invoked'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-mocks-3.11.2/lib/rspec/mocks/method_double.rb:64:in `block (2 levels) in define_proxy_method'
     # ./sample.rb:36:in `block (2 levels) in <top (required)>'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:263:in `instance_exec'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:263:in `block in run'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:511:in `block in with_around_and_singleton_context_hooks'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:468:in `block in with_around_example_hooks'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/hooks.rb:486:in `block in run'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/hooks.rb:626:in `block in run_around_example_hooks_for'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:352:in `call'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/webmock-3.18.1/lib/webmock/rspec.rb:37:in `block (2 levels) in <top (required)>'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:457:in `instance_exec'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:457:in `instance_exec'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/hooks.rb:390:in `execute_with'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/hooks.rb:628:in `block (2 levels) in run_around_example_hooks_for'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:352:in `call'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/hooks.rb:629:in `run_around_example_hooks_for'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/hooks.rb:486:in `run'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:468:in `with_around_example_hooks'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:511:in `with_around_and_singleton_context_hooks'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example.rb:259:in `run'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example_group.rb:646:in `block in run_examples'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example_group.rb:642:in `map'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example_group.rb:642:in `run_examples'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/example_group.rb:607:in `run'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:121:in `block (3 levels) in run_specs'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:121:in `map'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:121:in `block (2 levels) in run_specs'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/configuration.rb:2068:in `with_suite_hooks'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:116:in `block in run_specs'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/reporter.rb:74:in `report'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:115:in `run_specs'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:89:in `run'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:71:in `run'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/lib/rspec/core/runner.rb:45:in `invoke'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/gems/rspec-core-3.11.0/exe/rspec:4:in `<top (required)>'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/bin/rspec:23:in `load'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/bin/rspec:23:in `<main>'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/bin/ruby_executable_hooks:22:in `eval'
     # /Users/vbrown/.rvm/gems/ruby-2.7.4@captain/bin/ruby_executable_hooks:22:in `<main>'

[btalbot]

I have a similar issue when mocking a File.read for a test. The 'allow' is acting like an 'expect' and preventing all other uses of File.read that are not specifically mocked.

allow(File).to receive(:read).with('myfile').and_return('my file contents')

Then prevents all other uses of File.read. I can work around this by first including

allow(File).to receive(:read).and_call_original

[pirj]

This seems like an unrelated issue @btalbot .Can you please open a separate issue providing a reproduction example? So are you saying that if you File.read('someotherfile') in your spec, it would also return 'my file contents'?

[pirj]

@vibro Thanks for reporting.

started failing with the new version of rspec-mocks released

What was the previous version of rspec-mocks that worked as expected for you?

[rus-max]

I have a similar issue. Version 3.11.1 works without errors

[pirj]

@vibro At a glance, I could find #1394. Could you read comments there to see if there's an explanation to this behavioural change.

@rus-max What is the Ruby version that you're using? Can you provide an example of your code? I don't understand if "similar" refers to kwargs or allow...to receive...with with positional arguments.

[rus-max]

@pirj

ruby 3.1.2 allow(instance_double_class).to receive(:post).with(instance_of(MyClass), **params).and_return(stubbed_params)

     Failure/Error:
   
       #<InstanceDouble(InstanceDoubleClass) (anonymous)> received :post with unexpected arguments
         expected: (an_instance_of(MyClass), {:id=>0, :method=>"method", :params=>{:param1=>"param1"}})
              got: (#<Myclass foo="foo", bar="bar">, {:id=>0, :method=>"method", :params=>{:param1=>"param1"}})
       Diff:
       @@ -1,4 +1,4 @@
       -["an_instance_of(MyClass)",
       +[#<MyClass foo="foo", bar="bar">,
         {:id=>0,
          :method=>"method",
          :params=>{:param1=>"param1"}}]
       
        Please stub a default value first if message might be received with other args as well. 

[JonRowe]

Make sure you are not passing hashes as kwargs, this is not valid on Ruby 3 and is now detected correctly (but the diff doesn't show the delta correctly on 3.11, that will come in a minor version bump)

[pirj]

@rus-max I suggest you to check #1473 for some information regarding instance_double(...) changes.

[vibro]

What was the previous version of rspec-mocks that worked as expected for you?

3.11.1 works just fine.

I can update the code/change the behavior but this isn't something I would expect to change/break in a patch release.

[JonRowe]

Our support for keyword arguments isn't as good as it could be, a lot of usage such as yours has historically just worked due to implementation details of Ruby (mainly that keyword arguments carried over in splats and thus allowed usage in method calls and blocks transparently) with a bunch of edge case uses not being tested due to maintainers (including myself) not invisioning how they were used.

In Ruby 3 that changed and theres has had to be changes (which are mostly investigating and declaring ruby2_keywords correctly) to allow proper keyword arguments to be verified and treated differently to hashes (as in Ruby 3 you cannot mix splats).

In this case it appears that Net::SSH.start does not take keyword arguments, it takes three positonal arguments (from https://github.com/net-ssh/net-ssh/blob/master/lib/net/ssh.rb#L216):

def self.start(host, user = nil, options = {}, &block)

So its non standard to be passing keyword arguments to via **kwargs to this and is evidently tripping up some of our keyword detection, I won't revert that change, but as this seems to work on a standard method in Ruby 3.1:

def a_method(a, b, hash = {})
  puts hash.inspect
end
a_method(1, 2, **{is_this_a_hash: :or_is_this_just_fantasty})

I would accept a PR helping to improve that detection.

[JonRowe]

Oddly I can't replicate this within our test suite on either 2.7.4, 2.7.5 or 3.1.0

Edit: Removed output showing the snippet didn't work, it needed this adding for reproduction:

RSpec.configure do |config|
  config.mock_with :rspec do |mocks|
    mocks.verify_partial_doubles = true # changing this to false makes the issue go away
  end
end

An even futher odditiy is it seems to be specifically the Hash.new that is causing the issue, a non empty hash does not trigger the snippet.

[vibro]

I can confirm we have this in our spec helper:

  config.mock_with :rspec do |mocks|
    # Prevents you from mocking or stubbing a method that does not exist on
    # a real object. This is generally recommended, and will default to
    # `true` in RSpec 4.
    mocks.verify_partial_doubles = true
  end

[pirj]

An even futher odditiy is it seems to be specifically the Hash.new that is causing the issue, a non empty hash does not trigger the snippet.

delegate({}) # => [{}] in Ruby 2.6 & 3, but [] in Ruby 2.7! (with a warning)

See the famous blog post.

In Ruby 2.7:

def foo(*args)
  puts args.inspect
end

foo(**{}) # outputs `[]`

with is defined with (*args). This is the cause that

have_received(:start).with(host, user, **kwargs)

it the same as

have_received(:start).with(host, user)

But since the original method is defined with options = {}, and calling it with **{} sets options to its default {}.

According to git bisect it's not #1394 that breaks this case, but #1473

I've debugged this a bit, and it seems that the {} part of args was disappearing here. Adding ruby2_keywords :proxy_method_invoked fixes the snippet from the description. 🎉

This has already been fixed here.

But something else was fixed along the way, and the failure on main is now quite the opposite:

# 3.11.2
       Net::SSH received :start with unexpected arguments
         expected: ("host.com", "party_parrot", {})
              got: ("host.com", "party_parrot")

#main
       (Net::SSH).start("host.com", "party_parrot")
           expected: 1 time with arguments: ("host.com", "party_parrot")
           received: 0 times

[JonRowe]

I'm closing this during the monorepo migration as we do have a plan to further work on keyword argments in RSpec 4, and there are a bunch of issues I'm transfering but I think this is either fixed or a duplicate at this point.