rspec / rspec-mocks

RSpec's 'test double' framework, with support for stubbing and mocking
https://rspec.info
MIT License
1.16k stars 356 forks source link

Unexpected arguments error with correct hash value #1505

Closed dblock closed 2 years ago

dblock commented 2 years ago

Subject of the issue

Coming from https://github.com/ruby-grape/grape/issues/2247, getting unexpected arguments but expected value matches parameters.

Your environment

Steps to reproduce

I tried to get a small repro but so far have failed. If I have time will try to bisect next.

In Grape:

git clone git@github.com:ruby-grape/grape.git
rvm use 3.0.2
cd grape
git checkout 9687a88bafe8e75547f1fe5cb19e626b429ce475
bundle
rspec ./spec/grape/dsl/routing_spec.rb:19

Expected behavior

Arguments to match.

Actual behavior

  1) Grape::DSL::Routing.version sets a version for route
     Failure/Error: namespace_inheritable(:version_options, options)

       #<#<Class:0x000055f796d81f10> (class)> received :namespace_inheritable with unexpected arguments
         expected: (:version_options, {:using=>:path})
              got: (:version_options, {:using=>:path})
     # ./lib/grape/dsl/routing.rb:48:in `version'
     # ./spec/grape/dsl/routing_spec.rb:23:in `block (3 levels) in <module:DSL>'
$ bundle show rspec-expectations
/Users/dblock/.rvm/gems/ruby-3.0.2/gems/rspec-expectations-3.11.0
pirj commented 2 years ago

Error message for this is improved in this PR. Problem is this is using keyword arguments:

expect(subject).to receive(:namespace_inheritable).with(:version_options, using: :path)

and this is using positional hash:

options = options.reverse_merge(using: :path)
namespace_inheritable(:version_options, options)

I believe changing the spec to:

expect(subject).to receive(:namespace_inheritable).with(:version_options, {using: :path})

would fix it.

dblock commented 2 years ago

Spot on, thanks @pirj! Closing this since this is not a bug in rspec-expectations, and looking forward to #1461 to be merged.

DannyBen commented 2 years ago

This issue also baffled me. Here is a small repro case (passes in ruby < 3, fails in ruby >= 3)

# run with `rspec <this_file.rb>`
class Subject
  def method_missing(method_sym, *args)
    say method_sym, *args
  end

  def say(greeting, name, opts = {})
    puts "#{greeting} #{name} (opts: #{opts})"
  end
end

describe Subject do
  it "works" do
    expect(subject).to receive(:say).with :hello, "world", color: true
    subject.hello "world", color: true
  end
end

I understand how to work around it, and the trickiness of keyword arguments, but this is the error received:

expected: (:hello, "world", {:color=>true})
     got: (:hello, "world", {:color=>true})

and I wonder, if rspec already knows it is different, isn't there a way to specify how it is different? Just receiving strings that look identical, with a statement that they are not is confusing.

pirj commented 2 years ago

@DannyBen Error message for this is improved in https://github.com/rspec/rspec-mocks/pull/1461.

DannyBen commented 2 years ago

@DannyBen Error message for this is improved in rspec/rspec-mocks#1461.

Cool. So does it mean one of the upcoming versions will have this fixed? I am sorry, I am unfamiliar with which sub-gem is responsible for this behavior.

dblock commented 2 years ago

Once that PR is merged the error message will make sense. The spec is failing as expected though, so do fix your code as in https://github.com/ruby-grape/grape/pull/2248.

DannyBen commented 2 years ago

Cool, thanks!

The spec is failing as expected though

Of course, makes sense.