Added missing requirement to org.eclipse.equinox.launcher in test bundles #2116
Conversation
|
@laeubi the tests run properly with this change. |
trancexpress
force-pushed
the
gh2024
branch
2 times, most recently
from
2557916 to
805befa
Compare
|
One failure is: |
|
The instability in the current run is: |
What I'm wondering is, why is it needed now and was not needed before? |
Any specific version you want me to check with? I tried with 4.14 (when the test was first added) but there is some nonsense message there. |
Jenkins work, Github works, I-Build works ... or do I miss something here? So if it is only an issue in the IDE we need to check why its different there as it seems not needed elsewhere (and I haven seen it recently when looking into this tests as well). |
|
A recent change in one if the tests that fail is: bf28bcf
@HannesWell can you remember if executing this test from Eclipse worked? I assume it did. That would reduce the potential degradation interval to 04.2023 until now. |
|
With Eclipse 4.36 and At Eclipse 4.37 and Some tests fail with: Its hard to bisect this though... In the passing case, Last build I have without the bug: First build I have with the bug: I wasn't able to bisect with just PDE, likely something else changed. Maybe something in Equinox... |
I'm pretty sure it worked when I developed these tests. But I don't recall exactly when I executed them last from the IDE. |
|
Seems like From then on a lot of things are called, but essentially this code doesn't add the launcher bundle because its missing above: In particular Likely I cannot bisect this properly since its not a problem when tests are launched, its a problem when the launch is created... |
|
Bisected this to: 1a5255b Note that launching the test must be done from a debuggee Eclipse. The test configuration differs, not the test runtime itself. For the different configuration to be generated while bisecting, the intermediate Eclipse instance is necessary. Also existing launch configs have to be deleted before checking with each PDE state. See screen recording: vokoscreenNG-2025-11-12_00-26-19.mp4 |
I'm not completely sure what you mean by "differs" because before the change simply all bundles where always added (what of course include the launcher bundle somehow) but it does not explain why Tycho is able to compute the required sets and PDE not... maybe Tycho is including the launcher by default (possible) and PDE should do the same, but I think we first should understand why the launcher is required at all (what seems not obvious to me) and why it is not found by the automatic inclusion of requirements. |
Yes, that is the difference, the launch configuration differs now. That leads to the runtime error later on, even if the code executed at runtime isn't different.
That is fairly straightforward, in terms of code. For this exception to occur: The following code is called:
If both return
I'll see if I have time for this today. |
|
Alright it then seems that we really need the launcher here as it has to be either in the target or is expected to be present in the runtime. Adding it to the manifest then seems fine, but we should add an |
Should be this code in
But no
Considering that I.e. similar to how we add JUnit bundles in It does seem odd to have to add to the Or is |
You showed that the code actually need it or throws exception, this is similar as with classforname ... even if not explicitly required it is implicit.
Maybe yes, I remember I added equinox there recently as well (what obviously do not work without it)
That's the interesting question .. one would hope it is not required at all. |
@laeubi do you prefer this change then? From my POV, only a few PDE tests seem to test launching plug-in tests... And its those that require the launcher bundle. Aside from this bundle, there are two test methods in So we can add the launcher to also the dependencies of |
trancexpress
changed the title
|
Lets merge this, its probably the most simple fix for the test errors. |
bundles Launching org.eclipse.pde.junit.runtime.tests.JUnitRuntimeTests or org.eclipse.pde.ui.tests.launcher.PluginBasedLaunchTest from Eclipse results in the following exception: org.eclipse.core.runtime.CoreException: Launching failed. Bootstrap code cannot be found. Adding org.eclipse.equinox.launcher to the required bundles of org.eclipse.pde.junit.runtime.tests and org.eclipse.pde.ui.tests avoids these errors. Fixes: eclipse-pde#2024
If it is only in the IDE execution, we actually should not merge it but check what requirements are missing then (like for P2) as otherwhise this will soon be marked as unused and removed again if the verification builds do not fail. |
4 participants
Launching
org.eclipse.pde.junit.runtime.tests.JUnitRuntimeTestsororg.eclipse.pde.ui.tests.launcher.PluginBasedLaunchTestfrom Eclipseresults in the following exception:
org.eclipse.core.runtime.CoreException: Launching failed. Bootstrap codecannot be found.
Adding
org.eclipse.equinox.launcherto the required bundles oforg.eclipse.pde.junit.runtime.testsandorg.eclipse.pde.ui.testsavoidsthese errors.
Fixes: #2024