RFR(M) 8242165: SA sysprops support fails to dump all system properties

[serguei.spitsyn at oracle.com]

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