Skip to content

Commit 5c32d0f

Browse files
marcphilippmarcphilipp
committed
Report invalid lifecycle methods as discovery issues
Issue: #242
1 parent 4620988 commit 5c32d0f

File tree

6 files changed

+104
-39
lines changed

6 files changed

+104
-39
lines changed

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java

+47-23
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,12 @@
7171
import org.junit.platform.commons.util.ReflectionUtils;
7272
import org.junit.platform.commons.util.StringUtils;
7373
import org.junit.platform.commons.util.UnrecoverableExceptions;
74+
import org.junit.platform.engine.DiscoveryIssue;
7475
import org.junit.platform.engine.TestDescriptor;
7576
import org.junit.platform.engine.TestTag;
7677
import org.junit.platform.engine.UniqueId;
7778
import org.junit.platform.engine.support.descriptor.ClassSource;
79+
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
7880
import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
7981

8082
/**
@@ -84,28 +86,29 @@
8486
*/
8587
@API(status = INTERNAL, since = "5.5")
8688
public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor
87-
implements ResourceLockAware, TestClassAware {
89+
implements ResourceLockAware, TestClassAware, Validatable {
8890

8991
private static final InterceptingExecutableInvoker executableInvoker = new InterceptingExecutableInvoker();
9092

9193
protected final ClassInfo classInfo;
9294

95+
private LifecycleMethods lifecycleMethods;
9396
private TestInstanceFactory testInstanceFactory;
94-
private List<Method> beforeAllMethods;
95-
private List<Method> afterAllMethods;
9697

9798
ClassBasedTestDescriptor(UniqueId uniqueId, Class<?> testClass, Supplier<String> displayNameSupplier,
9899
JupiterConfiguration configuration) {
99100
super(uniqueId, testClass, displayNameSupplier, ClassSource.from(testClass), configuration);
100101

101102
this.classInfo = new ClassInfo(testClass, configuration);
103+
this.lifecycleMethods = new LifecycleMethods(this.classInfo);
102104
}
103105

104106
ClassBasedTestDescriptor(UniqueId uniqueId, Class<?> testClass, String displayName,
105107
JupiterConfiguration configuration) {
106108
super(uniqueId, displayName, ClassSource.from(testClass), configuration);
107109

108110
this.classInfo = new ClassInfo(testClass, configuration);
111+
this.lifecycleMethods = new LifecycleMethods(this.classInfo);
109112
}
110113

111114
// --- TestClassAware ------------------------------------------------------
@@ -127,6 +130,13 @@ public final String getLegacyReportingName() {
127130
return getTestClass().getName();
128131
}
129132

133+
// --- Validatable ---------------------------------------------------------
134+
135+
@Override
136+
public void validate(DiscoveryIssueReporter reporter) {
137+
this.lifecycleMethods.discoveryIssues.forEach(reporter::reportIssue);
138+
}
139+
130140
// --- Node ----------------------------------------------------------------
131141

132142
@Override
@@ -166,24 +176,15 @@ public final JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext
166176
registerExtensionsFromConstructorParameters(registry, getTestClass());
167177
}
168178

169-
this.beforeAllMethods = findBeforeAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD,
170-
issue -> {
171-
throw new JUnitException(issue.message());
172-
});
173-
this.afterAllMethods = findAfterAllMethods(getTestClass(), this.classInfo.lifecycle == Lifecycle.PER_METHOD,
174-
issue -> {
175-
throw new JUnitException(issue.message());
176-
});
177-
178-
this.beforeAllMethods.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
179+
this.lifecycleMethods.beforeAll.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
179180
// Since registerBeforeEachMethodAdapters() and registerAfterEachMethodAdapters() also
180181
// invoke registerExtensionsFromExecutableParameters(), we invoke those methods before
181182
// invoking registerExtensionsFromExecutableParameters() for @AfterAll methods,
182183
// thereby ensuring proper registration order for extensions registered via @ExtendWith
183184
// on parameters in lifecycle methods.
184185
registerBeforeEachMethodAdapters(registry);
185186
registerAfterEachMethodAdapters(registry);
186-
this.afterAllMethods.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
187+
this.lifecycleMethods.afterAll.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
187188
registerExtensionsFromInstanceFields(registry, getTestClass());
188189

189190
ThrowableCollector throwableCollector = createThrowableCollector();
@@ -258,6 +259,13 @@ public final void after(JupiterEngineExecutionContext context) {
258259
}
259260
}
260261

262+
@Override
263+
public void cleanUp(JupiterEngineExecutionContext context) throws Exception {
264+
super.cleanUp(context);
265+
this.lifecycleMethods = null;
266+
this.testInstanceFactory = null;
267+
}
268+
261269
private TestInstanceFactory resolveTestInstanceFactory(ExtensionRegistry registry) {
262270
List<TestInstanceFactory> factories = registry.getExtensions(TestInstanceFactory.class);
263271

@@ -410,7 +418,7 @@ private void invokeBeforeAllMethods(JupiterEngineExecutionContext context) {
410418
ThrowableCollector throwableCollector = context.getThrowableCollector();
411419
Object testInstance = extensionContext.getTestInstance().orElse(null);
412420

413-
for (Method method : this.beforeAllMethods) {
421+
for (Method method : this.lifecycleMethods.beforeAll) {
414422
throwableCollector.execute(() -> {
415423
try {
416424
executableInvoker.invoke(method, testInstance, extensionContext, registry,
@@ -439,7 +447,7 @@ private void invokeAfterAllMethods(JupiterEngineExecutionContext context) {
439447
ThrowableCollector throwableCollector = context.getThrowableCollector();
440448
Object testInstance = extensionContext.getTestInstance().orElse(null);
441449

442-
this.afterAllMethods.forEach(method -> throwableCollector.execute(() -> {
450+
this.lifecycleMethods.afterAll.forEach(method -> throwableCollector.execute(() -> {
443451
try {
444452
executableInvoker.invoke(method, testInstance, extensionContext, registry,
445453
ReflectiveInterceptorCall.ofVoidMethod(InvocationInterceptor::interceptAfterAllMethod));
@@ -472,17 +480,13 @@ private boolean isPerClassLifecycle(JupiterEngineExecutionContext context) {
472480
}
473481

474482
private void registerBeforeEachMethodAdapters(ExtensionRegistrar registrar) {
475-
List<Method> beforeEachMethods = findBeforeEachMethods(getTestClass(), issue -> {
476-
throw new JUnitException(issue.message());
477-
});
478-
registerMethodsAsExtensions(beforeEachMethods, registrar, this::synthesizeBeforeEachMethodAdapter);
483+
registerMethodsAsExtensions(this.lifecycleMethods.beforeEach, registrar,
484+
this::synthesizeBeforeEachMethodAdapter);
479485
}
480486

481487
private void registerAfterEachMethodAdapters(ExtensionRegistrar registrar) {
482488
// Make a local copy since findAfterEachMethods() returns an immutable list.
483-
List<Method> afterEachMethods = new ArrayList<>(findAfterEachMethods(getTestClass(), issue -> {
484-
throw new JUnitException(issue.message());
485-
}));
489+
List<Method> afterEachMethods = new ArrayList<>(this.lifecycleMethods.afterEach);
486490

487491
// Since the bottom-up ordering of afterEachMethods will later be reversed when the
488492
// synthesized AfterEachMethodAdapters are executed within TestMethodTestDescriptor,
@@ -523,6 +527,7 @@ private void invokeMethodInExtensionContext(Method method, ExtensionContext cont
523527
}
524528

525529
protected static class ClassInfo {
530+
526531
final Class<?> testClass;
527532
final Set<TestTag> tags;
528533
final Lifecycle lifecycle;
@@ -538,4 +543,23 @@ protected static class ClassInfo {
538543
}
539544
}
540545

546+
private static class LifecycleMethods {
547+
548+
private final List<DiscoveryIssue> discoveryIssues = new ArrayList<>();
549+
550+
private final List<Method> beforeAll;
551+
private final List<Method> afterAll;
552+
private final List<Method> beforeEach;
553+
private final List<Method> afterEach;
554+
555+
LifecycleMethods(ClassInfo classInfo) {
556+
Class<?> testClass = classInfo.testClass;
557+
boolean requireStatic = classInfo.lifecycle == Lifecycle.PER_METHOD;
558+
this.beforeAll = findBeforeAllMethods(testClass, requireStatic, discoveryIssues::add);
559+
this.afterAll = findAfterAllMethods(testClass, requireStatic, discoveryIssues::add);
560+
this.beforeEach = findBeforeEachMethods(testClass, discoveryIssues::add);
561+
this.afterEach = findAfterEachMethods(testClass, discoveryIssues::add);
562+
}
563+
}
564+
541565
}

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/Validatable.java

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
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.descriptor;
12+
13+
import static org.apiguardian.api.API.Status.INTERNAL;
14+
15+
import org.apiguardian.api.API;
16+
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
17+
18+
/**
19+
* Interface for descriptors that can be validated during discovery.
20+
*
21+
* @since 5.13
22+
*/
23+
@API(status = INTERNAL, since = "5.13")
24+
public interface Validatable {
25+
26+
/**
27+
* Validate the state of this descriptor and report any issues found to the
28+
* supplied {@link DiscoveryIssueReporter}.
29+
*/
30+
void validate(DiscoveryIssueReporter reporter);
31+
32+
}

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/DiscoverySelectorResolver.java

+6
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.junit.jupiter.engine.config.JupiterConfiguration;
1717
import org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor;
1818
import org.junit.jupiter.engine.descriptor.JupiterTestDescriptor;
19+
import org.junit.jupiter.engine.descriptor.Validatable;
1920
import org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests;
2021
import org.junit.platform.engine.EngineDiscoveryRequest;
2122
import org.junit.platform.engine.TestDescriptor;
@@ -47,6 +48,11 @@ public class DiscoverySelectorResolver {
4748
((JupiterTestDescriptor) descriptor).prunePriorToFiltering();
4849
}
4950
}) //
51+
.addTestDescriptorVisitor(ctx -> descriptor -> {
52+
if (descriptor instanceof Validatable) {
53+
((Validatable) descriptor).validate(ctx.getIssueReporter());
54+
}
55+
}) //
5056
.build();
5157

5258
private static JupiterConfiguration getConfiguration(InitializationContext<JupiterEngineDescriptor> context) {

‎junit-jupiter-engine/src/nativeImage/initialize-at-build-time

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ org.junit.jupiter.engine.config.InstantiatingConfigurationParameterConverter
66
org.junit.jupiter.engine.descriptor.ClassTestDescriptor
77
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor
88
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor$ClassInfo
9+
org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor$LifecycleMethods
910
org.junit.jupiter.engine.descriptor.DynamicDescendantFilter
1011
org.junit.jupiter.engine.descriptor.ExclusiveResourceCollector$1
1112
org.junit.jupiter.engine.descriptor.JupiterEngineDescriptor

‎jupiter-tests/src/test/java/org/junit/jupiter/engine/InvalidLifecycleMethodConfigurationTests.java

+13-13
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
import static org.assertj.core.api.Assertions.assertThat;
1515
import static org.junit.platform.commons.util.FunctionUtils.where;
1616

17+
import java.lang.annotation.Annotation;
18+
1719
import org.junit.jupiter.api.AfterAll;
1820
import org.junit.jupiter.api.AfterEach;
1921
import org.junit.jupiter.api.BeforeAll;
@@ -26,39 +28,37 @@
2628
* Integration tests that verify proper handling of invalid configuration for
2729
* lifecycle methods in conjunction with the {@link JupiterTestEngine}.
2830
*
29-
* <p>In general, configuration errors should not be thrown until the
30-
* execution phase, thereby giving all containers a chance to execute.
31-
*
3231
* @since 5.0
3332
*/
3433
class InvalidLifecycleMethodConfigurationTests extends AbstractJupiterTestEngineTests {
3534

3635
@Test
3736
void executeValidTestCaseAlongsideTestCaseWithInvalidNonStaticBeforeAllDeclaration() {
38-
assertReportsError(TestCaseWithInvalidNonStaticBeforeAllMethod.class);
37+
assertReportsError(TestCaseWithInvalidNonStaticBeforeAllMethod.class, BeforeAll.class);
3938
}
4039

4140
@Test
4241
void executeValidTestCaseAlongsideTestCaseWithInvalidNonStaticAfterAllDeclaration() {
43-
assertReportsError(TestCaseWithInvalidNonStaticAfterAllMethod.class);
42+
assertReportsError(TestCaseWithInvalidNonStaticAfterAllMethod.class, AfterAll.class);
4443
}
4544

4645
@Test
4746
void executeValidTestCaseAlongsideTestCaseWithInvalidStaticBeforeEachDeclaration() {
48-
assertReportsError(TestCaseWithInvalidStaticBeforeEachMethod.class);
47+
assertReportsError(TestCaseWithInvalidStaticBeforeEachMethod.class, BeforeEach.class);
4948
}
5049

5150
@Test
5251
void executeValidTestCaseAlongsideTestCaseWithInvalidStaticAfterEachDeclaration() {
53-
assertReportsError(TestCaseWithInvalidStaticAfterEachMethod.class);
52+
assertReportsError(TestCaseWithInvalidStaticAfterEachMethod.class, AfterEach.class);
5453
}
5554

56-
private void assertReportsError(Class<?> invalidTestClass) {
55+
private void assertReportsError(Class<?> invalidTestClass, Class<? extends Annotation> annotationType) {
5756
var results = discoverTestsForClass(invalidTestClass);
5857

5958
assertThat(results.getDiscoveryIssues()) //
6059
.filteredOn(where(DiscoveryIssue::severity, isEqual(Severity.ERROR))) //
61-
.isNotEmpty();
60+
.extracting(DiscoveryIssue::message) //
61+
.asString().contains("@%s method".formatted(annotationType.getSimpleName()));
6262
}
6363

6464
// -------------------------------------------------------------------------
@@ -67,7 +67,7 @@ private void assertReportsError(Class<?> invalidTestClass) {
6767
static class TestCaseWithInvalidNonStaticBeforeAllMethod {
6868

6969
// must be static
70-
@SuppressWarnings("JUnitMalformedDeclaration")
70+
@SuppressWarnings("unused")
7171
@BeforeAll
7272
void beforeAll() {
7373
}
@@ -81,7 +81,7 @@ void test() {
8181
static class TestCaseWithInvalidNonStaticAfterAllMethod {
8282

8383
// must be static
84-
@SuppressWarnings("JUnitMalformedDeclaration")
84+
@SuppressWarnings("unused")
8585
@AfterAll
8686
void afterAll() {
8787
}
@@ -95,7 +95,7 @@ void test() {
9595
static class TestCaseWithInvalidStaticBeforeEachMethod {
9696

9797
// must NOT be static
98-
@SuppressWarnings("JUnitMalformedDeclaration")
98+
@SuppressWarnings("unused")
9999
@BeforeEach
100100
static void beforeEach() {
101101
}
@@ -109,7 +109,7 @@ void test() {
109109
static class TestCaseWithInvalidStaticAfterEachMethod {
110110

111111
// must NOT be static
112-
@SuppressWarnings("JUnitMalformedDeclaration")
112+
@SuppressWarnings("unused")
113113
@AfterEach
114114
static void afterEach() {
115115
}

‎jupiter-tests/src/test/java/org/junit/jupiter/engine/TestInstanceLifecycleConfigurationTests.java

+5-3
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ void instancePerClassConfiguredViaSystemProperty() {
7373
Class<?> testClass = AssumedInstancePerClassTestCase.class;
7474

7575
// Should fail by default...
76-
performAssertions(testClass, 2, 1, 0);
76+
performAssertions(testClass, 1, 1, 0);
7777

7878
// Should pass with the system property set
7979
System.setProperty(KEY, PER_CLASS.name());
@@ -85,7 +85,7 @@ void instancePerClassConfiguredViaConfigParam() {
8585
Class<?> testClass = AssumedInstancePerClassTestCase.class;
8686

8787
// Should fail by default...
88-
performAssertions(testClass, 2, 1, 0);
88+
performAssertions(testClass, 1, 1, 0);
8989

9090
// Should pass with the config param
9191
performAssertions(testClass, singletonMap(KEY, PER_CLASS.name()), 2, 0, 1, "beforeAll", "test", "afterAll");
@@ -97,7 +97,7 @@ void instancePerClassConfiguredViaConfigParamThatOverridesSystemProperty() {
9797

9898
// Should fail with system property
9999
System.setProperty(KEY, PER_METHOD.name());
100-
performAssertions(testClass, 2, 1, 0);
100+
performAssertions(testClass, 1, 1, 0);
101101

102102
// Should pass with the config param
103103
performAssertions(testClass, singletonMap(KEY, PER_CLASS.name()), 2, 0, 1, "beforeAll", "test", "afterAll");
@@ -133,6 +133,8 @@ private void performAssertions(Class<?> testClass, Map<String, String> configPar
133133
);
134134
// @formatter:on
135135

136+
executionResults.allEvents().debug();
137+
136138
executionResults.containerEvents().assertStatistics(//
137139
stats -> stats.started(numContainers).finished(numContainers).failed(numFailedContainers));
138140
executionResults.testEvents().assertStatistics(//

0 commit comments

Comments
 (0)