Bug 351396 - Ignore test-fragments when adding requirements to launch
And unify detection of test-fragments.
Change-Id: Ia0a81f1f54c8f5de01cc33e2d3a068fd9f40420d
Signed-off-by: Hannes Wellmann <wellmann.hannes1@gmx.net>
Reviewed-on: https://git.eclipse.org/r/c/pde/eclipse.pde.ui/+/189927
Tested-by: PDE Bot <pde-bot@eclipse.org>
Reviewed-by: Julian Honnen <julian.honnen@vector.com>
diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java
index fe67cbf..db606bd 100644
--- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java
+++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/ClasspathComputer.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2005, 2020 IBM Corporation and others.
+ * Copyright (c) 2005, 2022 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -21,6 +21,7 @@
import java.util.Map;
import java.util.Map.Entry;
import java.util.regex.Pattern;
+import java.util.stream.Stream;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
@@ -87,9 +88,7 @@
}
private static void addSourceAndLibraries(IProject project, IPluginModelBase model, IBuild build, boolean clear, Map<?, ?> sourceLibraryMap, ArrayList<IClasspathEntry> result) throws CoreException {
- String testPluginPattern = PDECore.getDefault().getPreferencesManager().getString(ICoreConstants.TEST_PLUGIN_PATTERN);
- boolean isTestPlugin = testPluginPattern != null && testPluginPattern.length() > 0
- && Pattern.compile(testPluginPattern).matcher(project.getName()).find();
+ boolean isTestPlugin = hasTestPluginName(project);
HashSet<IPath> paths = new HashSet<>();
// keep existing source folders
@@ -141,21 +140,52 @@
}
}
- private static IClasspathEntry updateTestAttribute(boolean isTestPlugin, IClasspathEntry entry) {
+ public static boolean hasTestPluginName(IProject project) {
+ String pattern = PDECore.getDefault().getPreferencesManager().getString(ICoreConstants.TEST_PLUGIN_PATTERN);
+ return pattern != null && !pattern.isEmpty() && Pattern.compile(pattern).matcher(project.getName()).find();
+ }
+
+ /**
+ * Returns true if the given project is a java project that has
+ * {@code IClasspathEntry#CPE_SOURCE source classpath-entries} that are all
+ * marked as {@code IClasspathAttribute#TEST test sources}.
+ *
+ * @param project
+ * @return true if the given project is a test java project
+ */
+ public static boolean hasTestOnlyClasspath(IProject project) {
+ IJavaProject javaProject = JavaCore.create(project);
+ if (javaProject == null) {
+ return false;
+ }
+ try {
+ boolean hasSources = false;
+ for (IClasspathEntry entry : javaProject.getRawClasspath()) {
+ if (entry.getEntryKind() == IClasspathEntry.CPE_SOURCE) {
+ hasSources = true;
+ if (!entry.isTest()) {
+ return false;
+ }
+ }
+ }
+ return hasSources; // if it has sources, all are test-sources
+ } catch (JavaModelException e) { // assume no valid java-project
+ }
+ return false;
+ }
+
+ public static IClasspathEntry updateTestAttribute(boolean isTestPlugin, IClasspathEntry entry) {
if (isTestPlugin == entry.isTest() || entry.getEntryKind() != IClasspathEntry.CPE_SOURCE) {
return entry;
}
- IClasspathAttribute[] classpathAttributes = Arrays.stream(entry.getExtraAttributes())
- .filter(e -> !e.getName().equals(IClasspathAttribute.TEST)).toArray(IClasspathAttribute[]::new);
+ Stream<IClasspathAttribute> cpAttributes = Arrays.stream(entry.getExtraAttributes())
+ .filter(e -> !e.getName().equals(IClasspathAttribute.TEST));
if (isTestPlugin) {
- int length = classpathAttributes.length;
- System.arraycopy(classpathAttributes, 0, classpathAttributes = new IClasspathAttribute[length + 1], 0,
- length);
- classpathAttributes[length] = JavaCore.newClasspathAttribute(IClasspathAttribute.TEST, "true"); //$NON-NLS-1$
+ IClasspathAttribute testAttribute = JavaCore.newClasspathAttribute(IClasspathAttribute.TEST, "true"); //$NON-NLS-1$
+ cpAttributes = Stream.concat(cpAttributes, Stream.of(testAttribute));
}
- return JavaCore.newSourceEntry(entry.getPath(), entry.getInclusionPatterns(),
- entry.getExclusionPatterns(), entry.getOutputLocation(), classpathAttributes);
-
+ return JavaCore.newSourceEntry(entry.getPath(), entry.getInclusionPatterns(), entry.getExclusionPatterns(),
+ entry.getOutputLocation(), cpAttributes.toArray(IClasspathAttribute[]::new));
}
private static IClasspathAttribute[] getClasspathAttributes(IProject project, IPluginModelBase model) {
diff --git a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java
index ef8496a..971cd1d 100644
--- a/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java
+++ b/ui/org.eclipse.pde.core/src/org/eclipse/pde/internal/core/DependencyManager.java
@@ -24,10 +24,12 @@
import java.util.List;
import java.util.Queue;
import java.util.Set;
+import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.osgi.service.resolver.BundleDescription;
import org.eclipse.osgi.service.resolver.State;
import org.eclipse.pde.core.plugin.IPluginModelBase;
+import org.eclipse.pde.core.plugin.PluginRegistry;
import org.eclipse.pde.core.target.ITargetPlatformService;
import org.eclipse.pde.core.target.NameVersionDescriptor;
import org.osgi.framework.Constants;
@@ -53,8 +55,16 @@
/** Specifies to include all optional dependencies into the closure. */
INCLUDE_OPTIONAL_DEPENDENCIES,
- /** Specifies to include all fragments into the closure. */
+ /**
+ * Specifies to include all fragments into the closure (must not be
+ * combined with {@link #INCLUDE_NON_TEST_FRAGMENTS}).
+ */
INCLUDE_ALL_FRAGMENTS,
+ /**
+ * Specifies to include all non-test fragments into the closure (must
+ * not be combined with {@link #INCLUDE_ALL_FRAGMENTS}).
+ */
+ INCLUDE_NON_TEST_FRAGMENTS;
}
/**
@@ -153,6 +163,10 @@
Set<Options> optionSet = Set.of(options);
boolean includeOptional = optionSet.contains(Options.INCLUDE_OPTIONAL_DEPENDENCIES);
boolean includeAllFragments = optionSet.contains(Options.INCLUDE_ALL_FRAGMENTS);
+ boolean includeNonTestFragments = optionSet.contains(Options.INCLUDE_NON_TEST_FRAGMENTS);
+ if (includeAllFragments && includeNonTestFragments) {
+ throw new AssertionError("Cannot combine INCLUDE_ALL_FRAGMENTS and INCLUDE_NON_TEST_FRAGMENTS"); //$NON-NLS-1$
+ }
Set<BundleDescription> closure = new HashSet<>(bundles.size() * 4 / 3 + 1);
Queue<BundleDescription> pending = new ArrayDeque<>();
@@ -180,10 +194,12 @@
}
}
- if (includeAllFragments) {
+ if (includeAllFragments || includeNonTestFragments) {
// A fragment's host is already required by a wire
for (BundleDescription fragment : bundle.getFragments()) {
- addNewRequiredBundle(fragment, closure, pending);
+ if (includeAllFragments || !isTestWorkspaceProject(fragment)) {
+ addNewRequiredBundle(fragment, closure, pending);
+ }
}
}
}
@@ -208,6 +224,18 @@
return Constants.RESOLUTION_OPTIONAL.equals(requirement.getDirectives().get(Constants.RESOLUTION_DIRECTIVE));
}
+ private static boolean isTestWorkspaceProject(BundleDescription f) {
+ // Be defensive when declaring a fragment as 'test'-fragment
+ IPluginModelBase pluginModel = PluginRegistry.findModel(f);
+ if (pluginModel != null) {
+ IResource resource = pluginModel.getUnderlyingResource();
+ if (resource != null) {
+ return ClasspathComputer.hasTestOnlyClasspath(resource.getProject());
+ } // test-fragments are usually not part of the target-platform
+ }
+ return false;
+ }
+
/**
* Computes the set of implicit dependencies from the
* {@link PDEPreferencesManager}.
diff --git a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
index 4c9ad9f..77e0d3a 100644
--- a/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
+++ b/ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/launcher/BundleLauncherHelper.java
@@ -153,7 +153,7 @@
}
}
// Get all required plugins
- Set<BundleDescription> additionalBundles = DependencyManager.getDependencies(launchPlugins, DependencyManager.Options.INCLUDE_ALL_FRAGMENTS);
+ Set<BundleDescription> additionalBundles = DependencyManager.getDependencies(launchPlugins, DependencyManager.Options.INCLUDE_NON_TEST_FRAGMENTS);
for (BundleDescription bundle : additionalBundles) {
IPluginModelBase plugin = getRequiredPlugin(bundle.getSymbolicName(), bundle.getVersion().toString(), IMatchRules.PERFECT, defaultPluginResolution);
launchPlugins.add(Objects.requireNonNull(plugin));// should never be null
diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java
index 3cbb5ce..696a3d9 100644
--- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java
+++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/core/tests/internal/DependencyManagerTest.java
@@ -19,6 +19,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.pde.internal.core.DependencyManager.findRequirementsClosure;
import static org.eclipse.pde.internal.core.DependencyManager.Options.INCLUDE_ALL_FRAGMENTS;
+import static org.eclipse.pde.internal.core.DependencyManager.Options.INCLUDE_NON_TEST_FRAGMENTS;
import static org.eclipse.pde.internal.core.DependencyManager.Options.INCLUDE_OPTIONAL_DEPENDENCIES;
import static org.osgi.framework.Constants.EXPORT_PACKAGE;
import static org.osgi.framework.Constants.FRAGMENT_HOST;
@@ -29,11 +30,18 @@
import java.io.IOException;
import java.nio.file.Path;
-import java.util.Map;
+import java.util.*;
import java.util.Map.Entry;
-import java.util.Set;
+import org.eclipse.core.resources.IProject;
+import org.eclipse.core.runtime.CoreException;
+import org.eclipse.jdt.core.IClasspathEntry;
+import org.eclipse.jdt.core.JavaCore;
import org.eclipse.osgi.service.resolver.BundleDescription;
+import org.eclipse.osgi.service.resolver.VersionRange;
+import org.eclipse.pde.core.plugin.IPluginModelBase;
+import org.eclipse.pde.core.plugin.PluginRegistry;
import org.eclipse.pde.core.target.NameVersionDescriptor;
+import org.eclipse.pde.internal.core.ClasspathComputer;
import org.eclipse.pde.internal.core.PluginModelManager;
import org.eclipse.pde.ui.tests.launcher.AbstractLaunchTest;
import org.eclipse.pde.ui.tests.util.ProjectUtils;
@@ -189,6 +197,41 @@
}
@Test
+ public void testFindRequirementsClosure_includeNonTestFragments() throws Exception {
+
+ setTargetPlatform( //
+ bundle("bundle.a", "1.0.0", //
+ entry(EXPORT_PACKAGE, "bundle.a.pack" + version("1.0.0"))),
+
+ bundle("bundle.fragment", "1.0.0", //
+ entry(FRAGMENT_HOST, "bundle.a")),
+
+ bundle("other.tests", "1.0.0", //
+ entry(FRAGMENT_HOST, "bundle.a")));
+
+ BundleDescription bundleA = bundleDescription("bundle.a", "1.0.0");
+ BundleDescription bundleFragment = bundleDescription("bundle.fragment", "1.0.0");
+ BundleDescription otherFragmentWithTestName = bundleDescription("other.tests", "1.0.0");
+
+ BundleDescription testFragmentWithTestAttr = createFragmentProject("bundle.a1.tests", "bundle.a", true);
+ BundleDescription testFragmentWithOtherName = createFragmentProject("bundle.a.checks", "bundle.a", true);
+ BundleDescription testFragmentWithoutTestAttr = createFragmentProject("bundle.a2.tests", "bundle.a", false);
+
+ Set<BundleDescription> bundles = Set.of(bundleA);
+
+ Set<BundleDescription> noFragmentsClosure = findRequirementsClosure(bundles);
+ assertThat(noFragmentsClosure).isEqualTo(Set.of(bundleA));
+
+ Set<BundleDescription> allFragmentsClosure = findRequirementsClosure(bundles, INCLUDE_ALL_FRAGMENTS);
+ assertThat(allFragmentsClosure).isEqualTo(Set.of(bundleA, bundleFragment, otherFragmentWithTestName,
+ testFragmentWithTestAttr, testFragmentWithOtherName, testFragmentWithoutTestAttr));
+
+ Set<BundleDescription> nonTestFragmentsClosure = findRequirementsClosure(bundles, INCLUDE_NON_TEST_FRAGMENTS);
+ assertThat(nonTestFragmentsClosure)
+ .isEqualTo(Set.of(bundleA, bundleFragment, otherFragmentWithTestName, testFragmentWithoutTestAttr));
+ }
+
+ @Test
public void testFindRequirementsClosure_includeOptional() throws Exception {
setTargetPlatform( //
@@ -248,7 +291,22 @@
private static final String OPTIONAL = Constants.RESOLUTION_OPTIONAL;
- private BundleDescription bundleDescription(String id, String version) {
+ private static BundleDescription bundleDescription(String id, String version) {
return AbstractLaunchTest.findTargetModel(id, version).getBundleDescription();
}
+
+ private static BundleDescription createFragmentProject(String projectName, String hostName,
+ boolean setTestAttribute) throws CoreException {
+
+ IProject project = ProjectUtils.createPluginProject(projectName, projectName, "1.0.0", (d, s) -> {
+ d.setHost(s.newHost(hostName, VersionRange.emptyRange));
+ });
+ IPluginModelBase model = PluginRegistry.findModel(project);
+ if (setTestAttribute) { // set test attribute in classpath
+ IClasspathEntry[] classpath = ClasspathComputer.getClasspath(project, model, null, false, true);
+ var cpEntries = Arrays.stream(classpath).map(e -> ClasspathComputer.updateTestAttribute(true, e));
+ JavaCore.create(project).setRawClasspath(cpEntries.toArray(IClasspathEntry[]::new), null);
+ }
+ return model.getBundleDescription();
+ }
}
diff --git a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/util/ProjectUtils.java b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/util/ProjectUtils.java
index 05b2b26..51d51e8 100644
--- a/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/util/ProjectUtils.java
+++ b/ui/org.eclipse.pde.ui.tests/src/org/eclipse/pde/ui/tests/util/ProjectUtils.java
@@ -18,12 +18,14 @@
import java.net.URL;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiConsumer;
import org.eclipse.core.resources.*;
import org.eclipse.core.runtime.*;
import org.eclipse.jdt.core.IJavaProject;
import org.eclipse.jdt.core.JavaCore;
import org.eclipse.jdt.launching.environments.IExecutionEnvironment;
import org.eclipse.pde.core.project.IBundleProjectDescription;
+import org.eclipse.pde.core.project.IBundleProjectService;
import org.eclipse.pde.core.target.NameVersionDescriptor;
import org.eclipse.pde.internal.ui.wizards.IProjectProvider;
import org.eclipse.pde.internal.ui.wizards.plugin.NewProjectCreationOperation;
@@ -165,17 +167,26 @@
}
public static IProject createPluginProject(String bundleSymbolicName, String bundleVersion) throws CoreException {
- return createPluginProject(bundleSymbolicName + bundleVersion.replace('.', '_'), bundleSymbolicName, bundleVersion);
+ return createPluginProject(bundleSymbolicName + bundleVersion.replace('.', '_'), bundleSymbolicName,
+ bundleVersion);
}
public static IProject createPluginProject(String projectName, String bundleSymbolicName, String version)
throws CoreException {
+ return createPluginProject(projectName, bundleSymbolicName, version, (d, s) -> {
+ });
+ }
+
+ public static IProject createPluginProject(String projectName, String bundleSymbolicName, String version,
+ BiConsumer<IBundleProjectDescription, IBundleProjectService> setup) throws CoreException {
IProject project = ResourcesPlugin.getWorkspace().getRoot().getProject(projectName);
- IBundleProjectDescription description = ProjectCreationTests.getBundleProjectService().getDescription(project);
+ IBundleProjectService bundleProjectService = ProjectCreationTests.getBundleProjectService();
+ IBundleProjectDescription description = bundleProjectService.getDescription(project);
description.setSymbolicName(bundleSymbolicName);
if (version != null) {
description.setBundleVersion(Version.parseVersion(version));
}
+ setup.accept(description, bundleProjectService);
description.apply(null);
return project;
}
diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/product/PluginSection.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/product/PluginSection.java
index 7b4af13..8acbcca 100644
--- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/product/PluginSection.java
+++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/editor/product/PluginSection.java
@@ -356,8 +356,8 @@
}).filter(Objects::nonNull).map(IPluginModelBase::getBundleDescription).collect(Collectors.toList());
DependencyManager.Options[] options = includeOptional
- ? new Options[] { Options.INCLUDE_ALL_FRAGMENTS, Options.INCLUDE_OPTIONAL_DEPENDENCIES }
- : new Options[] { Options.INCLUDE_ALL_FRAGMENTS };
+ ? new Options[] { Options.INCLUDE_NON_TEST_FRAGMENTS, Options.INCLUDE_OPTIONAL_DEPENDENCIES }
+ : new Options[] { Options.INCLUDE_NON_TEST_FRAGMENTS };
Set<BundleDescription> dependencies = DependencyManager.findRequirementsClosure(list, options);
IProduct product = plugins[0].getProduct();
diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/AbstractPluginBlock.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/AbstractPluginBlock.java
index 810ace2..4c3c1b8 100644
--- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/AbstractPluginBlock.java
+++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/launcher/AbstractPluginBlock.java
@@ -800,8 +800,8 @@
.map(IPluginModelBase.class::cast).collect(Collectors.toList());
DependencyManager.Options[] options = fIncludeOptionalButton.getSelection()
- ? new Options[] { Options.INCLUDE_ALL_FRAGMENTS, Options.INCLUDE_OPTIONAL_DEPENDENCIES }
- : new Options[] { Options.INCLUDE_ALL_FRAGMENTS };
+ ? new Options[] { Options.INCLUDE_NON_TEST_FRAGMENTS, Options.INCLUDE_OPTIONAL_DEPENDENCIES }
+ : new Options[] { Options.INCLUDE_NON_TEST_FRAGMENTS };
Set<BundleDescription> additionalBundles = DependencyManager.getDependencies(toCheck, options);
additionalBundles.stream().map(PluginRegistry::findModel).filter(Objects::nonNull).forEach(toCheck::add);
diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/plugin/NewProjectCreationOperation.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/plugin/NewProjectCreationOperation.java
index 3c239984..27e0fe6 100644
--- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/plugin/NewProjectCreationOperation.java
+++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/plugin/NewProjectCreationOperation.java
@@ -1,5 +1,5 @@
/*******************************************************************************
- * Copyright (c) 2000, 2020 IBM Corporation and others.
+ * Copyright (c) 2000, 2022 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
@@ -20,7 +20,6 @@
import java.lang.reflect.InvocationTargetException;
import java.util.*;
-import java.util.regex.Pattern;
import org.eclipse.core.resources.*;
import org.eclipse.core.runtime.*;
import org.eclipse.core.runtime.preferences.IEclipsePreferences;
@@ -485,10 +484,7 @@
}
IClasspathEntry[] entries = new IClasspathEntry[1];
IPath path = project.getProject().getFullPath().append(data.getSourceFolderName());
- String testPluginPattern = PDECore.getDefault().getPreferencesManager()
- .getString(ICoreConstants.TEST_PLUGIN_PATTERN);
- boolean isTestPlugin = testPluginPattern != null && testPluginPattern.length() > 0
- && Pattern.compile(testPluginPattern).matcher(project.getProject().getName()).find();
+ boolean isTestPlugin = ClasspathComputer.hasTestPluginName(project.getProject());
if (isTestPlugin) {
IClasspathAttribute testAttribute = JavaCore.newClasspathAttribute(IClasspathAttribute.TEST, "true"); //$NON-NLS-1$
entries[0] = JavaCore.newSourceEntry(path, null, null, null, new IClasspathAttribute[] { testAttribute });
diff --git a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/product/ProductFromExtensionOperation.java b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/product/ProductFromExtensionOperation.java
index f5e2af4..041df04 100644
--- a/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/product/ProductFromExtensionOperation.java
+++ b/ui/org.eclipse.pde.ui/src/org/eclipse/pde/internal/ui/wizards/product/ProductFromExtensionOperation.java
@@ -68,7 +68,7 @@
}
}
Set<BundleDescription> bundles = DependencyManager.findRequirementsClosure(plugins,
- DependencyManager.Options.INCLUDE_ALL_FRAGMENTS);
+ DependencyManager.Options.INCLUDE_NON_TEST_FRAGMENTS);
return bundles.stream().map(BundleDescription::getSymbolicName).toArray(String[]::new);
}
