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.y...@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
