Hi Chris,
It looks good in general.
But a couple of comments.
http://cr.openjdk.java.net/~cjplummer/8242165/webrev.03/test/hotspot/jtreg/serviceability/sa/TestSysProps.java.html
I'm thinking if it'd make sense to do this kind of refactoring:
OutputAnalyzer runSACommand(String cmd, String argsStr, long pid)
throws Exception {
JDKToolLauncher launcher = JDKToolLauncher.createUsingTestJDK(cmd);
for (String arg : argsStr.split(" ")) {
launcher.addToolArg(arg);
}
launcher.addToolArg(Long.toString(pid));
ProcessBuilder pb = SATestUtils.createProcessBuilder(launcher);
System.out.println("> " + ProcessTools.getCommandLine(pb));
Process process = pb.start();
OutputAnalyzer out = new OutputAnalyzer(process);
process.waitFor();
System.out.println(out.getStdout());
System.err.println(out.getStderr());
return out;
}
OutputAnalyzer jhsdbOut = runSACommand("jhsdb", "jinfo --sysprops
--pid", app.getPid());
OutputAnalyzer jinfoOut = runSACommand("jinfo", "-sysprops",
app.getPid());
jhsdbOut.shouldMatch("Debugger attached successfully.");
jinfoOut.shouldMatch("Java System Properties:");
Even though it does not seem to give a big advantage for this particular
test it creates a useful pattern that can be replicated later if similar
tests are needed in the future.
Also, I wonder if the code at lines 158-178 is really needed.
We should get a RuntimeException if any of the properties is not found
in the jhsdb or jinfo output.
So, the number of printed options should match.
But maybe I'm missing something.
Thanks,
Serguei
On 4/5/20 22:49, Chris Plummer wrote:
Hello,
Please help review the following:
https://bugs.openjdk.java.net/browse/JDK-8242165
http://cr.openjdk.java.net/~cjplummer/8242165/webrev.03
Please see the CR for an explanation of the bug and the fix. If you
need some help with the SA code, let me know and I'll provide some
details. It was pretty much all new to me, so I tried to put the
needed details in the CR.
For the test, I compare the list of properties dumped using 3
different methods to make sure the same set is dumped for each:
1. jhsdb jinfo --sysprops
2. jinfo -sysprops
3. Simple app (LingeredAppSysProps) that calls
System.getProperties().list(System.out)
The app output is considered the master list that is compared against
the others, and I also make sure each list has the same number of
properties.
thanks,
Chris
