Import JUnit 6 bundles#146
Conversation
Currently the JUnit 6 Classpathcontainer is empty unless one installs the JUnit bundles into the IDE (what is not really feasible to expect). This now adds the JUnit 6 bundles as imports to make them install whenever the junit support is installed.
|
Its very likely we'll be using eclipse-jdt/eclipse.jdt.ui#2535. Aside from that I think the most pressing issue with adding JUnit 6 to the SDK (which e.g. this PR will do) is: eclipse-pde/eclipse.pde#2045 We need to decide what we want there, once we have the decision it should be simple to implement. |
This can/should be just merged as it declares the dependencies of the JUnit container (that can't do it directly), whatever will require any of these bundles additionally is totally independent.
I don't see what PDE has to do with JDT support, and having a user message is nothing "urgent" if there is an easy workaround! |
If this PR is merged (or eclipse-jdt/eclipse.jdt.ui#2535), won't JUnit 6 become part of the SDK? Or am I misunderstanding what this PR will do? Once JUnit 6 is in the SDK, launching a JUnit plug-in test will show obscure errors like this one: The workaround is simple, if the user knows the cause. But the above error doesn't mention the conflict. So that is urgent from my POV and we should address it before we have JUnit 6 in the SDK. |
Both wil likey result in JUnit 6 be part of the SDK yes, but the later is maybe not needed and currently seems hardly testable without making the first steps see:
I really dont see how one comes into this situation beside when using some obscure launch configs! If one uses the PDE default and a proper version range (what not only for JUnit is essential) then everything should just work. |
After eclipse-platform/eclipse.platform.releng.aggregator#3399 there were many discovered places where JUnit Jupiter bundles had no restricted version ranges. So using proper version ranges is not done everywhere. The launch is not obscure, its any JUnit Plug-in Test launch configured with a JUnit 5 test runner in the launch config page. |
Then please share an example, I always read about "duplicate" bundles and so on, by default PDE uses:
So in such combination there is actually no way for "wrong" Junit being part of the launch (or something is still broken) Then as a user one should use a target platform (and not the running target) so JUnit 6 will even never be part of the workspace bundles and therefore never will be found. So all these issues arise from using not recommended setup... and it does not help to delay "pure" JDT Junit support with starting at the end therefore I created #147 to outline the steps we need to get to there. So if we would have completed the two open PRs there would be an Ibuild tomorow containing that ground work and we could all easier devlop and verify patches than everyone is using a different setup. |
Run Technically there is only JUnit 6 that this launch will see, but in PDE we add JUnit 5 (since we see a JUnit 5 type of launch).
For a start I guess yes. The target platform in my work environment is the Eclipse SDK with some extra plug-ins on top. So as soon as the SDK is updated to a version that has both JUnit 5 and 6, we will have both in the target platform. I'm sure JUnit 5 and 6 will co-exist not just in our environment.
I agree, its not ideal. If you think not many will be affected by this, from my POV I have no issue with the merge. In all likelihood the next Eclipse release will have JUnit 6 launch support as default, so only old test launches will cause trouble with unrestricted JUnit Jupiter requirements (which honestly I expect to be the norm and not the exception). |
And that's why I wrote that's not recommended .. anyways I'll look into it.
Then we should do it ... I just have no committer powers here. |
|
Anyone against including JUnit 6 in the SDK? Alongside JUnit 5. Aside of the mentioned problem (eclipse-pde/eclipse.pde#2055) I can't think of anything else. I guess we can always revert short-term if we find some no-go type of problem. |
Not sure I understand this. What about this problem:
|
Assuming JUnit 6 and JUnit 5 are in the target/SDK (they will be in the SDK after this PR is merged): When you have no version restriction of This is because from the eclipse-pde/eclipse.pde#2055 just makes it clear what the user has to do in this situation, it doesn't make the error go away. Technically we can make the error go away and maybe cause more unexpected behavior later on, since we wouldn't tell the user to fix the problem (add version constraints). |
|
OK, thanks. So if there would be such problem, there will be at least workaround now, and for M3 we plan to fix that gap too. |
|
@merks @cdietrich : I wonder if that would affect Xtext tests? Xtext depends on JDT... |
|
It's not always entirely clear exactly what all needs to be constrained, e.g., eclipse-equinox/equinox#1176 (comment) There is resistance to actively trying to constrain the content, e.g., eclipse-pde/eclipse.pde@d1fe8d3 In any case, the slower this unfolds, the longer the painful runway because we can't really predict the impact of the full picture while only having a partial (stalled) picture. |
I read "let's merge now". |
|
In general it seems that adding sufficient imports with appropriate constraints (upper bounds) works around problems (though I still would hope that some of the underlying magic would be better at computing a "consistent" set of bundles). We really do need to move forward because so far the cautious approach is just extending the pain. So yes, merge. |
|
OK. So let's merge. |
|
@merks I'm currently working on a "non hacky way" of what you have proposed but I wanted to point out that we have two parts here:
By the way todays IBuild succeed, so for me it looks like we have no general problems during building... lets see what the tests show and this is maybe the most important part for me, that we get a state where it runs, so as @iloveeclipse say we can complement the support in m3. |
|
Only for the record: this PR triggered eclipse-platform/eclipse.platform.releng.aggregator#3430 and all subsequent fixes/workarounds. |
Yes this was somehow expected to show some irregularities. If all goes worse, we can quite easy revert this so better have it with a simple change than with an actual bigger feature. But I think it progress quite well and we have already identified some issues (in platform and tycho). |
It was for record only, so tickets have links to understand what is coming from where and all that dependencies. |
I didn't question that, just added something else, for the record, as I noticed it was not that clear in the initial PR description. |
4 participants
Currently the JUnit 6 Classpathcontainer is empty unless one installs the JUnit bundles into the IDE (what is not really feasible to expect).
This now adds the JUnit 6 bundles as imports to make them install whenever the junit support is installed.
FYI @merks @akurtakov @trancexpress @iloveeclipse