Consider bundle versions when adding JUnit to launch

and remove unnecessary separate attempts to add junit-launcher and
jupiter.engine bundles to the runtime, intending to support
@RunWith(JUnitPlatform.class), because these bundles always are already
added to a runtime.

This change adjusts JUnitLaunchConfigurationDelegate,
to consider bundle versions when adding required JUnit bundles.
E.g. when adding JUnit bundles for a JUnit 5 test, JUnit 5 versions are
added.
This avoids conflicts when both JUnit 5 and JUnit 6 bundles are in the
platform, since they share symbolic names.

Furthermore handle multiple versions of a bundle when searching them in
the host.

Fixes: #2006

Co-authored-by: Hannes Wellmann <[email protected]>
Co-authored-by: Ed Merks <[email protected]>

Author: 3 people

ui/org.eclipse.pde.launching/src/org/eclipse/pde/internal/launching/JUnitLaunchRequirements.java

@@ -15,14 +15,14 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
-import java.util.stream.Collectors;
+import java.util.function.Function;
+import java.util.stream.Stream;
 
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.Status;
@@ -34,69 +34,72 @@
 import org.eclipse.pde.internal.core.DependencyManager;
 import org.eclipse.pde.internal.core.PDECore;
 import org.eclipse.pde.internal.launching.launcher.BundleLauncherHelper;
+import org.osgi.framework.wiring.BundleRevision;
 
 public class JUnitLaunchRequirements {
 
 	public static final String JUNIT4_JDT_RUNTIME_PLUGIN = "org.eclipse.jdt.junit4.runtime"; //$NON-NLS-1$
 	public static final String JUNIT5_JDT_RUNTIME_PLUGIN = "org.eclipse.jdt.junit5.runtime"; //$NON-NLS-1$
+	private static final Comparator<IPluginModelBase> VERSION = Comparator.comparing(p -> p.getBundleDescription().getVersion());
 
 	public static void addRequiredJunitRuntimePlugins(ILaunchConfiguration configuration, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
-		Set<String> requiredPlugins = new LinkedHashSet<>(getRequiredJunitRuntimeEclipsePlugins(configuration));
-
-		if (allBundles.containsKey("junit-platform-runner")) { //$NON-NLS-1$
-			// add launcher and jupiter.engine to support @RunWith(JUnitPlatform.class)
-			requiredPlugins.add("junit-platform-launcher"); //$NON-NLS-1$
-			requiredPlugins.add("junit-jupiter-engine"); //$NON-NLS-1$
-		}
-
-		Set<BundleDescription> addedRequirements = new HashSet<>();
-		addAbsentRequirements(requiredPlugins, addedRequirements, allBundles, allModels);
-
-		Set<BundleDescription> requirementsOfRequirements = DependencyManager.findRequirementsClosure(addedRequirements);
-		Set<String> rorIds = requirementsOfRequirements.stream().map(BundleDescription::getSymbolicName).collect(Collectors.toSet());
-		addAbsentRequirements(rorIds, null, allBundles, allModels);
+		Collection<String> runtimePlugins = getRequiredJunitRuntimeEclipsePlugins(configuration);
+		Set<BundleDescription> addedRuntimeBundles = addAbsentRequirements(runtimePlugins, allBundles, allModels);
+		Set<BundleDescription> runtimeRequirements = DependencyManager.findRequirementsClosure(addedRuntimeBundles);
+		addAbsentRequirements(runtimeRequirements, allBundles, allModels);
 	}
 
 	@SuppressWarnings("restriction")
 	public static Collection<String> getRequiredJunitRuntimeEclipsePlugins(ILaunchConfiguration configuration) {
 		org.eclipse.jdt.internal.junit.launcher.ITestKind testKind = org.eclipse.jdt.internal.junit.launcher.JUnitLaunchConfigurationConstants.getTestRunnerKind(configuration);
 		if (testKind.isNull()) {
-			return Collections.emptyList();
+			return List.of();
 		}
 		List<String> plugins = new ArrayList<>();
 		plugins.add("org.eclipse.pde.junit.runtime"); //$NON-NLS-1$
-
-		if (org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT4_TEST_KIND_ID.equals(testKind.getId())) {
-			plugins.add("org.eclipse.jdt.junit4.runtime"); //$NON-NLS-1$
-		} else if (org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT5_TEST_KIND_ID.equals(testKind.getId())) {
-			plugins.add("org.eclipse.jdt.junit5.runtime"); //$NON-NLS-1$
+		switch (testKind.getId()) {
+			case org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT3_TEST_KIND_ID -> {
+			} // Nothing to add for JUnit-3
+			case org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT4_TEST_KIND_ID -> plugins.add(JUNIT4_JDT_RUNTIME_PLUGIN);
+			case org.eclipse.jdt.internal.junit.launcher.TestKindRegistry.JUNIT5_TEST_KIND_ID -> plugins.add(JUNIT5_JDT_RUNTIME_PLUGIN);
+			default -> throw new IllegalArgumentException("Unsupported junit test kind: " + testKind.getId()); //$NON-NLS-1$
 		}
 		return plugins;
 	}
 
-	private static void addAbsentRequirements(Collection<String> requirements, Set<BundleDescription> addedRequirements, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
+	private static Set<BundleDescription> addAbsentRequirements(Collection<String> requirements, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
+		Set<BundleDescription> addedRequirements = new LinkedHashSet<>();
 		for (String id : requirements) {
 			List<IPluginModelBase> models = allBundles.computeIfAbsent(id, k -> new ArrayList<>());
-			if (models.stream().noneMatch(m -> m.getBundleDescription().isResolved())) {
-				IPluginModelBase model = findRequiredPluginInTargetOrHost(id);
+			if (models.stream().noneMatch(p -> p.getBundleDescription().isResolved())) {
+				IPluginModelBase model = findRequiredPluginInTargetOrHost(PluginRegistry.findModel(id), plugins -> plugins.max(VERSION), id);
+				models.add(model);
+				BundleLauncherHelper.addDefaultStartingBundle(allModels, model);
+				addedRequirements.add(model.getBundleDescription());
+			}
+		}
+		return addedRequirements;
+	}
+
+	private static void addAbsentRequirements(Set<BundleDescription> requirements, Map<String, List<IPluginModelBase>> allBundles, Map<IPluginModelBase, String> allModels) throws CoreException {
+		for (BundleRevision bundle : requirements) {
+			String id = bundle.getSymbolicName();
+			List<IPluginModelBase> models = allBundles.computeIfAbsent(id, k -> new ArrayList<>());
+			if (models.stream().map(IPluginModelBase::getBundleDescription).noneMatch(b -> b.isResolved() && b.getVersion().equals(bundle.getVersion()))) {
+				IPluginModelBase model = findRequiredPluginInTargetOrHost(PluginRegistry.findModel(bundle), plgs -> plgs.filter(p -> p.getBundleDescription() == bundle).findFirst(), id);
 				models.add(model);
 				BundleLauncherHelper.addDefaultStartingBundle(allModels, model);
-				if (addedRequirements != null) {
-					addedRequirements.add(model.getBundleDescription());
-				}
 			}
 		}
 	}
 
-	private static IPluginModelBase findRequiredPluginInTargetOrHost(String id) throws CoreException {
-		IPluginModelBase model = PluginRegistry.findModel(id);
+	private static IPluginModelBase findRequiredPluginInTargetOrHost(IPluginModelBase model, Function<Stream<IPluginModelBase>, Optional<IPluginModelBase>> pluginSelector, String id) throws CoreException {
 		if (model == null || !model.getBundleDescription().isResolved()) {
 			// prefer bundle from host over unresolved bundle from target
-			model = PDECore.getDefault().findPluginInHost(id).max(Comparator.comparing(p -> p.getBundleDescription().getVersion())).orElse(null);
-		}
-		if (model == null) {
-			throw new CoreException(Status.error(NLS.bind(PDEMessages.JUnitLaunchConfiguration_error_missingPlugin, id)));
+			model = pluginSelector.apply(PDECore.getDefault().findPluginInHost(id)) //
+					.orElseThrow(() -> new CoreException(Status.error(NLS.bind(PDEMessages.JUnitLaunchConfiguration_error_missingPlugin, id))));
 		}
 		return model;
 	}
+
 }