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

Argument expectation is too permissive with actual keyword arguments #1478

Closed andrykonchin closed 2 years ago

andrykonchin commented 2 years ago

Subject of the issue

It's more a question and not an issue.

It seems to me that argument expectation (with with method) is too permissive and allows expecting successfully a Hash argument when actual is a keyword argument.

Your environment

Steps to reproduce

# 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 "rspec", "3.11.0" # Activate the gem and version you are reporting the issue against.
end

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

class A
  def foo(*args, **kw)
    [args, kw]
  end
end

RSpec.describe 'argument expectation' do
  it 'reproduces the issue' do
    a = A.new

    expect(a).to receive(:foo).with(1, 2, {c: 3})
    a.foo(1, 2, c: 3)
  end
end

Expected behavior

The test should fail because the last expected argument is a Hash, but the actual last argument is a keyword argument.

Actual behavior

The test passes successfully:

Fetching gem metadata from https://rubygems.org/...
Resolving dependencies...
Using bundler 2.2.32
Warning: the running version of Bundler (2.2.32) is older than the version that created the lockfile (2.3.19). We suggest you to upgrade to the version that created the lockfile by running `gem install bundler:2.3.19`.
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
Ruby version is: 3.0.3

Run options: include {:focus=>true}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 61040

argument expectation
  reproduces the issue

Finished in 0.00959 seconds (files took 0.21165 seconds to load)
1 example, 0 failures

Randomized with seed 61040
andrykonchin commented 2 years ago

The similar situation with

    expect(a).to receive(:foo).with(1, 2, c: 3)
    a.foo(1, 2, {c: 3})

(actual argument - Hash, expected - keyword argument)

Such test successfully passes.

pirj commented 2 years ago

Just by looking at those specs, do you think something is incorrect?

I tend to agree with what you say:

$ rvm use 3.0.1
$ irb
def x(a, b, c:)
  puts [a,b,c]
end
x(1, 2, {c:3})
ArgumentError: wrong number of arguments (given 3, expected 2; required keyword: c)

Would you like to send a PR to fix this case, or at least with failing examples?

andrykonchin commented 2 years ago

Ah, I see. Thank you.

Yes, I will try to find a workaround and create a PR.

andrykonchin commented 2 years ago

Looks like the issue is already fixed by https://github.com/rspec/rspec-mocks/pull/1473 on the main branch.

Thank you.