Skip to content

Commit

Permalink
Run StandaloneTests for Java 8 under Java 8
Browse files Browse the repository at this point in the history
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] c62cd6a (Fix package path computation in `ClasspathScanner`,
    2021-05-12) from #2613
  • Loading branch information
rybak authored and marcphilipp committed Sep 10, 2023
1 parent 3184a6e commit 977c85f
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -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 .+
Original file line number Diff line number Diff line change
@@ -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 ]

Original file line number Diff line number Diff line change
Expand Up @@ -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")) //
Expand Down Expand Up @@ -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") //
Expand All @@ -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"));

Expand All @@ -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") //
Expand All @@ -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"));

Expand Down Expand Up @@ -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";
}
};
}
}

0 comments on commit 977c85f

Please sign in to comment.