Ensure discovered descriptor graphs are acyclic

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));
+	}
+
+}