Ensure discovered descriptor graphs are acyclic

Prior this commit only a null-check was performed by the DefaultLauncher
against the root test descriptor returned by an engine. Other properties
where not checked, especially if the graph of that root descriptor
contained cycles it could lead to stack overflow errors when traversing
the graph for execution.

This commit addresses this issue by explicitly checking the returned
graph of the root test descriptor for cycles. If a cycle is detected a
PreconditionViolationException is thrown.

Fixes: #1447

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

@@ -13,6 +13,7 @@
 import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
+import java.util.Stack;
 
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.logging.Logger;
@@ -69,6 +70,27 @@ private static Iterable<TestEngine> validateUniqueIds(Iterable<TestEngine> testE
 		return testEngines;
 	}
 
+	/**
+	 * @return {@code true} if the graph does _not_ contain a cycle; else {@code false}.
+	 */
+	static boolean isAcyclic(TestDescriptor root) {
+		Set<UniqueId> visited = new HashSet<>();
+		visited.add(root.getUniqueId());
+		Stack<TestDescriptor> stack = new Stack<>();
+		stack.push(root);
+		while (!stack.isEmpty()) {
+			for (TestDescriptor child : stack.pop().getChildren()) {
+				if (!visited.add(child.getUniqueId())) {
+					return false; // id already known: cycle detected!
+				}
+				if (child.isContainer()) {
+					stack.add(child);
+				}
+			}
+		}
+		return true;
+	}
+
 	@Override
 	public void registerTestExecutionListeners(TestExecutionListener... listeners) {
 		Preconditions.notEmpty(listeners, "listeners array must not be null or empty");
@@ -132,6 +154,9 @@ private Optional<TestDescriptor> discoverEngineRoot(TestEngine testEngine,
 				() -> String.format(
 					"The discover() method for TestEngine with ID '%s' must return a non-null root TestDescriptor.",
 					testEngine.getId()));
+			Preconditions.condition(isAcyclic(engineRoot),
+				() -> String.format("The discover() method for TestEngine with ID '%s' returned a cyclic graph.",
+					testEngine.getId()));
 			return Optional.of(engineRoot);
 		}
 		catch (Throwable throwable) {

platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/AbstractTestDescriptorTests.java

@@ -34,7 +34,7 @@
  */
 class AbstractTestDescriptorTests {
 
-	EngineDescriptor engineDescriptor;
+	private EngineDescriptor engineDescriptor;
 
 	@BeforeEach
 	void initTree() {

platform-tests/src/test/java/org/junit/platform/launcher/core/DefaultLauncherTests.java

@@ -12,6 +12,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement;
@@ -164,6 +165,29 @@ public TestDescriptor discover(org.junit.platform.engine.EngineDiscoveryRequest
 		assertThat(testPlan.getRoots()).hasSize(0);
 	}
 
+	@Test
+	void detectCycleWithDoubleRoot() {
+		TestDescriptorStub root = new TestDescriptorStub(UniqueId.forEngine("root"), "root");
+		assertTrue(DefaultLauncher.isAcyclic(root));
+
+		root.addChild(root);
+		assertFalse(DefaultLauncher.isAcyclic(root));
+	}
+
+	@Test
+	void detectCycleWithDoubleGroup() {
+		UniqueId rootId = UniqueId.forEngine("root");
+		TestDescriptorStub root = new TestDescriptorStub(rootId, "root");
+		TestDescriptor group1 = new TestDescriptorStub(rootId.append("group", "1"), "1");
+		TestDescriptor group2 = new TestDescriptorStub(rootId.append("group", "2"), "2");
+		root.addChild(group1);
+		root.addChild(group2);
+		assertTrue(DefaultLauncher.isAcyclic(root));
+
+		group2.addChild(group1);
+		assertFalse(DefaultLauncher.isAcyclic(root));
+	}
+
 	@Test
 	void discoverTestPlanForSingleEngine() {
 		DemoHierarchicalTestEngine engine = new DemoHierarchicalTestEngine("myEngine");