Fix package path computation in ClasspathScanner#2613
Merged
Conversation
Fix `getRootUrisForPackage()` in class `ClasspathScanner` by looking for two "wordings" of a package name. For example, package `foo.bar` is expanded to `foo/bar` and `foo/bar/`. The latter allows find package `foo.bar` in a module called `foo.bar`, and not `foo`. Fixes #2500 Fixes #2600 Fixes #2612
Codecov Report
@@ Coverage Diff @@
## main #2613 +/- ##
===========================================
+ Coverage 0 90.72% +90.72%
- Complexity 0 4807 +4807
===========================================
Files 0 424 +424
Lines 0 11877 +11877
Branches 0 919 +919
===========================================
+ Hits 0 10776 +10776
- Misses 0 816 +816
- Partials 0 285 +285
Continue to review full report at Codecov.
|
marcphilipp
approved these changes
May 11, 2021
marcphilipp
left a comment
Member
There was a problem hiding this comment.
LGTM except for the missing entry in the release notes. Thanks for taking care of getting this fixed! 👍
Contributor
|
Hello @marcphilipp @sormuras I tested this fix on reproducer and it works. Thanks for your help. It was very reactive :) |
Member
Author
|
Thanks a bunch for testing early, @hosuaby! The new 1.8.0-SNAPSHOT build will soon contain this fix, too. Would you mind testing it against that version as well? |
Contributor
|
Hello @sormuras |
rybak
added a commit
to rybak/junit5
that referenced
this pull request
Aug 6, 2023
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 junit-team#2613
1 task
marcphilipp
pushed a commit
that referenced
this pull request
Sep 10, 2023
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
marcphilipp
pushed a commit
that referenced
this pull request
Nov 2, 2023
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
(cherry picked from commit 977c85f)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Fix
getRootUrisForPackage()in classClasspathScannerby looking for two "wordings" of a package name.For example, package
foo.baris expanded tofoo/barandfoo/bar/. The latter allows find packagefoo.barin a module calledfoo.bar, and notfoo.Fixes #2500
Fixes #2600
Fixes #2612
I hereby agree to the terms of the JUnit Contributor License Agreement.
Definition of Done
@APIannotations