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

@@ -44,6 +44,7 @@ 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;
 
 	/**
@@ -128,10 +129,7 @@ private Optional<TestDescriptor> discoverEngineRoot(TestEngine testEngine,
 		UniqueId uniqueEngineId = UniqueId.forEngine(testEngine.getId());
 		try {
 			TestDescriptor engineRoot = testEngine.discover(discoveryRequest, uniqueEngineId);
-			Preconditions.notNull(engineRoot,
-				() -> String.format(
-					"The discover() method for TestEngine with ID '%s' must return a non-null root TestDescriptor.",
-					testEngine.getId()));
+			discoveryResultValidator.validate(testEngine, engineRoot);
 			return Optional.of(engineRoot);
 		}
 		catch (Throwable throwable) {

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

@@ -0,0 +1,66 @@
+/*
+ * Copyright 2015-2018 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * http://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.platform.launcher.core;
+
+import java.util.ArrayDeque;
+import java.util.HashSet;
+import java.util.Queue;
+import java.util.Set;
+
+import org.junit.platform.commons.util.Preconditions;
+import org.junit.platform.engine.TestDescriptor;
+import org.junit.platform.engine.TestEngine;
+import org.junit.platform.engine.UniqueId;
+
+/**
+ * Perform common validation checks on the result from the `discover()` method.
+ *
+ * @since 1.3
+ */
+class EngineDiscoveryResultValidator {
+
+	/**
+	 *  Perform common validation checks.
+	 *
+	 * @throws org.junit.platform.commons.util.PreconditionViolationException if any check fails
+	 */
+	void validate(TestEngine testEngine, TestDescriptor root) {
+		Preconditions.notNull(root,
+			() -> String.format(
+				"The discover() method for TestEngine with ID '%s' must return a non-null root TestDescriptor.",
+				testEngine.getId()));
+		Preconditions.condition(isAcyclic(root),
+			() -> String.format("The discover() method for TestEngine with ID '%s' returned a cyclic graph.",
+				testEngine.getId()));
+	}
+
+	/**
+	 * @return {@code true} if the tree does <em>not</em> contain a cycle; else {@code false}.
+	 */
+	boolean isAcyclic(TestDescriptor root) {
+		Set<UniqueId> visited = new HashSet<>();
+		visited.add(root.getUniqueId());
+		Queue<TestDescriptor> queue = new ArrayDeque<>();
+		queue.add(root);
+		while (!queue.isEmpty()) {
+			for (TestDescriptor child : queue.remove().getChildren()) {
+				if (!visited.add(child.getUniqueId())) {
+					return false; // id already known: cycle detected!
+				}
+				if (child.isContainer()) {
+					queue.add(child);
+				}
+			}
+		}
+		return true;
+	}
+
+}

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/EngineDiscoveryResultValidatorTests.java

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2015-2018 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v2.0 which
+ * accompanies this distribution and is available at
+ *
+ * http://www.eclipse.org/legal/epl-v20.html
+ */
+
+package org.junit.platform.launcher.core;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.junit.jupiter.api.Test;
+import org.junit.platform.engine.TestDescriptor;
+import org.junit.platform.engine.UniqueId;
+import org.junit.platform.engine.test.TestDescriptorStub;
+
+/**
+ * @since 1.3
+ */
+class EngineDiscoveryResultValidatorTests {
+
+	private final EngineDiscoveryResultValidator validator = new EngineDiscoveryResultValidator();
+
+	@Test
+	void detectCycleWithDoubleRoot() {
+		TestDescriptorStub root = new TestDescriptorStub(UniqueId.forEngine("root"), "root");
+		assertTrue(validator.isAcyclic(root));
+
+		root.addChild(root);
+		assertFalse(validator.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(validator.isAcyclic(root));
+
+		group2.addChild(group1);
+		assertFalse(validator.isAcyclic(root));
+	}
+
+	@Test
+	void detectCycleWithDoubleTest() {
+		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);
+		TestDescriptor test1 = new TestDescriptorStub(rootId.append("test", "1"), "1-1");
+		TestDescriptor test2 = new TestDescriptorStub(rootId.append("test", "2"), "2-2");
+		group1.addChild(test1);
+		group2.addChild(test2);
+		assertTrue(validator.isAcyclic(root));
+
+		group2.addChild(test1);
+		assertFalse(validator.isAcyclic(root));
+	}
+
+}