SWT tests not executed in I20250325-1800 #1944
Comments
iloveeclipse
added
bug
|
From my understanding, the issue is that the The Maven builds of SWT succeed as the fragment is installed during the build and thus available. In I-Build tests, all dependencies need to be provided via the SDK or SDK Tests feature (if I am not mistaken). Obviously, the SVG fragment is not contained in any of them yet. Since we need to add the fragment to the platform feature soon anyway (as we want to use SVGs in platform), I propose to directly add the fragment to the What do you think? |
I think this is not the right approach, neither p2.inf nor adding it to a feature, this will make the fragment mandatory in all cases while it was promised to be optional and technically there can be other implementations. Instead the fragment should provide a capability, and bundles that require the SVG rendering should declare a requirement see here. I already did this for the transformer feature:
that way P2/OSGi will require to select at laest one provider. |
|
The fragment remains optional when using plain SWT. But for the platform itself it cannot be optional because as soon as we start replacing PNG with SVG icon paths, you will of course need an SVG renderer to process them. |
And then these bundles that contain SVGs could simply add the requirement to the SVG Rendering feature. This will then also work outside of Eclipse Managed builds. |
|
And what would be the benefit of requiring every bundle using SVGs to add that requirement rather than just adding it as a dependency to the feature containing those bundles? And wouldn't it still be necessary to add the fragment to a feature? How should the fragment be consumed by the build if it is not specified anywhere? |
Then svg support wouldn't be optional really anymore. If the fragment is needed or not is depends on the consumer/user. And adding that capability for all bundles that contain SVGs will probably be very cumbersome on the long run. To make a long story short, I think including the swt.svg fragment into the feature now is a solution that's fine for now.
That being said, that approach would be good for the o.e.swt host bundle to require a native fragment at runtime, but this will require a change in Tycho and P2 similar to eclipse-tycho/tycho#4743. |
The benefit would be that it works everywhere, even if the feature is not used. Making a bundle "silently fail" (like here) so the someone needs to find out what exactly is missing is a bad approach, and requirements are the OSGi way to solve this problem.
Every requirement is added to a launch / product automatically, so if everything is done in a proper way (what sadly is not always the case in Eclipse Platform) one would never need any features, you would just add your application bundle (and optional things like a log provider) and everything else would be pulled in automatically.
See previous comment. It is not required to explicitly name bundles you only need to specify requirements (e.g. a package, a java version or similar). |
I disagree here. This only needs to be done once and will prevent us from another part where Eclipse Bundles do not behave "correct", e.g. in Tycho I use P2 and it is horrible nightmare to find out what is really "needed" and if something is missing it simply badly fails. Another example is the compatibility fragment in PDE, without it it just fails completely. |
|
PDE doesn't provide support for adding a dependency to a fragment:
Even the package from that fragment doesn't seem visible/available:
|
Please read https://blog.osgi.org/2015/12/using-requirements-and-capabilities.html as mentioned above, no one ever have suggested to require the bundle (what is not possible for fragments) or a package (what is possible but PDE only allow it if the host is marked as extensible API). |
Okay, got that. That's a valid point, thanks! That would be an argument for me to add the provided/required capabilities for the productive usage of SVGs. And that should work for the SWT tests bundle as well, shouldn't it? I will give it a try.
I understand the point, but it's not a generic approach/goal as this is not how modular, extensible products work, but that's out of scope of this discussion anyway. |
I know that sometimes the test runtime is a bit special, I think the main problem here is that we assume that everything a test needs is also part of the Eclipse we build, so if nothing in Eclipse itself requires the SVG feature it might be missing or we can add some extra requirement @akurtakov ? By the way if we assume SVGs are only created through |
From my understanding, the I-Build test run uses a product based on the SDK test feature, which contains the SWT tests bundle, which in turn would have a requirement to the SVG capabitiliy. |
That's why I'm a bit confused, the error explicitly states that there is a requirement (through the p2.inf):
Another reason might be that it is not part of the I-Builds repo.... |
|
It is part of I-build repo https://www.eclipse.org/downloads/download.php?file=/eclipse/updates/4.36-I-builds/I20250325-1800/plugins/org.eclipse.swt.svg_3.130.0.v20250325-0759.jar , most likely it's included thanks to the swt.test bundle being published in it and it requiring it. Why the test framework doesn't find it remains to be found. |
Currently, requiring an SVG rasterizer needs manual placement of an according fragment on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to eclipse-platform#1944
Currently, requiring an SVG rasterizer needs manual placement of an according fragment providing a rasterizer on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to eclipse-platform#1944
Currently, requiring an SVG rasterizer needs manual placement of an according fragment providing a rasterizer on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to eclipse-platform#1944
Currently, requiring an SVG rasterizer needs manual placement of an according fragment providing a rasterizer on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to eclipse-platform#1944
Currently, requiring an SVG rasterizer needs manual placement of an according fragment providing a rasterizer on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to eclipse-platform#1944
Currently, requiring an SVG rasterizer needs manual placement of an according fragment providing a rasterizer on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to eclipse-platform#1944
Currently, requiring an SVG rasterizer needs manual placement of an according fragment providing a rasterizer on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to #1944
|
We have just merged Hopefully that fixes this issue. I propose to just wait for today's I-Build and will check its result. |
Unfortunately not. |
|
As mentioned yesterday by @akurtakov the bundle is in the ibuilds repo, but for some reason the install is not finding it, so it might be that it uses a different source for building the test-runtime? |
|
The SVG fragment is not present in any of the deliverables at the download page (SDK, runtime etc.): https://download.eclipse.org/eclipse/downloads/drops4/I20250326-1800/ In particular, it is not included in the Eclipse installation used for running I-Build tests: That why the fragment obviously cannot be found. I will have a look how those packages are generated. I guess that no on in the SDK has a dependency to the SVG fragment (no feature, no bundle), so it's not included. |
|
https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/tree/master/eclipse.platform.releng.tychoeclipsebuilder/eclipse.platform.repository defines the Platform and SDK products and the fragment should be included there (either directly or indirectly via some feature/bundle dependency.) |
|
Here is the specification for the platform runtime assembly, also referencing the IUs to be included: It simply includes the
The first is obviously bad. I would be in favor of the second, as the thirds introduces some implicitly necessity of using SVGs in platform to make tests run again. |
|
I vote for the second option. |
|
I have created an according PR (extracted from the original one that may be extended with other changes): |
|
By the way I think this is actually an issue of the aggregator not of SWT itself. SWT itself can perfectly run the tests (in Jenkins and Github on all supported Archs). So there is some hope that will fix the problem of missing included feature, as it is already part of the ibuild repository as well. |
|
It's definitely an aggregator issue, as we can see from the linked artifacts and scripts. And I am also quite optimistic that adding the fragment to the test feature will resolve the issue, as the test product is defined based on exactly that feature, so it would be strange if the generated product will not contain it. The question is whether productive use of the fragment will work just based on capabilities and without adding it to a fragment, as currently I see no difference in how the test product and the SDK or runtime products are built. In case there is no difference in how they are built, we will need to add the fragment to the e4.rcp feature once we start using SVGs in production bundles, which would then just be what we wanted to do from the beginning. Let's see :-) |
Currently, requiring an SVG rasterizer needs manual placement of an according fragment providing a rasterizer on the classpath. With this change, requiring SVG support can be defined explicitly via a required capability that is provided by the according SWT fragment. Contributes to eclipse-platform#1944
Out of curiosity I did this: So as mentioned earlier, I don't see a reason why it would not work to install given that the
It does not depend much on how they are build but more what is given as the build input, as shown in my example it works with the i-build repository as expected. But the failure we are seeing is in the execution of the individual test machines where things might vary (for history or technical or organizational reasons). |
|
Next I tried this minimal product: and it shows again everything is included as expected. |
|
Adding SVG fragment to SDK tests feature (eclipse-platform/eclipse.platform.releng.aggregator#2948) basically solved the issue. Only the transitive dependency to the JSVG bundle was not added: Will finally be fixed via: |
|
With latest changes, the SWT tests are executed again. Still, the SVG-related tests are failing: Seems like they don't simply execute the SDK test product but just execute a test bundle against an ordinary SDK product (or the like), which does not contain that fragment yet. Might be that we can extend the tooling, e.g., adding the fragment as an extra IU here in the Ant script (https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/blob/bae051ba858018dd4d76cf7a4fe0d544c87a390f/production/testScripts/configuration/sdk.tests/testScripts/test.xml#L40, proposed by @akurtakov). Since the test-specific I-Build tooling is obviously "incomplete" anyway, we could simply ensure that the fragment is contained in runtime/SDK products to avoid spending further efforts here, but that proposal is currently blocked: |
I only object to add it to a fragment used for different purpose. I also don't see that we should do such things to "fix" broken/incomplete testing framework. As mentioned elsewhere, for example jface bundle seem to be a good candidate to require the svg capability if we assume that images of SVGs are most likely created through ImageDescriptors. In any case adding an extra IU seems not that bad, I'm just not sure it will help, as already demonstrated (and shown by the failure) the things are included already, so there must be something else wrong. My suggestion would be to enhance the SWT test to check some preconditions, and possibly give some better feedback this could include:
Currently its a bit hard whats really missing as we are flying blind. |
|
Closing this one as the original issue has been resolved and the follow-up issue (specific failing tests) is documented separately: |
Most likely regression from #1638
See https://download.eclipse.org/eclipse/downloads/drops4/I20250325-1800/testResults.php
The tests page says:
The p2 director logs say
The text was updated successfully, but these errors were encountered: