rspec-mocks
rspec-mocks copied to clipboard
Verifying doubles do not differentiate between a Hash and keyword args in Ruby 3
Subject of the issue
When using an instance_double
of a class that defines an instance method that takes keyword arguments, then using allow
or expect
to stub that method in the verifying double, the method can be called on the verifying double with either keywords or a Hash object passed. This happens whether or not with
is used when setting up the stub.
When using live objects, passing a Hash without a double-splat raises ArgumentError
in Ruby 3 while it worked fine in Ruby 2.7. The fact that tests don't fail in this situation in Ruby 3 has made upgrading Ruby in a large project more complicated.
Your environment
- Ruby version: 3.1.2
- rspec-mocks version: 3.11.1
Steps to reproduce
# frozen_string_literal: true
require "rubygems/source"
require "bundler/inline"
gemfile(true) do
source "https://rubygems.org"
gem "rspec", "3.11.0"
gem "rspec-mocks", "3.11.1"
end
require "rspec/autorun"
RSpec.describe "args broken" do
let(:klass) do
Class.new do
def m(keyword:); end
end
end
it "works as expected in Ruby 3" do
# Pass
expect { klass.new.m(keyword: "value") }.to_not raise_error
expect { klass.new.m(**{ keyword: "value" }) }.to_not raise_error
expect { klass.new.m({ keyword: "value" }) }.to raise_error(ArgumentError)
end
it "works as expected with instance_double in Ruby 3" do
instance = instance_double(klass)
allow(instance).to receive(:m).with(keyword: "value")
# Pass
expect { instance.m(keyword: "value") }.to_not raise_error
expect { instance.m(**{ keyword: "value" }) }.to_not raise_error
# Fails
expect { instance.m({ keyword: "value" }) }.to raise_error(ArgumentError)
end
end
Expected behavior
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies...
Using bundler 2.3.10
Using diff-lcs 1.5.0
Using rspec-support 3.11.0
Using rspec-core 3.11.0
Using rspec-expectations 3.11.0
Using rspec-mocks 3.11.1
Using rspec 3.11.0
..
Finished in 0.00919 seconds (files took 0.0839 seconds to load)
2 examples, 0 failures
Actual behavior
Fetching gem metadata from https://rubygems.org/..
Resolving dependencies...
Using bundler 2.3.10
Using diff-lcs 1.5.0
Using rspec-support 3.11.0
Using rspec-mocks 3.11.1
Using rspec-core 3.11.0
Using rspec-expectations 3.11.0
Using rspec 3.11.0
.F
Failures:
1) args broken works as expected with instance_double in Ruby 3
Failure/Error: expect { instance.m({ keyword: "value" }) }.to raise_error(ArgumentError)
expected ArgumentError but nothing was raised
# rspec-mocks-bug.rb:37:in `block (2 levels) in <main>'
Finished in 0.01756 seconds (files took 0.07066 seconds to load)
2 examples, 1 failure
Failed examples:
rspec rspec-mocks-bug.rb:29 # args broken works as expected with instance_double in Ruby 3
This is at least partially fixed by https://github.com/rspec/rspec-mocks/pull/1473 (when with
is used to set up the stub, ArgumentError
isn't raised but RSpec::Mocks::MockExpectationError
is) so if the answer is to just wait for that to be released, I'd understand. When with
is not used, though, there's still something I find confusing even with 1473 applied:
it do
instance = instance_double(klass)
# .with is not used for the stub
allow(instance).to receive(:m)
# These pass, showing that arguments are verified at least a little bit
expect { instance.m(:totally_wrong) }.to raise_error(ArgumentError)
expect { instance.m(wrong: "value") }.to raise_error(ArgumentError)
expect { instance.m({ wrong: "value" }) }.to raise_error(ArgumentError)
# Fails
expect { instance.m({ keyword: "value" }) }.to raise_error(StandardError)
end
I'm running into this with the latest rspec-mocks (version 3.12.5). It's making the upgrade from Ruby 2.7 to 3.x, extra painful.
When
with
is not used, though, there's still something I find confusing even with 1473 applied:
@honigc - could you elaborate here? Is the confusing bit that RSpec::Mocks::MockExpectationError
is not a subclass of StandardError
? (I assume that's necessary for the broad error-handling rspec has to do to properly catch and expose the exceptions produced by the code under test as being separate from the exceptions produced by the mocks/matchers)
I'm curious if there's still any change needed here.
I assume that's necessary for the broad error-handling rspec has to do to properly catch and expose the exceptions produced by the code under test as being separate from the exceptions produced by the mocks/matchers
Correct, it is because rescue
by default catches StandardError
, so we raise errors inherited from Exception
to try to avoid your code swallowing spec failures.