Skip to content

Commit 07b8259

Browse files
marcphilippmarcphilipp
committed
Draft API for reporting discovery issues
1 parent 3d92bd6 commit 07b8259

20 files changed

+512
-45
lines changed

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,17 @@
6767
import org.junit.jupiter.engine.extension.ExtensionRegistry;
6868
import org.junit.jupiter.engine.extension.MutableExtensionRegistry;
6969
import org.junit.platform.commons.JUnitException;
70+
import org.junit.platform.commons.support.ModifierSupport;
7071
import org.junit.platform.commons.util.ExceptionUtils;
7172
import org.junit.platform.commons.util.ReflectionUtils;
7273
import org.junit.platform.commons.util.StringUtils;
7374
import org.junit.platform.commons.util.UnrecoverableExceptions;
75+
import org.junit.platform.engine.EngineDiscoveryIssue.Severity;
7476
import org.junit.platform.engine.TestDescriptor;
7577
import org.junit.platform.engine.TestTag;
7678
import org.junit.platform.engine.UniqueId;
7779
import org.junit.platform.engine.support.descriptor.ClassSource;
80+
import org.junit.platform.engine.support.descriptor.MethodSource;
7881
import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
7982

8083
/**
@@ -84,7 +87,7 @@
8487
*/
8588
@API(status = INTERNAL, since = "5.5")
8689
public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor
87-
implements ResourceLockAware, TestClassAware {
90+
implements ResourceLockAware, TestClassAware, Validatable {
8891

8992
private static final InterceptingExecutableInvoker executableInvoker = new InterceptingExecutableInvoker();
9093

@@ -95,8 +98,8 @@ public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor
9598

9699
private ExecutionMode defaultChildExecutionMode;
97100
private TestInstanceFactory testInstanceFactory;
98-
private List<Method> beforeAllMethods;
99-
private List<Method> afterAllMethods;
101+
private final List<Method> beforeAllMethods;
102+
private final List<Method> afterAllMethods;
100103

101104
ClassBasedTestDescriptor(UniqueId uniqueId, Class<?> testClass, Supplier<String> displayNameSupplier,
102105
JupiterConfiguration configuration) {
@@ -107,6 +110,8 @@ public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor
107110
this.lifecycle = getTestInstanceLifecycle(testClass, configuration);
108111
this.defaultChildExecutionMode = (this.lifecycle == Lifecycle.PER_CLASS ? ExecutionMode.SAME_THREAD : null);
109112
this.exclusiveResourceCollector = ExclusiveResourceCollector.from(testClass);
113+
this.beforeAllMethods = findBeforeAllMethods(this.testClass, this.lifecycle == Lifecycle.PER_METHOD);
114+
this.afterAllMethods = findAfterAllMethods(this.testClass, this.lifecycle == Lifecycle.PER_METHOD);
110115
}
111116

112117
ClassBasedTestDescriptor(UniqueId uniqueId, Class<?> testClass, String displayName,
@@ -118,6 +123,8 @@ public abstract class ClassBasedTestDescriptor extends JupiterTestDescriptor
118123
this.lifecycle = getTestInstanceLifecycle(testClass, configuration);
119124
this.defaultChildExecutionMode = (this.lifecycle == Lifecycle.PER_CLASS ? ExecutionMode.SAME_THREAD : null);
120125
this.exclusiveResourceCollector = ExclusiveResourceCollector.from(testClass);
126+
this.beforeAllMethods = findBeforeAllMethods(this.testClass, this.lifecycle == Lifecycle.PER_METHOD);
127+
this.afterAllMethods = findAfterAllMethods(this.testClass, this.lifecycle == Lifecycle.PER_METHOD);
121128
}
122129

123130
// --- TestClassAware ------------------------------------------------------
@@ -139,6 +146,22 @@ public final String getLegacyReportingName() {
139146
return this.testClass.getName();
140147
}
141148

149+
// --- Validatable ---------------------------------------------------------
150+
151+
@Override
152+
public void validate(IssueReporter reporter) {
153+
beforeAllMethods.stream() //
154+
.filter(ModifierSupport::isPrivate) //
155+
.forEach(method -> reporter.reportIssue(Severity.DEPRECATION,
156+
String.format("@BeforeAll method [%s] should not be private.", method.toGenericString()),
157+
b -> b.source(MethodSource.from(method))));
158+
afterAllMethods.stream() //
159+
.filter(ModifierSupport::isPrivate) //
160+
.forEach(method -> reporter.reportIssue(Severity.DEPRECATION,
161+
String.format("@AfterAll method [%s] should not be private.", method.toGenericString()),
162+
b -> b.source(MethodSource.from(method))));
163+
}
164+
142165
// --- Node ----------------------------------------------------------------
143166

144167
@Override
@@ -178,9 +201,6 @@ public final JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext
178201
registerExtensionsFromConstructorParameters(registry, this.testClass);
179202
}
180203

181-
this.beforeAllMethods = findBeforeAllMethods(this.testClass, this.lifecycle == Lifecycle.PER_METHOD);
182-
this.afterAllMethods = findAfterAllMethods(this.testClass, this.lifecycle == Lifecycle.PER_METHOD);
183-
184204
this.beforeAllMethods.forEach(method -> registerExtensionsFromExecutableParameters(registry, method));
185205
// Since registerBeforeEachMethodAdapters() and registerAfterEachMethodAdapters() also
186206
// invoke registerExtensionsFromExecutableParameters(), we invoke those methods before

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
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 java.util.function.UnaryOperator.identity;
14+
15+
import java.util.function.UnaryOperator;
16+
17+
import org.junit.platform.engine.EngineDiscoveryIssue;
18+
import org.junit.platform.engine.EngineDiscoveryIssue.Severity;
19+
20+
public interface Validatable {
21+
22+
void validate(IssueReporter issueReporter);
23+
24+
interface IssueReporter {
25+
26+
default void reportIssue(Severity severity, String message) {
27+
reportIssue(severity, message, identity());
28+
}
29+
30+
void reportIssue(Severity severity, String message, UnaryOperator<EngineDiscoveryIssue.Builder> issueBuilder);
31+
32+
}
33+
34+
}

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

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@
2323
import java.util.stream.Stream;
2424

2525
import org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor;
26-
import org.junit.platform.commons.logging.Logger;
27-
import org.junit.platform.commons.logging.LoggerFactory;
2826
import org.junit.platform.commons.util.UnrecoverableExceptions;
27+
import org.junit.platform.engine.EngineDiscoveryIssue;
28+
import org.junit.platform.engine.EngineDiscoveryIssue.Severity;
2929
import org.junit.platform.engine.TestDescriptor;
30+
import org.junit.platform.engine.support.discovery.EngineDiscoveryIssueReporter;
3031

3132
/**
3233
* Abstract base class for {@linkplain TestDescriptor.Visitor visitors} that
@@ -40,7 +41,11 @@
4041
abstract class AbstractOrderingVisitor<PARENT extends TestDescriptor, CHILD extends TestDescriptor, WRAPPER extends AbstractAnnotatedDescriptorWrapper<?>>
4142
implements TestDescriptor.Visitor {
4243

43-
private static final Logger logger = LoggerFactory.getLogger(AbstractOrderingVisitor.class);
44+
private final EngineDiscoveryIssueReporter issueReporter;
45+
46+
AbstractOrderingVisitor(EngineDiscoveryIssueReporter issueReporter) {
47+
this.issueReporter = issueReporter;
48+
}
4449

4550
@SuppressWarnings("unchecked")
4651
protected void doWithMatchingDescriptor(Class<PARENT> parentTestDescriptorType, TestDescriptor testDescriptor,
@@ -53,7 +58,12 @@ protected void doWithMatchingDescriptor(Class<PARENT> parentTestDescriptorType,
5358
}
5459
catch (Throwable t) {
5560
UnrecoverableExceptions.rethrowIfUnrecoverable(t);
56-
logger.error(t, () -> errorMessageBuilder.apply(parentTestDescriptor));
61+
String message = errorMessageBuilder.apply(parentTestDescriptor);
62+
AbstractOrderingVisitor.this.issueReporter.reportIssue(
63+
EngineDiscoveryIssue.builder(Severity.WARNING, message) //
64+
.source(testDescriptor.getSource()) //
65+
.cause(t) //
66+
.build());
5767
}
5868
}
5969
}
@@ -78,7 +88,7 @@ protected void orderChildrenTestDescriptors(TestDescriptor parentTestDescriptor,
7888
Stream<TestDescriptor> nonMatchingTestDescriptors = children.stream()//
7989
.filter(childTestDescriptor -> !matchingChildrenType.isInstance(childTestDescriptor));
8090

81-
descriptorWrapperOrderer.orderWrappers(matchingDescriptorWrappers);
91+
descriptorWrapperOrderer.orderWrappers(parentTestDescriptor, matchingDescriptorWrappers);
8292

8393
Stream<TestDescriptor> orderedTestDescriptors = matchingDescriptorWrappers.stream()//
8494
.map(AbstractAnnotatedDescriptorWrapper::getTestDescriptor);
@@ -149,18 +159,18 @@ private boolean canOrderWrappers() {
149159
return this.orderingAction != null;
150160
}
151161

152-
private void orderWrappers(List<WRAPPER> wrappers) {
162+
private void orderWrappers(TestDescriptor parentTestDescriptor, List<WRAPPER> wrappers) {
153163
List<WRAPPER> orderedWrappers = new ArrayList<>(wrappers);
154164
this.orderingAction.accept(orderedWrappers);
155165
Map<Object, Integer> distinctWrappersToIndex = distinctWrappersToIndex(orderedWrappers);
156166

157167
int difference = orderedWrappers.size() - wrappers.size();
158168
int distinctDifference = distinctWrappersToIndex.size() - wrappers.size();
159169
if (difference > 0) { // difference >= distinctDifference
160-
logDescriptorsAddedWarning(difference);
170+
logDescriptorsAddedWarning(parentTestDescriptor, difference);
161171
}
162172
if (distinctDifference < 0) { // distinctDifference <= difference
163-
logDescriptorsRemovedWarning(distinctDifference);
173+
logDescriptorsRemovedWarning(parentTestDescriptor, distinctDifference);
164174
}
165175

166176
wrappers.sort(comparing(wrapper -> distinctWrappersToIndex.getOrDefault(wrapper, -1)));
@@ -178,12 +188,18 @@ private Map<Object, Integer> distinctWrappersToIndex(List<?> wrappers) {
178188
return toIndex;
179189
}
180190

181-
private void logDescriptorsAddedWarning(int number) {
182-
logger.warn(() -> this.descriptorsAddedMessageGenerator.generateMessage(number));
191+
private void logDescriptorsAddedWarning(TestDescriptor parentTestDescriptor, int number) {
192+
reportWarning(parentTestDescriptor, this.descriptorsAddedMessageGenerator.generateMessage(number));
193+
}
194+
195+
private void logDescriptorsRemovedWarning(TestDescriptor parentTestDescriptor, int number) {
196+
reportWarning(parentTestDescriptor, this.descriptorsRemovedMessageGenerator.generateMessage(number));
183197
}
184198

185-
private void logDescriptorsRemovedWarning(int number) {
186-
logger.warn(() -> this.descriptorsRemovedMessageGenerator.generateMessage(Math.abs(number)));
199+
private void reportWarning(TestDescriptor parentTestDescriptor, String message) {
200+
AbstractOrderingVisitor.this.issueReporter //
201+
.reportIssue(EngineDiscoveryIssue.builder(Severity.WARNING, message) //
202+
.source(parentTestDescriptor.getSource()));
187203
}
188204

189205
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.junit.platform.commons.support.AnnotationSupport;
2323
import org.junit.platform.commons.support.ReflectionSupport;
2424
import org.junit.platform.engine.TestDescriptor;
25+
import org.junit.platform.engine.support.discovery.EngineDiscoveryIssueReporter;
2526

2627
/**
2728
* @since 5.8
@@ -31,7 +32,8 @@ class ClassOrderingVisitor
3132

3233
private final JupiterConfiguration configuration;
3334

34-
ClassOrderingVisitor(JupiterConfiguration configuration) {
35+
ClassOrderingVisitor(EngineDiscoveryIssueReporter issueReporter, JupiterConfiguration configuration) {
36+
super(issueReporter);
3537
this.configuration = configuration;
3638
}
3739

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ public class DiscoverySelectorResolver {
4040
.addClassContainerSelectorResolver(new IsTestClassWithTests()) //
4141
.addSelectorResolver(ctx -> new ClassSelectorResolver(ctx.getClassNameFilter(), getConfiguration(ctx))) //
4242
.addSelectorResolver(ctx -> new MethodSelectorResolver(getConfiguration(ctx))) //
43-
.addTestDescriptorVisitor(ctx -> new ClassOrderingVisitor(getConfiguration(ctx))) //
44-
.addTestDescriptorVisitor(ctx -> new MethodOrderingVisitor(getConfiguration(ctx))) //
43+
.addTestDescriptorVisitor(ctx -> new ClassOrderingVisitor(ctx.getIssueReporter(), getConfiguration(ctx))) //
44+
.addTestDescriptorVisitor(ctx -> new MethodOrderingVisitor(ctx.getIssueReporter(), getConfiguration(ctx))) //
45+
.addTestDescriptorVisitor(ctx -> new ValidatingVisitor(ctx.getIssueReporter())) //
4546
.addTestDescriptorVisitor(ctx -> descriptor -> {
4647
if (descriptor instanceof JupiterTestDescriptor) {
4748
((JupiterTestDescriptor) descriptor).prunePriorToFiltering();

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.junit.jupiter.engine.descriptor.MethodBasedTestDescriptor;
2525
import org.junit.platform.commons.support.ReflectionSupport;
2626
import org.junit.platform.engine.TestDescriptor;
27+
import org.junit.platform.engine.support.discovery.EngineDiscoveryIssueReporter;
2728

2829
/**
2930
* @since 5.5
@@ -33,7 +34,8 @@ class MethodOrderingVisitor
3334

3435
private final JupiterConfiguration configuration;
3536

36-
MethodOrderingVisitor(JupiterConfiguration configuration) {
37+
MethodOrderingVisitor(EngineDiscoveryIssueReporter issueReporter, JupiterConfiguration configuration) {
38+
super(issueReporter);
3739
this.configuration = configuration;
3840
}
3941

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

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,25 +41,25 @@
4141
import org.junit.jupiter.engine.discovery.predicates.IsTestFactoryMethod;
4242
import org.junit.jupiter.engine.discovery.predicates.IsTestMethod;
4343
import org.junit.jupiter.engine.discovery.predicates.IsTestTemplateMethod;
44-
import org.junit.platform.commons.logging.Logger;
45-
import org.junit.platform.commons.logging.LoggerFactory;
4644
import org.junit.platform.commons.util.ClassUtils;
4745
import org.junit.platform.engine.DiscoverySelector;
46+
import org.junit.platform.engine.EngineDiscoveryIssue;
47+
import org.junit.platform.engine.EngineDiscoveryIssue.Severity;
4848
import org.junit.platform.engine.TestDescriptor;
4949
import org.junit.platform.engine.UniqueId;
5050
import org.junit.platform.engine.discovery.DiscoverySelectors;
5151
import org.junit.platform.engine.discovery.IterationSelector;
5252
import org.junit.platform.engine.discovery.MethodSelector;
5353
import org.junit.platform.engine.discovery.NestedMethodSelector;
5454
import org.junit.platform.engine.discovery.UniqueIdSelector;
55+
import org.junit.platform.engine.support.descriptor.MethodSource;
5556
import org.junit.platform.engine.support.discovery.SelectorResolver;
5657

5758
/**
5859
* @since 5.5
5960
*/
6061
class MethodSelectorResolver implements SelectorResolver {
6162

62-
private static final Logger logger = LoggerFactory.getLogger(MethodSelectorResolver.class);
6363
private static final MethodFinder methodFinder = new MethodFinder();
6464
private static final Predicate<Class<?>> testClassPredicate = new IsTestClassWithTests().or(
6565
new IsNestedTestClass());
@@ -72,17 +72,17 @@ class MethodSelectorResolver implements SelectorResolver {
7272

7373
@Override
7474
public Resolution resolve(MethodSelector selector, Context context) {
75-
return resolve(context, emptyList(), selector.getJavaClass(), selector::getJavaMethod, Match::exact);
75+
return resolve(selector, context, emptyList(), selector.getJavaClass(), selector::getJavaMethod, Match::exact);
7676
}
7777

7878
@Override
7979
public Resolution resolve(NestedMethodSelector selector, Context context) {
80-
return resolve(context, selector.getEnclosingClasses(), selector.getNestedClass(), selector::getMethod,
81-
Match::exact);
80+
return resolve(selector, context, selector.getEnclosingClasses(), selector.getNestedClass(),
81+
selector::getMethod, Match::exact);
8282
}
8383

84-
private Resolution resolve(Context context, List<Class<?>> enclosingClasses, Class<?> testClass,
85-
Supplier<Method> methodSupplier,
84+
private Resolution resolve(DiscoverySelector selector, Context context, List<Class<?>> enclosingClasses,
85+
Class<?> testClass, Supplier<Method> methodSupplier,
8686
BiFunction<TestDescriptor, Supplier<Set<? extends DiscoverySelector>>, Match> matchFactory) {
8787
if (!testClassPredicate.test(testClass)) {
8888
return unresolved();
@@ -97,14 +97,15 @@ private Resolution resolve(Context context, List<Class<?>> enclosingClasses, Cla
9797
.collect(toSet());
9898
// @formatter:on
9999
if (matches.size() > 1) {
100-
logger.warn(() -> {
101-
Stream<TestDescriptor> testDescriptors = matches.stream().map(Match::getTestDescriptor);
102-
return String.format(
103-
"Possible configuration error: method [%s] resulted in multiple TestDescriptors %s. "
104-
+ "This is typically the result of annotating a method with multiple competing annotations "
105-
+ "such as @Test, @RepeatedTest, @ParameterizedTest, @TestFactory, etc.",
106-
method.toGenericString(), testDescriptors.map(d -> d.getClass().getName()).collect(toList()));
107-
});
100+
Stream<TestDescriptor> testDescriptors = matches.stream().map(Match::getTestDescriptor);
101+
String message = String.format(
102+
"Possible configuration error: method [%s] resulted in multiple TestDescriptors %s. " //
103+
+ "This is typically the result of annotating a method with multiple competing annotations " //
104+
+ "such as @Test, @RepeatedTest, @ParameterizedTest, @TestFactory, etc.",
105+
method.toGenericString(), testDescriptors.map(d -> d.getClass().getName()).collect(toList()));
106+
context.reportIssue(EngineDiscoveryIssue.builder(Severity.NOTICE, message) //
107+
.selector(selector) //
108+
.source(MethodSource.from(testClass, method)));
108109
}
109110
return matches.isEmpty() ? unresolved() : matches(matches);
110111
}
@@ -139,7 +140,7 @@ public Resolution resolve(UniqueIdSelector selector, Context context) {
139140
public Resolution resolve(IterationSelector selector, Context context) {
140141
if (selector.getParentSelector() instanceof MethodSelector) {
141142
MethodSelector methodSelector = (MethodSelector) selector.getParentSelector();
142-
return resolve(context, emptyList(), methodSelector.getJavaClass(), methodSelector::getJavaMethod,
143+
return resolve(selector, context, emptyList(), methodSelector.getJavaClass(), methodSelector::getJavaMethod,
143144
(testDescriptor, childSelectorsSupplier) -> {
144145
if (testDescriptor instanceof Filterable) {
145146
Filterable filterable = (Filterable) testDescriptor;

0 commit comments

Comments
 (0)