Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-10 Thread Naoto Sato
Looks good to me. Naoto On 1/10/17 2:26 AM, Sergei Kovalev wrote: Hi Naoto, Thank you for review. I looked throw sources mode carefully and found some inconsistency you mentioned. I cleaned out all possible dependencies on jdk.localedata. Probably previously the cases passed because they

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-10 Thread Sergei Kovalev
Hi Naoto, Thank you for review. I looked throw sources mode carefully and found some inconsistency you mentioned. I cleaned out all possible dependencies on jdk.localedata. Probably previously the cases passed because they doing only number formatting. However I agree that it meaningful to

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-09 Thread Naoto Sato
Hi Sergei, java.base only supports Locale.US locale (and its parents Locale.ENGLISH/Locale.ROOT). I still see tests that use other locales, such as Locale.UK, Locale.FRENCH (eg. in TestDateTimeFormatterBuilder.java). Those test should also need to be separated. Naoto On 1/9/17 1:45 AM,

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-09 Thread Rachna Goel
Hi Sergei, Just for curiosity, what is the purpose of moving package statement down. Sorry for pointing out, Copyright year needs to be updated to 2017. Thanks, Rachna On 09/01/17 3:15 PM, Sergei Kovalev wrote: Hi All, Re-sending request because I'm still looking for reviewers. In addition

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2017-01-09 Thread Sergei Kovalev
Hi All, Re-sending request because I'm still looking for reviewers. In addition for TEST.properties modification I'd like to put citation from Jon G. email. FWIW, I note your TEST.properties file looks malformed. It has 2 modules entries (line 4, line 7), which by the standard rules of

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2016-12-28 Thread Sergei Kovalev
Hi Rachna, Thank you for the comment. I've reviewed usage of module declaration in TEST.properties file and find that it could be removed without any impact. In both cases (with original and modified TEST.properties file) I get same result. Test results: passed: 142; failed: 27 The reason

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2016-12-27 Thread Rachna Goel
Hi Sergei, Though I am not a reviewer, But I have one comment on this fix. test/java/time/TEST.properties declares "modules = jdk.localedata" , so that all tests for java.time can have access to "jdk.localedata" module. If we are restricting usage of jdk.localedata module for different

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2016-12-27 Thread Sergei Kovalev
Hi Nadeesh, I cannot say for sure does tck tests have same issue or not because now they broken and failing as with limited JRE as well with full JDK. To make sure if we have same issue with tck tests we need to fix them first. I can take a look at the problem in separate CR. An example of

Re: RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2016-12-26 Thread nadeesh tv
Hi Sergei, I could see you modified tests only in /test/java/time/*test*/java/time/format/ directory. Won't the tests from test/java/time/*tck*/java/time/format/ directory fail with same issue? Thanks and Regards, Nadeesh On 12/26/2016 8:27 PM, Sergei Kovalev wrote: Hello Team, Please

RFR(s): 8171958: Several tests from java/time/test/java/time/format requiring jdk.localedata for execution

2016-12-26 Thread Sergei Kovalev
Hello Team, Please review below fix for tests. Bug ID: https://bugs.openjdk.java.net/browse/JDK-8171958 Web review: http://cr.openjdk.java.net/~skovalev/8171958/webrev.00/ Issue: some tests fails in case of module limitation by '--limit-module java.base' command line option. Root cause: The