-
-
Notifications
You must be signed in to change notification settings - Fork 303
Fix Junit Platform 1.13+ Support (up to JUnit Jupiter and Platform to 5.14.1/1.14.1) #7000
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
base: master
Are you sure you want to change the base?
Fix Junit Platform 1.13+ Support (up to JUnit Jupiter and Platform to 5.14.1/1.14.1) #7000
Conversation
|
Hi @kriegfrj , could you have a look at this PR if this contains something useful for #6651 ? I have tried using Copilot for it as a learning experience for me, since this whole interaction between bnd and JUnit is unfamiliar to me. And it came up with this + two minor commits by me. (A first attempt in https://github.com/bndtools/bnd/pull/6999/changes led to a dead end I guess) So let me know what you think, and if it at least contains some useful clues. |
Thanks for trying, @chrisrueger - I'll see if I can have a look tomorrow. |
babd508 to
f170708
Compare
|
@kriegfrj I did some additional work to support up to JUnit Jupiter and Platform to 5.14.1/1.14.1 (which is the current latest version from that 5.x / 1.x range and the version Eclipse uses eclipse-platform/eclipse.platform.releng.aggregator#3579 (comment)) The recent commit f170708 changes Copilots suggestion from using the Also interesting: |
|
Sorry @chrisrueger, Christmas is a very busy time for me. I'll try and look when I can. Thank you so much for being willing to have a look at this, it looks promising! The static create() method didn't exist when I first wrote this, I think. |
20de661 to
c89ceb8
Compare
|
Thanks @kriegfrj based on the discussion in #7024 (comment) I think this fix would be really appreciated by all downstream consumers like Eclipse / AssertJ who currently have trouble with the narrow version range (1.13) introduced in 7.2.0 (because they are on >= 1.13 already. Since my debugging session yesterday I now have a better understanding how this Junit Tester works and the |
c89ceb8 to
7c6ad38
Compare
Just to fill you in a little on the background of why the tester is written the way that it is written: It mostly came down to how the class/method resolvers would work. The built-in JUnit platform resolvers for method and class selectors would always try and use the system classloader to resolve them. This obviously caused issues. Initially I thought it would be sufficient to resolve the classes by loading them from the corrent bundle classloader, and then hand off the resolved classes to the JUnit Platform resolving/execution infrastructure for method resolution. However then I discovered that we had to do more due to the difference between class initialisation and class loading: the classloader is lazy and won't load all of the dependency classes (eg, method parameters) until you use the class for the first time ("initialisation"). So it was not sufficient to merely load the class with the correct classloader, but you also had to ensure that it got initialised from within the correct classloader context too or else the dependencies would fail to load. So I had to fully resolve all of the method selectors as well as the class selectors. The I raised a number of bugs/feature requests with the JUnit team as a result of this work so that the built-in resolvers could be made more class aware (one example: the method resolver will now use the class's classloader rather than the system classloader to resolve the method parameters). So it is possible that a lot of the workarounds I had to put in place could be bypassed now. I'll try and have a look at your implementation in the coming days. |
…1/1.14.1) - Modified BundleEngine.executeBundle() to accept and pass ExecutionRequest parameter - Updated BundleDescriptor.executeChild() to accept ExecutionRequest instead of separate listener and params Add ExecutionRequestFactory to propagate execution context for JUnit Platform 1.13+ Implemented ExecutionRequestFactory that uses reflection to properly propagate the internal NamespacedHierarchicalStore from parent ExecutionRequest to child requests. This is necessary for JUnit Platform 1.13+ compatibility. The factory: - Detects if reflection is available and needed - Uses internal Store API if available (JUnit Platform 1.13+) - Falls back to public constructor for older versions - Handles failures gracefully Updated BundleDescriptor to use the factory instead of directly creating ExecutionRequest. - Add detailed comment explaining why internal JUnit Platform API is accessed Updated BundleEngineTest to include ExecutionRequestFactory class in the engine bundle for tests. This class was added in the JUnit Platform 1.13+ compatibility changes but wasn't included in the test bundle setup. All tests now pass successfully. Update JUnit Jupiter/Platform versions to 5.11.4/1.11.4 Updated junit.jupiter.version and junit.platform.version to 5.11.4/1.11.4 to test with newer versions. These versions are compatible and all 136 tests pass. Fix JUnit5 test classes to use static @BeforeAll methods for 5.13+ compatibility Made @BeforeAll methods static in JUnit5ContainerFailure and JUnit5ContainerError test classes. In JUnit 5.13+, non-static @BeforeAll methods are now detected as discovery errors (causing DiscoveryIssueException) rather than execution failures. This was causing the exitCode_countsJupiterContainerErrorsAndFailures() test to fail - expecting exit code 2 (two container failures) but getting exit code 1 (discovery error). Making the @BeforeAll methods static ensures they properly execute and fail as intended by the test, maintaining compatibility with JUnit Platform 1.13+. fix ConfigVersionsTest replace deprecated method getParameterTypeNames Replaces calls to getMethodParameterTypes with getParameterTypeNames when creating and resolving MethodSelectors. This aligns with updated API usage and ensures correct parameter type handling. Refactor ExecutionRequest to use static create() methods Replaces use of internal constructors with reflection-based invocation of static create() methods for ExecutionRequest, improving compatibility with JUnit Platform 1.13+ and future versions. This change also updates the logic to detect and use the appropriate static method signatures, and removes reliance on direct constructor access. Signed-off-by: Christoph Rueger <chrisrueger@gmail.com> Update JUnit Jupiter and Platform to 5.14.1/1.14.1 Bump JUnit Jupiter and Platform versions to 5.14.1 and 1.14.1 in cnf/ext/junit.bnd, Gradle, and Maven plugin parent POM to keep dependencies up to date and consistent across the project. This is the latest version in the 5.x / 1.x range and also the version used by Eclipse Signed-off-by: Christoph Rueger <chrisrueger@gmail.com> Co-Authored-By: chrisrueger <188422+chrisrueger@users.noreply.github.com>
7c6ad38 to
3f562ba
Compare
Closes #6651
Copied from fork PR chrisrueger#1 which was created via Copilot.