Skip to content

Commit 1aa837d

Browse files
marcphilippmarcphilipp
committed
Report discovery issues for invalid suite lifecycle methods
Issue: #242
1 parent fbbce44 commit 1aa837d

File tree

8 files changed

+111
-66
lines changed

8 files changed

+111
-66
lines changed

‎junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/ClassSelectorResolver.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.junit.platform.engine.discovery.ClassSelector;
2727
import org.junit.platform.engine.discovery.UniqueIdSelector;
2828
import org.junit.platform.engine.reporting.OutputDirectoryProvider;
29+
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
2930
import org.junit.platform.engine.support.discovery.SelectorResolver;
3031

3132
/**
@@ -41,13 +42,16 @@ final class ClassSelectorResolver implements SelectorResolver {
4142
private final SuiteEngineDescriptor suiteEngineDescriptor;
4243
private final ConfigurationParameters configurationParameters;
4344
private final OutputDirectoryProvider outputDirectoryProvider;
45+
private final DiscoveryIssueReporter issueReporter;
4446

4547
ClassSelectorResolver(Predicate<String> classNameFilter, SuiteEngineDescriptor suiteEngineDescriptor,
46-
ConfigurationParameters configurationParameters, OutputDirectoryProvider outputDirectoryProvider) {
48+
ConfigurationParameters configurationParameters, OutputDirectoryProvider outputDirectoryProvider,
49+
DiscoveryIssueReporter issueReporter) {
4750
this.classNameFilter = classNameFilter;
4851
this.suiteEngineDescriptor = suiteEngineDescriptor;
4952
this.configurationParameters = configurationParameters;
5053
this.outputDirectoryProvider = outputDirectoryProvider;
54+
this.issueReporter = issueReporter;
5155
}
5256

5357
@Override
@@ -106,7 +110,8 @@ private Optional<SuiteTestDescriptor> newSuiteDescriptor(Class<?> suiteClass, Te
106110
return Optional.empty();
107111
}
108112

109-
return Optional.of(new SuiteTestDescriptor(id, suiteClass, configurationParameters, outputDirectoryProvider));
113+
return Optional.of(
114+
new SuiteTestDescriptor(id, suiteClass, configurationParameters, outputDirectoryProvider, issueReporter));
110115
}
111116

112117
private static boolean containsCycle(UniqueId id) {

‎junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/DiscoverySelectorResolver.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ final class DiscoverySelectorResolver {
2626
context.getClassNameFilter(),
2727
context.getEngineDescriptor(),
2828
context.getDiscoveryRequest().getConfigurationParameters(),
29-
context.getDiscoveryRequest().getOutputDirectoryProvider()))
29+
context.getDiscoveryRequest().getOutputDirectoryProvider(),
30+
context.getIssueReporter()))
3031
.build();
3132
// @formatter:on
3233

‎junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/LifecycleMethodUtils.java‎

Lines changed: 56 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,20 @@
1111
package org.junit.platform.suite.engine;
1212

1313
import static org.junit.platform.commons.support.AnnotationSupport.findAnnotatedMethods;
14-
import static org.junit.platform.commons.util.ReflectionUtils.returnsPrimitiveVoid;
14+
import static org.junit.platform.commons.util.CollectionUtils.toUnmodifiableList;
15+
import static org.junit.platform.engine.support.discovery.DiscoveryIssueReporter.Condition.allOf;
1516

1617
import java.lang.annotation.Annotation;
1718
import java.lang.reflect.Method;
1819
import java.util.List;
1920

20-
import org.junit.platform.commons.JUnitException;
2121
import org.junit.platform.commons.support.HierarchyTraversalMode;
2222
import org.junit.platform.commons.support.ModifierSupport;
23-
import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
23+
import org.junit.platform.commons.util.ReflectionUtils;
24+
import org.junit.platform.engine.DiscoveryIssue;
25+
import org.junit.platform.engine.DiscoveryIssue.Severity;
26+
import org.junit.platform.engine.support.descriptor.MethodSource;
27+
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
2428
import org.junit.platform.suite.api.AfterSuite;
2529
import org.junit.platform.suite.api.BeforeSuite;
2630

@@ -35,56 +39,68 @@ private LifecycleMethodUtils() {
3539
/* no-op */
3640
}
3741

38-
static List<Method> findBeforeSuiteMethods(Class<?> testClass, ThrowableCollector throwableCollector) {
39-
return findMethodsAndAssertStaticAndNonPrivate(testClass, BeforeSuite.class, HierarchyTraversalMode.TOP_DOWN,
40-
throwableCollector);
42+
static List<Method> findBeforeSuiteMethods(Class<?> testClass, DiscoveryIssueReporter issueReporter) {
43+
return findMethodsAndCheckStaticAndNonPrivate(testClass, BeforeSuite.class, HierarchyTraversalMode.TOP_DOWN,
44+
issueReporter);
4145
}
4246

43-
static List<Method> findAfterSuiteMethods(Class<?> testClass, ThrowableCollector throwableCollector) {
44-
return findMethodsAndAssertStaticAndNonPrivate(testClass, AfterSuite.class, HierarchyTraversalMode.BOTTOM_UP,
45-
throwableCollector);
47+
static List<Method> findAfterSuiteMethods(Class<?> testClass, DiscoveryIssueReporter issueReporter) {
48+
return findMethodsAndCheckStaticAndNonPrivate(testClass, AfterSuite.class, HierarchyTraversalMode.BOTTOM_UP,
49+
issueReporter);
4650
}
4751

48-
private static List<Method> findMethodsAndAssertStaticAndNonPrivate(Class<?> testClass,
52+
private static List<Method> findMethodsAndCheckStaticAndNonPrivate(Class<?> testClass,
4953
Class<? extends Annotation> annotationType, HierarchyTraversalMode traversalMode,
50-
ThrowableCollector throwableCollector) {
51-
52-
List<Method> methods = findAnnotatedMethods(testClass, annotationType, traversalMode);
53-
throwableCollector.execute(() -> methods.forEach(method -> {
54-
assertVoid(annotationType, method);
55-
assertStatic(annotationType, method);
56-
assertNonPrivate(annotationType, method);
57-
assertNoParameters(annotationType, method);
58-
}));
59-
return methods;
54+
DiscoveryIssueReporter issueReporter) {
55+
56+
return findAnnotatedMethods(testClass, annotationType, traversalMode).stream() //
57+
.filter(allOf( //
58+
returnsPrimitiveVoid(annotationType, issueReporter), //
59+
isStatic(annotationType, issueReporter), //
60+
isNotPrivate(annotationType, issueReporter), //
61+
hasNoParameters(annotationType, issueReporter) //
62+
)) //
63+
.collect(toUnmodifiableList());
64+
}
65+
66+
private static DiscoveryIssueReporter.Condition<Method> isStatic(Class<? extends Annotation> annotationType,
67+
DiscoveryIssueReporter issueReporter) {
68+
return issueReporter.createReportingCondition(ModifierSupport::isStatic, method -> {
69+
String message = String.format("@%s method '%s' must be static.", annotationType.getSimpleName(),
70+
method.toGenericString());
71+
return createError(message, method);
72+
});
6073
}
6174

62-
private static void assertStatic(Class<? extends Annotation> annotationType, Method method) {
63-
if (ModifierSupport.isNotStatic(method)) {
64-
throw new JUnitException(String.format("@%s method '%s' must be static.", annotationType.getSimpleName(),
65-
method.toGenericString()));
66-
}
75+
private static DiscoveryIssueReporter.Condition<Method> isNotPrivate(Class<? extends Annotation> annotationType,
76+
DiscoveryIssueReporter issueReporter) {
77+
return issueReporter.createReportingCondition(ModifierSupport::isNotPrivate, method -> {
78+
String message = String.format("@%s method '%s' must not be private.", annotationType.getSimpleName(),
79+
method.toGenericString());
80+
return createError(message, method);
81+
});
6782
}
6883

69-
private static void assertNonPrivate(Class<? extends Annotation> annotationType, Method method) {
70-
if (ModifierSupport.isPrivate(method)) {
71-
throw new JUnitException(String.format("@%s method '%s' must not be private.",
72-
annotationType.getSimpleName(), method.toGenericString()));
73-
}
84+
private static DiscoveryIssueReporter.Condition<Method> returnsPrimitiveVoid(
85+
Class<? extends Annotation> annotationType, DiscoveryIssueReporter issueReporter) {
86+
return issueReporter.createReportingCondition(ReflectionUtils::returnsPrimitiveVoid, method -> {
87+
String message = String.format("@%s method '%s' must not return a value.", annotationType.getSimpleName(),
88+
method.toGenericString());
89+
return createError(message, method);
90+
});
7491
}
7592

76-
private static void assertVoid(Class<? extends Annotation> annotationType, Method method) {
77-
if (!returnsPrimitiveVoid(method)) {
78-
throw new JUnitException(String.format("@%s method '%s' must not return a value.",
79-
annotationType.getSimpleName(), method.toGenericString()));
80-
}
93+
private static DiscoveryIssueReporter.Condition<Method> hasNoParameters(Class<? extends Annotation> annotationType,
94+
DiscoveryIssueReporter issueReporter) {
95+
return issueReporter.createReportingCondition(method -> method.getParameterCount() == 0, method -> {
96+
String message = String.format("@%s method '%s' must not accept parameters.",
97+
annotationType.getSimpleName(), method.toGenericString());
98+
return createError(message, method);
99+
});
81100
}
82101

83-
private static void assertNoParameters(Class<? extends Annotation> annotationType, Method method) {
84-
if (method.getParameterCount() > 0) {
85-
throw new JUnitException(String.format("@%s method '%s' must not accept parameters.",
86-
annotationType.getSimpleName(), method.toGenericString()));
87-
}
102+
private static DiscoveryIssue createError(String message, Method method) {
103+
return DiscoveryIssue.builder(Severity.ERROR, message).source(MethodSource.from(method)).build();
88104
}
89105

90106
}

‎junit-platform-suite-engine/src/main/java/org/junit/platform/suite/engine/SuiteTestDescriptor.java‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.junit.platform.engine.reporting.OutputDirectoryProvider;
3030
import org.junit.platform.engine.support.descriptor.AbstractTestDescriptor;
3131
import org.junit.platform.engine.support.descriptor.ClassSource;
32+
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
3233
import org.junit.platform.engine.support.hierarchical.OpenTest4JAwareThrowableCollector;
3334
import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
3435
import org.junit.platform.launcher.LauncherDiscoveryRequest;
@@ -57,17 +58,19 @@ final class SuiteTestDescriptor extends AbstractTestDescriptor {
5758
private final OutputDirectoryProvider outputDirectoryProvider;
5859
private final Boolean failIfNoTests;
5960
private final Class<?> suiteClass;
61+
private final LifecycleMethods lifecycleMethods;
6062

6163
private LauncherDiscoveryResult launcherDiscoveryResult;
6264
private SuiteLauncher launcher;
6365

6466
SuiteTestDescriptor(UniqueId id, Class<?> suiteClass, ConfigurationParameters configurationParameters,
65-
OutputDirectoryProvider outputDirectoryProvider) {
67+
OutputDirectoryProvider outputDirectoryProvider, DiscoveryIssueReporter issueReporter) {
6668
super(id, getSuiteDisplayName(suiteClass), ClassSource.from(suiteClass));
6769
this.configurationParameters = configurationParameters;
6870
this.outputDirectoryProvider = outputDirectoryProvider;
6971
this.failIfNoTests = getFailIfNoTests(suiteClass);
7072
this.suiteClass = suiteClass;
73+
this.lifecycleMethods = new LifecycleMethods(suiteClass, issueReporter);
7174
}
7275

7376
private static Boolean getFailIfNoTests(Class<?> suiteClass) {
@@ -134,24 +137,21 @@ void execute(EngineExecutionListener parentEngineExecutionListener) {
134137
parentEngineExecutionListener.executionStarted(this);
135138
ThrowableCollector throwableCollector = new OpenTest4JAwareThrowableCollector();
136139

137-
List<Method> beforeSuiteMethods = LifecycleMethodUtils.findBeforeSuiteMethods(suiteClass, throwableCollector);
138-
List<Method> afterSuiteMethods = LifecycleMethodUtils.findAfterSuiteMethods(suiteClass, throwableCollector);
139-
140-
executeBeforeSuiteMethods(beforeSuiteMethods, throwableCollector);
140+
executeBeforeSuiteMethods(throwableCollector);
141141

142142
TestExecutionSummary summary = executeTests(parentEngineExecutionListener, throwableCollector);
143143

144-
executeAfterSuiteMethods(afterSuiteMethods, throwableCollector);
144+
executeAfterSuiteMethods(throwableCollector);
145145

146146
TestExecutionResult testExecutionResult = computeTestExecutionResult(summary, throwableCollector);
147147
parentEngineExecutionListener.executionFinished(this, testExecutionResult);
148148
}
149149

150-
private void executeBeforeSuiteMethods(List<Method> beforeSuiteMethods, ThrowableCollector throwableCollector) {
150+
private void executeBeforeSuiteMethods(ThrowableCollector throwableCollector) {
151151
if (throwableCollector.isNotEmpty()) {
152152
return;
153153
}
154-
for (Method beforeSuiteMethod : beforeSuiteMethods) {
154+
for (Method beforeSuiteMethod : lifecycleMethods.beforeSuite) {
155155
throwableCollector.execute(() -> ReflectionSupport.invokeMethod(beforeSuiteMethod, null));
156156
if (throwableCollector.isNotEmpty()) {
157157
return;
@@ -173,8 +173,8 @@ private TestExecutionSummary executeTests(EngineExecutionListener parentEngineEx
173173
return launcher.execute(discoveryResult, parentEngineExecutionListener);
174174
}
175175

176-
private void executeAfterSuiteMethods(List<Method> afterSuiteMethods, ThrowableCollector throwableCollector) {
177-
for (Method afterSuiteMethod : afterSuiteMethods) {
176+
private void executeAfterSuiteMethods(ThrowableCollector throwableCollector) {
177+
for (Method afterSuiteMethod : lifecycleMethods.afterSuite) {
178178
throwableCollector.execute(() -> ReflectionSupport.invokeMethod(afterSuiteMethod, null));
179179
}
180180
}
@@ -198,4 +198,15 @@ public boolean mayRegisterTests() {
198198
return true;
199199
}
200200

201+
private static class LifecycleMethods {
202+
203+
final List<Method> beforeSuite;
204+
final List<Method> afterSuite;
205+
206+
LifecycleMethods(Class<?> suiteClass, DiscoveryIssueReporter issueReporter) {
207+
beforeSuite = LifecycleMethodUtils.findBeforeSuiteMethods(suiteClass, issueReporter);
208+
afterSuite = LifecycleMethodUtils.findAfterSuiteMethods(suiteClass, issueReporter);
209+
}
210+
}
211+
201212
}

‎junit-platform-suite-engine/src/nativeImage/initialize-at-build-time‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
org.junit.platform.suite.engine.SuiteEngineDescriptor
22
org.junit.platform.suite.engine.SuiteLauncher
33
org.junit.platform.suite.engine.SuiteTestDescriptor
4+
org.junit.platform.suite.engine.SuiteTestDescriptor$LifecycleMethods
45
org.junit.platform.suite.engine.SuiteTestEngine

‎platform-tests/src/test/java/org/junit/platform/suite/engine/BeforeAndAfterSuiteTests.java‎

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.assertj.core.api.Assertions.assertThat;
1414
import static org.junit.jupiter.api.Named.named;
1515
import static org.junit.jupiter.params.provider.Arguments.arguments;
16+
import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement;
1617
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
1718
import static org.junit.platform.suite.engine.SuiteEngineDescriptor.ENGINE_ID;
1819
import static org.junit.platform.suite.engine.testsuites.LifecycleMethodsSuites.FailingAfterSuite;
@@ -47,7 +48,7 @@
4748
import org.junit.jupiter.params.ParameterizedTest;
4849
import org.junit.jupiter.params.provider.Arguments;
4950
import org.junit.jupiter.params.provider.MethodSource;
50-
import org.junit.platform.commons.JUnitException;
51+
import org.junit.platform.engine.DiscoveryIssue.Severity;
5152
import org.junit.platform.suite.api.AfterSuite;
5253
import org.junit.platform.suite.api.BeforeSuite;
5354
import org.junit.platform.suite.engine.testcases.StatefulTestCase;
@@ -192,14 +193,12 @@ void severalFailingBeforeAndAfterSuite() {
192193
@ParameterizedTest(name = "{0}")
193194
@MethodSource
194195
void invalidBeforeOrAfterSuiteMethod(Class<?> testSuiteClass, Predicate<String> failureMessagePredicate) {
195-
// @formatter:off
196-
executeSuite(testSuiteClass)
197-
.allEvents()
198-
.assertThatEvents()
199-
.haveExactly(1, event(
200-
container(testSuiteClass),
201-
finishedWithFailure(instanceOf(JUnitException.class), message(failureMessagePredicate))));
202-
// @formatter:on
196+
var results = engineWithSelectedSuite(testSuiteClass).discover();
197+
198+
var issue = getOnlyElement(results.getDiscoveryIssues());
199+
assertThat(issue.severity()).isEqualTo(Severity.ERROR);
200+
assertThat(issue.message()).matches(failureMessagePredicate);
201+
assertThat(issue.source()).containsInstanceOf(org.junit.platform.engine.support.descriptor.MethodSource.class);
203202
}
204203

205204
private static Stream<Arguments> invalidBeforeOrAfterSuiteMethod() {
@@ -219,11 +218,19 @@ private static Stream<Arguments> invalidBeforeOrAfterSuiteMethod() {
219218
private static Arguments invalidBeforeOrAfterSuiteCase(Class<?> suiteClass, String failureMessageStart,
220219
String failureMessageEnd) {
221220
return arguments(named(suiteClass.getSimpleName(), suiteClass),
222-
(Predicate<String>) s -> s.startsWith(failureMessageStart) && s.endsWith(failureMessageEnd));
221+
expectedMessage(failureMessageStart, failureMessageEnd));
222+
}
223+
224+
private static Predicate<String> expectedMessage(String failureMessageStart, String failureMessageEnd) {
225+
return message -> message.startsWith(failureMessageStart) && message.endsWith(failureMessageEnd);
223226
}
224227

225228
private static EngineExecutionResults executeSuite(Class<?> suiteClass) {
226-
return EngineTestKit.engine(ENGINE_ID).selectors(selectClass(suiteClass)).execute();
229+
return engineWithSelectedSuite(suiteClass).execute();
230+
}
231+
232+
private static EngineTestKit.Builder engineWithSelectedSuite(Class<?> suiteClass) {
233+
return EngineTestKit.engine(ENGINE_ID).selectors(selectClass(suiteClass));
227234
}
228235

229236
}

‎platform-tests/src/test/java/org/junit/platform/suite/engine/SuiteTestDescriptorTests.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import static org.assertj.core.api.Assertions.assertThat;
1414
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
1515
import static org.junit.jupiter.api.Assertions.assertAll;
16+
import static org.mockito.Mockito.mock;
1617

1718
import java.util.Collections;
1819
import java.util.Optional;
@@ -28,6 +29,7 @@
2829
import org.junit.platform.engine.TestDescriptor;
2930
import org.junit.platform.engine.UniqueId;
3031
import org.junit.platform.engine.reporting.OutputDirectoryProvider;
32+
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
3133
import org.junit.platform.launcher.core.OutputDirectoryProviders;
3234
import org.junit.platform.suite.api.Suite;
3335
import org.junit.platform.suite.engine.testcases.SingleTestTestCase;
@@ -48,8 +50,9 @@ class SuiteTestDescriptorTests {
4850

4951
final ConfigurationParameters configurationParameters = new EmptyConfigurationParameters();
5052
final OutputDirectoryProvider outputDirectoryProvider = OutputDirectoryProviders.dummyOutputDirectoryProvider();
53+
final DiscoveryIssueReporter discoveryIssueReporter = DiscoveryIssueReporter.create(mock(), engineId);
5154
final SuiteTestDescriptor suite = new SuiteTestDescriptor(suiteId, TestSuite.class, configurationParameters,
52-
outputDirectoryProvider);
55+
outputDirectoryProvider, discoveryIssueReporter);
5356

5457
@Test
5558
void suiteIsEmptyBeforeDiscovery() {
@@ -68,7 +71,7 @@ void suiteDiscoversTestsFromClass() {
6871
}
6972

7073
@Test
71-
void suitDiscoversTestsFromUniqueId() {
74+
void suiteDiscoversTestsFromUniqueId() {
7275
suite.addDiscoveryRequestFrom(methodId);
7376
suite.discover();
7477

‎platform-tests/src/test/java/org/junit/platform/suite/engine/testsuites/LifecycleMethodsSuites.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
*
3030
* @since 1.11
3131
*/
32+
@SuppressWarnings("NewClassNamingConvention")
3233
public class LifecycleMethodsSuites {
3334

3435
@Retention(RetentionPolicy.RUNTIME)

0 commit comments

Comments
 (0)