Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 3.11.2 issue with kwargs #1492

Open
vibro opened this issue Oct 25, 2022 · 13 comments
Open

Version 3.11.2 issue with kwargs #1492

vibro opened this issue Oct 25, 2022 · 13 comments

Comments

@vibro
Copy link

vibro commented Oct 25, 2022

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
Copy link

btalbot commented Oct 26, 2022

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
Copy link
Member

pirj commented Oct 26, 2022

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
Copy link
Member

pirj commented Oct 26, 2022

@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
Copy link

rus-max commented Oct 26, 2022

I have a similar issue. Version 3.11.1 works without errors

@pirj
Copy link
Member

pirj commented Oct 26, 2022

@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
Copy link

rus-max commented Oct 26, 2022

@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
Copy link
Member

JonRowe commented Oct 26, 2022

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
Copy link
Member

pirj commented Oct 26, 2022

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

@vibro
Copy link
Author

vibro commented Oct 26, 2022

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
Copy link
Member

JonRowe commented Oct 26, 2022

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
Copy link
Member

JonRowe commented Oct 26, 2022

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
Copy link
Author

vibro commented Oct 26, 2022

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
Copy link
Member

pirj commented Jan 11, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants