[15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

[Joe Wang]

Hi Naoto,

The patch looks good to fix the failure. I'm just curious whether the 
100-time comparison is necessary because of the existence of this 
assertion outside the loop that allowed the test to pass if the 
different was within a certain period of time. None of the tests had 
commented on the purpose of the test,  it looks like it's testing the 
assertion that (for the now method) "This will query the system clock to 
obtain the current time." The 100-loop therefore was a compromise for 
lack of a better way to prove that. I agree with what you said that 
"inherently those two objects could have different times". The 
outside-loop assertion  therefore makes better sense, and the loop was 
kind of just wasting time to me (I mean you could get lucky to have the 
two returning the same time down to a nanosecond, but that didn't make 
the test better than just the out-of-loop assertion.

my 2 cents

-Joe

On 6/1/2020 12:31 PM, naoto.sato at oracle.com wrote:
> Hello,
>
> Please review the fix to the following issue:
>
> https://bugs.openjdk.java.net/browse/JDK-8246261
>
> The proposed changeset is located at:
>
> https://cr.openjdk.java.net/~naoto/8246261/webrev.00/
>
> The test case compares two LocalTime objects, created with 
> LocalTime.now(Clock/ZoneId). So inherently those two objects could 
> have different times. The test tries to compare them 100 times for the 
> exact match, and if not, then falls back to compare those times by 
> truncating nanoseconds. The failure could occur when those two 
> LocalTimes are around the whole second, e.g., expected == 
> 18:14:22.999999 and test == 18:14:23.000001. To fix this, check the 
> difference of those objects and ensure it is less than a second.
>
> Similar test cases exist in TCKLocalDateTime.java and 
> TCKZonedDateTime.java so they should also be fixed. It is ok to leave 
> the similar test case in TCKLocalDate.java, as multiple tries do exact 
> match.
>
> Naoto