-
Notifications
You must be signed in to change notification settings - Fork 78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for specify test requirements of a bundle #914
base: master
Are you sure you want to change the base?
Conversation
d126b73
to
d5be63b
Compare
43df917
to
a77bb15
Compare
a77bb15
to
11d5b47
Compare
Thanks for this. I'll review it tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for bringing this forward, this will definitively help to reduce the need for Test-Fragments.
But what I think is totally missing is support for launching the Plugin projects with embedded tests as JUnit-Plugin test, isn't it?
In that case we need to adjust the manifest and add the test-requirements as well.
The launched Equinox runtime would then need to be pointed to the modified manifest, not sure if a dev-classpath entry is sufficient?
Content-Assist would also be nice to have.
And if I read the code correct, one can only add bundles?
It would be convenient if features could be added as well. I have seen a few test-projects in the Eclipse-SDK that require entire features.
The IU syntax could be used for that (i.e. if a requirement ends with .feature.group it could at least be attempted to get the corresponding feature).
Nevertheless this could also be done in a subsequent PR.
package org.eclipse.pde.internal.core.bnd; | ||
|
||
import aQute.bnd.osgi.PluginsContainer; | ||
|
||
public class PDEPluginsContainer extends PluginsContainer { | ||
|
||
public PDEPluginsContainer() { | ||
add(TargetRepository.getTargetRepository()); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary?
I cannot see any reference to it, neither in code nor in metadata?
// Add any library entries contributed via classpath contributor | ||
// extension (Bug 363733) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this and the other unrelated white-space changes.
Repository repository = TargetRepository.getTargetRepository(); | ||
Map<Requirement, Collection<Capability>> providers = repository.findProviders(requirements); | ||
List<BundleCap> testBundles = providers.values().stream().flatMap(Collection::stream) | ||
.map(cap -> cap.getResource()).distinct().map(res -> ResourceUtils.getBundleCapability(res)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.map(cap -> cap.getResource()).distinct().map(res -> ResourceUtils.getBundleCapability(res)) | |
.map(Capability::getResource).distinct().map(ResourceUtils::getBundleCapability) |
ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/builders/BuildErrorReporter.java
Show resolved
Hide resolved
Currently there is only limited support for "test only" dependencies of a bundle, the usual way is to add an (optional) import/require bundle to the test but this is not very suitable when one wants to have tests in the bundle itself, also some requirements can't be expressed as a package/bundle without binding to a particular implementation and even if, these are still added as regular dependencies and not with test scope. This ads a way to specify a list of 'test.requirements' in the build.properties to account for this.
11d5b47
to
aee286a
Compare
This only covers the definition of requirements everything else has to be supplied on top of it. |
Features would already work (if we can have them defined e.g. with something like bndtools/bnd#5805) because a feature is nothing we want/need to fetch here, a feature just require other things and then the bundle set will just grow with the additional requirements so in the end it all is bundles only, but this is definitely something we need more support in PDE with additional PRs. |
Yes of course, in the end a feature is only a group of bundles and one could simply replace it with all (transitively) contained bundles. |
The problem is not collecting features, this already would work, but features are currently not represented as resources and therefore can't be required. |
Currently there is only limited support for "test only" dependencies of a bundle, the usual way is to add an (optional) import/require bundle to the test but this is not very suitable when one wants to have tests in the bundle itself, also some requirements can't be expressed as a package/bundle without binding to a particular implementation and even if, these are still added as regular dependencies and not with test scope.
This ads a way to specify a list of 'test.requirements' in the build.properties to account for this.
Will require