Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hash_including deviates from a_hash_including #1558

Open
jasonkarns opened this issue Nov 14, 2023 · 1 comment
Open

hash_including deviates from a_hash_including #1558

jasonkarns opened this issue Nov 14, 2023 · 1 comment

Comments

@jasonkarns
Copy link

jasonkarns commented Nov 14, 2023

hash_including does not duck-type

a_hash_including from rspec-expectations (via the underlying include matcher) supports matching expected hashes against hash-like things (so long as they implement #to_hash). However, hash_including argument matcher from rspec-mocks does not support this hash-like comparison. This is primarily a problem because, it's not immediately obvious that hash_including and a_hash_including are different matchers with different implementations. (owing to the fact that rspec supports such a wide array of aliased matchers, one could be forgiven for assuming the a_ prefix is simply an alias)

Since there have been previous discussion about unifying the matchers between rspec-mocks and rspec-expectations, it seems to me that there may be an expectation that these two matchers should behave similarly?

a_hash_including gained duck-typing in rspec/rspec-expectations#1012

and currently casts to Hash via: https://github.com/rspec/rspec-expectations/blob/562d327cb951575d13db2a43b232c9ca1456ea6a/lib/rspec/matchers/built_in/include.rb#L78

Even if hash_including and a_hash_including are not expected to be inter-changeable, I think it would be valuable for hash_including to duck-type. The ability of rspec's matchers to nest is one of the most powerful features, IMO. Being able to leverage the nestable matchers against objects that are convertable to Hash makes tests very clean (and the diffs are great!).

The current workaround in our test suites, of course, is to properly use a_hash_including instead of hash_including. However, as mentioned before, lots of devs likely believe (as I did) these to be aliases. I've seen devs run into the no-method :key? error with hash_including and immediately bail to some rather gross assertions instead of thinking to try a_hash_including.

Your environment

  • Ruby version: ruby 3.1.3p185 (2022-11-24 revision 1a6b16756e) [arm64-darwin22]
  • rspec-mocks version: 3.12.6
    rspec-core (3.12.2)
    rspec-expectations (3.12.3)
    rspec-its (1.3.0)
    rspec-mocks (3.12.6)
    rspec-parameterized (1.0.0)
    rspec-parameterized-core (1.0.0)
    rspec-parameterized-table_syntax (1.0.0)
    rspec-rails (6.0.3)
    rspec-sidekiq (4.0.2)
    rspec-support (3.12.1)

Steps to reproduce

where foo and bar are custom objects that implement #to_hash

expect([foo,bar]).to contain_exactly(
hash_including(thing: "val"),
hash_including(thing: "other")
)

Expected behavior

I'd expect the hash_including matcher to send #to_hash to the actuals before fuzzy matching.

Actual behavior

       expected collection contained:  [hash_including(thing: "val"), hash_including(thing: "other")]
       actual collection contained:    [#<GraphitiSpecHelpers::Node:0x000000011b037958 @attributes={"id"=>"130", "jsonapi_type"=>"versions",... not create a versions record for document attachments" (./spec/api/v1/services/show_spec.rb:256)>>]
       the missing elements were:      [hash_including(thing: "val"), hash_including(thing: "other")]
       the extra elements were:        [#<GraphitiSpecHelpers::Node:0x000000011b037958 @attributes={"id"=>"130", "jsonapi_type"=>"versions",... not create a versions record for document attachments" (./spec/api/v1/services/show_spec.rb:256)>>]

(Another implementation of the custom classes would raise

    GraphitiSpecHelpers::Errors::NoAttribute:
      No attribute 'key?' in JSON response node!

because the GraphitiSpecHelpers::Node class implements method_missing, but not respond_to_missing properly.)

@JonRowe
Copy link
Member

JonRowe commented Dec 6, 2023

Sorry for the delayed response, we never did really settle on anything beyond "behaviour in mocks without expectations" is to be considered "lite" and really argument matchers are expected to leverage their expectations companions if possible, if you want to either bring mocks up to par with expectations or actualy check for the prescence of the expectations matcher and use that if possible I'd review either PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants