RFR: 8321812: Update GC tests to use execute[Limited]TestJava

Thomas Schatzl tschatzl at openjdk.org
Mon Dec 18 13:48:40 UTC 2023


On Mon, 11 Dec 2023 19:31:49 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> A lot of our tests use a multi-step recipe to spawn and wait for a process. Here's an example
> 
>     ProcessBuilder pb = ProcessTools.createTestJavaProcessBuilder(
>          "-XX:-UseTLAB",
>          "-XX:+UnlockDiagnosticVMOptions",
>          "-XX:+VerifyDuringStartup",
>          "-Xlog:gc+verify=debug",
>          "-version");
>     OutputAnalyzer output = new OutputAnalyzer(pb.start());
>     ... do something with output and wait for the process to complete ...
> 
> 
> These are the steps involved:
> 
> 1) Create a `ProcessBuilder`
> 2) Call `ProcessBuilder::start`
> 3) Create an `OutputAnalyzer`
> 4) Perform an operation that finally waits for the process to, at least partially, complete (OutputAnalyzer::getOutput, OutputAnalyzer::shouldHaveExitValue(), and more).
> 
> Almost all our tests could be converted to use a single call to `ProcessTools.executeTestJava` (or `executeLimitedTestJava`), which spawns the process, makes sure that it has fully completed, and then returns a filled-in OutputAnalyzer to the caller. The above example would become:
> 
> 
>     OutputAnalyzer output = ProcessTools.executeTestJava(
>          "-XX:-UseTLAB",
>          "-XX:+UnlockDiagnosticVMOptions",
>          "-XX:+VerifyDuringStartup",
>          "-Xlog:gc+verify=debug",
>          "-version");
> 
> 
> I propose that we make this change in the GC tests, to make our code simpler and hopefully easier to read.
> 
> Note: There's a few changes to the throws statements because some ProcessTools APIs throws IOException while others throw Exception.
> 
> Testing: I've done testing on a similar set of changes, but I'm going to run the appropriate, final tests while this is being considered/reviewed.

Marked as reviewed by tschatzl (Reviewer).

test/hotspot/jtreg/gc/arguments/GCArguments.java line 101:

> 99:         return ProcessTools.executeTestJava(withDefaults(arguments));
> 100:     }
> 101: 

Suggestion:

test/hotspot/jtreg/gc/g1/numa/TestG1NUMATouchRegions.java line 184:

> 182:         }
> 183: 
> 184:         OutputAnalyzer output =  ProcessTools.executeLimitedTestJava(

Suggestion:

        OutputAnalyzer output = ProcessTools.executeLimitedTestJava(

-------------

PR Review: https://git.openjdk.org/jdk/pull/17067#pullrequestreview-1786908880
PR Review Comment: https://git.openjdk.org/jdk/pull/17067#discussion_r1430152105
PR Review Comment: https://git.openjdk.org/jdk/pull/17067#discussion_r1430158652


More information about the hotspot-gc-dev mailing list