RFR: 8244010: Simplify usages of ProcessTools.createJavaProcessBuilder in our tests

[Stefan Karlsson]

Hi again,

I realized that we probably want to give ProcessTools.executeTestJvm the 
same treatment.

Side-note: It's very awkard that createJavaProcessBuilder defaults to 
not adding user-specifed flags, but executeTestJvm does. I think it 
would be good to unify this as a separate RFE. I think *a lot* of 
callers to createJavaProcessBuilder could be simplified by either using 
executeTestJvm directly, or a simplified version of that.

I'm running testing through mach5 and found a few things to fix, I might 
find more when the testing has proceeded further.

This is the current patch:
https://cr.openjdk.java.net/~stefank/8244010/webrev.02.delta
https://cr.openjdk.java.net/~stefank/8244010/webrev.02

Thanks,
StefanK

On 2020-04-28 13:58, Stefan Karlsson wrote:
> Hi all,
>
> Please review this patch to simplify usages of 
> ProcessTools.createJavaProcessBuilder in our tests.
>
> https://cr.openjdk.java.net/~stefank/8244010/webrev.01/
> https://bugs.openjdk.java.net/browse/JDK-8244010
>
> I saw all this code when reviewing changes to how we pass flags in our 
> tests. There are a many places where arguments are converted and 
> passed back and forth in String[] and Collections.
>
> For example:
>   ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
>           argsList.toArray(new String[argsList.size()]));
>
> If we add an overload the createJavaProcessBuilder, that takes a 
> Collection<String> as an argument, then we can write the code above as:
>    ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(argsList);
>
> Other places temporarily put the flags in a String[], where most calls 
> simply lists the arguments in the call:
>   String[] opts = {Xmx, "-XX:NativeMemoryTracking=detail", 
> "-XX:+UseParallelGC", "-version"};
>   ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(opts);
>
> And some places put the args in a temporary Collection:
>   LinkedList<String> vmOptions = new LinkedList<>();
>   vmOptions.add(gc);
>   vmOptions.add("-Xmx" + minMaxHeap);
>   vmOptions.add("-XX:+PrintFlagsFinal");
>   vmOptions.add(VerifyHeapSize.class.getName());
>
>   ProcessBuilder pb = 
> ProcessTools.createJavaProcessBuilder(vmOptions.toArray(new String[0]));
>
> I'd like to cleanup, simplify, and unify many of these usages.
>
> I've tested this by running all the changed tests locally.
>
> Thanks,
> StefanK