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

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

-- 
With best regards,
Sergei