Add have_json_subset/have_json_superset matcher

[myitcv]

With due reference to #61 and #62

This is a first cut of a new be_json_subset matcher.

Tests and potential update to README included.

The one thing I found whilst doing this (leaning heavily on the tests for be_json_eql) is that the notion of at_path reads one way be is interpreted another. Here is a working test against the new matcher:

%({"json":{"laser":"lemon"}}).should be_json_subset(%({"laser":"lemon"})).at_path("json")

The following pseudo code however reads better given what was intended:

%({"json":{"laser":"lemon"}}).at_path("json").should be_json_subset(%({"laser":"lemon"}))

So whilst I have implemented at_path (again leaning heavily on the be_json_eql implementation) it doesn't read well to my mind. But this is really a separate issue.

Thoughts welcomed on the PR.

[charlierudolph]

1. My initial thought for this was naming it have_json_subset. I like to hear your thoughts comparing that with be_json_subset.
2. In order to put at_path on the object as you've suggested, we would need to write a core extension to the string class that decodes the json, traverses down to the path, and encodes the json at that path. That does read better but it also adds an extra encode and decode.

[charlierudolph]

Thinking more on the name, I mistook what you were implementing.

a.should be_json_subset b     # a should be a subset of b
a.should have_json_subset b   # b should be a subset of a

I wonder what is needed more. Once we implement one, the other can be easily implemented by simply calling the other.

Personally I assume have_json_subset is more likely to be used as you are checking that at some specific key-value pairs are present. Have you found a specific time when be_json_subset is more specifically what you want?

[myitcv]

You pre-empted my next comment/PR.... when I was going to request that we add superset.

But have_json_subset is equivalent to be_json_superset:

b.should be_json_subset a === a.should be_json_superset b
a.should have_json_subset b === a.should be_json_superset b

I wonder too whether be_json_subset is actually clearer as be_json_subset_of - but then that's maybe too verbose. Ditto for be_json_superset -> be_json_superset_of... if we implement that.

[myitcv]

@charlierudolph - any thoughts/updates on this?

[ryansch]

I took a pass at something similar in #57. I originally agreed that it wasn't needed but I've hit some other use cases and we're actually using that branch in our application currently.

[charlierudolph]

Personally have_json_subset makes more sense to me as you will be testing that the actual has expected as subset. Testing that actual is a subset of expected seems weird to me. Do you have an example where you want to write a test that way?

[myitcv]

Would tend to agree with your comments on have_json_subset. Right now I can't think of a good scenario where the reverse is true - indeed every scenario so far has caused me to write things the 'wrong' way round. If we had have_json_subset it would be back to the 'right' way around.

I will update my commits on this PR.

[ryansch]

I also agree. That's a cleaner way of thinking about it.

[myitcv]

I've revisited this with the following in mind:

1. the move to RSpec 3 - should-based syntax won't be around for ever
2. the language used when describing subsets and supersets - a is a subset of b, rather than b has a subset a

Consider two 'bits' of JSON, a and b. If a is a subset of b (language from point 2 above), the best matcher equivalent, I suggest, is:

a = %({ "apple": "green" })
b = %({ "apple": "green", "banana": "yellow" })

expect(a).to be_json_subset_of b
expect(b).to be_json_superset_of a

Using both superset and subset we can, as shown, expect the actual to be a subset of the expected without needing to expect the expected to be a subset of actual (if that makes sense)

Updated commits pushed

Thoughts?

[ryansch]

This still feels backwards to me. When I'm testing our json api, I usually want to assert that the response has an object which has a particular json subset in it.

expect(response.body).to have_json_subset({id: 123}.to_json)

Your proposed naming would lead to:

expect({id: 123}.to_json).to be_json_subset_of response.body
# or
expect(response.body).to be_json_superset_of({id: 123}.to_json)

The be_json_superset_of example isn't bad but I still feel like the have_json_subset matcher more closely aligns with my intent during the test.

[myitcv]

@ryansch - thanks for the feedback. After a more careful reading of the link, I'm going to take back my comments in point 2 above. So will move be_json_superset_of -> have_json_subset and be_json_subset_of -> have_json_superset... almost back to where we started :)

[ryansch]

😄

[myitcv]

And with that commit we are hopefully done :-)

a = %({ "apple": "green" })
b = %({ "apple": "green", "banana": "yellow" })

expect(b).to have_json_subset a
expect(a).to have_json_superset b

Thoughts welcomed

[charlierudolph]

Code looks good. The main thing I'm wondering is do we need the have_json_superset. As you last comment shows if you have one, you don't really need the other. And personally I can't think of a use case where someone would use have_json_superset.

[myitcv]

Quite agree we could get away with one. But in some situations I need one and not the other so my test code 'looks' (and I realise that's very subjective) more readable.

For example:

it 'should work' do
  example = Fabricate.to_params(:example)
  response = make_request req_body: example.to_json

  # consistency in terms of expectations if
  # we can use either subset/superset
  expect(response.status).to eq 201
  expect(response.body).to have_json_subset example.to_json

  # as opposed to (slightly contrived in the case of my example)
  example = Fabricate.to_params(:example)
  response = make_request req_body: example.to_json
  expect(response.status).to eq 201
  expect(example.to_json).to have_json_superset response.body
  #      ^^^^^^^^^^^^^^^ <- 'wrong' way round -> ^^^^^^^^^^^^^
end

Make sense? If it's cheap to keep both then that would be my vote.

[charlierudolph]

Okay. Can you give me an example where have_json_superset makes your code more readable?

[myitcv]

Hands up - no, I can't, not right now :) Largely because all our JSON tests are linked to testing an API!

So you keen to leave have_json_superset out for now.... and add in when there's a use case? I can get on board with that.

[charlierudolph]

Yep that sounds good to me

[myitcv]

Ok - this finally should be done. Sorry for the delay

[myitcv]

Closing as this is very old...