lukas-krecan / JsonUnit

Compare JSON in your Unit Tests
Apache License 2.0
894 stars 115 forks source link

whenIgnoringPaths doesn't ignore missing paths #182

Closed joschi closed 5 years ago

joschi commented 5 years ago

I'm using the AssertJ integration of JsonUnit (net.javacrumbs.json-unit:json-unit-assertj:2.6.1) and expected JsonAssert#whenIgnoringPaths(String ...) to ignore missing paths/keys as well.

The documentation at https://github.com/lukas-krecan/JsonUnit/tree/json-unit-parent-2.6.1#ignorepaths says (emphasize by me):

whenIgnoringPaths configuration option makes JsonUnit ignore the specified paths in the actual value. If the path matches, it's completely ignored. It may be missing, null or have any value.

The following test is failing, though:

assertThatJson("{\"root\":{\"foo\":1}}")
        .whenIgnoringPaths("root.bar", "missing")
        .isEqualTo("{\"root\":{\"foo\":1, \"bar\":2}, \"missing\":{\"quux\":\"test\"}}");
net.javacrumbs.jsonunit.core.internal.JsonAssertError: JSON documents are different:
Different keys found in node "", missing: "missing", expected: <{"missing":{"quux":"test"},"root":{"bar":2,"foo":1}}> but was: <{"root":{"foo":1}}>
Different keys found in node "root", missing: "root.bar", expected: <{"bar":2,"foo":1}> but was: <{"foo":1}>

    at net.javacrumbs.jsonunit.core.internal.ExceptionUtils.createException(ExceptionUtils.java:50)
    at net.javacrumbs.jsonunit.core.internal.Diff.failIfDifferent(Diff.java:612)
    at net.javacrumbs.jsonunit.assertj.JsonAssert.isEqualTo(JsonAssert.java:106)
lukas-krecan commented 5 years ago

Hi, thanks for feedback. The comparison is not symmetric and it seems you have switched expected and actual value.

This should work

assertThatJson("{\"root\":{\"foo\":1, \"bar\":2}, \"missing\":{\"quux\":\"test\"}}")
        .whenIgnoringPaths("root.bar", "missing")
        .isEqualTo("{\"root\":{\"foo\":1}}");
joschi commented 5 years ago

@lukas-krecan Thanks for your response!

In my example "{\"root\":{\"foo\":1}}" is the actual value and "{\"root\":{\"foo\":1, \"bar\":2}, \"missing\":{\"quux\":\"test\"}}" is the expected value.

I would expect the "root.bar" and "missing" paths which do not exist in the actual value to be ignored in this case. Isn't that how it's supposed to work? At least the error message says it in the correct way with actual/expected value.

lukas-krecan commented 5 years ago

JsonUnit main focus are tests, not comparing two arbitrary JSONs. It's therefore built around the assumption that you have the expected value under control. So it does not make sense to ignore paths from expected value, because if you do not want some part of the JSON in the expected value, you simple do not put it there.

joschi commented 5 years ago

So it does not make sense to ignore paths from expected value, because if you do not want some part of the JSON in the expected value, you simple do not put it there.

Okay, got it. In this case, the documentation should be updated, because it says at https://github.com/lukas-krecan/JsonUnit/tree/json-unit-parent-2.6.1#ignorepaths:

whenIgnoringPaths configuration option makes JsonUnit ignore the specified paths in the actual value. If the path matches, it's completely ignored. It may be missing, null or have any value.

If I understood you correctly, whenIgnoringPaths will only ignore additional paths, i. e. keys in the actual JSON which do not exist in the expected JSON.

Alternatively, would you accept a PR to match the code to the documentation?

lukas-krecan commented 5 years ago

whenIgnoringPaths configuration option makes JsonUnit ignore the specified paths in the actual value. If the path matches, it's completely ignored. It may be missing, null or have any value.

The first sentence clearly states that it applies only to the actual value. I could repeat the words actual value in the second sentence to make it even more clear, but it feels redundant. In my opinion, the code matches the documentation, so no code changes are needed.

joschi commented 5 years ago

The first sentence clearly states that it applies only to the actual value.

Exactly, and whenIgnoringPaths doesn't ignore missing paths in the actual value. 😉

Anyway, thanks for your time!

lukas-krecan commented 5 years ago

Ah, I see. I still do not understand why would you put something you want to ignore to the expected value, but I agree that the current behavior might be confusing if you do.

joschi commented 5 years ago

@lukas-krecan Thanks for re-opening the issue!

I still do not understand why would you put something you want to ignore to the expected value

In the specific use case we're deserializing a JSON payload using Jackson into a POJO which doesn't map all the keys and check afterwards, if the mapped fields are there when serializing the POJO back into JSON. When serializing the POJO back into JSON, the unmapped fields are missing, and the original JSON payload still has them.

lukas-krecan commented 5 years ago

Released as 2.6.3

joschi commented 5 years ago

@lukas-krecan Thanks a lot!

joschi commented 5 years ago

Fixed via #184