From 808493ab09b30970b506a48fda3d616ac1ba4fff Mon Sep 17 00:00:00 2001 From: Andrei Rybak Date: Sun, 6 Aug 2023 23:47:02 +0200 Subject: [PATCH] Run StandaloneTests for Java 8 under Java 8 Problem ------- Tests that claim by their name to run on Java 8 don't actually run on Java 8. This can be clear from the output for tests that add option `--show-version` to the arguments and _don't_ fail -- they all print the version for the current JDK. The tool `java` of JDK 8 does _not_ have the option `--show-version`. The actual option that exists in JDK 8 has fewer hyphens, as per documentation of Java 8 [1]: -showversion Displays version information and continues execution of the application. This option is equivalent to the `-version` option except that the latter instructs the JVM to exit after displaying version information. And when I actually run Java 8 binary with the incorrect option `--show-version` used by the affected tests, I get: $ /usr/lib/jvm/java-8-openjdk-amd64/bin/java --show-version Unrecognized option: --show-version Error: Could not create the Java Virtual Machine. Error: A fatal exception has occurred. Program will exit. The option `--show-version` was only added in Java 9 [2]. Explanation ----------- In actuality, the tests are being run on whatever the current JDK is. These tests create an instance of class `de.sormuras.bartholdy.tool.Java`, which importantly has the following method: @Override public Path getHome() { return Bartholdy.currentJdkHome(); } When the `bartholdy` library creates the process, class `AbstractTool` does the following: protected Path createPathToProgram() { return getHome().resolve("bin").resolve(getProgram()); } The string `"java"` returned from method `getProgram` of class `Java` gets resolved against `Bartholdy.currentJdkHome()`. As far as I can tell, the library doesn't promise to look up the `java` binary in the `JAVA_HOME` supplied in the environment. In fact, just before consuming library user's environment, method `run()` of class `de.sormuras.bartholdy.tool.AbstractTool` puts the current JDK as `JAVA_HOME` into the environment to correspond to the behavior of class `de.sormuras.bartholdy.tool.Java` described above: builder.environment().put("JAVA_HOME", Bartholdy.currentJdkHome().toString()); The issue has been present since commit [3] where these tests were introduced. Fix --- Fix affected tests to run them under actual Java 8 by overriding method `de.sormuras.bartholdy.tool.Java#getHome`. Replace erroneous option `--show-version` with `-showversion`. To make tests executeOnJava8() and executeOnJava8SelectPackage() see the class files, update test compile() to use option `--release 8`. Because compiling to release 8 is deprecated, add a linter option to disable the warning to make compile() pass. Because option `-showversion` of Java 8 behaves slightly differently to option `--show-version` of later versions of Java, prepare two new files for expected stdout and stderr: expected-out-java8.txt and expected-err-java8.txt, which are similar to existing files expected-out.txt and expected-err.txt, but have different layout of fastforward lines "JAVA VERSION" and "TREE". Footnotes --------- [1] "Java Platform, Standard Edition Tools Reference", "java" https://docs.oracle.com/javase/8/docs/technotes/tools/unix/java.html https://docs.oracle.com/javase/8/docs/technotes/tools/windows/java.html [2] https://docs.oracle.com/javase/9/tools/java.htm > `--show-version` or `-showversion` > > Displays version information and continues execution of the application. > This option is equivalent to the `-version` option except that the latter > instructs the JVM to exit after displaying version information. [3] c62cd6ab11 (Fix package path computation in `ClasspathScanner`, 2021-05-12) from https://github.com/junit-team/junit5/pull/2613 (cherry picked from commit 977c85fc31ad6825b4c68f6c6c972a93356ffe74) --- .../standalone/expected-err-java8.txt | 21 ++++++++++ .../standalone/expected-out-java8.txt | 18 ++++++++ .../support/tests/StandaloneTests.java | 42 ++++++++++++++----- 3 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 platform-tooling-support-tests/projects/standalone/expected-err-java8.txt create mode 100644 platform-tooling-support-tests/projects/standalone/expected-out-java8.txt diff --git a/platform-tooling-support-tests/projects/standalone/expected-err-java8.txt b/platform-tooling-support-tests/projects/standalone/expected-err-java8.txt new file mode 100644 index 000000000000..70a3db689cc9 --- /dev/null +++ b/platform-tooling-support-tests/projects/standalone/expected-err-java8.txt @@ -0,0 +1,21 @@ +>> JAVA VERSION >> +.+ org.junit.platform.launcher.core.ServiceLoaderRegistry load +.+ Loaded LauncherInterceptor instances: .. +.+ org.junit.platform.launcher.core.ServiceLoaderRegistry load +.+ Loaded LauncherSessionListener instances: .. +.+ org.junit.platform.launcher.core.ServiceLoaderTestEngineRegistry loadTestEngines +.+ Discovered TestEngines: +- junit-jupiter .+ +- junit-vintage .+ +- junit-platform-suite .+ +.+ org.junit.platform.launcher.core.ServiceLoaderRegistry load +.+ Loaded PostDiscoveryFilter instances: .. +.+ org.junit.platform.launcher.core.ServiceLoaderRegistry load +.+ Loaded LauncherDiscoveryListener instances: .. +.+ org.junit.platform.launcher.core.ServiceLoaderRegistry load +.+ Loaded TestExecutionListener instances: .+ +.+ org.junit.platform.launcher.core.ServiceLoaderTestEngineRegistry loadTestEngines +.+ Discovered TestEngines: +- junit-jupiter .+ +- junit-vintage .+ +- junit-platform-suite .+ diff --git a/platform-tooling-support-tests/projects/standalone/expected-out-java8.txt b/platform-tooling-support-tests/projects/standalone/expected-out-java8.txt new file mode 100644 index 000000000000..0a1cc1ecfba8 --- /dev/null +++ b/platform-tooling-support-tests/projects/standalone/expected-out-java8.txt @@ -0,0 +1,18 @@ +>> TREE >> +Failures (2): +>> STACKTRACE >> + +Test run finished after \d+ ms +[ 11 containers found ] +[ 0 containers skipped ] +[ 11 containers started ] +[ 0 containers aborted ] +[ 11 containers successful ] +[ 0 containers failed ] +[ 10 tests found ] +[ 2 tests skipped ] +[ 8 tests started ] +[ 1 tests aborted ] +[ 5 tests successful ] +[ 2 tests failed ] + diff --git a/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java b/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java index 3747e6d6dfeb..33c5051de2c0 100644 --- a/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java +++ b/platform-tooling-support-tests/src/test/java/platform/tooling/support/tests/StandaloneTests.java @@ -89,6 +89,8 @@ void compile() throws Exception { var result = Request.builder() // .setTool(new Javac()) // .setProject("standalone") // + .addArguments("-Xlint:-options") // + .addArguments("--release", "8") // .addArguments("-proc:none") // .addArguments("-d", workspace.resolve("bin")) // .addArguments("--class-path", MavenRepo.jar("junit-platform-console-standalone")) // @@ -368,11 +370,12 @@ void execute() throws IOException { @Test @Order(4) void executeOnJava8() throws IOException { + Java java8 = getJava8(); var result = Request.builder() // - .setTool(new Java()) // - .setJavaHome(Helper.getJavaHome("8").orElseThrow(TestAbortedException::new)) // + .setTool(java8) // + .setJavaHome(java8.getHome()) // .setProject("standalone") // - .addArguments("--show-version") // + .addArguments("-showversion") // .addArguments("-enableassertions") // .addArguments("-Djava.util.logging.config.file=logging.properties") // .addArguments("-Djunit.platform.launcher.interceptors.enabled=true") // @@ -387,8 +390,8 @@ void executeOnJava8() throws IOException { assertEquals(1, result.getExitCode(), () -> getExitCodeMessage(result)); var workspace = Request.WORKSPACE.resolve("standalone"); - var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); - var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err.txt")); + var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out-java8.txt")); + var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err-java8.txt")); assertLinesMatch(expectedOutLines, result.getOutputLines("out")); assertLinesMatch(expectedErrLines, result.getOutputLines("err")); @@ -404,11 +407,12 @@ void executeOnJava8() throws IOException { @Order(5) // https://github.com/junit-team/junit5/issues/2600 void executeOnJava8SelectPackage() throws IOException { + Java java8 = getJava8(); var result = Request.builder() // - .setTool(new Java()) // - .setJavaHome(Helper.getJavaHome("8").orElseThrow(TestAbortedException::new)) // + .setTool(java8) // + .setJavaHome(java8.getHome()) // .setProject("standalone") // - .addArguments("--show-version") // + .addArguments("-showversion") // .addArguments("-enableassertions") // .addArguments("-Djava.util.logging.config.file=logging.properties") // .addArguments("-Djunit.platform.launcher.interceptors.enabled=true") // @@ -423,8 +427,8 @@ void executeOnJava8SelectPackage() throws IOException { assertEquals(1, result.getExitCode(), () -> getExitCodeMessage(result)); var workspace = Request.WORKSPACE.resolve("standalone"); - var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); - var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err.txt")); + var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out-java8.txt")); + var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err-java8.txt")); assertLinesMatch(expectedOutLines, result.getOutputLines("out")); assertLinesMatch(expectedErrLines, result.getOutputLines("err")); @@ -468,4 +472,22 @@ private static String getExitCodeMessage(Result result) { return "Exit codes don't match. Stdout:\n" + result.getOutput("out") + // "\n\nStderr:\n" + result.getOutput("err") + "\n"; } + + /** + * Special override of class {@link Java} to resolve against a different {@code JAVA_HOME}. + */ + private static Java getJava8() { + Path java8Home = Helper.getJavaHome("8").orElseThrow(TestAbortedException::new); + return new Java() { + @Override + public Path getHome() { + return java8Home; + } + + @Override + public String getVersion() { + return "8"; + } + }; + } }