Provide better names for the test plan specification

- created EngineIdFilter as a new filter
- let the Launcher execute the EngineIdFilter

Issue: #116

junit-engine-api/src/main/java/org/junit/gen5/engine/DiscoveryRequest.java

@@ -26,6 +26,9 @@ public final class DiscoveryRequest {
 	// Selectors provided to the engines to be used for finding tests
 	private final List<DiscoverySelector> selectors = new LinkedList<>();
 
+	// Filter based on the engine id
+	private final List<EngineIdFilter> engineIdFilters = new LinkedList<>();
+
 	// Discovery filters are handed through to all test engines to be applied during discovery
 	private final List<DiscoveryFilter> discoveryFilters = new LinkedList<>();
 
@@ -40,6 +43,14 @@ public void addSelectors(Collection<DiscoverySelector> selectors) {
 		selectors.forEach(this::addSelector);
 	}
 
+	public void addEngineIdFilter(EngineIdFilter engineIdFilter) {
+		this.engineIdFilters.add(engineIdFilter);
+	}
+
+	public void addEngineIdFilters(Collection<EngineIdFilter> engineIdFilters) {
+		this.engineIdFilters.addAll(engineIdFilters);
+	}
+
 	public void addFilter(DiscoveryFilter discoveryFilter) {
 		this.discoveryFilters.add(discoveryFilter);
 	}
@@ -64,6 +75,10 @@ public <T extends DiscoverySelector> List<T> getSelectoryByType(Class<T> selecto
 		return this.selectors.stream().filter(selectorType::isInstance).map(selectorType::cast).collect(toList());
 	}
 
+	public List<EngineIdFilter> getEngineIdFilters() {
+		return unmodifiableList(this.engineIdFilters);
+	}
+
 	public <T extends DiscoveryFilter> List<T> getFilterByType(Class<T> filterType) {
 		return this.discoveryFilters.stream().filter(filterType::isInstance).map(filterType::cast).collect(toList());
 	}
@@ -79,6 +94,6 @@ public boolean acceptDescriptor(TestDescriptor testDescriptor) {
 	}
 
 	public void accept(DiscoverySelectorVisitor visitor) {
-		this.getSelectors().forEach(element -> element.accept(visitor));
+		this.getSelectors().forEach(selector -> selector.accept(visitor));
 	}
 }

junit-engine-api/src/main/java/org/junit/gen5/engine/specification/dsl/DiscoveryRequestBuilder.java

@@ -17,6 +17,7 @@
 import org.junit.gen5.engine.DiscoveryFilter;
 import org.junit.gen5.engine.DiscoveryRequest;
 import org.junit.gen5.engine.DiscoverySelector;
+import org.junit.gen5.engine.EngineIdFilter;
 import org.junit.gen5.engine.PostDiscoveryFilter;
 
 /**
@@ -42,18 +43,16 @@
  *       uniqueId("unique-id-1"),
  *       uniqueId("unique-id-2")
  *     )
- *     .filterBy(
- *       engineIds("junit5"),
- *       classNamePattern(".*Test[s]"),
- *       tagsIncluded("fast"),
- *       tagsExcluded("slow")
- *     )
+ *     .filterBy(engineIds("junit5"))
+ *     .filterBy(classNamePattern("org.junit.gen5.tests"), classNamePattern("org.junit.sample"))
+ *     .filterBy(tagsIncluded("Fast"), tagsExcluded("Slow"))
  *   ).build();
  * </pre>
  */
 public final class DiscoveryRequestBuilder {
 
 	private List<DiscoverySelector> selectors = new LinkedList<>();
+	private List<EngineIdFilter> engineIdFilters = new LinkedList<>();
 	private List<DiscoveryFilter> filters = new LinkedList<>();
 	private List<PostDiscoveryFilter> postFilters = new LinkedList<>();
 
@@ -75,6 +74,13 @@ public DiscoveryRequestBuilder select(List<DiscoverySelector> elements) {
 		return this;
 	}
 
+	public DiscoveryRequestBuilder filterBy(EngineIdFilter... filters) {
+		if (filters != null) {
+			this.engineIdFilters.addAll(Arrays.asList(filters));
+		}
+		return this;
+	}
+
 	public DiscoveryRequestBuilder filterBy(DiscoveryFilter... filters) {
 		if (filters != null) {
 			this.filters.addAll(Arrays.asList(filters));
@@ -93,7 +99,9 @@ public DiscoveryRequestBuilder filterBy(PostDiscoveryFilter... filters) {
 	public DiscoveryRequest build() {
 		DiscoveryRequest discoveryRequest = new DiscoveryRequest();
 		discoveryRequest.addSelectors(this.selectors);
+		discoveryRequest.addEngineIdFilters(this.engineIdFilters);
 		discoveryRequest.addFilters(this.filters);
+		discoveryRequest.addPostFilters(this.postFilters);
 		return discoveryRequest;
 	}
 

junit-engine-api/src/main/java/org/junit/gen5/engine/specification/dsl/EngineFilterBuilder.java

@@ -11,14 +11,8 @@
 package org.junit.gen5.engine.specification.dsl;
 
 import org.junit.gen5.engine.EngineIdFilter;
-import org.junit.gen5.engine.FilterResult;
-import org.junit.gen5.engine.PostDiscoveryFilter;
 
 public class EngineFilterBuilder {
-	public static PostDiscoveryFilter filterByEngineId(String engineId) {
-		return descriptor -> FilterResult.result(descriptor.getUniqueId().startsWith(engineId));
-	}
-
 	public static EngineIdFilter byEngineId(String engineId) {
 		return new EngineIdFilter(engineId);
 	}

junit-engine-api/src/main/java/org/junit/gen5/engine/specification/dsl/NameBasedSelectorBuilder.java

@@ -13,7 +13,7 @@
 import static java.util.stream.Collectors.toList;
 import static org.junit.gen5.engine.specification.dsl.ClassSelectorBuilder.forClass;
 import static org.junit.gen5.engine.specification.dsl.MethodSelectorBuilder.byMethod;
-import static org.junit.gen5.engine.specification.dsl.PackageTestPlanSpecificationElementBuilder.byPackageName;
+import static org.junit.gen5.engine.specification.dsl.PackageSelectorBuilder.byPackageName;
 
 import java.lang.reflect.Method;
 import java.util.*;

junit-engine-api/src/main/java/org/junit/gen5/engine/specification/dsl/PackageTestPlanSpecificationElementBuilder.java → junit-engine-api/src/main/java/org/junit/gen5/engine/specification/dsl/PackageSelectorBuilder.java

@@ -21,12 +21,12 @@
 /**
  * @since 5.0
  */
-public class PackageTestPlanSpecificationElementBuilder {
+public class PackageSelectorBuilder {
 	public static DiscoverySelector byPackageName(String packageName) {
 		return new PackageNameSelector(packageName);
 	}
 
 	public static List<DiscoverySelector> byPackageNames(Collection<String> packageNames) {
-		return packageNames.stream().map(PackageTestPlanSpecificationElementBuilder::byPackageName).collect(toList());
+		return packageNames.stream().map(PackageSelectorBuilder::byPackageName).collect(toList());
 	}
 }

junit-launcher/src/main/java/org/junit/gen5/launcher/Launcher.java

@@ -14,7 +14,6 @@
 import java.util.logging.Logger;
 
 import org.junit.gen5.engine.*;
-import org.junit.gen5.engine.DiscoveryRequest;
 
 /**
  * Facade for <em>discovering</em> and <em>executing</em> tests using
@@ -99,6 +98,14 @@ public void execute(DiscoveryRequest specification) {
 	private RootTestDescriptor discoverRootDescriptor(DiscoveryRequest specification, String phase) {
 		RootTestDescriptor root = new RootTestDescriptor();
 		for (TestEngine testEngine : testEngineRegistry.getTestEngines()) {
+			if (specification.getEngineIdFilters().stream().map(
+				engineIdFilter -> engineIdFilter.filter(testEngine.getId())).anyMatch(FilterResult::isFiltered)) {
+				LOG.fine(
+					() -> String.format("Test discovery for engine '%s' was skipped due to a filter in phase '%s'.",
+						testEngine.getId(), phase));
+				continue;
+			}
+
 			LOG.fine(() -> String.format("Discovering tests during launcher %s phase in engine '%s'.", phase,
 				testEngine.getId()));
 			EngineAwareTestDescriptor engineRoot = testEngine.discoverTests(specification);

junit-tests/src/test/java/org/junit/gen5/engine/DiscoveryRequestBuilderTests.java

@@ -16,7 +16,7 @@
 import static org.junit.gen5.engine.specification.dsl.ClassSelectorBuilder.forClassName;
 import static org.junit.gen5.engine.specification.dsl.DiscoveryRequestBuilder.request;
 import static org.junit.gen5.engine.specification.dsl.MethodSelectorBuilder.byMethod;
-import static org.junit.gen5.engine.specification.dsl.PackageTestPlanSpecificationElementBuilder.byPackageName;
+import static org.junit.gen5.engine.specification.dsl.PackageSelectorBuilder.byPackageName;
 import static org.junit.gen5.engine.specification.dsl.UniqueIdSelectorBuilder.byUniqueId;
 
 import java.io.File;

junit-tests/src/test/java/org/junit/gen5/engine/junit4/JUnit4TestEngineDiscoveryTests.java

@@ -24,7 +24,7 @@
 import static org.junit.gen5.engine.specification.dsl.ClasspathSelectorBuilder.byPaths;
 import static org.junit.gen5.engine.specification.dsl.DiscoveryRequestBuilder.request;
 import static org.junit.gen5.engine.specification.dsl.MethodSelectorBuilder.byMethod;
-import static org.junit.gen5.engine.specification.dsl.PackageTestPlanSpecificationElementBuilder.byPackageName;
+import static org.junit.gen5.engine.specification.dsl.PackageSelectorBuilder.byPackageName;
 import static org.junit.gen5.engine.specification.dsl.UniqueIdSelectorBuilder.byUniqueId;
 
 import java.io.File;

junit-tests/src/test/java/org/junit/gen5/launcher/LauncherTests.java

@@ -12,12 +12,11 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.gen5.engine.specification.dsl.DiscoveryRequestBuilder.request;
-import static org.junit.gen5.engine.specification.dsl.EngineFilterBuilder.filterByEngineId;
+import static org.junit.gen5.engine.specification.dsl.EngineFilterBuilder.byEngineId;
 import static org.junit.gen5.engine.specification.dsl.UniqueIdSelectorBuilder.byUniqueId;
 import static org.junit.gen5.launcher.LauncherFactory.createLauncher;
 
 import org.junit.gen5.api.Test;
-import org.junit.gen5.engine.DiscoveryRequest;
 import org.junit.gen5.engine.DummyTestEngine;
 import org.junit.gen5.engine.TestDescriptor;
 
@@ -63,13 +62,14 @@ public void discoverTestPlanForSingleEngineWithASingleTests() {
 	@Test
 	public void discoverTestPlanForMultipleEngines() {
 		DummyTestEngine firstEngine = new DummyTestEngine("engine1");
-		firstEngine.addTest("test1", noOp());
+		TestDescriptor test1 = firstEngine.addTest("test1", noOp());
 		DummyTestEngine secondEngine = new DummyTestEngine("engine2");
 		TestDescriptor test2 = secondEngine.addTest("test2", noOp());
 
 		Launcher launcher = createLauncher(firstEngine, secondEngine);
 
-		TestPlan testPlan = launcher.discover(request().select(byUniqueId(test2.getUniqueId())).build());
+		TestPlan testPlan = launcher.discover(
+			request().select(byUniqueId(test1.getUniqueId()), byUniqueId(test2.getUniqueId())).build());
 
 		assertThat(testPlan.getRoots()).hasSize(1);
 		TestIdentifier rootIdentifier = testPlan.getRoots().iterator().next();
@@ -79,20 +79,22 @@ public void discoverTestPlanForMultipleEngines() {
 	}
 
 	@Test
-	public void discoverPrunesEnginesWithoutTestsFromTestPlan() {
-		DummyTestEngine engine = new DummyTestEngine();
-		TestDescriptor test = engine.addTest("test", noOp());
-
-		Launcher launcher = createLauncher(engine);
+	public void launcherWillNotCallEnginesThatAreFilterByAnEngineIdFilter() {
+		DummyTestEngine firstEngine = new DummyTestEngine("first");
+		TestDescriptor test1 = firstEngine.addTest("test1", noOp());
+		DummyTestEngine secondEngine = new DummyTestEngine("second");
+		TestDescriptor test2 = secondEngine.addTest("test2", noOp());
 
-		DiscoveryRequest specification = request().select(byUniqueId(test.getUniqueId())).build();
-		specification.addPostFilter(filterByEngineId("doesNotExist"));
+		Launcher launcher = createLauncher(firstEngine, secondEngine);
 
-		TestPlan testPlan = launcher.discover(specification);
+		TestPlan testPlan = launcher.discover(
+			request().select(byUniqueId(test1.getUniqueId()), byUniqueId(test2.getUniqueId())).filterBy(
+				byEngineId("first")).build());
 
 		assertThat(testPlan.getRoots()).hasSize(1);
 		TestIdentifier rootIdentifier = testPlan.getRoots().iterator().next();
-		assertThat(testPlan.getChildren(rootIdentifier.getUniqueId())).isEmpty();
+		assertThat(testPlan.getChildren(rootIdentifier.getUniqueId())).hasSize(1);
+		assertThat(testPlan.getChildren(new TestId("first"))).hasSize(1);
 	}
 
 	private static Runnable noOp() {

junit4-runner/src/main/java/org/junit/gen5/junit4/runner/JUnit5.java

@@ -13,6 +13,7 @@
 import static java.util.Arrays.stream;
 import static java.util.stream.Collectors.toList;
 import static org.junit.gen5.engine.specification.dsl.ClassFilters.classNameMatches;
+import static org.junit.gen5.engine.specification.dsl.ClassSelectorBuilder.forClass;
 import static org.junit.gen5.engine.specification.dsl.DiscoveryRequestBuilder.request;
 
 import java.lang.annotation.Annotation;
@@ -22,7 +23,6 @@
 import java.util.function.Function;
 
 import org.junit.gen5.commons.util.Preconditions;
-import org.junit.gen5.commons.util.StringUtils;
 import org.junit.gen5.engine.DiscoveryRequest;
 import org.junit.gen5.engine.DiscoverySelector;
 import org.junit.gen5.engine.PostDiscoveryFilter;
@@ -95,31 +95,30 @@ private JUnit5TestTree generateTestTree() {
 	}
 
 	private DiscoveryRequest createSpecification() {
-		List<DiscoverySelector> specElements = getSpecElementsFromAnnotations();
+		List<DiscoverySelector> selectors = getSpecElementsFromAnnotations();
 
 		// Allows to simply add @RunWith(JUnit5.class) to any JUnit5 test case
-		if (specElements.isEmpty()) {
-			specElements.add(ClassSelectorBuilder.forClass(this.testClass));
+		if (selectors.isEmpty()) {
+			selectors.add(forClass(this.testClass));
 		}
 
-		DiscoveryRequest spec = request().select(specElements).build();
-		addFiltersFromAnnotations(spec);
-		return spec;
+		DiscoveryRequest request = request().select(selectors).build();
+		addFiltersFromAnnotations(request);
+		return request;
 	}
 
-	private void addFiltersFromAnnotations(DiscoveryRequest spec) {
-		addClassNameMatchesFilter(spec);
-		addIncludeTagsFilter(spec);
-		addExcludeTagsFilter(spec);
-		addEngineIdFilter(spec);
+	private void addFiltersFromAnnotations(DiscoveryRequest request) {
+		addClassNameMatchesFilter(request);
+		addIncludeTagsFilter(request);
+		addExcludeTagsFilter(request);
 	}
 
 	private List<DiscoverySelector> getSpecElementsFromAnnotations() {
-		List<DiscoverySelector> specElements = new ArrayList<>();
-		specElements.addAll(getClassSpecificationElements());
-		specElements.addAll(getUniqueIdSpecificationElements());
-		specElements.addAll(getPackageSpecificationElements());
-		return specElements;
+		List<DiscoverySelector> selectors = new ArrayList<>();
+		selectors.addAll(getClassSpecificationElements());
+		selectors.addAll(getUniqueIdSpecificationElements());
+		selectors.addAll(getPackageSpecificationElements());
+		return selectors;
 	}
 
 	private List<DiscoverySelector> getClassSpecificationElements() {
@@ -131,8 +130,7 @@ private List<DiscoverySelector> getUniqueIdSpecificationElements() {
 	}
 
 	private List<DiscoverySelector> getPackageSpecificationElements() {
-		return stream(getPackageNames()).map(PackageTestPlanSpecificationElementBuilder::byPackageName).collect(
-			toList());
+		return stream(getPackageNames()).map(PackageSelectorBuilder::byPackageName).collect(toList());
 	}
 
 	private void addClassNameMatchesFilter(DiscoveryRequest plan) {
@@ -158,14 +156,6 @@ private void addExcludeTagsFilter(DiscoveryRequest plan) {
 		}
 	}
 
-	private void addEngineIdFilter(DiscoveryRequest plan) {
-		String engineId = getExplicitEngineId();
-		if (StringUtils.isNotBlank(engineId)) {
-			PostDiscoveryFilter engineFilter = EngineFilterBuilder.filterByEngineId(engineId);
-			plan.addPostFilter(engineFilter);
-		}
-	}
-
 	private Class<?>[] getTestClasses() {
 		return getValueFromAnnotation(Classes.class, Classes::value, EMPTY_CLASS_ARRAY);
 	}