Skip to content

Commit 442aac1

Browse files
authored
Run non-inner @Nested classes to restore backward compatibility (#4700)
Since the introduction of discovery issues in 5.13, top-level and static member classes annotated with `@Nested` were no longer executed because the validation logic excluded them. This commit restores the old behavior and improves the generated discovery issues for such cases. Fixes #4686.
1 parent cc9b919 commit 442aac1

File tree

7 files changed

+136
-17
lines changed

7 files changed

+136
-17
lines changed

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import static java.util.stream.Collectors.toCollection;
1616
import static java.util.stream.Collectors.toSet;
1717
import static org.junit.jupiter.engine.descriptor.NestedClassTestDescriptor.getEnclosingTestClasses;
18+
import static org.junit.jupiter.engine.discovery.predicates.TestClassPredicates.NestedClassInvalidityReason.NOT_INNER;
1819
import static org.junit.platform.commons.support.HierarchyTraversalMode.TOP_DOWN;
1920
import static org.junit.platform.commons.support.ReflectionSupport.findMethods;
2021
import static org.junit.platform.commons.util.FunctionUtils.where;
@@ -86,13 +87,22 @@ public Resolution resolve(ClassSelector selector, Context context) {
8687

8788
if (this.predicates.isAnnotatedWithNested.test(testClass)) {
8889
// Class name filter is not applied to nested test classes
89-
if (this.predicates.isValidNestedTestClass(testClass)) {
90+
var invalidityReason = this.predicates.validateNestedTestClass(testClass);
91+
if (invalidityReason == null) {
9092
return toResolution(
9193
context.addToParent(() -> DiscoverySelectors.selectClass(testClass.getEnclosingClass()),
9294
parent -> Optional.of(newMemberClassTestDescriptor(parent, testClass))));
9395
}
96+
if (invalidityReason == NOT_INNER) {
97+
return resolveStandaloneTestClass(context, testClass);
98+
}
99+
return unresolved();
94100
}
95-
else if (isAcceptedStandaloneTestClass(testClass)) {
101+
return resolveStandaloneTestClass(context, testClass);
102+
}
103+
104+
private Resolution resolveStandaloneTestClass(Context context, Class<?> testClass) {
105+
if (isAcceptedStandaloneTestClass(testClass)) {
96106
return toResolution(
97107
context.addToParent(parent -> Optional.of(newStandaloneClassTestDescriptor(parent, testClass))));
98108
}

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

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.function.Predicate;
2828

2929
import org.apiguardian.api.API;
30+
import org.jspecify.annotations.Nullable;
3031
import org.junit.jupiter.api.ClassTemplate;
3132
import org.junit.jupiter.api.Nested;
3233
import org.junit.platform.commons.util.ReflectionUtils;
@@ -55,15 +56,16 @@ public class TestClassPredicates {
5556
candidate) || looksLikeIntendedTestClass(candidate);
5657
public final Predicate<Method> isTestOrTestFactoryOrTestTemplateMethod;
5758

58-
private final Condition<Class<?>> isValidNestedTestClass;
59+
private final Condition<Class<?>> isNotPrivateUnlessAbstractNestedClass;
60+
private final Condition<Class<?>> isInnerNestedClass;
5961
private final Condition<Class<?>> isValidStandaloneTestClass;
6062

6163
public TestClassPredicates(DiscoveryIssueReporter issueReporter) {
6264
this.isTestOrTestFactoryOrTestTemplateMethod = new IsTestMethod(issueReporter) //
6365
.or(new IsTestFactoryMethod(issueReporter)) //
6466
.or(new IsTestTemplateMethod(issueReporter));
65-
this.isValidNestedTestClass = isNotPrivateUnlessAbstract("@Nested", issueReporter) //
66-
.and(isInner(issueReporter));
67+
this.isNotPrivateUnlessAbstractNestedClass = isNotPrivateUnlessAbstract("@Nested", issueReporter);
68+
this.isInnerNestedClass = isInner(issueReporter);
6769
this.isValidStandaloneTestClass = isNotPrivateUnlessAbstract("Test", issueReporter) //
6870
.and(isNotLocal(issueReporter)) //
6971
.and(isNotInnerUnlessAbstract(issueReporter)) //
@@ -84,8 +86,16 @@ private boolean looksLikeIntendedTestClass(Class<?> candidate, Set<Class<?>> see
8486
}
8587

8688
public boolean isValidNestedTestClass(Class<?> candidate) {
87-
return this.isValidNestedTestClass.check(candidate) //
88-
&& isNotAbstract(candidate);
89+
return validateNestedTestClass(candidate) == null;
90+
}
91+
92+
public @Nullable NestedClassInvalidityReason validateNestedTestClass(Class<?> candidate) {
93+
boolean isInner = this.isInnerNestedClass.check(candidate);
94+
boolean isNotPrivateUnlessAbstract = this.isNotPrivateUnlessAbstractNestedClass.check(candidate);
95+
if (isNotPrivateUnlessAbstract && isNotAbstract(candidate)) {
96+
return isInner ? null : NestedClassInvalidityReason.NOT_INNER;
97+
}
98+
return NestedClassInvalidityReason.OTHER;
8999
}
90100

91101
public boolean isValidStandaloneTestClass(Class<?> candidate) {
@@ -124,9 +134,13 @@ private static Condition<Class<?>> isNotLocal(DiscoveryIssueReporter issueReport
124134
private static Condition<Class<?>> isInner(DiscoveryIssueReporter issueReporter) {
125135
return issueReporter.createReportingCondition(ReflectionUtils::isInnerClass, testClass -> {
126136
if (testClass.getEnclosingClass() == null) {
127-
return createIssue("@Nested", testClass, "must not be a top-level class");
137+
return createIssue("Top-level", testClass, "must not be annotated with @Nested",
138+
"It will be executed anyway for backward compatibility. "
139+
+ "You should remove the @Nested annotation to resolve this warning.");
128140
}
129-
return createIssue("@Nested", testClass, "must not be static");
141+
return createIssue("@Nested", testClass, "must not be static",
142+
"It will only be executed if discovered as a standalone test class. "
143+
+ "You should remove the annotation or make it non-static to resolve this warning.");
130144
});
131145
}
132146

@@ -141,8 +155,11 @@ private static Condition<Class<?>> isNotAnonymous(DiscoveryIssueReporter issueRe
141155
}
142156

143157
private static DiscoveryIssue createIssue(String prefix, Class<?> testClass, String detailMessage) {
144-
String message = "%s class '%s' %s. It will not be executed.".formatted(prefix, testClass.getName(),
145-
detailMessage);
158+
return createIssue(prefix, testClass, detailMessage, "It will not be executed.");
159+
}
160+
161+
private static DiscoveryIssue createIssue(String prefix, Class<?> testClass, String detailMessage, String effect) {
162+
String message = "%s class '%s' %s. %s".formatted(prefix, testClass.getName(), detailMessage, effect);
146163
return DiscoveryIssue.builder(DiscoveryIssue.Severity.WARNING, message) //
147164
.source(ClassSource.from(testClass)) //
148165
.build();
@@ -151,4 +168,9 @@ private static DiscoveryIssue createIssue(String prefix, Class<?> testClass, Str
151168
private static boolean isAnnotatedButNotComposed(Class<?> candidate, Class<? extends Annotation> annotationType) {
152169
return !candidate.isAnnotation() && isAnnotated(candidate, annotationType);
153170
}
171+
172+
@API(status = INTERNAL, since = "5.13.3")
173+
public enum NestedClassInvalidityReason {
174+
NOT_INNER, OTHER
175+
}
154176
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,40 @@ private void assertNestedCycle(Class<?> start, Class<?> from, Class<?> to) {
274274
.haveExactly(1, finishedWithFailure(message(it -> it.contains(expectedMessage))));
275275
}
276276

277+
@Test
278+
void discoversButWarnsAboutTopLevelNestedTestClasses() {
279+
var results = discoverTestsForClass(TopLevelNestedTestCase.class);
280+
281+
var engineDescriptor = results.getEngineDescriptor();
282+
assertEquals(2, engineDescriptor.getDescendants().size(), "# resolved test descriptors");
283+
284+
var discoveryIssues = results.getDiscoveryIssues();
285+
assertThat(discoveryIssues).hasSize(1);
286+
assertThat(discoveryIssues.getFirst().message()) //
287+
.isEqualTo(
288+
"Top-level class '%s' must not be annotated with @Nested. "
289+
+ "It will be executed anyway for backward compatibility. "
290+
+ "You should remove the @Nested annotation to resolve this warning.",
291+
TopLevelNestedTestCase.class.getName());
292+
}
293+
294+
@Test
295+
void discoversButWarnsAboutStaticNestedTestClasses() {
296+
var results = discoverTestsForClass(StaticNestedTestCase.TestCase.class);
297+
298+
var engineDescriptor = results.getEngineDescriptor();
299+
assertEquals(2, engineDescriptor.getDescendants().size(), "# resolved test descriptors");
300+
301+
var discoveryIssues = results.getDiscoveryIssues();
302+
assertThat(discoveryIssues).hasSize(1);
303+
assertThat(discoveryIssues.getFirst().message()) //
304+
.isEqualTo(
305+
"@Nested class '%s' must not be static. "
306+
+ "It will only be executed if discovered as a standalone test class. "
307+
+ "You should remove the annotation or make it non-static to resolve this warning.",
308+
StaticNestedTestCase.TestCase.class.getName());
309+
}
310+
277311
// -------------------------------------------------------------------
278312

279313
@SuppressWarnings("JUnitMalformedDeclaration")
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright 2015-2025 the original author or authors.
3+
*
4+
* All rights reserved. This program and the accompanying materials are
5+
* made available under the terms of the Eclipse Public License v2.0 which
6+
* accompanies this distribution and is available at
7+
*
8+
* https://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.jupiter.engine;
12+
13+
import org.junit.jupiter.api.Nested;
14+
import org.junit.jupiter.api.Test;
15+
16+
class StaticNestedTestCase {
17+
18+
@SuppressWarnings("JUnitMalformedDeclaration")
19+
@Nested
20+
static class TestCase {
21+
@Test
22+
void test() {
23+
}
24+
}
25+
26+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright 2015-2025 the original author or authors.
3+
*
4+
* All rights reserved. This program and the accompanying materials are
5+
* made available under the terms of the Eclipse Public License v2.0 which
6+
* accompanies this distribution and is available at
7+
*
8+
* https://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.jupiter.engine;
12+
13+
import org.junit.jupiter.api.Nested;
14+
import org.junit.jupiter.api.Test;
15+
16+
@Nested
17+
public class TopLevelNestedTestCase {
18+
19+
@Test
20+
void test() {
21+
}
22+
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,8 @@ void reportsWarningForInvalidNestedTestClass(LauncherDiscoveryRequest request) {
305305
.isEqualTo("@Nested class '%s' must not be private. It will not be executed.",
306306
InvalidTestCases.InvalidTestClassTestCase.Inner.class.getName());
307307
assertThat(discoveryIssues.getLast().message()) //
308-
.isEqualTo("@Nested class '%s' must not be static. It will not be executed.",
309-
InvalidTestCases.InvalidTestClassTestCase.Inner.class.getName());
308+
.startsWith("@Nested class '%s' must not be static.".formatted(
309+
InvalidTestCases.InvalidTestClassTestCase.Inner.class.getName()));
310310
}
311311

312312
static List<Named<LauncherDiscoveryRequest>> requestsForTestClassWithInvalidNestedTestClass() {

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,9 @@ void staticNestedClassEvaluatesToFalse() {
254254
assertThat(predicates.isAnnotatedWithNestedAndValid).rejects(candidate);
255255

256256
var issue = DiscoveryIssue.builder(Severity.WARNING,
257-
"@Nested class '%s' must not be static. It will not be executed.".formatted(candidate.getName())) //
257+
"@Nested class '%s' must not be static. ".formatted(candidate.getName())
258+
+ "It will only be executed if discovered as a standalone test class. "
259+
+ "You should remove the annotation or make it non-static to resolve this warning.") //
258260
.source(ClassSource.from(candidate)) //
259261
.build();
260262
assertThat(discoveryIssues.stream().distinct()).containsExactly(issue);
@@ -268,8 +270,9 @@ void topLevelClassEvaluatesToFalse() {
268270
assertThat(predicates.isAnnotatedWithNestedAndValid).rejects(candidate);
269271

270272
var issue = DiscoveryIssue.builder(Severity.WARNING,
271-
"@Nested class '%s' must not be a top-level class. It will not be executed.".formatted(
272-
candidate.getName())) //
273+
("Top-level class '%s' must not be annotated with @Nested. ".formatted(candidate.getName())
274+
+ "It will be executed anyway for backward compatibility. "
275+
+ "You should remove the @Nested annotation to resolve this warning.")) //
273276
.source(ClassSource.from(candidate)) //
274277
.build();
275278
assertThat(discoveryIssues.stream().distinct()).containsExactly(issue);
@@ -312,7 +315,9 @@ class LocalClass {
312315
assertThat(predicates.isAnnotatedWithNestedAndValid).rejects(candidate);
313316

314317
var issue = DiscoveryIssue.builder(Severity.WARNING,
315-
"@Nested class '%s' must not be static. It will not be executed.".formatted(candidate.getName())) //
318+
"@Nested class '%s' must not be static. ".formatted(candidate.getName())
319+
+ "It will only be executed if discovered as a standalone test class. "
320+
+ "You should remove the annotation or make it non-static to resolve this warning.") //
316321
.source(ClassSource.from(candidate)) //
317322
.build();
318323
assertThat(discoveryIssues.stream().distinct()).containsExactly(issue);

0 commit comments

Comments
 (0)