On Wed, 9 Oct 2024 01:10:54 GMT, Ramkumar Sunderbabu <[email protected]>
wrote:
>> test/jdk/java/time/tck/java/time/TCKInstant.java line 193:
>>
>>> 191: Instant test = Instant.now();
>>> 192: long diff = Math.abs(test.toEpochMilli() -
>>> expected.toEpochMilli());
>>> 193: assertTrue(diff < 10_000); // less than 10 secs
>>
>> Given arbitrary delays between the two executions; the premise of the test
>> itself is suspect; especially if the allowed time is increased. I think the
>> test is supposed to be testing that the default clock for Instant.now() is
>> the SystemUTC clock.
>> I'd expect expected to be less than or equal to test.
>> The use of math.abs allows the clock to go backwards; that might happen if
>> the time was re-set manually.
>
> Thanks @RogerRiggs for the comments. I increased the diff allowance to absorb
> any compilation related delays. What would you propose here?
If your objection is primarily on Math.abs, is this ok?
`
long diff = test.toEpochMilli() - expected.toEpochMilli();
assertTrue(diff >= 0 && diff < 10_000); // less than 10 secs
`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21413#discussion_r1792673474
