Support JUnit 6 plug-in test launches#2109
Merged
iloveeclipse merged 2 commits intoeclipse-pde:masterfrom Nov 11, 2025
Merged
Conversation
Contributor
Author
|
First error I run into: The available bundles seem fine... Maybe something to do with the removal of |
Test Results 771 files ± 0 771 suites ±0 58m 30s ⏱️ + 6m 51s Results for commit 3faacd2. ± Comparison against base commit b00ef59. This pull request removes 3 and adds 14 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
Author
|
Next error is: |
Contributor
|
@trancexpress we have added some import restirctions on some other parts of PDE/JDT that are used here. Beside that, I would like to first see a successful run in pure JDT (is this already possible?) and maybe one should delay PDE support to next release... it really do not bring any value right now. |
Contributor
Author
I'll check what needs to change, no problem.
Only with the 2 PRs linked in #2108:
The SDK build is broken, so we cannot merge either. But once we merge them, yes, plain JUnit tests run with JUnit 6.
We would have to add some sort of validation on the launch, and more. Right now the launch defaults to JUnit 6, as soon as eclipse-jdt/eclipse.jdt.ui#2560 is used. For plug-in tests we'll need to filter out the JUnit 6 option, if we are delaying PDE support by a release. How much time do we have @iloveeclipse ? |
Member
Ideally "bigger" changes should be done before M3 planned for next thursday/friday. |
Contributor
Author
|
With the latest changes here, as well as eclipse-jdt/eclipse.jdt#145 and |
Member
With JUnit 5, correct? Because it is what manifest says. |
Contributor
Author
|
@laeubi it would be great if you can take a look. If you think we should delay JUnit 6 support until the next release, so that we get more manual testing, I think that would be fine as well. But we'll need to disable the plug-in test launch JUnit 6 choice. Unless we delay the entire JUnit 6 choice in launches. |
Contributor
Author
One set of manifest and sources with JUnit 5, the other set of manifest and sources with JUnit 6. |
trancexpress
force-pushed
the
gh2108
branch
2 times, most recently
from
7af4a00 to
a9767d3
Compare
22 tasks
Contributor
Author
|
Seems like we don't have a build with the latest JUnit 6 changes in JDT UI. We'll have to wait more here. I also see problems with #2113, maybe it will be best to include the new tests here. Similar to the JDT UI PR/tests. |
Member
Yes, tonight's I-build failed but I have already prepared a fix and I'm currently in the process of verifying it.
I think that makes sense to have the new functionality tested immediately. This also makes it simpler to review and test this change. |
Contributor
Author
|
Looks like JUnit 4 vintage test plug-in launches are broken with changes here. Among other problems I see in tests. It might be good to look into disabling the JUnit 6 choice for plug-in test launches, in case we don't manage to fix everything that doesn't work until the end of the next week. |
trancexpress
force-pushed
the
gh2108
branch
3 times, most recently
from
8a9777c to
92f906d
Compare
Contributor
Author
|
There are still some fails, but I have no idea what the problem is: The I find no exception stack traces anywhere. I also can't find the log files from the test workspace... In case there is some logged errors. Even the workspace passed with Copying the test projects and manually launching the test cases / suites / packages works fine... |
Member
|
Triggered https://ci.eclipse.org/releng/job/Builds/job/I-build-4.38/113/ to see eclipse-jdt/eclipse.jdt.ui#2626 change in PDE. |
Contributor
Author
|
@laeubi lets move the discussion from #2111 (comment) to here. See the current state: Not copying the projects in the test resources (in If you insist on this, IMO lets start that rewrite as a separate ticket. |
Member
trancexpress
force-pushed
the
gh2108
branch
2 times, most recently
from
656853c to
5589556
Compare
Member
|
Looks like Jenkins can't fetch the commit from fork again:
Could be related to https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/6858 but could be another issue... |
Contributor
Author
|
The Windows job ran into this failure: Here the same test fails but for JUnit 5 and for the Linux job: Will have to investigate this, I guess we open an issue for it. |
Contributor
Author
|
Another instability, this time in the Mac tests: @iloveeclipse I suggest we merge this. |
Member
iloveeclipse
left a comment
iloveeclipse
left a comment
There was a problem hiding this comment.
I only reviewed in browser, change sounds good, just small style issue.
| private static final Bundle BUNDLE = FrameworkUtil.getBundle(Caller.class); | ||
| private static final Bundle loaderBundle; | ||
| private static final Bundle BUNDLE; | ||
| private static final Bundle loaderBundleJUnit5; |
Member
There was a problem hiding this comment.
Two constants should be upper case
HannesWell
reviewed
Nov 10, 2025
...pse.pde.junit.runtime/src/org/eclipse/pde/internal/junit/runtime/RemotePluginTestRunner.java
Show resolved
Hide resolved
HannesWell
approved these changes
Nov 10, 2025
Member
HannesWell
left a comment
HannesWell
left a comment
There was a problem hiding this comment.
Thank you @trancexpress for this enhancement. Great work!
And I'm glad to see that we have JUnit-6 support for PDE as well (soon).
I have reviewed and tested this and as far as I can tell, it works well.
I have just pushed a small update to address a few minor style issues (besides other the one from Andrey), also to make the code a bit more strict.
This update also includes the missing consideration of the version range I mentioned before.
With that I think this is ready.
If you want to apply more changes, please make sure to fetch the latest state before.
iloveeclipse
approved these changes
Nov 11, 2025
Member
iloveeclipse
left a comment
iloveeclipse
left a comment
There was a problem hiding this comment.
LGTM.
I've also smoke tested various use cases, with/without launch config dialogs, with/without JUnit5/6 dependencies, so far no problems found.
Let's merge.
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
4 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.
Fixes: #2108