TestSuiteHelper: Include felix.scr if osgi.component is required#1970
TestSuiteHelper: Include felix.scr if osgi.component is required#1970sratz wants to merge 3 commits intoeclipse-pde:masterfrom
Conversation
|
This works for me, but isn't very pretty. |
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
Test Results 324 files - 441 324 suites - 441 41m 13s ⏱️ - 12m 47s For more details on these errors, see this check. Results for commit ad88220. ± Comparison against base commit 9213517. This pull request removes 2650 tests.♻️ This comment has been updated with latest results. |
sratz
force-pushed
the
apitools-test-scr
branch
from
ffe47f7 to
5fbac9f
Compare
| public static void addAllRequired(IApiBaseline baseline, Set<String> done, IApiComponent component, List<IApiComponent> collection) throws CoreException { | ||
| IRequiredComponentDescription[] descriptions = component.getRequiredComponents(); | ||
| if (component instanceof BundleComponent bc) { | ||
| for (BundleRequirement extenderRequirements : bc.getBundleDescription() |
There was a problem hiding this comment.
I have not tested it but
there is BundleDescription#getGenericRequires and we use it in P2 BundlesAction it should include the extender requirement.
The reverse is BundleDescription#getGenericCapabilities see BundlesAction so one could of course try to match those instead, so if there is a generic requires find a bundle that has a generic capability matching those.
There was a problem hiding this comment.
Yes, but we do not have any pool of bundles available to query their capabilities here and match() them against the requirements.
That's the way these apitooling tests work:
They only load bundles of explicit names via getBundle(String) from a directory specified via -DrequiredBundles=... that this logic in addAllRequired() determines to load.
So I came up with this quick-and-dirty check.
There was a problem hiding this comment.
They only load bundles of explicit names via getBundle(String) from a directory specified via -DrequiredBundles=... that this logic in addAllRequired() determines to load.
One might of course thing about reading all bundles from that directory and building an index...
At least it would get you the method GenericSpecification.getType() / getFilter() so a litte bit less parsing maybe...
There was a problem hiding this comment.
I pushed an improved algorithm that requires no parsing:
- If we find an osgi.extender requirement
- then try to load the felix.scr bundle and see if it's providing a capability for that requirement using the standard API
- if it matches, add it + its dependencies
Some test cases use 'org.eclipse.core.runtime' as dependency. One of the runtime's dependencies (org.eclipse.core.contenttype) uses declarative services to provide a service component, but was missing the Require-Capability: header to require the osgi.extender=osgi.component. In eclipse-platform/eclipse.platform#2162 this was corrected. But as a result, for the resolution of org.eclipse.core.runtime to succeed, a OSGI extender provider must now strictly be present. This cannot be auto-resolved by the PDE API tooling tests, as these tests are quite special and the bundles available in the resolver state are hand-picked by TestSuiteHelper.addAllRequired(IApiBaseline, Set<String>, IApiComponent, List<IApiComponent>) which currently does not support this kind of dependency. To work around that, we check if we find a dependency on SCR and if so, add an implementation for it to the baseline fixture: org.apache.felix.scr and its dependencies.
sratz
force-pushed
the
apitools-test-scr
branch
from
9dd2ed5 to
04e1f4c
Compare
| for (String bundleId : List.of("org.osgi.service.event", //$NON-NLS-1$ | ||
| "org.osgi.service.component", "org.osgi.util.promise", //$NON-NLS-1$ //$NON-NLS-2$ | ||
| "org.osgi.util.function")) { //$NON-NLS-1$ |
There was a problem hiding this comment.
Can't we just use the DependencyManager to compute all dependencies instead of hard-coding them?
There was a problem hiding this comment.
In principle, yes. In practice, no, as DependencyManager uses TargetPlatformHelper.getState() for the list of all available bundles. But here, in TestHelper, in this method we are in process of manually curating such a resolver state based on a directory passed in via -DrequiredBundles=.... So I don't see how we could do that easily.
|
What is the current state here? I see PDE tests are still failing, contributions to PDE are affected. |
|
I didn't know this PR was festering in the backlog and fixed this with a different hack: |
6 participants
Some test cases use 'org.eclipse.core.runtime' as dependency.
One of the runtime's dependencies (org.eclipse.core.contenttype) uses declarative services to provide a service component, but was missing the Require-Capability: header to require the osgi.extender=osgi.component.
In eclipse-platform/eclipse.platform#2162 this was corrected.
But as a result, for the resolution of org.eclipse.core.runtime to succeed, a OSGI extender provider must now strictly be present.
This cannot be auto-resolved as the PDE API tooling tests, as these tests are quite special and the bundles available in the resolver state are hand-picked by
which currently does not support this kind of dependency.
To work around that, we check if we find a dependency on SCR and if so, add an implementation for it to the baseline fixture: org.apache.felix.scr and its dependencies.
See also: eclipse-equinox/equinox#1146