rspec / rspec-expectations

Provides a readable API to express expected outcomes of a code example
https://rspec.info
MIT License
1.26k stars 397 forks source link

hash check in `change` matcher causes unexpected failures #1283

Closed mockdeep closed 3 years ago

mockdeep commented 3 years ago

Subject of the issue

When comparing has_many associations in Rails, the change matcher fails unexpectedly:

     Failure/Error: expect {}.not_to change { user.reload.tasks }.from([])
       expected `user.reload.tasks` not to have changed, but did change from #<ActiveRecord::Associations::CollectionProxy []> to #<ActiveRecord::Associations::CollectionProxy []>

While the results are functionally equivalent, their #hash values are different:

From: /home/fletch/.rvm/gems/ruby-2.6.6/gems/rspec-expectations-3.10.1/lib/rspec/matchers/built_in/change.rb:394 RSpec::Matchers::BuiltIn::ChangeDetails#changed?:

    380: def changed?
    381:   # Consider it changed if either:
    382:   #
    383:   # - The before/after values are unequal
    384:   # - The before/after values have different hash values
    385:   #
    386:   # The latter case specifically handles the case when the value proc
    387:   # returns the exact same object, but it has been mutated.
    388:   #
    389:   # Note that it is not sufficient to only check the hashes; it is
    390:   # possible for two values to be unequal (and of different classes)
    391:   # but to return the same hash value. Also, some objects may change
    392:   # their hash after being compared with `==`/`!=`.
    393:   binding.pry
 => 394:   @actual_before != @actual_after || @before_hash != @actual_hash
    395: end

[3] Ik12 test (#<RSpec::Matchers::BuiltIn::ChangeDetails>)> @actual_before == @actual_after
=> true
[4] Ik12 test (#<RSpec::Matchers::BuiltIn::ChangeDetails>)> @actual_before
=> []
[5] Ik12 test (#<RSpec::Matchers::BuiltIn::ChangeDetails>)> @actual_after
=> []
[6] Ik12 test (#<RSpec::Matchers::BuiltIn::ChangeDetails>)> @actual_before.class
=> Campaign::ActiveRecord_Associations_CollectionProxy
[7] Ik12 test (#<RSpec::Matchers::BuiltIn::ChangeDetails>)> @actual_after.class
=> Campaign::ActiveRecord_Associations_CollectionProxy
[8] Ik12 test (#<RSpec::Matchers::BuiltIn::ChangeDetails>)> @actual_before.hash
=> -687641799465686215
[9] Ik12 test (#<RSpec::Matchers::BuiltIn::ChangeDetails>)> @actual_after.hash
=> 2644692918628454529

Your environment

Steps to reproduce

I tested this on a couple of Rails projects and saw the same thing. As far as I can tell, it's any has_many relationship:

class User < ActiveRecord::Base
  has_many :tasks
end

class Task < ActiveRecord::Base
  belongs_to :user
end

Expected behavior

The test should not fail when collections are equal.

Actual behavior

The test fails.

pirj commented 3 years ago

Thanks for reporting. This is a well-known issue, see https://github.com/rspec/rspec-rails/issues/1173. I'll close this, as this is specific to Rails mostly.