Avoid discovery errors for inner classes not annotated with @Nested

documentation/src/docs/asciidoc/release-notes/release-notes-5.13.2.adoc

@@ -35,7 +35,8 @@ on GitHub.
 [[release-notes-5.13.2-junit-jupiter-bug-fixes]]
 ==== Bug Fixes
 
-* ❓
+* Stop reporting discovery issues for cyclic inner class hierarchies not annotated with
+  `@Nested`.
 
 [[release-notes-5.13.2-junit-jupiter-deprecations-and-breaking-changes]]
 ==== Deprecations and Breaking Changes

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/ClassSelectorResolver.java

@@ -17,9 +17,9 @@
 import static org.junit.jupiter.engine.descriptor.NestedClassTestDescriptor.getEnclosingTestClasses;
 import static org.junit.platform.commons.support.HierarchyTraversalMode.TOP_DOWN;
 import static org.junit.platform.commons.support.ReflectionSupport.findMethods;
-import static org.junit.platform.commons.support.ReflectionSupport.streamNestedClasses;
 import static org.junit.platform.commons.util.FunctionUtils.where;
 import static org.junit.platform.commons.util.ReflectionUtils.isInnerClass;
+import static org.junit.platform.commons.util.ReflectionUtils.streamNestedClasses;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
 import static org.junit.platform.engine.support.discovery.SelectorResolver.Resolution.unresolved;
 
@@ -46,6 +46,7 @@
 import org.junit.jupiter.engine.discovery.predicates.TestClassPredicates;
 import org.junit.platform.commons.support.ReflectionSupport;
 import org.junit.platform.commons.util.ReflectionUtils;
+import org.junit.platform.commons.util.ReflectionUtils.CycleErrorHandling;
 import org.junit.platform.engine.DiscoveryIssue;
 import org.junit.platform.engine.DiscoveryIssue.Severity;
 import org.junit.platform.engine.DiscoverySelector;
@@ -289,9 +290,13 @@ private Supplier<Set<? extends DiscoverySelector>> expansionCallback(TestDescrip
 			Stream<DiscoverySelector> methods = findMethods(testClass,
 				this.predicates.isTestOrTestFactoryOrTestTemplateMethod, TOP_DOWN).stream() //
 						.map(method -> selectMethod(testClasses, method));
-			Stream<NestedClassSelector> nestedClasses = streamNestedClasses(testClass,
-				this.predicates.isAnnotatedWithNested.or(ReflectionUtils::isInnerClass)) //
-						.map(nestedClass -> DiscoverySelectors.selectNestedClass(testClasses, nestedClass));
+			Stream<Class<?>> annotatedNestedClasses = streamNestedClasses(testClass,
+				this.predicates.isAnnotatedWithNested);
+			Stream<Class<?>> notAnnotatedInnerClasses = streamNestedClasses(testClass,
+				this.predicates.isAnnotatedWithNested.negate().and(ReflectionUtils::isInnerClass),
+				CycleErrorHandling.ABORT_VISIT);
+			var nestedClasses = Stream.concat(annotatedNestedClasses, notAnnotatedInnerClasses) //
+					.map(nestedClass -> DiscoverySelectors.selectNestedClass(testClasses, nestedClass));
 			return Stream.concat(methods, nestedClasses).collect(
 				toCollection((Supplier<Set<DiscoverySelector>>) LinkedHashSet::new));
 		};

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/predicates/TestClassPredicates.java

@@ -20,12 +20,15 @@
 import static org.junit.platform.commons.util.ReflectionUtils.isNestedClassPresent;
 
 import java.lang.reflect.Method;
+import java.util.HashSet;
+import java.util.Set;
 import java.util.function.Predicate;
 
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.ClassTemplate;
 import org.junit.jupiter.api.Nested;
 import org.junit.platform.commons.util.ReflectionUtils;
+import org.junit.platform.commons.util.ReflectionUtils.CycleErrorHandling;
 import org.junit.platform.engine.DiscoveryIssue;
 import org.junit.platform.engine.support.descriptor.ClassSource;
 import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
@@ -65,9 +68,16 @@ public TestClassPredicates(DiscoveryIssueReporter issueReporter) {
 	}
 
 	public boolean looksLikeIntendedTestClass(Class<?> candidate) {
-		return this.isAnnotatedWithClassTemplate.test(candidate) //
-				|| hasTestOrTestFactoryOrTestTemplateMethods(candidate) //
-				|| hasNestedTests(candidate);
+		return looksLikeIntendedTestClass(candidate, new HashSet<>());
+	}
+
+	private boolean looksLikeIntendedTestClass(Class<?> candidate, Set<Class<?>> seen) {
+		if (seen.add(candidate)) {
+			return this.isAnnotatedWithClassTemplate.test(candidate) //
+					|| hasTestOrTestFactoryOrTestTemplateMethods(candidate) //
+					|| hasNestedTests(candidate, seen);
+		}
+		return false;
 	}
 
 	public boolean isValidNestedTestClass(Class<?> candidate) {
@@ -84,15 +94,17 @@ private boolean hasTestOrTestFactoryOrTestTemplateMethods(Class<?> candidate) {
 		return isMethodPresent(candidate, this.isTestOrTestFactoryOrTestTemplateMethod);
 	}
 
-	private boolean hasNestedTests(Class<?> candidate) {
+	private boolean hasNestedTests(Class<?> candidate, Set<Class<?>> seen) {
+		var hasAnnotatedClass = isNestedClassPresent(candidate, this.isAnnotatedWithNested,
+			CycleErrorHandling.THROW_EXCEPTION);
+		if (hasAnnotatedClass) {
+			return true;
+		}
 		return isNestedClassPresent( //
 			candidate, //
-			isNotSame(candidate).and(
-				this.isAnnotatedWithNested.or(it -> isInnerClass(it) && looksLikeIntendedTestClass(it))));
-	}
-
-	private static Predicate<Class<?>> isNotSame(Class<?> candidate) {
-		return clazz -> candidate != clazz;
+			it -> isInnerClass(it) && looksLikeIntendedTestClass(it, seen), //
+			CycleErrorHandling.ABORT_VISIT //
+		);
 	}
 
 	private static Condition<Class<?>> isNotPrivateUnlessAbstract(String prefix, DiscoveryIssueReporter issueReporter) {

junit-platform-commons/src/main/java/org/junit/platform/commons/util/ReflectionUtils.java

@@ -130,7 +130,7 @@ public enum HierarchyTraversalMode {
 	 * <p>This serves as a cache to avoid repeated cycle detection for classes
 	 * that have already been checked.
 	 * @since 1.6
-	 * @see #detectInnerClassCycle(Class)
+	 * @see #detectInnerClassCycle(Class, CycleErrorHandling)
 	 */
 	private static final Set<String> noCyclesDetectedCache = ConcurrentHashMap.newKeySet();
 
@@ -1117,11 +1117,17 @@ public static Stream<Resource> streamAllResourcesInModule(String moduleName, Pre
 	 * @see org.junit.platform.commons.support.ReflectionSupport#findNestedClasses(Class, Predicate)
 	 */
 	public static List<Class<?>> findNestedClasses(Class<?> clazz, Predicate<Class<?>> predicate) {
+		return findNestedClasses(clazz, predicate, CycleErrorHandling.THROW_EXCEPTION);
+	}
+
+	@API(status = INTERNAL, since = "5.13.2")
+	public static List<Class<?>> findNestedClasses(Class<?> clazz, Predicate<Class<?>> predicate,
+			CycleErrorHandling errorHandling) {
 		Preconditions.notNull(clazz, "Class must not be null");
 		Preconditions.notNull(predicate, "Predicate must not be null");
 
 		Set<Class<?>> candidates = new LinkedHashSet<>();
-		visitAllNestedClasses(clazz, predicate, candidates::add);
+		visitAllNestedClasses(clazz, predicate, candidates::add, errorHandling);
 		return List.copyOf(candidates);
 	}
 
@@ -1144,13 +1150,15 @@ public static List<Class<?>> findNestedClasses(Class<?> clazz, Predicate<Class<?
 	 * @return {@code true} if such a nested class is present
 	 * @throws JUnitException if a cycle is detected within an inner class hierarchy
 	 */
-	@API(status = INTERNAL, since = "1.13")
-	public static boolean isNestedClassPresent(Class<?> clazz, Predicate<Class<?>> predicate) {
+	@API(status = INTERNAL, since = "1.13.2")
+	public static boolean isNestedClassPresent(Class<?> clazz, Predicate<Class<?>> predicate,
+			CycleErrorHandling errorHandling) {
 		Preconditions.notNull(clazz, "Class must not be null");
 		Preconditions.notNull(predicate, "Predicate must not be null");
+		Preconditions.notNull(errorHandling, "CycleErrorHandling must not be null");
 
 		AtomicBoolean foundNestedClass = new AtomicBoolean(false);
-		visitAllNestedClasses(clazz, predicate, __ -> foundNestedClass.setPlain(true));
+		visitAllNestedClasses(clazz, predicate, __ -> foundNestedClass.setPlain(true), errorHandling);
 		return foundNestedClass.getPlain();
 	}
 
@@ -1162,27 +1170,37 @@ public static Stream<Class<?>> streamNestedClasses(Class<?> clazz, Predicate<Cla
 		return findNestedClasses(clazz, predicate).stream();
 	}
 
+	@API(status = INTERNAL, since = "5.13.2")
+	public static Stream<Class<?>> streamNestedClasses(Class<?> clazz, Predicate<Class<?>> predicate,
+			CycleErrorHandling errorHandling) {
+		return findNestedClasses(clazz, predicate, errorHandling).stream();
+	}
+
 	/**
 	 * Visit <em>all</em> nested classes without support for short-circuiting
 	 * in order to ensure all of them are checked for class cycles.
 	 */
 	private static void visitAllNestedClasses(Class<?> clazz, Predicate<Class<?>> predicate,
-			Consumer<Class<?>> consumer) {
+			Consumer<Class<?>> consumer, CycleErrorHandling errorHandling) {
 
 		if (!isSearchable(clazz)) {
 			return;
 		}
 
 		if (isInnerClass(clazz) && predicate.test(clazz)) {
-			detectInnerClassCycle(clazz);
+			if (detectInnerClassCycle(clazz, errorHandling)) {
+				return;
+			}
 		}
 
 		try {
 			// Candidates in current class
 			for (Class<?> nestedClass : clazz.getDeclaredClasses()) {
 				if (predicate.test(nestedClass)) {
-					detectInnerClassCycle(nestedClass);
 					consumer.accept(nestedClass);
+					if (detectInnerClassCycle(nestedClass, errorHandling)) {
+						return;
+					}
 				}
 			}
 		}
@@ -1191,48 +1209,47 @@ private static void visitAllNestedClasses(Class<?> clazz, Predicate<Class<?>> pr
 		}
 
 		// Search class hierarchy
-		visitAllNestedClasses(clazz.getSuperclass(), predicate, consumer);
+		visitAllNestedClasses(clazz.getSuperclass(), predicate, consumer, errorHandling);
 
 		// Search interface hierarchy
 		for (Class<?> ifc : clazz.getInterfaces()) {
-			visitAllNestedClasses(ifc, predicate, consumer);
+			visitAllNestedClasses(ifc, predicate, consumer, errorHandling);
 		}
 	}
 
 	/**
 	 * Detect a cycle in the inner class hierarchy in which the supplied class
 	 * resides &mdash; from the supplied class up to the outermost enclosing class
 	 * &mdash; and throw a {@link JUnitException} if a cycle is detected.
-	 *
 	 * <p>This method does <strong>not</strong> detect cycles within inner class
 	 * hierarchies <em>below</em> the supplied class.
-	 *
 	 * <p>If the supplied class is not an inner class and does not have a
 	 * searchable superclass, this method is effectively a no-op.
 	 *
 	 * @since 1.6
 	 * @see #isInnerClass(Class)
 	 * @see #isSearchable(Class)
 	 */
-	private static void detectInnerClassCycle(Class<?> clazz) {
+	private static boolean detectInnerClassCycle(Class<?> clazz, CycleErrorHandling errorHandling) {
 		Preconditions.notNull(clazz, "Class must not be null");
 		String className = clazz.getName();
 
 		if (noCyclesDetectedCache.contains(className)) {
-			return;
+			return false;
 		}
 
 		Class<?> superclass = clazz.getSuperclass();
 		if (isInnerClass(clazz) && isSearchable(superclass)) {
 			for (Class<?> enclosing = clazz.getEnclosingClass(); enclosing != null; enclosing = enclosing.getEnclosingClass()) {
 				if (superclass.equals(enclosing)) {
-					throw new JUnitException(String.format("Detected cycle in inner class hierarchy between %s and %s",
-						className, enclosing.getName()));
+					errorHandling.handle(clazz, enclosing);
+					return true;
 				}
 			}
 		}
 
 		noCyclesDetectedCache.add(className);
+		return false;
 	}
 
 	/**
@@ -1936,4 +1953,25 @@ static Throwable getUnderlyingCause(Throwable t) {
 		return t;
 	}
 
+	@API(status = INTERNAL, since = "5.13.2")
+	public enum CycleErrorHandling {
+
+		THROW_EXCEPTION {
+			@Override
+			void handle(Class<?> clazz, Class<?> enclosing) {
+				throw new JUnitException(String.format("Detected cycle in inner class hierarchy between %s and %s",
+					clazz.getName(), enclosing.getName()));
+			}
+		},
+
+		ABORT_VISIT {
+			@Override
+			void handle(Class<?> clazz, Class<?> enclosing) {
+				// ignore
+			}
+		};
+
+		abstract void handle(Class<?> clazz, Class<?> enclosing);
+	}
+
 }

jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java

@@ -48,6 +48,7 @@
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.platform.engine.DiscoveryIssue;
+import org.junit.platform.engine.DiscoveryIssue.Severity;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.support.descriptor.ClassSource;
 import org.junit.platform.launcher.LauncherDiscoveryRequest;
@@ -289,7 +290,35 @@ void reportsWarningForTestClassWithPotentialNestedTestClasses() {
 	}
 
 	@Test
-	void reportsWarningsForInvalidTags() throws NoSuchMethodException {
+	void ignoresUnrelatedClassDefinitionCycles() {
+		var results = discoverTestsForClass(UnrelatedRecursiveHierarchyTestCase.class);
+
+		assertThat(results.getDiscoveryIssues()).isEmpty();
+	}
+
+	@Test
+	void ignoresRecursiveNonTestHierarchyCycles() {
+		var results = discoverTestsForClass(NonTestRecursiveHierarchyTestCase.class);
+
+		assertThat(results.getDiscoveryIssues()).isEmpty();
+	}
+
+	@Test
+	void reportsMissingNestedAnnotationOnRecursiveHierarchy() {
+		var results = discoverTestsForClass(RecursiveHierarchyWithoutNestedTestCase.class);
+
+		var discoveryIssues = results.getDiscoveryIssues();
+		assertThat(discoveryIssues).hasSize(1);
+		assertThat(discoveryIssues.getFirst().severity()) //
+				.isEqualTo(Severity.WARNING);
+		assertThat(discoveryIssues.getFirst().message()) //
+				.isEqualTo(
+					"Inner class '%s' looks like it was intended to be a test class but will not be executed. It must be static or annotated with @Nested.",
+					RecursiveHierarchyWithoutNestedTestCase.Inner.class.getName());
+	}
+
+	@Test
+	void reportsWarningsForInvalidTags() throws Exception {
 
 		var results = discoverTestsForClass(InvalidTagsTestCase.class);
 
@@ -311,7 +340,7 @@ void reportsWarningsForInvalidTags() throws NoSuchMethodException {
 	}
 
 	@Test
-	void reportsWarningsForBlankDisplayNames() throws NoSuchMethodException {
+	void reportsWarningsForBlankDisplayNames() throws Exception {
 
 		var results = discoverTestsForClass(BlankDisplayNamesTestCase.class);
 
@@ -441,6 +470,39 @@ private class InvalidTestClassSubclassTestCase extends InvalidTestClassTestCase
 
 	}
 
+	@SuppressWarnings("JUnitMalformedDeclaration")
+	static class UnrelatedRecursiveHierarchyTestCase {
+
+		@Test
+		void test() {
+		}
+
+		@SuppressWarnings({ "InnerClassMayBeStatic", "unused" })
+		class Inner {
+			class Recursive extends Inner {
+			}
+		}
+	}
+
+	@SuppressWarnings("JUnitMalformedDeclaration")
+	static class RecursiveHierarchyWithoutNestedTestCase {
+
+		@Test
+		void test() {
+		}
+
+		@SuppressWarnings({ "InnerClassMayBeStatic", "unused" })
+		class Inner extends RecursiveHierarchyWithoutNestedTestCase {
+		}
+	}
+
+	@SuppressWarnings("unused")
+	static class NonTestRecursiveHierarchyTestCase {
+		@SuppressWarnings("InnerClassMayBeStatic")
+		class Inner extends NonTestRecursiveHierarchyTestCase {
+		}
+	}
+
 	@SuppressWarnings("JUnitMalformedDeclaration")
 	@Tag("")
 	static class InvalidTagsTestCase {

jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/predicates/TestClassPredicatesTests.java

@@ -11,7 +11,6 @@
 package org.junit.jupiter.engine.discovery.predicates;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
@@ -24,7 +23,6 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.TestFactory;
 import org.junit.jupiter.api.TestTemplate;
-import org.junit.platform.commons.JUnitException;
 import org.junit.platform.engine.DiscoveryIssue;
 import org.junit.platform.engine.DiscoveryIssue.Severity;
 import org.junit.platform.engine.support.descriptor.ClassSource;
@@ -218,11 +216,7 @@ void privateStaticTestClassEvaluatesToFalse() {
 		 */
 		@Test
 		void recursiveHierarchies() {
-			assertThatExceptionOfType(JUnitException.class)//
-					.isThrownBy(() -> predicates.looksLikeIntendedTestClass(TestCases.OuterClass.class))//
-					.withMessage("Detected cycle in inner class hierarchy between %s and %s",
-						TestCases.OuterClass.RecursiveInnerClass.class.getName(), TestCases.OuterClass.class.getName());
-
+			assertTrue(predicates.looksLikeIntendedTestClass(TestCases.OuterClass.class));
 			assertTrue(predicates.isValidStandaloneTestClass(TestCases.OuterClass.class));
 			assertThat(discoveryIssues).isEmpty();