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

[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, Sergei Kovalev wrote:
> 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 Java
>> properties files, means that "last one wins".
>>
>> line source
>> # Threeten test uses TestNG
>> TestNG.dirs = .
>> othervm.dirs = tck/java/time/chrono test/java/time/chrono
>> test/java/time/format
>> modules = jdk.localedata
>> lib.dirs = ../../lib/testlibrary
>> lib.build = jdk.testlibrary.RandomFactory
>> modules = java.base/java.time:open java.base/java.time.chrono:open
>> java.base/java.time.zone:open
> This mean that first modules declaration could be safely removed as I did.
>
>
> 28.12.16 17:20, Sergei Kovalev wrote:
>>
>> 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 of this behavior is the lack of jtreg header in test
>> sources. In case test source has no "@test" tag jtreg ignores all
>> properties that exists in TEST.properties file.
>>
>> I recreated the review by adding TEST.properties modification:
>> http://cr.openjdk.java.net/~skovalev/8171958/webrev.01/
>>
>>
>> 27.12.16 13:55, Rachna Goel wrote:
>>>
>>> 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
>>> tests, then TEST.properties need to be updated as well.
>>>
>>> Thanks,
>>> Rachna
>>>
>>>
>>>
>>> On 26/12/16 8:27 PM, Sergei Kovalev wrote:
>>>> 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 tests uses locale data that stored in separate
>>>> resource file "jdk.localedata".
>>>> Solution: Add declaration of required module. In same cases a test
>>>> file contains many test items, part of which could be executed with
>>>> java.base module only. In this cases we can separate the items and
>>>> extract that items which depend on locale data and run them
>>>> individually. Therefore this proposal contains additional test files
>>>> where added "WithLocale" suffix which demonstrate dependency on
>>>> resources. Alternative solution could be add a required module
>>>> declaration "jdk.localedata" to all files. However this will lead to
>>>> unnecessary test coverage reduction.
>>>>
>>>
>>
>> --
>> With best regards,
>> Sergei
>