Set matching

[odobroiu]

Played a little with the compare as set option. From my point of view, not quite sure how I feel about this; so it's still in progress( did not do a few things yet) as I wanted to get some feedback/suggestions.

If you think that this can be improved and eventually merged, will probably do it in the next several days.

[andreineculau]

Ovidiu,

thanks for the PR, although I probably "broke" (merge conflicts) it in https://github.com/for-GET/katt/commit/5bd49a31c2b21d1f4783dbf313ba6e74cb191b64#diff-2

I think my commit was needed though - the compare function was esoteric at most - before we move forward with extending the comparison section even more.

1. Could you rebase and fix the conflicts of your PR? Thanks!
2. I don't know of any POV, but mine is: no -author, no copyright header. I will add you to the AUTHORS file and also update/add the copyright header to include a reference to the list in the AUTHORS file. Please add the emacs footer though :s
3. I am not a total fan, but please observe the 80-char limit
4. In https://github.com/for-GET/katt/pull/5/files#diff-0ca8b609d9263432236ebef8184c89c0R217 could you check instead if a {{type}} exists, and if it does, call katt_callbacks:validate_type (a new fun, which is very similar to katt_callbacks:validate_body but takes an extra argument Type /// it's fine if you don't follow me all the way, we can take this later
5. I haven't looked carefully at the katt_compare_as_set module, but is this as simple as it can get? :)

Thanks again for the PR!

[odobroiu]

Edited: added 5.

Started looking into your suggestions, when I realized that now you sort the items in the arrays by default. I do not think that is a good idea since now:

-> If you match [ 1, {{unexpected}} ] with [2,1], it will actually compare it with [1,2] and will return { fail, unexpected, "../body/../1 instead of "../body/../0 which might be confusing -> It will match [2,1] with [1,2] -> Unfortunately, json.org says specifically that the array is ordered.

I will address all your sugestions as soon as you decide what to do with the sorting. Related to those:

1. the author thing is something my ide adds up, will add your klarna copyright
2. not quite sure what needs to be done in this moment, but will take a closer look
3. the thing with that is that since we have to do the comparison using katt_util:compare ( did some changes earlier, in the PR that is missing ) we have to do a lot of comparison, so I tried to limit those. The first code i wrote for this went through all the expected elements, and for every one of them it would go through all the actual elements trying to find a match. We could go back to that, and maybe we should since I am not sure what is currently there is 100% functionally correct ( mainly due the ordering of elements which might not always hold).

[andreineculau]

Thanks for noticing the sorting issue. Should be fixed now. I added support for the validate_type extension (what i was explaining in the initial point 4) and based on that, I refactored your PR - see the https://github.com/for-GET/katt/tree/validate_type_set branch

That could be a starting point for future iterations. PS: I haven't digested your update (point 3)

[andreineculau]

@odobroiu Have a look at the new HEAD of the https://github.com/for-GET/katt/tree/validate_type_set branch - https://github.com/for-GET/katt/tree/9a66ffed80b54f2e082af89888be8c36106de633

I refactored the comparison to use katt_util:compare (now renamed to katt_util:validate). Does that do the job? In haste, I haven't "refactored" your eunit module for validate_type_set, which would/should still be good to have, before merging the branch into develop.

[odobroiu]

You were right about the unexpected thing, sorry for that. Reverted that change, and also rebased my last commits.

[andreineculau]

Nice! Merged into develop

Thank you for the contribution, Ovidiu!