lukas-krecan / JsonUnit

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

How to validate a java.time.Duration? #653

Closed hjohn closed 1 year ago

hjohn commented 1 year ago

Describe the bug

My problem is that none of these assertions pass:

    assertThatJson(Duration.ofSeconds(10)).isEqualTo("10.0");  // Different value found in node "", expected: <10.0> but was: <1E+1>
    assertThatJson(Duration.ofSeconds(10)).isEqualTo("1E+1");  // Different value found in node "", expected: <10.0> but was: <1E+1>
    assertThatJson(Duration.ofSeconds(10)).isEqualTo("10");  // Different value found in node "", expected: <10> but was: <1E+1>.

So I think there is either a bug in:

  1. The code that is validating this
  2. The message that is returned as it is not helpful

Note that this is a simplified example; the problem also occurs when the Duration is nested in some other JSON node.

lukas-krecan commented 1 year ago

Hi, the following is happening:

  1. You are trying to pretend that Duration is JSON and compare it to a string
  2. The only thing JsonUnit can do is to serialize Duration to JSON. Dependening on the JSON library being used and its configuraiton the result may be different. Here it seems that it was serialized to a float. If you are using Jackson, you can configure it to serialize it in a different way.
  3. If you really think it's a good idea to have a test like this, read the JsonUnit documentation about Numerical comparison.
  4. I agree that the error message is confusing but the example is so contrieved my brain refuses to think about it 🙂
hjohn commented 1 year ago

Look at this line:

assertThatJson(Duration.ofSeconds(10)).isEqualTo("1E+1");

Note that I expect: 1E+1

But this was apparently converted somewhere, because the error message says I expected 10.0, which is not accurate. I'm fine with that as they are equivalent. However, this then no longer matches how Duration was converted (which is 1E+1). This really does seem like an internal problem?

Edit: there should be something I can put in isEqualTo that matches, shouldn't there be? I mean, JSON is text, I should be able to match this text.

hjohn commented 1 year ago

Also:

You are trying to pretend that Duration is JSON and compare it to a string

Shouldn't this then throw an exception instead of pretending to work in a way users won't expect? I think it's perfectly fine to offer an option to serialize arbitrary Java trees -- my problem is that I'd expect some (I don't care what) JSON output to result from this, that I can match with an expected JSON. However, apparently there are cases where the resulting "JSON" can not be matched in any way with another expected JSON -- this is due to some internal representation, which apparently isn't quite JSON, because if it was JSON, I could find a matching expected JSON for it...

hjohn commented 1 year ago

Let's say I call assertThatJson(Duration.ofSeconds(10)). What would the JSON be in which this results? For example, it could be a single value, which is valid JSON:

 10

Or it could be some kind of toString call on it:

 "PT10S"

Or anything else really.

So, you would think I could match this by providing the same representation:

 10

Or

 "PT10S"

But no matter what I provide, be it 10, 10.0, 1E+1, nothing matches. This is odd because a JSON document, if you manage to generate one, should always have a match (its duplicate).

So, internally, it seems that it is not quite represented as actual JSON to make the match, because it seems to be calling Duration#equals. What this has to do with anything is unclear to me.

I realize that a Duration is not a JsonNode or one of Map, List, integer, float, boolean, text. But if you accept this parameter for assertThatJson, then you must be converting it into one of these. The error message seems to indicate it is converting it to a float 1E+1, yet does not allow me to match it. I only want to validate the contents of a tree of java objects to see that it never changes due to a new change that is introduced. I could care less about the accuracy concerns, or that a Duration isn't JSON. I'm not asking for silly stuff like 1.00 should not match 1.0, just that 1E+1 will actually match 1E+1 if I provide the exact same thing.

lukas-krecan commented 1 year ago

Can you please specify which JSON library you are using?

This is how it works in Jackson with jsr310 module. Duration.ofSeconds(10) get's serialized as DecimalNode("10.000000000"). That's why the assertion assertThatJson(Duration.ofSeconds(10)).isEqualTo("10.000000000"); passes.

If you do not care about the representation but just about the value, you can use assertThatJson(Duration.ofSeconds(10)).isNumber().isEqualByComparingTo("10");

lukas-krecan commented 1 year ago

In some configurations, Jackson serializes it as 1E+1, in which case comparison with 1E+1 works

hjohn commented 1 year ago

Thanks for your reply, it looks like there is a lot more magic going on the background than I expected, especially with the detection of a suitable JSON library.

I can't really see which library JsonUnit is using; both Jackson and gson are there (thank you Spring), but read on for a separate test as well...

In the large Spring project, none of these tests pass, except test7. The failing tests all complain about Different value found in node "time", expected: XXX but was: <1E+1>. No matter what I do, I can't get that 1E+1 matched (aside from "${json-unit.ignore}")

private Map<String, Object> data = Map.of(
    "time", Duration.ofSeconds(10)
);

@Test
public void test1() {
    assertThatJson(data).isEqualTo("""
        {"time": 10}
    """);
}

@Test
public void test2() {
    assertThatJson(data).isEqualTo("""
        {"time": 1E+1}
    """);
}

@Test
public void test3() {
    assertThatJson(data).isEqualTo("""
        {"time": 10.0}
    """);
}

@Test
public void test4() {
    assertThatJson(data).isEqualTo("""
        {"time": 10.000000000}
    """);
}

@Test
public void test5() {
    assertThatJson(data).isEqualTo("""
        {"time": "10.000000000"}
    """);
}

@Test
public void test6() {
    assertThatJson(data).isEqualTo("""
        {"time": "1E+1"}
    """);
}

@Test
public void test7() throws JsonProcessingException {
    assertThatJson(new ObjectMapper().registerModule(new JavaTimeModule()).writeValueAsString(data)).isEqualTo("""
        {"time": 10.0}
    """);
}

@Test
public void test8() throws JsonProcessingException {
    assertThatJson(new ObjectMapper().registerModule(new JavaTimeModule()).writeValueAsString(data)).isEqualTo("""
        {"time": 10.000000000}
    """);
}

@Test
public void test9() throws JsonProcessingException {
    assertThatJson(new ObjectMapper().registerModule(new JavaTimeModule()).convertValue(data, JsonNode.class)).isEqualTo("""
        {"time": 1E+1}
    """);
}

I've since then isolated these tests into a new project with only Jackson and its time module:

    <dependency>
      <groupId>net.javacrumbs.json-unit</groupId>
      <artifactId>json-unit-assertj</artifactId>
      <version>3.0.0</version>
    </dependency>
    <dependency>
      <groupId>org.junit.jupiter</groupId>
      <artifactId>junit-jupiter</artifactId>
      <version>5.10.0</version>
    </dependency>
    <dependency>
      <groupId>com.fasterxml.jackson.core</groupId>
      <artifactId>jackson-databind</artifactId>
      <version>2.15.2</version>
    </dependency>
    <dependency>
      <groupId>com.fasterxml.jackson.datatype</groupId>
      <artifactId>jackson-datatype-jsr310</artifactId>
      <version>2.15.2</version>
    </dependency>

Now the same tests above all fail except test4, test8, test9. The error message for the failing tests has changed to: Different value found in node "time", expected: XXX but was: <10.000000000>. The test converting to JsonNode requires 1E+1 to pass.

Since test7 and test8 are now behaving differently, it seems JsonUnit is picking up something other than Jackson (probably gson?) in the large project. What is also however very strange is the different behavior of test7 and test8. It seems that even Jackson itself is somehow working differently in the two projects, despite using the exact same Maven version -- still, I assume that JsonUnit will take the given String and still use gson/Jackson on it, so that might explain the difference still.

All in all this is not what I expected, and it seems to be hard to get consistent behavior even when specifically serializing the Json data yourself (using writeValueAsString or convert) -- I haven't been able to find one yet that would work consistently in both projects. Even when I add:

static {
    System.setProperty("json-unit.libraries", "jackson2");
}

The tests that pass still differ in both projects. In the Spring project, none pass except test7 and test8 now (an improvement I suppose), while in the stripped down project nothing changes (but that's expected since only Jackson is available). At least I have one test now (test8) that works in both projects.