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

Ruby 3.2.0 and keyword arguments vs options hash #1513

Closed
schinery opened this issue Dec 31, 2022 · 14 comments · Fixed by #1514
Closed

Ruby 3.2.0 and keyword arguments vs options hash #1513

schinery opened this issue Dec 31, 2022 · 14 comments · Fixed by #1514

Comments

@schinery
Copy link

Subject of the issue

I'm currently in the process of upgrading some Rails apps from Ruby 3.1.3 to 3.2.0 and am now getting 'expected keyword arguments, received options hash' failures in a bunch of specs that pass in 3.1.3.

Wasn't sure if it is related to rspec/rspec-expectations#1370 or rspec/rspec-expectations#1350.

Your environment

  • Ruby version: 3.2.0
  • rspec-expectations version: 3.12.1

Steps to reproduce

Rough basic example pulled from one of the Rails apps where the spec is now failing after the Ruby upgrade.

# app/models/app.rb

class App
  def initialize(app_name, do_something: false)
    # ...
  end

  def things
    # ...
  end
end

# app/controllers/things_controller.rb

class ThingsController < ApplicationController
  def index
    @app = App.new(app_name, do_something: true)
    @things = @app.things

    render partial: "index"
  end

  private

  def app_name
    @app_name ||= params[:app_name].to_sym
  end
end

# spec/requests/things_spec.rb

RSpec.describe "/things" do
  describe "GET /" do
    before do
      app = instance_double(App)
      allow(App).to receive(:new).with(:jimmy, do_something: true).and_return(app)
      allow(app).to receive(:things).and_return([])
      
      get "/things", params: { app_name: "jimmy" }
    end

    it "returns status 200 OK" do
      expect(response).to have_http_status(:ok)
    end
  end
end

Expected behavior

The spec passes with the expectations met.

Actual behavior

Failures:

  1) /things GET / returns status 200 OK
     Failure/Error: @app = App.new(app_name, do_something: true)
     
       #<App (class)> received :new with unexpected arguments
         expected: (:jimmy, {:do_something=>true}) (keyword arguments)
              got: (:jimmy, {:do_something=>true}) (options hash)
        Please stub a default value first if message might be received with other args as well.
@schinery
Copy link
Author

Hmmm... if I do the following as an inline example then it works as expected.

# frozen_string_literal: true

begin
  require 'bundler/inline'
rescue LoadError => e
  warn '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' # Activate the gem and version you are reporting the issue against.
end

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

class App
  def things
    Thing.new(:jimmy, do_something: true)
  end
end

class Thing
  def initialize(name, do_something: false)
    # ...
  end
end

RSpec.describe 'keyword args' do
  before do
    thing = instance_double(Thing)
    allow(Thing).to receive(:new).with(:jimmy, do_something: true).and_return(thing)
    App.new.things
  end

  it { expect(Thing).to have_received(:new).with(:jimmy, do_something: true) }
end

Then it passes...

Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Using bundler 2.4.1
Using diff-lcs 1.5.0
Using rspec-support 3.12.0
Using rspec-mocks 3.12.1
Using rspec-core 3.12.0
Using rspec-expectations 3.12.1
Using rspec 3.12.0
Ruby version is: 3.2.0
.

Finished in 0.00938 seconds (files took 0.07401 seconds to load)
1 example, 0 failures

Tried this with using both rspec-core and rspec-rails just to see if it made any difference. Also tried creating an inline Rails app to try but so far failing at producing that.

Also worth noting, in my original example if I change allow(App).to receive(:new).with(:jimmy, do_something: true) to allow(App).to receive(:new).with(:jimmy, { do_something: true }) then the spec passes but that doesn't seem like the fix.

@pirj
Copy link
Member

pirj commented Dec 31, 2022 via email

@schinery
Copy link
Author

Can you please try allow(App).to receive(:new).with(:jimmy, {do_something: true})

@pirj yeah if I do that the spec passes, but am trying to understand why it doesn’t need to be changed when run in Ruby 3.1.3.

Been looking up the Ruby 3.2 keyword argument changes and stumbled on #1495 and #1502, so assume it’s the same thing here?

@pirj
Copy link
Member

pirj commented Dec 31, 2022

Actually you're right, I've missed this line in your code.

@app = App.new(app_name, do_something: true)

@pirj pirj transferred this issue from rspec/rspec-expectations Dec 31, 2022
@marcqualie
Copy link

This is also happening will keywords args only when uprading from ruby 3.1 to 3.2.

MyClass.call(content: content, url: url)
expect(MyClass).to receive(:call).with(content: content, url: url).and_return mock_result

# failure
#<MyClass(class)> received :call with unexpected arguments
  expected: ({:content=>"somestring", :url=>"https://example.com"}) (keyword arguments)
       got: ({:content=>"somestring", :url=>"https://example.com"}) (options hash)

As suggested above, converting the test expectation to a hash makes the test pass:

expect(MyClass).to receive(:call).with({ content: content, url: url }).and_return mock_result

This is asserting incorrect behaviour though.. if actually changing the underlying code to call with a hash, the test still passes, but runtime error occurs:

wrong number of arguments (given 1, expected 0; required keywords: content, url)

I hope this helps narorw down the issue.

@mblumtritt
Copy link

I can confirm the issue described above. It prevents me from updating to Ruby v3.2.0, which is important for me :/

@ric2b
Copy link

ric2b commented Jan 2, 2023

This might be related to #1512, .withshares the argument customization implementation with .and_call_original and other methods:

[:with, :and_return, :and_invoke, :and_throw, :and_raise, :and_yield, :and_call_original].each do |msg|
define_method(msg) do |*args, &block|
@recorded_customizations << ExpectationCustomization.new(msg, args, block)
self
end
end

@schinery
Copy link
Author

schinery commented Jan 3, 2023

@pirj I've just tried applying the ruby2_keywords :proxy_method_invoked if respond_to?(:ruby2_keywords, true) change from #1502 to my locally installed rspec-mocks and the tests that were failing now pass as expected.

@benoittgt
Copy link
Member

@schinery could you try this #1514 ?

@schinery
Copy link
Author

schinery commented Jan 3, 2023

@benoittgt just tried but am getting the following error when adding gem "rspec-mocks", git: "https://github.com/rspec/rspec-mocks", branch: "ruby-3.2" to the Gemfile:

bundle update
Fetching https://github.com/rspec/rspec-mocks
Fetching gem metadata from https://rubygems.org/..........
Resolving dependencies...
Could not find compatible versions

Because every version of rspec-mocks depends on rspec-support = 3.13.0.pre
  and rspec-support = 3.13.0.pre could not be found in rubygems repository https://rubygems.org/ or installed locally,
  every version of rspec-mocks is forbidden.
So, because Gemfile depends on rspec-mocks >= 0,
  version solving has failed.

However this was the change I applied locally which resolved the particular issue I was having.

@benoittgt
Copy link
Member

@schinery Do you have better result with this

gem 'rspec-mocks', git: 'https://github.com/rspec/rspec-mocks.git', branch: 'ruby-3.2'
%w[rspec-core rspec-expectations rspec-rails rspec-support].each do |lib|
  gem lib, git: "https://github.com/rspec/#{lib}.git", branch: 'main'
end

Thanks for the feedback.

@schinery
Copy link
Author

schinery commented Jan 3, 2023

@benoittgt that did the trick, and can confirm that the failing specs are passing again as expected locally.

I've got some more Rails apps that also have failing specs in, will try in some of those too.

@schinery
Copy link
Author

schinery commented Jan 3, 2023

I've got some more Rails apps that also have failing specs in, will try in some of those too.

Having some more dependency issues in the other projects (with rspec-html-matchers for example) so might be awhile confirming in them...

@adsteel
Copy link

adsteel commented Jan 3, 2023

Been looking up the Ruby 3.2 keyword argument changes and stumbled on #1495 and #1502, so assume it’s the same thing here?

@schinery I believe so. I think we could merge this issue and that issue together.

@pirj pirj closed this as completed in #1514 Jan 4, 2023
peteryates added a commit to DFE-Digital/early-careers-framework that referenced this issue Apr 14, 2023
This prevents us from hitting some incompatibility issues between Ruby
3.1 and Ruby 3.2

rspec/rspec-mocks#1513
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

Successfully merging a pull request may close this issue.

7 participants