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

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

-- 
Thanks,
Rachna