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

have_json_fields matcher #25

Closed
wants to merge 1 commit into from
Closed

have_json_fields matcher #25

wants to merge 1 commit into from

Conversation

inossidabile
Copy link

This matcher allows to check existence of fields set ignoring their values. Can be useful for highly-random large hashes acceptance testing.

%({"a": "a", "b": "b", "c": "c"}).should have_json_fields(%w(a b c))

@laserlemon
Copy link
Contributor

This can be accomplished using the have_json_path matcher. For your example:

json = %({"a": "a", "b": "b", "c": "c"})
json.should have_json_path("a")
json.should have_json_path("b")
json.should have_json_path("c")

I hope that helps! Thank you for the pull request, but I think the existing functionality will suffice.

@laserlemon laserlemon closed this Apr 9, 2012
@inossidabile
Copy link
Author

And now imagine that with "foo/bar/0/a", "foo/bar/0/b" etc. And for 20 fields. Like 10x more code to write. Are you quite sure? :)

@inossidabile
Copy link
Author

I mean with have_json_path you can not use .at_path. With the new matcher you are supposed to.

@laserlemon
Copy link
Contributor

That's a good point. There are a few things I would change. To be consistent with JavaScript objects and Ruby hashes, I would rename the matcher to have_json_keys. I think it also needs some more bulletproof testing. And it would be great to have a Cucumber step that exposes the new matcher. Then to top it all off, the README would need some love to explain the new features.

Please feel free if you want to take a stab at those things. Otherwise, I will get to pulling this in and making those changes when I'm able.

Thank you! 👏

@laserlemon laserlemon reopened this Apr 9, 2012
@inossidabile
Copy link
Author

Yep, makes sense. I'll handle that tomorrow :)

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

Successfully merging this pull request may close these issues.

2 participants