RFR: 7903507: jtreg runs into race conditions for multi-modules tests

Jonathan Gibbons jjg at openjdk.org
Mon Jul 24 20:23:06 UTC 2023


On Mon, 24 Jul 2023 13:44:39 GMT, Christian Stein <cstein at openjdk.org> wrote:

> This change puts the necessary JAR files onto the module path as (automatic) modules - without copying them. Therefore, no race condition can happen when multiple tests that require modules of test frameworks are compiling or executing in parallel.

Suggest minor changes to improve code style

src/share/classes/com/sun/javatest/regtest/exec/Action.java line 528:

> 526:             for (Path file: FileUtils.listFiles(element)) {
> 527:                 getModule(file, results);
> 528:             }

The `continue` seems a bit unnecessary; I suggest

if ... isRegularFile ... {
    getModule(...)
} else if ... isDirectory ... {
    for ... {
       getModule
    }
}

src/share/classes/com/sun/javatest/regtest/exec/Action.java line 536:

> 534:         if (isModule(file)) {
> 535:             results.add(file.getFileName().toString());
> 536:             return;

I suggest using `if ... else ...` without embedded `return`

src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java line 848:

> 846:                 // other jar files which are not valid as automatic modules.
> 847:                 if (needJUnit) params.getJUnitPath().asList().forEach(mp::append);
> 848:                 if (needTestNG) params.getTestNGPath().asList().forEach(mp::append);

✅

src/share/classes/com/sun/javatest/regtest/exec/RegressionScript.java line 1000:

> 998:     }
> 999: 
> 1000:     private void install(SearchPath path, Path dir) throws TestRunException {

yay!

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

PR Review: https://git.openjdk.org/jtreg/pull/161#pullrequestreview-1544201493
PR Review Comment: https://git.openjdk.org/jtreg/pull/161#discussion_r1272711305
PR Review Comment: https://git.openjdk.org/jtreg/pull/161#discussion_r1272713132
PR Review Comment: https://git.openjdk.org/jtreg/pull/161#discussion_r1272713752
PR Review Comment: https://git.openjdk.org/jtreg/pull/161#discussion_r1272714073


More information about the jtreg-dev mailing list