Please review two corrections for java.time

[Peter Levart]

On 09/09/2013 09:42 PM, roger riggs wrote:
> Hi Peter,
>
> Right, max doesn't solve the issue but I'm not keen on a test that 
> retries
> until it gets a better answer.

Hi Roger,

If java.time logic is correct, it should only ever retry once when 
roll-over or DST jump-back happens, so the test could be made to fail if 
it tries to retry the 2nd time, indicating unexpected behaviour. The 
"jumps" in LocalTime should be very far-apart so the test should only 
encounter one of them, if any.

>
> Adding nanosPerDay if the difference comes out negative would adjust
> for the crossing of midnight and not require looping on a more complex
> test condition.

That's ok for midnight roll-over, but what about DST jumps? They only 
happen two times a year, so you expect the test will never encounter them?

Regards, Peter

>
> The longish delay in the now() method is due to first-time initialization
> that reads the timezone data file.  Introducing the loop it would change
> the test condition so that it is not testing the 'cold' startup.
> However, the purpose of the test in not to measure the initialization 
> overhead
> so  adding an extra sampling of now(Clock) before the test will remove 
> the first time
> initialization.
>
> Updated webrev at:
> http://cr.openjdk.java.net/~rriggs/webrev-localtime-now-8023639/
>
> Thanks, Roger
>
>
> On 9/9/2013 11:14 AM, Peter Levart wrote:
>>
>> On 09/09/2013 03:12 PM, roger riggs wrote:
>>> Hi Peter,
>>>
>>> The possible wrap-around caused by crossing midnight is handled by 
>>> Math.max
>>> so a retry is not needed.
>>>
>>> Math.abs(test.toNanoOfDay() - expected.toNanoOfDay())
>>
>> Hi Roger,
>>
>> In case there is a wrap-around, the 'diff' is much more than 
>> 500,000,000 ns (about 24*60*60*1,000,000,000 ns - delay), which fails 
>> the test.
>>
>> But what do you think about testing before <= test <= after ? It 
>> should not be timing dependent, like it is now. Does it test the same 
>> thing?
>>
>> Regards, Peter
>>
>>>
>>> Thanks, Roger
>>>
>>>
>>>
>>>
>>> On 9/9/2013 2:14 AM, Peter Levart wrote:
>>>> On 09/06/2013 07:58 PM, roger riggs wrote:
>>>>> Please review for two corrections:
>>>>>
>>>>> -  The java/time/tck/java/time/TCKLocalTime test failed on a slow 
>>>>> machine;
>>>>>     the test should be more lenient.   The test is not appropriate 
>>>>> for a conformance test
>>>>>     and is moved to java/time/test/java/time/TestLocalTime.
>>>>>
>>>>> - The javadoc for the JapaneseEra.MEIJI era should indicate the 
>>>>> start date is 1868-01-01
>>>>>   to be consistent with java.util.Calendar.  Note that java.time 
>>>>> does not permit dates before Meiji 6
>>>>>   to be created since the calendar is not clearly defined until then.
>>>>>
>>>>> Webrev: 
>>>>> http://cr.openjdk.java.net/~rriggs/webrev-localtime-now-8023639/
>>>>>
>>>>> Thanks, Roger
>>>>>
>>>>>
>>>> Hi Roger,
>>>>
>>>> Although very in-probable, the test can fail when 'expected' is 
>>>> sampled before and 'test' is sampled after midnight. I'm guessing 
>>>> the test is trying to prove that LocalTime.now() is equivalent to 
>>>> LocalTime.now(Clock.systemDefaultZone()), right?
>>>>
>>>> In that case, what about the following:
>>>>
>>>>     public void now() {
>>>>         LocalTime before, test, after;
>>>>         do {
>>>>             before = LocalTime.now(Clock.systemDefaultZone());
>>>>             test = LocalTime.now();
>>>>             after = LocalTime.now(Clock.systemDefaultZone());
>>>>           // retry in case the samples were obtained around midnight
>>>>         } while (before.compareTo(after) > 0);
>>>>
>>>>         assertTrue(before.compareTo(test) <= 0 && 
>>>> test.compareTo(after) <= 0);
>>>>     }
>>>>
>>>>
>>>> Regards, Peter
>>>>
>>>
>>
>