Please review two corrections for java.time

[roger riggs]

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.

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.

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
>>>
>>
>