Make include_json a subset relation #62

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@myitcv

myitcv commented Dec 2, 2013

Rewrite of #61 which included a wrong branch name and commit message

@charlierudolph

This comment has been minimized.

Show comment Hide comment
@charlierudolph

charlierudolph Dec 2, 2013

Since you are looking for specifically a subset test, how about a new matcher instead of adding this into the include_json matcher. Maybe have_json_subset which returns false if actual is not a hash, otherwise returns whether or not expected is a subset of actual. The matcher should also allow at_path like a few of the other matchers. What do you think?

Since you are looking for specifically a subset test, how about a new matcher instead of adding this into the include_json matcher. Maybe have_json_subset which returns false if actual is not a hash, otherwise returns whether or not expected is a subset of actual. The matcher should also allow at_path like a few of the other matchers. What do you think?

@myitcv

This comment has been minimized.

Show comment Hide comment
@myitcv

myitcv Dec 2, 2013

Sounds good. I suspect that's probably the best way to go if you want to retain the include semantics.

Want me to go ahead and add the implementation to my pull request?

myitcv commented Dec 2, 2013

Sounds good. I suspect that's probably the best way to go if you want to retain the include semantics.

Want me to go ahead and add the implementation to my pull request?

@myitcv

This comment has been minimized.

Show comment Hide comment
@myitcv

myitcv Dec 2, 2013

Actually, probably makes more sense to close this PR and create another one with correct naming (given that we are no longer going to modify include_json)

myitcv commented Dec 2, 2013

Actually, probably makes more sense to close this PR and create another one with correct naming (given that we are no longer going to modify include_json)

@charlierudolph

This comment has been minimized.

Show comment Hide comment
@charlierudolph

charlierudolph Dec 2, 2013

Yeah, probably cleaner as a fresh PR. Thank you.

Yeah, probably cleaner as a fresh PR. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment