Skip to content

Commit 2aefea2

Browse files
marcphilippmarcphilipp
committed
Report discovery errors for ineffectual lifecycle methods
Declaring `Before`/`AfterParameterizedClassInvocation` in regular test classes not annotated with `ParameterizedClass` now causes a discovery issue with severity `ERROR` to be reported.
1 parent ec697a2 commit 2aefea2

File tree

7 files changed

+114
-24
lines changed

7 files changed

+114
-24
lines changed

‎junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ClassTemplateInvocationLifecycleMethod.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,14 @@
3232
public @interface ClassTemplateInvocationLifecycleMethod {
3333

3434
/**
35-
* The actual annotation class for reporting of discovery issues.
35+
* The corresponding {@link org.junit.jupiter.api.ClassTemplate}-derived
36+
* annotation class.
3637
*/
37-
Class<? extends Annotation> value();
38+
Class<? extends Annotation> classTemplateAnnotation();
39+
40+
/**
41+
* The actual lifecycle method annotation class.
42+
*/
43+
Class<? extends Annotation> lifecycleMethodAnnotation();
3844

3945
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,21 @@ public final String getLegacyReportingName() {
133133
// --- Validatable ---------------------------------------------------------
134134

135135
@Override
136-
public void validate(DiscoveryIssueReporter reporter) {
136+
public final void validate(DiscoveryIssueReporter reporter) {
137+
validateCoreLifecycleMethods(reporter);
138+
validateClassTemplateInvocationLifecycleMethods(reporter);
139+
}
140+
141+
protected void validateCoreLifecycleMethods(DiscoveryIssueReporter reporter) {
137142
List<DiscoveryIssue> discoveryIssues = this.lifecycleMethods.discoveryIssues;
138143
discoveryIssues.forEach(reporter::reportIssue);
139144
discoveryIssues.clear();
140145
}
141146

147+
protected void validateClassTemplateInvocationLifecycleMethods(DiscoveryIssueReporter reporter) {
148+
LifecycleMethodUtils.validateNoClassTemplateInvocationLifecycleMethodsAreDeclared(getTestClass(), reporter);
149+
}
150+
142151
// --- Node ----------------------------------------------------------------
143152

144153
@Override

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import static java.util.stream.Collectors.toList;
1515
import static org.apiguardian.api.API.Status.INTERNAL;
1616
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_METHOD;
17-
import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.validateClassTemplateInvocationLifecycleMethods;
17+
import static org.junit.jupiter.engine.descriptor.LifecycleMethodUtils.validateClassTemplateInvocationLifecycleMethodsAreDeclaredCorrectly;
1818

1919
import java.util.ArrayList;
2020
import java.util.Collection;
@@ -80,10 +80,14 @@ public Set<TestTag> getTags() {
8080
// --- Validatable ---------------------------------------------------------
8181

8282
@Override
83-
public void validate(DiscoveryIssueReporter reporter) {
84-
this.delegate.validate(reporter);
83+
protected void validateCoreLifecycleMethods(DiscoveryIssueReporter reporter) {
84+
this.delegate.validateCoreLifecycleMethods(reporter);
85+
}
86+
87+
@Override
88+
protected void validateClassTemplateInvocationLifecycleMethods(DiscoveryIssueReporter reporter) {
8589
boolean requireStatic = this.classInfo.lifecycle == PER_METHOD;
86-
validateClassTemplateInvocationLifecycleMethods(getTestClass(), requireStatic, reporter);
90+
validateClassTemplateInvocationLifecycleMethodsAreDeclaredCorrectly(getTestClass(), requireStatic, reporter);
8791
}
8892

8993
// --- Filterable ----------------------------------------------------------

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

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@
1111
package org.junit.jupiter.engine.descriptor;
1212

1313
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotatedMethods;
14+
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;
1415
import static org.junit.platform.commons.util.CollectionUtils.toUnmodifiableList;
1516
import static org.junit.platform.engine.support.discovery.DiscoveryIssueReporter.Condition.allOf;
1617

1718
import java.lang.annotation.Annotation;
1819
import java.lang.reflect.Method;
1920
import java.util.List;
21+
import java.util.Optional;
2022
import java.util.function.Function;
2123
import java.util.function.Supplier;
2224
import java.util.stream.Stream;
@@ -26,7 +28,6 @@
2628
import org.junit.jupiter.api.BeforeAll;
2729
import org.junit.jupiter.api.BeforeEach;
2830
import org.junit.jupiter.api.extension.ClassTemplateInvocationLifecycleMethod;
29-
import org.junit.platform.commons.support.AnnotationSupport;
3031
import org.junit.platform.commons.support.HierarchyTraversalMode;
3132
import org.junit.platform.commons.support.ModifierSupport;
3233
import org.junit.platform.commons.util.ReflectionUtils;
@@ -69,17 +70,24 @@ static List<Method> findAfterEachMethods(Class<?> testClass, DiscoveryIssueRepor
6970
issueReporter);
7071
}
7172

72-
static void validateClassTemplateInvocationLifecycleMethods(Class<?> testClass, boolean requireStatic,
73+
static void validateNoClassTemplateInvocationLifecycleMethodsAreDeclared(Class<?> testClass,
7374
DiscoveryIssueReporter issueReporter) {
7475

75-
Stream<Method> allMethods = Stream.concat( //
76-
findAnnotatedMethods(testClass, ClassTemplateInvocationLifecycleMethod.class,
77-
HierarchyTraversalMode.TOP_DOWN).stream(), //
78-
findAnnotatedMethods(testClass, ClassTemplateInvocationLifecycleMethod.class,
79-
HierarchyTraversalMode.BOTTOM_UP).stream() //
80-
);
81-
allMethods //
82-
.distinct() //
76+
findAllClassTemplateInvocationLifecycleMethods(testClass) //
77+
.forEach(method -> findClassTemplateInvocationLifecycleMethodAnnotation(method) //
78+
.ifPresent(annotation -> {
79+
String message = String.format(
80+
"@%s method '%s' must not be declared in test class '%s' because it is not annotated with @%s.",
81+
annotation.lifecycleMethodAnnotation().getSimpleName(), method.toGenericString(),
82+
testClass.getName(), annotation.classTemplateAnnotation().getSimpleName());
83+
issueReporter.reportIssue(createIssue(Severity.ERROR, message, method));
84+
}));
85+
}
86+
87+
static void validateClassTemplateInvocationLifecycleMethodsAreDeclaredCorrectly(Class<?> testClass,
88+
boolean requireStatic, DiscoveryIssueReporter issueReporter) {
89+
90+
findAllClassTemplateInvocationLifecycleMethods(testClass) //
8391
.forEach(allOf( //
8492
isNotPrivateError(issueReporter), //
8593
returnsPrimitiveVoid(issueReporter,
@@ -91,6 +99,16 @@ static void validateClassTemplateInvocationLifecycleMethods(Class<?> testClass,
9199
));
92100
}
93101

102+
private static Stream<Method> findAllClassTemplateInvocationLifecycleMethods(Class<?> testClass) {
103+
Stream<Method> allMethods = Stream.concat( //
104+
findAnnotatedMethods(testClass, ClassTemplateInvocationLifecycleMethod.class,
105+
HierarchyTraversalMode.TOP_DOWN).stream(), //
106+
findAnnotatedMethods(testClass, ClassTemplateInvocationLifecycleMethod.class,
107+
HierarchyTraversalMode.BOTTOM_UP).stream() //
108+
);
109+
return allMethods.distinct();
110+
}
111+
94112
private static List<Method> findMethodsAndCheckStatic(Class<?> testClass, boolean requireStatic,
95113
Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
96114
DiscoveryIssueReporter issueReporter) {
@@ -168,11 +186,17 @@ private static Condition<Method> returnsPrimitiveVoid(DiscoveryIssueReporter iss
168186
}
169187

170188
private static String classTemplateInvocationLifecycleMethodAnnotationName(Method method) {
171-
return AnnotationSupport.findAnnotation(method, ClassTemplateInvocationLifecycleMethod.class) //
172-
.map(ClassTemplateInvocationLifecycleMethod::value).map(Class::getSimpleName) //
189+
return findClassTemplateInvocationLifecycleMethodAnnotation(method) //
190+
.map(ClassTemplateInvocationLifecycleMethod::lifecycleMethodAnnotation) //
191+
.map(Class::getSimpleName) //
173192
.orElseGet(ClassTemplateInvocationLifecycleMethod.class::getSimpleName);
174193
}
175194

195+
private static Optional<ClassTemplateInvocationLifecycleMethod> findClassTemplateInvocationLifecycleMethodAnnotation(
196+
Method method) {
197+
return findAnnotation(method, ClassTemplateInvocationLifecycleMethod.class);
198+
}
199+
176200
private static DiscoveryIssue createIssue(Severity severity, String message, Method method) {
177201
return DiscoveryIssue.builder(severity, message).source(MethodSource.from(method)).build();
178202
}

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/AfterParameterizedClassInvocation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@
158158
@Retention(RetentionPolicy.RUNTIME)
159159
@Documented
160160
@API(status = EXPERIMENTAL, since = "5.13")
161-
@ClassTemplateInvocationLifecycleMethod(AfterParameterizedClassInvocation.class)
161+
@ClassTemplateInvocationLifecycleMethod(classTemplateAnnotation = ParameterizedClass.class, lifecycleMethodAnnotation = AfterParameterizedClassInvocation.class)
162162
public @interface AfterParameterizedClassInvocation {
163163

164164
/**

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/BeforeParameterizedClassInvocation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@
159159
@Retention(RetentionPolicy.RUNTIME)
160160
@Documented
161161
@API(status = EXPERIMENTAL, since = "5.13")
162-
@ClassTemplateInvocationLifecycleMethod(BeforeParameterizedClassInvocation.class)
162+
@ClassTemplateInvocationLifecycleMethod(classTemplateAnnotation = ParameterizedClass.class, lifecycleMethodAnnotation = BeforeParameterizedClassInvocation.class)
163163
public @interface BeforeParameterizedClassInvocation {
164164

165165
/**

‎jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedClassIntegrationTests.java‎

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
package org.junit.jupiter.params;
1212

13+
import static java.util.Comparator.comparing;
1314
import static org.assertj.core.api.Assertions.assertThat;
1415
import static org.assertj.core.api.Assertions.tuple;
1516
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -48,6 +49,7 @@
4849
import java.lang.annotation.Target;
4950
import java.util.List;
5051
import java.util.Map;
52+
import java.util.Optional;
5153
import java.util.concurrent.atomic.AtomicInteger;
5254
import java.util.function.Supplier;
5355
import java.util.stream.Stream;
@@ -95,6 +97,7 @@
9597
import org.junit.jupiter.params.support.ParameterDeclarations;
9698
import org.junit.platform.commons.util.StringUtils;
9799
import org.junit.platform.engine.DiscoveryIssue;
100+
import org.junit.platform.engine.DiscoveryIssue.Severity;
98101
import org.junit.platform.engine.TestDescriptor;
99102
import org.junit.platform.engine.TestExecutionResult;
100103
import org.junit.platform.engine.UniqueId;
@@ -613,7 +616,7 @@ void lifecycleMethodsNeedToBeStaticByDefault(String simpleClassName, String anno
613616

614617
var issue = getOnlyElement(results.getDiscoveryIssues());
615618
assertThat(issue.severity()) //
616-
.isEqualTo(DiscoveryIssue.Severity.ERROR);
619+
.isEqualTo(Severity.ERROR);
617620
assertThat(issue.message()) //
618621
.isEqualTo(
619622
"%s method 'void %s.%s()' must be static unless the test class is annotated with @TestInstance(Lifecycle.PER_CLASS).",
@@ -629,7 +632,7 @@ void lifecycleMethodsMustNotBePrivate() {
629632

630633
var issue = getOnlyElement(results.getDiscoveryIssues());
631634
assertThat(issue.severity()) //
632-
.isEqualTo(DiscoveryIssue.Severity.ERROR);
635+
.isEqualTo(Severity.ERROR);
633636
assertThat(issue.message()) //
634637
.isEqualTo(
635638
"@BeforeParameterizedClassInvocation method 'private static void %s.beforeParameterizedClassInvocation()' must not be private.",
@@ -645,7 +648,7 @@ void lifecycleMethodsMustNotDeclareReturnType() {
645648

646649
var issue = getOnlyElement(results.getDiscoveryIssues());
647650
assertThat(issue.severity()) //
648-
.isEqualTo(DiscoveryIssue.Severity.ERROR);
651+
.isEqualTo(Severity.ERROR);
649652
assertThat(issue.message()) //
650653
.isEqualTo(
651654
"@BeforeParameterizedClassInvocation method 'static int %s.beforeParameterizedClassInvocation()' must not return a value.",
@@ -755,6 +758,35 @@ void failsForLifecycleMethodWithParameterAfterAggregator() {
755758
.haveExactly(1, finishedWithFailure(
756759
message(it -> it.contains("No ParameterResolver registered for parameter [int value]"))));
757760
}
761+
762+
@Test
763+
void lifecycleMethodsMustNotBeDeclaredInRegularTestClasses() {
764+
var testClassName = RegularClassWithLifecycleMethodsTestCase.class.getName();
765+
766+
var results = discoverTestsForClass(RegularClassWithLifecycleMethodsTestCase.class);
767+
768+
assertThat(results.getDiscoveryIssues()).hasSize(2);
769+
770+
var issues = results.getDiscoveryIssues().stream() //
771+
.sorted(comparing(DiscoveryIssue::message)) //
772+
.toList();
773+
774+
assertThat(issues) //
775+
.extracting(DiscoveryIssue::severity) //
776+
.containsOnly(Severity.ERROR);
777+
assertThat(issues) //
778+
.extracting(DiscoveryIssue::source) //
779+
.extracting(Optional::orElseThrow) //
780+
.allMatch(org.junit.platform.engine.support.descriptor.MethodSource.class::isInstance);
781+
assertThat(issues.getFirst().message()) //
782+
.isEqualTo(
783+
"@AfterParameterizedClassInvocation method 'static void %s.after()' must not be declared in test class '%s' because it is not annotated with @ParameterizedClass.",
784+
testClassName, testClassName);
785+
assertThat(issues.getLast().message()) //
786+
.isEqualTo(
787+
"@BeforeParameterizedClassInvocation method 'static void %s.before()' must not be declared in test class '%s' because it is not annotated with @ParameterizedClass.",
788+
testClassName, testClassName);
789+
}
758790
}
759791

760792
// -------------------------------------------------------------------
@@ -2174,4 +2206,19 @@ protected Object convert(Object source, Class<?> targetType) throws ArgumentConv
21742206
static class ConcreteInheritanceTestCase extends BaseInheritanceTestCase {
21752207
}
21762208

2209+
static class RegularClassWithLifecycleMethodsTestCase {
2210+
2211+
@BeforeParameterizedClassInvocation
2212+
static void before() {
2213+
}
2214+
2215+
@AfterParameterizedClassInvocation
2216+
static void after() {
2217+
}
2218+
2219+
@Test
2220+
void test() {
2221+
}
2222+
}
2223+
21772224
}

0 commit comments

Comments
 (0)