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

[Lance Andersen]

The change looks OK also  with the loop removed.

> On Jun 2, 2020, at 12:13 AM, naoto.sato at oracle.com wrote:
> 
> Updated webrev: http://cr.openjdk.java.net/~naoto/8246261/webrev.01/
> 
> Naoto
> 
> On 6/1/20 6:30 PM, naoto.sato at oracle.com wrote:
>> 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.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
>>> 

 <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
 <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>