Hi Joe,
In fact, this bug was possibly revealed by the fix to 8242504, where the
system clock precision is now nanoseconds. Before that, it used to be
millisecond precision, so the first try for the exact match succeeded
for most of the cases. Even with the nano precision fix, most of the
cases the test exits with exact match in the loop. But you are right,
exact match or not does not matter in this test case, so I think we can
just eliminate these exact match try loops. I will remove them and do
some sniff testing on it.
Naoto
On 6/1/20 5:58 PM, Joe Wang wrote:
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.s...@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
