11

I'm currently in the process of upgrading a few projects from Java 8 to Java 11 where one of the unit tests for a converter failed. Basically the problem stems from the equality check failing due to a a date precision which previously passed with JDK 8.

Here's a section sample of the test, I've moved the contents of the converter for clarity:

@Test
public void testDateTime() {
    LocalDateTime expected = LocalDateTime.now().plusDays(1L);

    // converter contents
    long epochMillis = expected.atZone(ZoneId.systemDefault())
            .toInstant().toEpochMilli();
    LocalDateTime actual = LocalDateTime.ofInstant(Instant.ofEpochMilli(epochMillis),
            TimeZone.getDefault().toZoneId());


    assertThat(actual, equalTo(expected));
}

This results to an assetion error due to:

Expected :<2021-06-02T14:06:21.820299>
Actual   :<2021-06-02T14:06:21.820>

I can trunkate the expected with assertThat(actual, equalTo(expected.truncatedTo(ChronoUnit.MILLIS))) for them to be equal, however, this means each time a comparison is made (isAfter, isBefore, equals) to the converter class being tested, trunkating will have to be applied.

Is there a proper way to do conversions for between LocalDateTime to Long and vice versa for JDK 11 (or a documentation I might have missed perhaps :))?

Update:

As pointed out in the comments, the representation is not the same for Java 8 and 11 thus resulting to the failing test. To give more context on what is being asked by this post, here are the 2 methods being verified by the test (which i moved to the test itself to only capture what is being executed because the unit test that failed belongs to a class that uses the utility method)

public Long localDateTimeToEpochMillis(LocalDateTime ldt) {
    Instant instant = ldt.atZone(ZoneId.systemDefault()).toInstant();
        return ldt.atZone(ZoneId.systemDefault())
           .toInstant().toEpochMilli();
}

and

public LocalDateTime epochMillisToLocalDateTime(long epochMillis) {
    return LocalDateTime.ofInstant(
            Instant.ofEpochMilli(epochMillis), 
            ZoneId.systemDefault());
}

What the existng tests seems to verify is that given a long value, i should get the same LocalDateTime equivalent and this was done by using the Given (LocalDateTime converted to Long value) then back to LocalDateTime for comparison.

geneqew
  • 2,401
  • 5
  • 33
  • 48
  • did you try Instant.ofEpochSecond​ and pass Instant.getNano() as second parameter? Or better just work with Instant rather than milli and nano seconds. – Puce Jun 01 '21 at 06:26
  • 2
    I believe the question is more pivoted around the expectation mismatch between Java-8 and Java-11. But for that, you might need to state the assumptions or documentation on which the current tests rely upon. – Naman Jun 01 '21 at 06:39
  • Unfortunately i cant, there are instances where long values would be needed (ie protobuf related conversion etc). On the other hand, using nanoAdjustment as 2nd param for ofEpochSecond works. – geneqew Jun 01 '21 at 06:43
  • Narrowing it down, the actual comparison fails for the `toLocalTime()` representation of the `LocalDateTime` which is not same in Java-8 and Java-11. @geneqew What about in your actual code getting rid of the `toMillis..` conversion? Does that impact existing test cases? – Naman Jun 01 '21 at 06:51
  • 2
    You haven't explained what this is actually supposed to test. Therefore we can't tell you whether your test code is correct or not. – Stephen C Jun 01 '21 at 06:56
  • @Naman, ive updated the post to give more context specially for the 'toMillis' conversion. As for the question if it impacts existing test, yes it does. For this specific test however, it seems it's more appropriate to update assertion setting the expected to truncated by millis. – geneqew Jun 01 '21 at 07:47
  • 1
    By the way, I cannot imagine any case where calling `LocalDateTime.now` is the right thing to do. That class purposely lacks the context of a time zone or offset-from-UTC, so it cannot represent a moment, is *not* a specific point on the timeline. `Instant.now()` should be used for capturing the current moment. – Basil Bourque Jun 01 '21 at 15:08

1 Answers1

10

If you take a look at the difference:

Expected :<2021-06-02T14:06:21.820299>
Actual   :<2021-06-02T14:06:21.820>

You can see that it removes anything less than a millisecond.

This happens because you convert the LocalDateTime to milliseconds:

.toInstant().toEpochMilli();

In order to avoid that, you can use Instant#getNano:

Gets the number of nanoseconds, later along the time-line, from the start of the second. The nanosecond-of-second value measures the total number of nanoseconds from the second returned by getEpochSecond().

It could look like this:

Instant instant=expected.atZone(ZoneId.systemDefault())
                .toInstant();
long epochMillis = instant.toEpochMilli();
long nanos=instant.getNano()%1000000;//get nanos of Millisecond

LocalDateTime actual = LocalDateTime.ofInstant(Instant.ofEpochMilli(epochMillis).plusNanos(nanos),
        TimeZone.getDefault().toZoneId());

Why did it work in Java 8?

As this post and JDK-8068730 Increase the precision of the implementation of java.time.Clock.systemUTC() describes, Java 8 did not capture time units smaller than a millisecond. Since Java 9, LocalDateTime.now (and similar) get the time with microseconds.

dan1st
  • 12,568
  • 8
  • 34
  • 67
  • 1
    are you saying the current tests are wrong? or else how is this a convenient way to migrate to Java-11? – Naman Jun 01 '21 at 06:31
  • The test seems to be "wrong" at least for Java 11 as it truncates the nanoseconds. – dan1st Jun 01 '21 at 06:40
  • 2
    The current test is inaccurate. It’s a bit like comparing `double` values and converting one of them to an integer first. @Naman – Ole V.V. Jun 01 '21 at 07:28
  • 1
    The problem also existed in Java 8 but you weren't measuring a time with microseconds/nanoseconds. See my edit. – dan1st Jun 01 '21 at 07:39