Alexander,
The fix is pretty good.
Some comments besides the bug 8164490 we already discussed.
test/serviceability/jdwp/AllModulesCommandTest.java
75 System.err.println("Could not launch the debuggee. Error: '" + line +
"'");
A suggestion to replace "Error: " => Error at line: ".
Otherwise, the line number will be confused with the error code.
81 // Etsablish JDWP socket connection
A typo: Etsablish => Establish
The verification in the assertClassLoader is pretty weak.
It'd be nice to have more sophisticated verification.
For instance, the following commands can be used for such verification:
cmdset: ClassLoaderReference=14 , cmd: VisibleClasses=1
cmdset: ReferenceType=2, cmd: ClassLoader=2
cmdset: ReferenceType=2, cmd: Module=19
It is possible to iterate over all classes of the class loader to find
at least one class with the given module and check if the class's
class loader
is the same as module's class loader.
BTW, this would test the new jdwp command added for Jigsaw support:
cmdset: ReferenceType=2, cmd: Module=19
But I leave it up to you as it looks unreasonably complicated.
Maybe, you find a better approach.
test/serviceability/jdwp/AllModulesCommandTestDebuggee.java
41 Set<String> modNames = new HashSet<>();
42 for (Module mod : Layer.boot().modules()) {
43 modNames.add(mod.getName());
44 String info = String.format("module %s", mod.getName());
45 write(info);
46 }
The local modNames is not really used and can be removed (lines 41, 43).
test/serviceability/jdwp/Arch.java
It seems, this file/class is not really needed and can be removed.
test/serviceability/jdwp/JdwpReply.java
Nit: the empty lines 39 and 54 are not needed.
Thanks,
Serguei
On 8/12/16 05:55, Alexander Kulyakhtin wrote:
Hi,
Could you, please, review the following test-only change (adding a new test):
CR: https://bugs.openjdk.java.net/browse/JDK-8148103
Webrev: http://cr.openjdk.java.net/~akulyakh/8148103_02/
The new test verifies the new JDWP commands: AllModules, Module, Name,
ClassLoader, CanRead.
It does so by launching a debuggee java program with the necessary
JDWP-related options, so that a JDWP session can be established between the
debuggee and the test.
When started the debuggee reports its loaded modules to the test.
The test then initiates the JDWP session and issues AllModules command to get
the modules info by means of the JDWP.
For each module the test issues Name command. It then verifies that the modules
names reported via the JDWP are the same as reported by the debuggee using the
Java API.
Additionally, for each module the test issues CanRead and Classloader commands
and verifies that the corresponding replies are correct.
Since all the previous JDWP tests were implemented using the deprecated closed
test framework, the amount of the code for this test is slightly larger than
usually for a single test.
The simple JDWP framework, created for this test, allows for porting the other
JDWP tests to the jtreg in the same manner.
Best regards,
Alexander
