rspec / rspec-mocks

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

Unexpected arguments error (keywords vs hash) has misleading message with another argument #1504

Closed mvidner closed 1 year ago

mvidner commented 1 year ago

Subject of the issue

This is a continuation of rspec/rspec-mocks#1505. The fix https://github.com/rspec/rspec-mocks/pull/1461 seems incomplete.

When my test contains a wrong with expectation, where the actual arguments are keywords and I use a hash (or the other way around), RSpec reports an error. And in most cases the message correctly points out the problem, per 1461.

But in my case I got a misleading message:

Failures:

  1) MyClass#outer calls #inner with a tag and a hash
     Failure/Error: inner(Tag.new("a tag"), a_hash)

       #<MyClass:0x00007fca6cf24638> received :inner with unexpected arguments
         expected: (#<IdTag:0x00007fca69849668 @id="a tag">, {:one=>1})
              got: (#<IdTag:0x00007fca69831298 @id="a tag">, {:one=>1})
       Diff:
       @@ -1 +1 @@
       -[#<IdTag:0x00007fca69849668 @id="a tag">, {:one=>1}]
       +[#<IdTag:0x00007fca69831298 @id="a tag">, {:one=>1}]

     # ./spec/bug_spec.rb:22:in `outer'
     # ./spec/bug_spec.rb:30:in `block (3 levels) in <top (required)>'

I've traced it down to IdTag instances comparing as equal yet having different inspect representations.

Your environment

Steps to reproduce

spec/bug_spec.rb:

class IdTag
  attr_accessor :id

  def initialize(id)
    @id = id
  end

  def ==(other)
    other.class == self.class && other.id == id
  end
end

Tag = IdTag
# these would make the bug go away
# Tag = String
# Tag = Struct.new(:id)

class MyClass
  def inner(tag, hash)
    puts "Inner: #{tag.inspect}, #{hash.inspect}"
  end

  def outer
    a_hash = { one: 1 }
    inner(Tag.new("a tag"), a_hash)
  end
end

describe MyClass do
  describe "#outer" do
    it "calls #inner with a tag and a hash" do
      # This expectation is wrong: it actually expects keyword arguments
      expect(subject).to receive(:inner).with(Tag.new("a tag"), one: 1)
      subject.outer
    end
  end
end

Gemfile:

source "https://rubygems.org"

gem "rspec-mocks", "= 3.12.0"
gem "rspec", "= 3.12.0"
gem "rspec-core", "= 3.12.0"
gem "rspec-expectations", "= 3.12.0"
gem "rspec-support", "= 3.12.0"

Expected behavior

Even in this case it should point out the difference between keyword and hash arguments

pirj commented 1 year ago

A PR is welcome

JonRowe commented 1 year ago

So theres a couple of things going on here, firstly the reason why this:

Tag = IdTag
# these would make the bug go away
# Tag = String
# Tag = Struct.new(:id)

Is the case, is because implementing #== is not enough for RSpec to match arguments here, you need to implement #=== this means your spec is failing because of your implementation as well as the keyword vs hash difference, this is why RSpec is not producing the keyword / hash message, it thinks there is a failure already.

Now as it turns out this is also a keyword vs hash mismatch, which is shortcutting the failure process but thats not why the spec is failing from the perspective of the differ. This is behaviour we can improve on however, see #1506 which will output the keyword/hash difference whenever it is different rather than only when they are otherwise equal.

(Also transfered this issue and the original linked issue because they are rspec-mocks issues, not rspec-expectations)

mvidner commented 1 year ago

OK, thanks, the removal of && expected_args == actual_args in #1506 was also my planned direction of the fix.

Now,

Is the case, is because implementing #== is not enough for RSpec to match arguments here, you need to implement #=== this means your spec is failing because of your implementation as well as the keyword vs hash difference, this is why RSpec is not producing the keyword / hash message, it thinks there is a failure already.

  1. Ah, good point, a careful reading of the rspec-mocks README reveals:

    Argument Matchers: Arguments that are passed to with are compared with actual arguments received using ===.

    Which makes sense, with wants a matcher which can work in more ways than simple == equality.

  2. But if I change the expectation to really use a hash, it passes

    -      expect(subject).to receive(:inner).with(Tag.new("a tag"), one: 1)
    +      expect(subject).to receive(:inner).with(Tag.new("a tag"), { one: 1 })

    and apparently it's because IdTag#=== calls IdTag#== by default: https://docs.ruby-lang.org/en/3.0/Object.html#method-i-3D-3D-3D

    Which also makes sense: if I don't want special matching, equal objects should match.