Reuse Launcher orchestration in EngineTestKit (#2242)

Resolves #2109.

Author: marcphilipp

documentation/src/docs/asciidoc/release-notes/release-notes-5.7.0-M1.adoc

@@ -22,6 +22,10 @@ on GitHub.
 * In the `EngineTestKit` API, the `all()`, `containers()`, and `tests()` methods in
   `EngineExecutionResults` that were deprecated in JUnit Platform 1.6.0 have been removed
   in favor of `allEvents()`, `containerEvents()`, and `testEvents()`, respectively.
+* The following methods in `EngineTestKit` are now deprecated with replacements:
+  - `execute(String, EngineDiscoveryRequest)` → `execute(String, LauncherDiscoveryRequest)`
+  - `execute(TestEngine, EngineDiscoveryRequest)` → `execute(TestEngine, LauncherDiscoveryRequest)`
+  - `Builder.filters(DiscoveryFilter...)` → `Builder.filters(Filter...)`
 
 ==== New Features and Improvements
 
@@ -36,6 +40,9 @@ on GitHub.
   execution of a submitted test via the returned `Future`.
 * Add `EngineExecutionListener.NOOP` and change all declared methods to have empty default
   implementations.
+* The `EngineTestKit` now reuses the same test discovery and execution logic as the
+  `Launcher`. Thus, it's now possible to test that an engine's behavior in the presence of
+  post-discovery filters (e.g. tag filters) and with regard to pruning.
 * The TestKit now allows to match conditions with events loosely, i.e. an incomplete match
   with or without a fixed order.
 

documentation/src/docs/asciidoc/user-guide/testkit.adoc

@@ -17,10 +17,10 @@ given `{TestEngine}` running on the JUnit Platform and then accessing the result
 fluent API to verify the expected results. The key entry point into this API is the
 `{EngineTestKit}` which provides static factory methods named `engine()` and `execute()`.
 It is recommended that you select one of the `engine()` variants to benefit from the
-fluent API for building an `EngineDiscoveryRequest`.
+fluent API for building a `LauncherDiscoveryRequest`.
 
 NOTE: If you prefer to use the `LauncherDiscoveryRequestBuilder` from the `Launcher` API
-to build your `EngineDiscoveryRequest`, you must use one of the `execute()` variants in
+to build your `LauncherDiscoveryRequest`, you must use one of the `execute()` variants in
 `EngineTestKit`.
 
 The following test class written using JUnit Jupiter will be used in subsequent examples.

junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/NestedTestClassesTests.java

@@ -12,6 +12,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.util.Throwables.getRootCause;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
@@ -182,7 +183,7 @@ private void assertNestedCycle(Class<?> start, Class<?> from, Class<?> to) {
 		assertThatExceptionOfType(JUnitException.class)//
 				.isThrownBy(() -> executeTestsForClass(start))//
 				.withCauseExactlyInstanceOf(JUnitException.class)//
-				.satisfies(ex -> assertThat(ex.getCause()).hasMessageMatching(
+				.satisfies(ex -> assertThat(getRootCause(ex)).hasMessageMatching(
 					String.format("Detected cycle in inner class hierarchy between .+%s and .+%s", from.getSimpleName(),
 						to.getSimpleName())));
 	}

junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/DefaultLauncher.java

@@ -10,27 +10,9 @@
 
 package org.junit.platform.launcher.core;
 
-import java.util.HashSet;
-import java.util.Optional;
-import java.util.Set;
-import java.util.function.Consumer;
-
-import org.junit.platform.commons.JUnitException;
-import org.junit.platform.commons.logging.Logger;
-import org.junit.platform.commons.logging.LoggerFactory;
-import org.junit.platform.commons.util.BlacklistedExceptions;
 import org.junit.platform.commons.util.Preconditions;
-import org.junit.platform.engine.ConfigurationParameters;
-import org.junit.platform.engine.EngineExecutionListener;
-import org.junit.platform.engine.ExecutionRequest;
-import org.junit.platform.engine.FilterResult;
-import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestEngine;
-import org.junit.platform.engine.TestExecutionResult;
-import org.junit.platform.engine.UniqueId;
-import org.junit.platform.launcher.EngineDiscoveryResult;
 import org.junit.platform.launcher.Launcher;
-import org.junit.platform.launcher.LauncherDiscoveryListener;
 import org.junit.platform.launcher.LauncherDiscoveryRequest;
 import org.junit.platform.launcher.TestExecutionListener;
 import org.junit.platform.launcher.TestPlan;
@@ -47,11 +29,9 @@
  */
 class DefaultLauncher implements Launcher {
 
-	private static final Logger logger = LoggerFactory.getLogger(DefaultLauncher.class);
-
 	private final TestExecutionListenerRegistry listenerRegistry = new TestExecutionListenerRegistry();
-	private final EngineDiscoveryResultValidator discoveryResultValidator = new EngineDiscoveryResultValidator();
-	private final Iterable<TestEngine> testEngines;
+	private final EngineExecutionOrchestrator executionOrchestrator = new EngineExecutionOrchestrator(listenerRegistry);
+	private final EngineDiscoveryOrchestrator discoveryOrchestrator;
 
 	/**
 	 * Construct a new {@code DefaultLauncher} with the supplied test engines.
@@ -62,53 +42,7 @@ class DefaultLauncher implements Launcher {
 		Preconditions.condition(testEngines != null && testEngines.iterator().hasNext(),
 			() -> "Cannot create Launcher without at least one TestEngine; "
 					+ "consider adding an engine implementation JAR to the classpath");
-		this.testEngines = validateEngineIds(testEngines);
-	}
-
-	private static Iterable<TestEngine> validateEngineIds(Iterable<TestEngine> testEngines) {
-		Set<String> ids = new HashSet<>();
-		for (TestEngine testEngine : testEngines) {
-			// check usage of reserved id prefix
-			if (!validateReservedIds(testEngine)) {
-				logger.warn(() -> String.format(
-					"Third-party TestEngine implementations are forbidden to use the reserved 'junit-' prefix for their ID: '%s'",
-					testEngine.getId()));
-			}
-
-			// check uniqueness
-			if (!ids.add(testEngine.getId())) {
-				throw new JUnitException(String.format(
-					"Cannot create Launcher for multiple engines with the same ID '%s'.", testEngine.getId()));
-			}
-		}
-		return testEngines;
-	}
-
-	// https://github.com/junit-team/junit5/issues/1557
-	private static boolean validateReservedIds(TestEngine testEngine) {
-		String engineId = testEngine.getId();
-		if (!engineId.startsWith("junit-")) {
-			return true;
-		}
-		if (engineId.equals("junit-jupiter")) {
-			validateWellKnownClassName(testEngine, "org.junit.jupiter.engine.JupiterTestEngine");
-			return true;
-		}
-		if (engineId.equals("junit-vintage")) {
-			validateWellKnownClassName(testEngine, "org.junit.vintage.engine.VintageTestEngine");
-			return true;
-		}
-		return false;
-	}
-
-	private static void validateWellKnownClassName(TestEngine testEngine, String expectedClassName) {
-		String actualClassName = testEngine.getClass().getName();
-		if (actualClassName.equals(expectedClassName)) {
-			return;
-		}
-		throw new JUnitException(
-			String.format("Third-party TestEngine '%s' is forbidden to use the reserved '%s' TestEngine ID.",
-				actualClassName, testEngine.getId()));
+		this.discoveryOrchestrator = new EngineDiscoveryOrchestrator(EngineIdValidator.validate(testEngines));
 	}
 
 	@Override
@@ -121,15 +55,15 @@ public void registerTestExecutionListeners(TestExecutionListener... listeners) {
 	@Override
 	public TestPlan discover(LauncherDiscoveryRequest discoveryRequest) {
 		Preconditions.notNull(discoveryRequest, "LauncherDiscoveryRequest must not be null");
-		return InternalTestPlan.from(discoverRoot(discoveryRequest, "discovery"));
+		return InternalTestPlan.from(discover(discoveryRequest, "discovery"));
 	}
 
 	@Override
 	public void execute(LauncherDiscoveryRequest discoveryRequest, TestExecutionListener... listeners) {
 		Preconditions.notNull(discoveryRequest, "LauncherDiscoveryRequest must not be null");
 		Preconditions.notNull(listeners, "TestExecutionListener array must not be null");
 		Preconditions.containsNoNullElements(listeners, "individual listeners must not be null");
-		execute(InternalTestPlan.from(discoverRoot(discoveryRequest, "execution")), listeners);
+		execute(InternalTestPlan.from(discover(discoveryRequest, "execution")), listeners);
 	}
 
 	@Override
@@ -145,114 +79,12 @@ TestExecutionListenerRegistry getTestExecutionListenerRegistry() {
 		return listenerRegistry;
 	}
 
-	private Root discoverRoot(LauncherDiscoveryRequest discoveryRequest, String phase) {
-		Root root = new Root(discoveryRequest.getConfigurationParameters());
-
-		for (TestEngine testEngine : this.testEngines) {
-			// @formatter:off
-			boolean engineIsExcluded = discoveryRequest.getEngineFilters().stream()
-					.map(engineFilter -> engineFilter.apply(testEngine))
-					.anyMatch(FilterResult::excluded);
-			// @formatter:on
-
-			if (engineIsExcluded) {
-				logger.debug(() -> String.format(
-					"Test discovery for engine '%s' was skipped due to an EngineFilter in phase '%s'.",
-					testEngine.getId(), phase));
-				continue;
-			}
-
-			logger.debug(() -> String.format("Discovering tests during Launcher %s phase in engine '%s'.", phase,
-				testEngine.getId()));
-
-			TestDescriptor rootDescriptor = discoverEngineRoot(testEngine, discoveryRequest);
-			root.add(testEngine, rootDescriptor);
-		}
-		root.applyPostDiscoveryFilters(discoveryRequest);
-		root.prune();
-		return root;
-	}
-
-	private TestDescriptor discoverEngineRoot(TestEngine testEngine, LauncherDiscoveryRequest discoveryRequest) {
-		LauncherDiscoveryListener discoveryListener = discoveryRequest.getDiscoveryListener();
-		UniqueId uniqueEngineId = UniqueId.forEngine(testEngine.getId());
-		try {
-			discoveryListener.engineDiscoveryStarted(uniqueEngineId);
-			TestDescriptor engineRoot = testEngine.discover(discoveryRequest, uniqueEngineId);
-			discoveryResultValidator.validate(testEngine, engineRoot);
-			discoveryListener.engineDiscoveryFinished(uniqueEngineId, EngineDiscoveryResult.successful());
-			return engineRoot;
-		}
-		catch (Throwable throwable) {
-			BlacklistedExceptions.rethrowIfBlacklisted(throwable);
-			String message = String.format("TestEngine with ID '%s' failed to discover tests", testEngine.getId());
-			JUnitException cause = new JUnitException(message, throwable);
-			discoveryListener.engineDiscoveryFinished(uniqueEngineId, EngineDiscoveryResult.failed(cause));
-			return new EngineDiscoveryErrorDescriptor(uniqueEngineId, testEngine, cause);
-		}
+	private LauncherDiscoveryResult discover(LauncherDiscoveryRequest discoveryRequest, String phase) {
+		return discoveryOrchestrator.discover(discoveryRequest, phase);
 	}
 
 	private void execute(InternalTestPlan internalTestPlan, TestExecutionListener[] listeners) {
-		Root root = internalTestPlan.getRoot();
-		ConfigurationParameters configurationParameters = root.getConfigurationParameters();
-		TestExecutionListenerRegistry listenerRegistry = buildListenerRegistryForExecution(listeners);
-		withInterceptedStreams(configurationParameters, listenerRegistry, testExecutionListener -> {
-			testExecutionListener.testPlanExecutionStarted(internalTestPlan);
-			ExecutionListenerAdapter engineExecutionListener = new ExecutionListenerAdapter(internalTestPlan,
-				testExecutionListener);
-			for (TestEngine testEngine : root.getTestEngines()) {
-				TestDescriptor engineDescriptor = root.getTestDescriptorFor(testEngine);
-				if (engineDescriptor instanceof EngineDiscoveryErrorDescriptor) {
-					engineExecutionListener.executionStarted(engineDescriptor);
-					engineExecutionListener.executionFinished(engineDescriptor,
-						TestExecutionResult.failed(((EngineDiscoveryErrorDescriptor) engineDescriptor).getCause()));
-				}
-				else {
-					execute(engineDescriptor, engineExecutionListener, configurationParameters, testEngine);
-				}
-			}
-			testExecutionListener.testPlanExecutionFinished(internalTestPlan);
-		});
-	}
-
-	private void withInterceptedStreams(ConfigurationParameters configurationParameters,
-			TestExecutionListenerRegistry listenerRegistry, Consumer<TestExecutionListener> action) {
-
-		TestExecutionListener testExecutionListener = listenerRegistry.getCompositeTestExecutionListener();
-		Optional<StreamInterceptingTestExecutionListener> streamInterceptingTestExecutionListener = StreamInterceptingTestExecutionListener.create(
-			configurationParameters, testExecutionListener::reportingEntryPublished);
-		streamInterceptingTestExecutionListener.ifPresent(listenerRegistry::registerListeners);
-		try {
-			action.accept(testExecutionListener);
-		}
-		finally {
-			streamInterceptingTestExecutionListener.ifPresent(StreamInterceptingTestExecutionListener::unregister);
-		}
-	}
-
-	private TestExecutionListenerRegistry buildListenerRegistryForExecution(TestExecutionListener... listeners) {
-		if (listeners.length == 0) {
-			return this.listenerRegistry;
-		}
-		TestExecutionListenerRegistry registry = new TestExecutionListenerRegistry(this.listenerRegistry);
-		registry.registerListeners(listeners);
-		return registry;
-	}
-
-	private void execute(TestDescriptor engineDescriptor, EngineExecutionListener listener,
-			ConfigurationParameters configurationParameters, TestEngine testEngine) {
-
-		OutcomeDelayingEngineExecutionListener delayingListener = new OutcomeDelayingEngineExecutionListener(listener,
-			engineDescriptor);
-		try {
-			testEngine.execute(new ExecutionRequest(engineDescriptor, delayingListener, configurationParameters));
-			delayingListener.reportEngineOutcome();
-		}
-		catch (Throwable throwable) {
-			BlacklistedExceptions.rethrowIfBlacklisted(throwable);
-			delayingListener.reportEngineFailure(new JUnitException(
-				String.format("TestEngine with ID '%s' failed to execute tests", testEngine.getId()), throwable));
-		}
+		executionOrchestrator.execute(internalTestPlan, listeners);
 	}
 
 }