kinbiko / jsonassert

A Go test assertion library for verifying that two representations of JSON are semantically equal
MIT License
125 stars 16 forks source link

Feature Request: support for partial JSON matching #20

Open pauloavelar opened 4 years ago

pauloavelar commented 4 years ago

Hello. First of all, great package!
It really reduces boilerplate code in tests and makes everything more readable.

I have a feature request/consideration I hope would get included to this project and I may help implement it if that's something that makes sense for this library: Partial JSON matching.

It can be behind a flag or in another method, but it basically means a JSON with 4 properties, when checked against a JSON with 2 properties, would not return the following error:

expected 2 keys at '$' but got 4 keys unexpected object key(s) ["status","cause"] found at '$'

More often than not, we need to check for the overall structure and main fields and not the whole thing, and having to check for presence of every single field can take a lot of time.

Is this something that makes sense for you as the author of this library?
As much as I like creating my own fork, I think it's best for the community if we keep things centralized.

Thanks.

kinbiko commented 4 years ago

Hi @pauloavelar,

Thanks for your kind words, and your interest in the package.

I think your proposal makes sense, and it's something I've been thinking of implementing myself. I'm probably going to be really busy for the next couple of months, but if you have the opportunity to, please feel free to implement it, and I'll make time to review it.

I'm thinking perhaps the following method can be implemented:

func (a *Asserter) AssertPartialf(path, actualJSON, expectedJSON string, fmtArgs ...interface{})

where path is the JSON-path. I think as long as it can handle a path like $.data.issues[3].name that will do the trick. We would also have to implement validation against the path parameter:

etc.

If find you don't have the time either, I might get back to it later. If you do find the time, please have a look at the developer guide I whipped up a long time ago. I think it's still accurate. I'll be sure to review it ASAP if you do raise a PR that passes the build.

How does that sound?

pauloavelar commented 4 years ago

Deal. I read the developer guide and it makes sense.
While I am not sure if I will have the time to get it done myself, I might as well try and see how it goes.

As for the function signature, it looks good as a check for a single property, which I think is also very useful. I had a different approach in mind (below) and depending on how it goes I will try to get both working.

func (a *Asserter) AssertContainsf(actualJSON, expectedJSON string, fmtArgs ...interface{})

The variable names perhaps could change to be clearer as to what they represent, but the idea is asserting that the expectedJSON is contained in the actualJSON, something along the lines of Spring's ContentResultMatchers.

kinbiko commented 4 years ago

If I understand correctly, that sounds like it would be a more powerful solution than my initial suggestion, and therefore better.

My understanding is something like this:

ja.AssertContainsf(`{"a": 1, "b": 2}`, `{"b": 2}`) // passes as everything in the expected JSON matches
ja.AssertContainsf(`{"a": 1, "b": 2}`, `{"c": 3}`) // fails as the actual JSON has no "c" property

Could you clarify how it would work with arrays? I'm not sure where to draw the line If the expected JSON contains an array with a single element, and the actual JSON contains multiple elements. Is it a case of:

  1. "does the element in the expected JSON appear anywhere in the array?"

or is it more strict?

  1. "does the element in the expected JSON appear at index 0 of the actual JSON?"

In the case of 1, partial assertions cannot be used if the order matters -- and this is invisible to a reader of the code that has not read the documentation of AssertContainsf (which presumably would mention that the order doesn't matter in arrays), leading to tests passing when they should be failing.

In the case of 2, we'd have to know exactly where in an array the expected data is. We could do something similar to "<<PRESENCE>>" (e.g. "<<UNORDERED>>") as a 0th-element-visual-cue for readers of the test code to indicate that we're ignoring the ordering of this particular array.

Out of these two, I much prefer the second option, but I'm guessing there's probably a better solution that I don't see just yet.

Perhaps you have an idea @pauloavelar? Do you know how Spring does it?

pauloavelar commented 4 years ago

I did not know how Spring handles arrays because it is not an assertion I often do.

When handling collections I usually deserialize the JSON into a model, iterate and assert its properties. Things can get verbose, but they are usually clearer that way.

That said, I have prepared a small example of how Spring's ResultMatchers work if you want to check it out.
Apparently they don't do any partial matching with arrays and completely disregard ordering.

Interesting notes:

kinbiko commented 4 years ago

Thank you @pauloavelar for doing this research and getting back to me. I'm not convinced I agree with the way Spring does things in this regard.

I'm starting work on this feature, following the approach I proposed in my previous comment (case 2.). I'll raise a PR, at the earliest next week (but probably later), with a proposed approach. Just to be clear, I'm implementing it to try it out in other projects I'm working on to verify the design. I might not like what I write, and end up dropping it.

If you have the time and once/if I'm personally happy with the solution I would love your opinion on it as well, seeing as you've contributed a great deal to designing this feature. If I end up scrapping it, I'll let you know.

kinbiko commented 4 years ago

Just a quick update: I ended up doing a bit of refactoring (currently in master, not released) that would make this change easier and I've started working on the implement-assertf branch, but life has gotten in the way of me progressing this beyond that. If anyone wants to help that'd be most welcome, otherwise I'll get around it eventually.

Tl;dr: Not scrapped, but also not expecting progress in the next few months.