[15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

[naoto.sato at oracle.com]

Thanks!

On 6/9/20 11:48 AM, Roger Riggs wrote:
> Hi Naoto,
> 
> Looks fine.
> 
> fyi,  Objects.equals(a,b) can replace DateTimeFormatter: 1510-1511.

Right. I will replace it before the push.

Naoto

> 
> Thanks, Roger
> 
> On 6/9/20 1:01 PM, naoto.sato at oracle.com wrote:
>> Hi Roger,
>>
>> Thanks for the review.
>>
>> On 6/9/20 8:52 AM, Roger Riggs wrote:
>>> Hi Naoto,
>>>
>>> Since the default locale is being changed even briefly, the test 
>>> should be run in /othervm.
>>> Add:  @run testng/othervm ...
>>
>> All java/time/test/java/time/format tests are run in othervm mode by 
>> default, defined in java/time/test/TEST.properties.
> ok,  non-local control. ugh
>>
>>>
>>> DateTimeFormatter:1498:
>>>     Can the optimization be retained in the case where z.equals(zone)?
>>>     Move it down to 1508+ and check the zone vs z.
>>
>> It's not only zone, but also has to check chrono and decimalStyle. In 
>> the previous version, I intentionally did not keep the optimization, 
>> as it would no longer be "fast" since it would need to check those 
>> equality, but at least it would save creating the new 
>> DateTimeFormatter instance, so I resurrected it. Here is the updated 
>> webrev:
> 
> 
>>
>> http://cr.openjdk.java.net/~naoto/8246662/webrev.01/
>>
>> Naoto
>>
>>>
>>> Thanks, Roger
>>>
>>>
>>>
>>> On 6/8/20 5:06 PM, naoto.sato at oracle.com wrote:
>>>> Hello,
>>>>
>>>> Please review the fix to the following issue:
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8246662
>>>>
>>>> The proposed changeset is located at:
>>>>
>>>> https://cr.openjdk.java.net/~naoto/8246662/webrev.00/
>>>>
>>>> This is a regression caused by the fix to JDK-8244245, where the 
>>>> locale related fields in the formatter are now overridden by 
>>>> localizedBy() method. It was only comparing the locale object for 
>>>> the fast path, but it was not sufficient.
>>>>
>>>> Naoto
>>>
>