Avoid reporting the same discovery issues multiple times

When encountering a method through different subclasses multiple times,
any encountered discovery issues are now only reported once.

Author: marcphilipp

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

@@ -19,6 +19,7 @@
 import org.junit.jupiter.engine.discovery.predicates.IsTestClassWithTests;
 import org.junit.platform.engine.EngineDiscoveryRequest;
 import org.junit.platform.engine.TestDescriptor;
+import org.junit.platform.engine.support.discovery.DiscoveryIssueReporter;
 import org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolver;
 import org.junit.platform.engine.support.discovery.EngineDiscoveryRequestResolver.InitializationContext;
 
@@ -56,7 +57,9 @@ private static JupiterConfiguration getConfiguration(InitializationContext<Jupit
 	}
 
 	public void resolveSelectors(EngineDiscoveryRequest request, JupiterEngineDescriptor engineDescriptor) {
-		resolver.resolve(request, engineDescriptor);
+		DiscoveryIssueReporter issueReporter = DiscoveryIssueReporter.deduplicating(
+			DiscoveryIssueReporter.forwarding(request.getDiscoveryListener(), engineDescriptor.getUniqueId()));
+		resolver.resolve(request, engineDescriptor, issueReporter);
 	}
 
 }

junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/DiscoveryIssueReporter.java

@@ -12,6 +12,8 @@
 
 import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
+import java.util.HashSet;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -41,12 +43,31 @@ public interface DiscoveryIssueReporter {
 	 * {@code null}
 	 * @param engineId the unique identifier of the engine; never {@code null}
 	 */
-	static DiscoveryIssueReporter create(EngineDiscoveryListener engineDiscoveryListener, UniqueId engineId) {
+	static DiscoveryIssueReporter forwarding(EngineDiscoveryListener engineDiscoveryListener, UniqueId engineId) {
 		Preconditions.notNull(engineDiscoveryListener, "engineDiscoveryListener must not be null");
 		Preconditions.notNull(engineId, "engineId must not be null");
 		return issue -> engineDiscoveryListener.issueEncountered(engineId, issue);
 	}
 
+	/**
+	 * Create a new {@code DiscoveryIssueReporter} that avoids reporting
+	 * duplicate issues.
+	 *
+	 * <p>The implementation returned by this method is not thread-safe.
+	 *
+	 * @param delegate the delegate to forward issues to; never {@code null}
+	 */
+	static DiscoveryIssueReporter deduplicating(DiscoveryIssueReporter delegate) {
+		Preconditions.notNull(delegate, "delegate must not be null");
+		Set<DiscoveryIssue> seen = new HashSet<>();
+		return issue -> {
+			boolean notSeen = seen.add(issue);
+			if (notSeen) {
+				delegate.reportIssue(issue);
+			}
+		};
+	}
+
 	/**
 	 * Build the supplied {@link DiscoveryIssue.Builder Builder} and report the
 	 * resulting {@link DiscoveryIssue}.

junit-platform-engine/src/main/java/org/junit/platform/engine/support/discovery/EngineDiscoveryRequestResolver.java

@@ -65,7 +65,9 @@ private EngineDiscoveryRequestResolver(List<Function<InitializationContext<T>, S
 
 	/**
 	 * Resolve the supplied {@link EngineDiscoveryRequest} and collect the
-	 * results into the supplied {@link TestDescriptor}.
+	 * results into the supplied {@link TestDescriptor} while forwarding
+	 * encountered discovery issues to the {@link EngineDiscoveryRequest}'s
+	 * {@link org.junit.platform.engine.EngineDiscoveryListener}.
 	 *
 	 * <p>The algorithm works as follows:
 	 *
@@ -110,8 +112,35 @@ private EngineDiscoveryRequestResolver(List<Function<InitializationContext<T>, S
 	public void resolve(EngineDiscoveryRequest request, T engineDescriptor) {
 		Preconditions.notNull(request, "request must not be null");
 		Preconditions.notNull(engineDescriptor, "engineDescriptor must not be null");
-		DiscoveryIssueReporter issueReporter = DiscoveryIssueReporter.create(request.getDiscoveryListener(),
+		DiscoveryIssueReporter issueReporter = DiscoveryIssueReporter.forwarding(request.getDiscoveryListener(),
 			engineDescriptor.getUniqueId());
+		resolve(request, engineDescriptor, issueReporter);
+	}
+
+	/**
+	 * Resolve the supplied {@link EngineDiscoveryRequest} and collect the
+	 * results into the supplied {@link TestDescriptor} using the supplied
+	 * {@link DiscoveryIssueReporter} to report issues encountered during
+	 * resolution.
+	 *
+	 * <p>The algorithm works as described in
+	 * {@link #resolve(EngineDiscoveryRequest, TestDescriptor)}.
+	 *
+	 * @param request the request to be resolved; never {@code null}
+	 * @param engineDescriptor the engine's {@code TestDescriptor} to be used
+	 * for adding direct children
+	 * @param issueReporter the {@link DiscoveryIssueReporter} to report issues
+	 * encountered during resolution
+	 * @since 1.13
+	 * @see #resolve(EngineDiscoveryRequest, TestDescriptor)
+	 * @see SelectorResolver
+	 * @see TestDescriptor.Visitor
+	 */
+	@API(status = EXPERIMENTAL, since = "1.13")
+	public void resolve(EngineDiscoveryRequest request, T engineDescriptor, DiscoveryIssueReporter issueReporter) {
+		Preconditions.notNull(request, "request must not be null");
+		Preconditions.notNull(engineDescriptor, "engineDescriptor must not be null");
+		Preconditions.notNull(issueReporter, "issueReporter must not be null");
 		InitializationContext<T> initializationContext = new DefaultInitializationContext<>(request, engineDescriptor,
 			issueReporter);
 		List<SelectorResolver> resolvers = instantiate(resolverCreators, initializationContext);

jupiter-tests/src/test/java/org/junit/jupiter/engine/discovery/DiscoveryTests.java

@@ -197,7 +197,11 @@ static List<Named<LauncherDiscoveryRequest>> requestsForTestClassWithInvalidTest
 			named("indirectly selected", request() //
 					.selectors(selectPackage(InvalidTestMethodTestCase.class.getPackageName())) //
 					.filters(
-						includeClassNamePatterns(Pattern.quote(InvalidTestMethodTestCase.class.getName()))).build()) //
+						includeClassNamePatterns(Pattern.quote(InvalidTestMethodTestCase.class.getName()))).build()), //
+			named("subclasses", request() //
+					.selectors(selectClass(InvalidTestMethodSubclass1TestCase.class),
+						selectClass(InvalidTestMethodSubclass2TestCase.class)) //
+					.build()) //
 		);
 	}
 
@@ -277,4 +281,10 @@ private static int test() {
 		}
 	}
 
+	static class InvalidTestMethodSubclass1TestCase extends InvalidTestMethodTestCase {
+	}
+
+	static class InvalidTestMethodSubclass2TestCase extends InvalidTestMethodTestCase {
+	}
+
 }

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

@@ -50,7 +50,7 @@ class SuiteTestDescriptorTests {
 
 	final ConfigurationParameters configurationParameters = new EmptyConfigurationParameters();
 	final OutputDirectoryProvider outputDirectoryProvider = OutputDirectoryProviders.dummyOutputDirectoryProvider();
-	final DiscoveryIssueReporter discoveryIssueReporter = DiscoveryIssueReporter.create(mock(), engineId);
+	final DiscoveryIssueReporter discoveryIssueReporter = DiscoveryIssueReporter.forwarding(mock(), engineId);
 	final SuiteTestDescriptor suite = new SuiteTestDescriptor(suiteId, TestSuite.class, configurationParameters,
 		outputDirectoryProvider, discoveryIssueReporter);