From 4d51cd6d396387da57d534611fdc2162fd4d11da Mon Sep 17 00:00:00 2001 From: Andrei Rybak Date: Sun, 6 Aug 2023 19:27:17 +0200 Subject: [PATCH 1/4] Drop misleading messages in StandaloneTests Test `execute()` in StandaloneTests asserts matching of lines between expected standard output and actual output lines. It also passes the same actual lines (but in a `String` form) as a message to method assertLinesMatch, which takes three arguments. Same situation for standard error. Drop this confusing argument from the two calls to assertLinesMatch and let class `AssertLinesMatch` construct the error message for us. --- .../java/platform/tooling/support/tests/StandaloneTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 eb8eb20254f2..bea82965ae4e 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 @@ -354,8 +354,8 @@ void execute() throws IOException { var workspace = Request.WORKSPACE.resolve("standalone"); var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err.txt")); - assertLinesMatch(expectedOutLines, result.getOutputLines("out"), result.getOutput("out")); - assertLinesMatch(expectedErrLines, result.getOutputLines("err"), result.getOutput("err")); + assertLinesMatch(expectedOutLines, result.getOutputLines("out")); + assertLinesMatch(expectedErrLines, result.getOutputLines("err")); var jupiterVersion = Helper.version("junit-jupiter-engine"); var vintageVersion = Helper.version("junit-vintage-engine"); From 3f2590dd32138e9558179fda7932b08e2287dd52 Mon Sep 17 00:00:00 2001 From: Andrei Rybak Date: Sun, 6 Aug 2023 19:27:17 +0200 Subject: [PATCH 2/4] Unify messages about exit codes in StandaloneTests Some tests in StandaloneTests provide concatenation of standard output and standard error as a message for the assertion about exit code, while other tests provide only standard output. Some use method de.sormuras.bartholdy.Result#getOutput while others use method de.sormuras.bartholdy.Result#getOutputLines. Unify these approaches by replacing the messages with a supplier that gives a) a human-readable message about exit codes, and b) both standard output and standard error. --- .../support/tests/StandaloneTests.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) 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 bea82965ae4e..3747e6d6dfeb 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 @@ -69,7 +69,7 @@ void listAllObservableEngines() { .addArguments("engines", "--disable-banner").build() // .run(false); - assertEquals(0, result.getExitCode(), String.join("\n", result.getOutputLines("out"))); + assertEquals(0, result.getExitCode(), () -> getExitCodeMessage(result)); var jupiterVersion = Helper.version("junit-jupiter-engine"); var suiteVersion = Helper.version("junit-platform-suite-engine"); @@ -98,7 +98,7 @@ void compile() throws Exception { .addArguments(workspace.resolve("src/standalone/VintageIntegration.java")).build() // .run(); - assertEquals(0, result.getExitCode(), result.getOutput("out") + result.getOutput("err")); + assertEquals(0, result.getExitCode(), () -> getExitCodeMessage(result)); assertTrue(result.getOutput("out").isEmpty()); assertTrue(result.getOutput("err").isEmpty()); @@ -327,7 +327,7 @@ private static Result discover(String... args) { .build() // .run(false); - assertEquals(0, result.getExitCode(), String.join("\n", result.getOutputLines("out"))); + assertEquals(0, result.getExitCode(), () -> getExitCodeMessage(result)); return result; } @@ -349,7 +349,7 @@ void execute() throws IOException { .addArguments("--classpath", "bin").build() // .run(false); - assertEquals(1, result.getExitCode(), String.join("\n", result.getOutputLines("out"))); + assertEquals(1, result.getExitCode(), () -> getExitCodeMessage(result)); var workspace = Request.WORKSPACE.resolve("standalone"); var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); @@ -384,7 +384,7 @@ void executeOnJava8() throws IOException { .addArguments("--classpath", "bin").build() // .run(false); - assertEquals(1, result.getExitCode(), String.join("\n", result.getOutputLines("out"))); + assertEquals(1, result.getExitCode(), () -> getExitCodeMessage(result)); var workspace = Request.WORKSPACE.resolve("standalone"); var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); @@ -420,7 +420,7 @@ void executeOnJava8SelectPackage() throws IOException { .addArguments("--classpath", "bin").build() // .run(false); - assertEquals(1, result.getExitCode(), String.join("\n", result.getOutputLines("out"))); + assertEquals(1, result.getExitCode(), () -> getExitCodeMessage(result)); var workspace = Request.WORKSPACE.resolve("standalone"); var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); @@ -461,6 +461,11 @@ void executeWithJarredTestClasses() { .build() // .run(false); - assertEquals(1, result.getExitCode(), String.join("\n", result.getOutputLines("out"))); + assertEquals(1, result.getExitCode(), () -> getExitCodeMessage(result)); + } + + private static String getExitCodeMessage(Result result) { + return "Exit codes don't match. Stdout:\n" + result.getOutput("out") + // + "\n\nStderr:\n" + result.getOutput("err") + "\n"; } } From 3fad8c6c2a3829e2e329b334cd49b19f179d5f1f Mon Sep 17 00:00:00 2001 From: Andrei Rybak Date: Sun, 6 Aug 2023 23:47:02 +0200 Subject: [PATCH 3/4] 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 --- .../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"; + } + }; + } } From 060f2c4bec1ee260b44aafa2ac9b9e9a424e126f Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Sun, 10 Sep 2023 12:37:53 +0200 Subject: [PATCH 4/4] Use same expected files for all JDK versions --- .../standalone/expected-err-java8.txt | 21 ------------------- .../standalone/expected-out-java8.txt | 18 ---------------- .../support/tests/StandaloneTests.java | 16 ++++++++++---- 3 files changed, 12 insertions(+), 43 deletions(-) delete mode 100644 platform-tooling-support-tests/projects/standalone/expected-err-java8.txt delete 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 deleted file mode 100644 index 70a3db689cc9..000000000000 --- a/platform-tooling-support-tests/projects/standalone/expected-err-java8.txt +++ /dev/null @@ -1,21 +0,0 @@ ->> 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 deleted file mode 100644 index 0a1cc1ecfba8..000000000000 --- a/platform-tooling-support-tests/projects/standalone/expected-out-java8.txt +++ /dev/null @@ -1,18 +0,0 @@ ->> 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 33c5051de2c0..d4775c42f64c 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 @@ -21,6 +21,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.List; import de.sormuras.bartholdy.Result; import de.sormuras.bartholdy.jdk.Jar; @@ -390,8 +391,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-java8.txt")); - var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err-java8.txt")); + var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); + var expectedErrLines = getExpectedErrLinesOnJava8(workspace); assertLinesMatch(expectedOutLines, result.getOutputLines("out")); assertLinesMatch(expectedErrLines, result.getOutputLines("err")); @@ -427,8 +428,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-java8.txt")); - var expectedErrLines = Files.readAllLines(workspace.resolve("expected-err-java8.txt")); + var expectedOutLines = Files.readAllLines(workspace.resolve("expected-out.txt")); + var expectedErrLines = getExpectedErrLinesOnJava8(workspace); assertLinesMatch(expectedOutLines, result.getOutputLines("out")); assertLinesMatch(expectedErrLines, result.getOutputLines("err")); @@ -440,6 +441,13 @@ void executeOnJava8SelectPackage() throws IOException { + " (group ID: org.junit.vintage, artifact ID: junit-vintage-engine, version: " + vintageVersion)); } + private static List getExpectedErrLinesOnJava8(Path workspace) throws IOException { + var expectedErrLines = new ArrayList(); + expectedErrLines.add(">> JAVA VERSION >>"); + expectedErrLines.addAll(Files.readAllLines(workspace.resolve("expected-err.txt"))); + return expectedErrLines; + } + @Test @Order(6) @Disabled("https://github.com/junit-team/junit5/issues/1724")