Look good.

Thanks, Roger


On 11/21/16 5:08 AM, Bhanu Gopularam wrote:
Hi Nadeesh,

Thanks for the review. Please find the updated webrev:
http://cr.openjdk.java.net/~bgopularam/JDK-8158880/webrev.02/

Please note that I have used Locale.US instead of Locale.UK as test parameters 
(variables) depend on that locale.

Thank you,
Bhanu


-----Original Message-----
From: nadeesh tv
Sent: Friday, November 11, 2016 2:09 PM
To: core-libs-dev@openjdk.java.net
Subject: Re: RFR 8158880: 
java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java fail with zh_CN 
locale

Hi Bhanu,


I think adding Locale to the formatter will be better

For eg:

DateTimeFormatter f = 
builder.parseLenient().appendValue(HOUR_OF_DAY).appendValue(MINUTE_OF_HOUR, 
2).appendLiteral('9').toFormatter(Locale.UK);

Since other test cases use Locale.UK may be here also you can use UK instead of 
locale.US.

Thanks and Regards,
Nadeesh


On 11/11/2016 10:57 AM, Bhanu Gopularam wrote:
Hi Roger,

Thanks for the review. I have updated the test to set default locale (and 
restore it) in only those methods which were causing problem in non English 
locales.

Please review the updated webrev below:

Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8158880/webrev.01/

Thanks,

Bhanu

From: Roger Riggs
Sent: Tuesday, June 21, 2016 8:36 PM
To: Bhanu Gopularam; core-libs-dev@openjdk.java.net
Cc: Stephen Colebourne
Subject: Re: RFR 8158880:
java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java fail
with zh_CN locale

Hi Bhanu,

It can be problematic to change the default Locale for the entire
process, especially since many tests are run in the same process.  It
would be preferable to set the locale in the DateTimeFormatter builder instead 
of changing the process wide Locale.

Please identify the specific test case to apply the fix only where needed.

Is it only
tck.java.time.format.TCKDateTimeFormatterBuilder.test_appendZoneText_p
arseGenericTimeZonePatterns
where the failure is seen?

This would be an issue testing any pattern letter with locale dependent 
behavior.
The locale should be set explicitly in the test.

BTW,  There is a predefined Locale.US that can be used, no need to construct a 
new Locale.

Roger

On 6/21/2016 6:31 AM, Bhanu Gopularam wrote:

Hi all,
May I request you to review fix for following issue Bug id:
https://bugs.openjdk.java.net/browse/JDK-8158880
Solution: To avoid test failure in non english locales, set the
default locale while initial test setup
Webrev: http://cr.openjdk.java.net/~bgopularam/JDK-8158880/webrev.00/ Thanks,
Bhanu

--
Thanks and Regards,
Nadeesh TV


Reply via email to