jsonapi-rb / jsonapi-rspec

RSpec matchers for JSON:API spec
https://rubygems.org/gems/jsonapi-rspec
MIT License
123 stars 24 forks source link

No way to match on a subset of meta #10

Closed mykecameron closed 4 years ago

mykecameron commented 6 years ago

have_meta will only succeed on EXACT matches. Often I have shared examples that test for some omnipresent meta in responses across many endpoints (example, a UUID identifying the request which we use for debugging), but then some meta contents which are specific to a particular resource or API (example: pagination data for an index endpoint).

We added a contain_meta matcher for this in our applications, happy to share back w/ a PR.

Check out: https://github.com/jsonapi-rb/jsonapi-rspec/compare/master...mykecameron:add_partial_meta_matcher, I didn't want to open an unsolicited PR since its unclear to me what the best practices around contribution to this project are, or even if it is actively maintained. Any guidance would be appreciated!

stas commented 4 years ago

@mykecameron, apologies for the late reply.

I think it's better to make have_meta work as contain rather than equal. I don't think we should introduce a new matcher just for this. Alternatively, let's add an option for the matcher to chain on exactly this behavior.

I'll be happy to take a look if you send a PR. Thanks :bowing_man:

stas commented 4 years ago

Related #14 if somebody wants to pick it up :smiley:

sa73917 commented 4 years ago

@stas - something like this? will make have_meta work as a contain rather than an equal with the exactly chain. Though this does risk breaking people's existing tests who are expecting have_meta to be an exact match.

stas commented 4 years ago

@sa73917 yup, though the way you're checking for the subset could be a bit optimized, take a look at these sugestions (probably worth looking at the ruby v2.4+ compatibility though, so <= operator should be more than enough).

Thanks!

sa73917 commented 4 years ago

Changed the subset logic as suggested and raised a PR

stas commented 4 years ago

Fixed in #22 thanks to @sa73917!