Skip to content

Report discovery issue for competing test method annotations #4401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 19, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class DiscoverySelectorResolver {
private static final EngineDiscoveryRequestResolver<JupiterEngineDescriptor> resolver = EngineDiscoveryRequestResolver.<JupiterEngineDescriptor> builder() //
.addClassContainerSelectorResolver(new IsTestClassWithTests()) //
.addSelectorResolver(ctx -> new ClassSelectorResolver(ctx.getClassNameFilter(), getConfiguration(ctx))) //
.addSelectorResolver(ctx -> new MethodSelectorResolver(getConfiguration(ctx))) //
.addSelectorResolver(ctx -> new MethodSelectorResolver(getConfiguration(ctx), ctx.getIssueReporter())) //
.addTestDescriptorVisitor(ctx -> TestDescriptor.Visitor.composite( //
new ClassOrderingVisitor(getConfiguration(ctx)), //
new MethodOrderingVisitor(getConfiguration(ctx)), //
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@
import org.junit.jupiter.engine.discovery.predicates.IsTestFactoryMethod;
import org.junit.jupiter.engine.discovery.predicates.IsTestMethod;
import org.junit.jupiter.engine.discovery.predicates.IsTestTemplateMethod;
import org.junit.platform.commons.logging.Logger;
import org.junit.platform.commons.logging.LoggerFactory;
import org.junit.platform.commons.util.ClassUtils;
import org.junit.platform.engine.DiscoveryIssue;
import org.junit.platform.engine.DiscoveryIssue.Severity;
import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.TestDescriptor;
import org.junit.platform.engine.UniqueId;
Expand All @@ -52,22 +52,25 @@
import org.junit.platform.engine.discovery.MethodSelector;
import org.junit.platform.engine.discovery.NestedMethodSelector;
import org.junit.platform.engine.discovery.UniqueIdSelector;
import org.junit.platform.engine.support.descriptor.MethodSource;
import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
import org.junit.platform.engine.support.discovery.SelectorResolver;

/**
* @since 5.5
*/
class MethodSelectorResolver implements SelectorResolver {

private static final Logger logger = LoggerFactory.getLogger(MethodSelectorResolver.class);
private static final MethodFinder methodFinder = new MethodFinder();
private static final Predicate<Class<?>> testClassPredicate = new IsTestClassWithTests().or(
new IsNestedTestClass());

protected final JupiterConfiguration configuration;
private final JupiterConfiguration configuration;
private final DiscoveryIssueReporter issueReporter;

MethodSelectorResolver(JupiterConfiguration configuration) {
MethodSelectorResolver(JupiterConfiguration configuration, DiscoveryIssueReporter issueReporter) {
this.configuration = configuration;
this.issueReporter = issueReporter;
}

@Override
Expand Down Expand Up @@ -97,14 +100,14 @@ private Resolution resolve(Context context, List<Class<?>> enclosingClasses, Cla
.collect(toSet());
// @formatter:on
if (matches.size() > 1) {
logger.warn(() -> {
Stream<TestDescriptor> testDescriptors = matches.stream().map(Match::getTestDescriptor);
return String.format(
"Possible configuration error: method [%s] resulted in multiple TestDescriptors %s. "
+ "This is typically the result of annotating a method with multiple competing annotations "
+ "such as @Test, @RepeatedTest, @ParameterizedTest, @TestFactory, etc.",
method.toGenericString(), testDescriptors.map(d -> d.getClass().getName()).collect(toList()));
});
Stream<TestDescriptor> testDescriptors = matches.stream().map(Match::getTestDescriptor);
String message = String.format(
"Possible configuration error: method [%s] resulted in multiple TestDescriptors %s. "
+ "This is typically the result of annotating a method with multiple competing annotations "
+ "such as @Test, @RepeatedTest, @ParameterizedTest, @TestFactory, etc.",
method.toGenericString(), testDescriptors.map(d -> d.getClass().getName()).collect(toList()));
issueReporter.reportIssue(
DiscoveryIssue.builder(Severity.WARNING, message).source(MethodSource.from(method)));
}
return matches.isEmpty() ? unresolved() : matches(matches);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@

import org.junit.platform.commons.JUnitException;
import org.junit.platform.commons.util.UnrecoverableExceptions;
import org.junit.platform.engine.DiscoveryIssue;
import org.junit.platform.engine.DiscoverySelector;
import org.junit.platform.engine.EngineDiscoveryListener;
import org.junit.platform.engine.EngineDiscoveryRequest;
Expand Down Expand Up @@ -262,11 +261,6 @@ private <T extends TestDescriptor> Optional<T> createAndAdd(TestDescriptor paren
return child;
}

@Override
public void reportIssue(DiscoveryIssue issue) {
issueReporter.reportIssue(issue);
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ default Resolution resolve(DiscoverySelector selector, Context context) {
* @see SelectorResolver
*/
@API(status = STABLE, since = "1.10")
interface Context extends DiscoveryIssueReporter {
interface Context {

/**
* Resolve the supplied {@link TestDescriptor}, if possible.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,15 @@

package org.junit.jupiter.engine;

import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;

import java.util.logging.Level;
import java.util.logging.LogRecord;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.platform.commons.util.CollectionUtils.getOnlyElement;

import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.RepetitionInfo;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.fixtures.TrackLogRecords;
import org.junit.platform.commons.logging.LogRecordListener;
import org.junit.platform.engine.DiscoveryIssue.Severity;
import org.junit.platform.engine.support.descriptor.MethodSource;

/**
* Integration tests that verify the correct behavior for methods annotated
Expand All @@ -32,23 +29,27 @@
class MultipleTestableAnnotationsTests extends AbstractJupiterTestEngineTests {

@Test
void testAndRepeatedTest(@TrackLogRecords LogRecordListener listener) {
discoverTests(request().selectors(selectClass(TestCase.class)).build());

// @formatter:off
assertTrue(listener.stream(Level.WARNING)
.map(LogRecord::getMessage)
.anyMatch(m -> m.matches("Possible configuration error: method .+ resulted in multiple TestDescriptors .+")));
// @formatter:on
void testAndRepeatedTest() throws Exception {
var results = discoverTestsForClass(TestCase.class);

var discoveryIssue = getOnlyElement(results.getDiscoveryIssues());

assertThat(discoveryIssue.severity()) //
.isEqualTo(Severity.WARNING);
assertThat(discoveryIssue.message()) //
.matches("Possible configuration error: method .+ resulted in multiple TestDescriptors .+");
assertThat(discoveryIssue.source()) //
.contains(
MethodSource.from(TestCase.class.getDeclaredMethod("testAndRepeatedTest", RepetitionInfo.class)));
}

@SuppressWarnings("JUnitMalformedDeclaration")
static class TestCase {

@SuppressWarnings("JUnitMalformedDeclaration")
@Test
@RepeatedTest(1)
void testAndRepeatedTest(RepetitionInfo repetitionInfo) {
assertNotNull(repetitionInfo);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ public class EngineDiscoveryRequestResolverTests {
@Test
void allowsSelectorResolversToReportDiscoveryIssues() {
var resolver = EngineDiscoveryRequestResolver.builder() //
.addSelectorResolver(new SelectorResolver() {
.addSelectorResolver(ctx -> new SelectorResolver() {
@Override
public Resolution resolve(ClassSelector selector, Context context) {
context.reportIssue(DiscoveryIssue.builder(NOTICE, "test") //
.source(ClassSource.from(selector.getClassName())));
ctx.getIssueReporter() //
.reportIssue(DiscoveryIssue.builder(NOTICE, "test") //
.source(ClassSource.from(selector.getClassName())));
return unresolved();
}
}) //
Expand Down