RFR 8087307/9, new tests for ServiceLoader updates for module

[Hamlin Li]

Hi Felix,

I have some comments as below:


1. Possible test gaps for Locating Order
         1.1 providers in named module have high priority than providers 
in unnamed
         1.2 ServiceLoader.load​(ModuleLayer layer, Class<S> service) is 
not tested
2. Possible test gap for automatic module, it is not tested

3. Could you remove unnecessary module dependency on java.scripting, 
replace it with a customized module or java.base? it will make your 
tests run under as many as conditions.

4. For [a|b]filesystem
     could you use URLStreamHandlerProvider rather than 
FileSystemProvider, it will make your test files(FileSystemForInstr, 
AFileSystem, BFileSystem) more simple.

5. MisUsesTest.java and missuse
     missuses: miss => mis? (missuses => mis.uses)
     Possible test gap: add similar use case for unnamed module?

6. OrderingWithInstrTest.java
     Could you use jtreg tag "driver RedefineModuleAgent" rather than 
"@run main/othervm OrderingWithInstrTest" to trigger the preparation of 
test? and move createAgentJar() and main() into RedefineModuleAgent.java.

7. ReloadTest.java
     The test success depends on execution order of tests, so need to 
add priority=N to every @Test method, or separate tests into 2 classes, 
one for negative, one for normal tests.

8. Some additional minor comments:
     8.1 split module name with ".", for e.g. multiprovides => 
multi.provides
     8.2 wrap long lines, and revert unnecessary wrapping too, in 
several files.
     8.3 correct indent, e.g. in PermissionTest.java
     8.4 Is default constructor "public ProviderX() { }" necessary in 
multi.ProviderX ?
     8.5 rename Service to OrderedService?

Thank you

-Hamlin


On 2017/5/19 9:38, Felix Yang wrote:
> Ping:)
>
> -Felix
>> On 16 May 2017, at 4:59 PM, Felix Yang <felix.yang at oracle.com> wrote:
>>
>> Hi there,
>>
>>     please review the new tests added for ServiceLoader updates for module system.
>>
>> Test bug:
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8087307
>>
>> Webrev:
>>
>>     http://cr.openjdk.java.net/~xiaofeya/8087307/webrev.00/
>>
>> Related product Changes:
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8132026
>>
>>     https://bugs.openjdk.java.net/browse/JDK-8047814
>>
>> Thanks,
>>
>> Felix
>>
>>