waterlink / rspec-json_expectations

Set of matchers and helpers to allow you test your APIs responses like a pro.
https://www.relishapp.com/waterlink/rspec-json-expectations/docs
MIT License
140 stars 23 forks source link

include_unordered_json does not take missing items into account #22

Closed jeroenj closed 7 years ago

jeroenj commented 7 years ago

If the expected Array has less values than the actual Array the matcher succeeds:

expect(%(["foo", "bar", "baz"])).to include_unordered_json(["foo", "bar"]) # this will pass

/cc @esjee (since it was your contribution in #17)

jeroenj commented 7 years ago

I might try to look into this this week if I find the time.

esjee commented 7 years ago

Oh no! Nice find! I'll also try to find time for this if you don't beat me to it. I'll post a message here if I pick it up, to avoid wasting your time.

jeroenj commented 7 years ago

Great. I'll do that too. I have quite some work on my plate this week (and the following weeks). We've worked around it for now, but I'm still planning to fix it. :)

esjee commented 7 years ago

Upon closer look, I'm wondering if this isn't expected behavior for something that "includes". It looks like you want stricter matching, similar to match_array/contain_exactly.

What do you think of adding a match_unordered_json matcher that does this?

# Pass
expect(%(["foo", "bar", "baz"])).to include_unordered_json(["foo", "bar"])  # rspec-json_expectations
expect(["foo", "bar", "baz"]).to include("foo", "bar")  # built-in matcher

# Fail
expect(%(["foo", "bar", "baz"])).to match_unordered_json(["foo", "bar"])  # example, this matcher does not exist
expect(["foo", "bar", "baz"]).to contain_exactly("foo", "bar")  # built-in matcher
expect(["foo", "bar", "baz"]).to match_array(["foo", "bar"]) # built-in matcher

@jeroenj What do you think?

jeroenj commented 7 years ago

Looks like a good idea but it might add some confusion. Maybe the naming wasn't optimal to start with. I haven't checked but I'd say that match_json and match_unordered_json would have been more clear to start with.

I'd see what @waterlink has to say about it. Having match_unordered_json next to include_unordered_json would definately be a step in the right direction. 👍

esjee commented 7 years ago

Hey @jeroenj, just wanted to say I'm trying to create a PR that implements match_unordered_json. While you may expect some rough edges, I figured a PR would be a good way to get @waterlink's thoughts on this, and see where can go.

jeroenj commented 7 years ago

Awesome. I didn't got the time yet to look into it, but I'll give your PR a try when it's available. 👍

waterlink commented 7 years ago

@jeroenj @esjee I have released this as a part of 2.0.0 version