-
-
Notifications
You must be signed in to change notification settings - Fork 356
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
Mocks dont raise keyword argument errors #1425
Comments
Can you add to your reproduction the missing The problem with your fix is it forces use of keyword arguments, we tried a similar setup for fixing the warnings generated internally but it caused issues on Ruby < 3 where hashes were used and were meant to be used as a last placed positional argument. This is still valid in Ruby 2.x even if you are warned on 2.7, but we can't change our implementation as both uses are valid. The solution elsewhere was to use the |
Hi @JonRowe, thank you for your fast response. I was on vacation, away from keyboard. I used main branch (had to tweak it to look like 3.10.3 otherwise it would not bundle in my project). After that, and using ruby 3, the test did not break (I was expecting it to break). My next step will be to prepare a gist that exposes the problem. Moreover, for this case I was using dry-initializer (could be an edge case). Will get back to you soon. Thank you very much. |
Can you reproduce it in a small snippet rather than a cloneable repo, and definietly without Rails? |
In the following script there are two types of test, one with mocks, the other without mocks. executing this code in ruby 3:
Currently, throughout our codebase, we use the version with mocks/spies. require 'rspec/core'
class Multiplier
def initialize(quotient:, data:)
@quotient = quotient
@data = data
end
def call
@data.map { |element| element * @quotient }
end
end
class Engine
attr_reader :multiplier
def initialize(data:)
@data = data
end
def call
# to work in ruby 3: Multiplier.new(**multiplier_params).call
Multiplier.new(multiplier_params).call
end
def multiplier_params
{
data: @data,
quotient: 2
}
end
end
RSpec.describe Engine do
let(:data) { [1, 2, 3] }
subject { Engine.new(data: data) }
describe "#call" do
it "returns the result of multipling by two" do
expect(subject.call).to match_array([2, 4, 6])
end
context "with mocks" do
let(:multiplier) { instance_double(Multiplier) }
let(:multiplier_result) { 'multiplier_result' }
before do
allow(Multiplier).to receive(:new).and_return(multiplier)
allow(multiplier).to receive(:call).and_return(multiplier_result)
end
it "calls multiplier" do
subject.call
expect(Multiplier).to have_received(:new).with(
data: data,
quotient: 2
)
expect(multiplier).to have_received(:call)
end
it "returns the multiplier result" do
expect(subject.call).to eq(multiplier_result)
end
end
end
end Gemfile:
|
In order to fetch the required params for the initialize method, its possible to use this:
source: https://bugs.ruby-lang.org/issues/17596 I think rspec-mocks could leverage from matching the initializer arity (as it currently does for doubles instance methods) |
Apologies for the delay here, I've been meaning to look into this more, we do in fact already validate arguments for We don't raise the original error because of implementation details (there isn't an original error to raise, we are checking captured arguments vs expected arguments) and changing that behaviour is out of scope really, as an aside I note our error diff could be improved to indicate that kw args and hashes are different |
I got bitten by something similar on keypup-io/cloudtasker#32 comment... I couldn't fix a "false positive" test that should been failing on Ruby 3... I've got this gist exposing the problem... Even |
The problem is that keyword arguments are hashes everywhere in Ruby except "strict keyword" ruby method definitions. Even on 3.1 a keyword splat is a hash.
|
They do not support it because all information about kwargs are not being preserved on method_missing calls. Proof: https://github.com/rspec/rspec-mocks/blob/main/lib/rspec/mocks/test_double.rb#L74 Ruby 3.0 and higher is not going to be able to determine if Hash.ruby2_keywords_hash? if it does not know that it should expect them there. Flag will be trimmed and keyword arguments will be treated as positional arguments. Same story goes to the HaveRecived Matcher which is doing neither https://github.com/rspec/rspec-mocks/blob/main/lib/rspec/mocks/matchers/have_received.rb#L53-L56 You need to wrap your arguments into Hash.ruby2_keywords_hash to make that working and even with this, you still cannot have proper match on the called method because method_missing is skipping that flag too. |
Subject of the issue
When preparing for ruby 3, I want the following deprecation warnings to be thrown when using mocks, so that I can update the code:
Your environment
configure application to show deprecations
via env variable:
or add to config/environments/test.rb
Steps to reproduce
Expected behavior
Moreover, I would expect it to fail in ruby 3 (as the code would not be executed in production).
and the block version can detect the kwargs problem, so it should work like it:
When using this approach we can correctly detect deprecated version ^
Actual behavior
All tests pass
Fix
monkey patching allows to throw errors
The text was updated successfully, but these errors were encountered: