-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix jdk21 tests #11605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix jdk21 tests #11605
Conversation
|
The first commit of this pull request is #11604. Not necessarily a problem, but it should be noted that #11604 would need to be merged first. Also those two pull requests should be on the top of the |
|
Corrections to my previous comment: #11398 said that the issue is reported against Maven 3.9.9. In such case, this pull request needs to be rebased not on |
|
@desruisseaux |
|
No need to create a new pull request. You can cherry-pick, push force and then click on the "edit" button of this pull request. |
Fix JDK 21 test failures by propagating --add-opens to forked JVM
Ensure Surefire passes required --add-opens options alongside the JaCoCo
agent using the supported @{jacocoArgLine} mechanism. This fixes
InaccessibleObjectException failures on JDK 21+ while preserving existing
test and coverage behavior.
Fixes apache#11398
e88939d to
3982070
Compare
|
Hi @desruisseaux , I’ve updated the pull request based on the discussion and rebased it onto the Could you please review the updated changes when you have a moment? |
|
@Jayesh45-master , @desruisseaux I missed something here .... which test in Maven we try to fix by this PR? We have a build with JDK 25 and I don't see a failing tests? |
| <forkNode implementation="org.apache.maven.plugin.surefire.extensions.SurefireForkNodeFactory" /> | ||
| <argLine>-Xmx256m @{jacocoArgLine} --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED</argLine> | ||
| <systemPropertyVariables combine.children="append"> | ||
| <version.toolbox>${toolboxVersion}</version.toolbox> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where property toolboxVersion is defianed?
Fixes #11398
Pull Request: Fix JDK 21+ test failures by propagating
--add-opensto forked test JVMFixes #11398
Overview
This pull request fixes test failures on JDK 21+ caused by missing module opens
in the forked Surefire JVM.
The change ensures that required
--add-opensoptions are propagated correctlyalongside the JaCoCo agent using Surefire’s supported
@{jacocoArgLine}mechanism.What this PR does
--add-opensJVM options to the existing SurefireargLineWhy this change is needed
Starting with JDK 21, Java enforces strong module encapsulation more strictly.
Tests that rely on reflective access to JDK internals (for example
java.langandjava.lang.reflect) fail withInaccessibleObjectExceptionunless the corresponding modules are explicitly opened at JVM startup.
Although the build already used
@{jacocoArgLine}to inject the JaCoCo agent,the forked test JVM did not receive the required
--add-opensoptions.As a result, tests passed on JDK 17 but failed on JDK 21+.
This change ensures the required JVM options are present at fork time,
which is the only point where module opens are honored by the JVM.
How the change works
The Surefire
argLineconfiguration is extended to include the required--add-opensoptions in addition to@{jacocoArgLine}.This approach:
No production code behavior is affected.
Testing
mvn verifyon JDK 17 and JDK 21unrelated to this change
Checklist
mvn verifyrun (JDK 17 / 21)License Declaration
Apache License Version 2.0, January 2004