Draft API for reporting discovery issues #4380

marcphilipp · 2025-03-10T14:29:26Z

Overview

This is a proof-of-concept draft for #242 to get early feedback on the API.

I hereby agree to the terms of the JUnit Contributor License Agreement.

Definition of Done

There are no TODOs left in the code
Method preconditions are checked and documented in the method's Javadoc
Coding conventions (e.g. for logging) have been followed
Change is covered by automated tests including corner cases, errors, and exception handling
Public API has Javadoc and @API annotations
Change is documented in the User Guide and Release Notes

marcphilipp · 2025-03-10T14:31:50Z

junit-platform-engine/src/main/java/org/junit/platform/engine/EngineDiscoveryListener.java

 	default void selectorProcessed(UniqueId engineId, DiscoverySelector selector, SelectorResolutionResult result) {
 	}

+	default void issueFound(UniqueId engineId, EngineDiscoveryIssue issue) {


💬 This is the top-level API for engines to report discovery issues.

marcphilipp · 2025-03-10T14:33:47Z

...org/junit/platform/launcher/listeners/discovery/AbortOnFailureLauncherDiscoveryListener.java

+		Set<Severity> failureIssueSeverities = Arrays.stream(configValue.split(",")) //
+				.map(Severity::valueOf) // TODO case-insensitive parsing
+				.collect(toCollection(() -> EnumSet.noneOf(Severity.class)));
+		// TODO Collect errors and throw exception or log all issues


💬 The idea here is that the default abortOnFailure listener would treat the configured issue severities as failures and throw an exception containing all of them after the launcher has finished test discovery. In 5.13 the list would be empty (?) but in 6.0 we could make it strict and fail on deprecations and warnings.

Alternatively, a discovery listener could collect all issues and report the engine descriptor as failed during execution.

see #4380 (comment)

marcphilipp · 2025-03-10T14:35:04Z

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

+			context.reportIssue(EngineDiscoveryIssue.builder(Severity.NOTICE, message) //
+					.selector(selector) //
+					.source(MethodSource.from(testClass, method)));


💬 Engines using the EngineDiscoveryRequestResolver framework work discovery can report issues via the Context passed to each SelectorResolver.

marcphilipp · 2025-03-10T14:36:12Z

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

-			.addTestDescriptorVisitor(ctx -> new MethodOrderingVisitor(getConfiguration(ctx))) //
+			.addTestDescriptorVisitor(ctx -> new ClassOrderingVisitor(ctx.getIssueReporter(), getConfiguration(ctx))) //
+			.addTestDescriptorVisitor(ctx -> new MethodOrderingVisitor(ctx.getIssueReporter(), getConfiguration(ctx))) //
+			.addTestDescriptorVisitor(ctx -> new ValidatingVisitor(ctx.getIssueReporter())) //


💬 Visitors registered via EngineDiscoveryRequestResolver can use the DiscoveryIssueReporter to report issues.

marcphilipp · 2025-03-10T14:37:15Z

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

+		if (descriptor instanceof Validatable) {
+			((Validatable) descriptor).validate(new Reporter(descriptor));
+		}


💬 Engines such as Jupiter can use any custom logic to validate during discovery. Here a visitor is used that checks for the Jupiter-specific Validatable marker interface.

marcphilipp · 2025-03-10T14:38:17Z

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

+		afterAllMethods.stream() //
+				.filter(ModifierSupport::isPrivate) //
+				.forEach(method -> reporter.reportIssue(Severity.DEPRECATION,
+					String.format("@AfterAll method [%s] should not be private.", method.toGenericString()),
+					b -> b.source(MethodSource.from(method))));


💬 Example of a engine-specific "post-discovery" validation that reports private lifecycle methods as deprecated (also would need to be done for @BeforeEach etc.).

marcphilipp · 2025-03-10T14:39:26Z

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

+			AbstractOrderingVisitor.this.issueReporter //
+					.reportIssue(EngineDiscoveryIssue.builder(Severity.WARNING, message) //
+							.source(parentTestDescriptor.getSource()));


💬 Another example of a custom "post-discovery" validation that reports a warning for issues during class/method ordering.

junit-platform-engine/src/main/java/org/junit/platform/engine/EngineDiscoveryIssue.java

leonard84

some preliminary comments

junit-platform-engine/src/main/java/org/junit/platform/engine/EngineDiscoveryIssue.java

leonard84 · 2025-03-10T14:50:06Z

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

+
+	void validate(IssueReporter issueReporter);
+
+	interface IssueReporter {


🚲🏠 Why did you choose issue instead of problem?

...n/java/org/junit/platform/launcher/listeners/discovery/LoggingLauncherDiscoveryListener.java

mpkorstanje

This looks very workable. It would cover all use cases Cucumber has for it now.

marcphilipp · 2025-03-11T17:13:59Z

Alternatively, a discovery listener could collect all issues and report the engine descriptor as failed during execution.

I've spiked that approach. Here are the preliminary results:

Gradle

Maven

IntelliJ

Run via Gradle

Eclipse

leonard84 · 2025-03-11T18:52:25Z

Was this just for testing, or will deprecations fail the discovery? IMHO, only Severity.ERROR should cause a failure unless you set something like the equivalent of -Werror.

sbrannen · 2025-03-12T09:14:55Z

Was this just for testing, or will deprecations fail the discovery? IMHO, only Severity.ERROR should cause a failure unless you set something like the equivalent of -Werror.

That was just for testing.

We haven't yet decided what severity would fail the build, and that may differ in 5.x and 6.0.

We also plan to make it configurable.

marcphilipp · 2025-03-12T11:34:16Z

Correct, that was just for testing. All "non-critical" issues (depending on the severity configured as "critical") will just be logged.

- Rename to `DiscoveryIssue` - Remove `selector()` property - Add `ERROR` severity - Rename listener method to `issueEncountered`

Since those are now reported as discovery issues

It expects engine descriptors to be containers

marcphilipp · 2025-03-13T16:37:52Z

Superseded by #4385

marcphilipp self-assigned this Mar 10, 2025

marcphilipp added type: enhancement theme: reporting component: Jupiter component: Platform theme: discovery theme: diagnostics labels Mar 10, 2025

marcphilipp commented Mar 10, 2025

View reviewed changes

marcphilipp force-pushed the marc/242-discovery-issues branch from 07b8259 to f848139 Compare March 10, 2025 14:42

marcphilipp commented Mar 10, 2025

View reviewed changes

junit-platform-engine/src/main/java/org/junit/platform/engine/EngineDiscoveryIssue.java Outdated Show resolved Hide resolved

marcphilipp force-pushed the marc/242-discovery-issues branch from f848139 to 7827335 Compare March 10, 2025 14:56

leonard84 reviewed Mar 10, 2025

View reviewed changes

mpkorstanje reviewed Mar 10, 2025

View reviewed changes

marcphilipp linked an issue Mar 11, 2025 that may be closed by this pull request

Introduce error handling mechanism for validation and discovery errors #242

Closed

36 tasks

marcphilipp force-pushed the marc/242-discovery-issues branch from 6f121de to 09ddba0 Compare March 11, 2025 13:40

marcphilipp added 11 commits March 13, 2025 09:36

Draft API for reporting discovery issues

f0807a1

Apply feedback from team call

a5b4ff5

- Rename to `DiscoveryIssue` - Remove `selector()` property - Add `ERROR` severity - Rename listener method to `issueEncountered`

Add missing API annotations

0dae85d

Remove issue collecting from abortOnFailure listener

232466a

Implement new method

97aba67

Handle error severity when logging

d79419a

Report discovery issues as engine-level execution failure

57d5170

No longer abort on selector processing issues

dfaa88a

Since those are now reported as discovery issues

List issues in exception message

7c8d2ba

Avoid failed assert in IntelliJ

8b76090

It expects engine descriptors to be containers

Use original display name from engine descriptor

f3946a9

marcphilipp added 3 commits March 13, 2025 09:36

Don't log by default to avoid issues from being reported multiple times

e414c39

Print IDE-compatible links for method/class sources

cee6064

Polishing

a89235c

marcphilipp force-pushed the marc/242-discovery-issues branch from 08b24e0 to a89235c Compare March 13, 2025 08:37

marcphilipp closed this Mar 13, 2025

marcphilipp removed a link to an issue Mar 16, 2025

Introduce error handling mechanism for validation and discovery errors #242

Closed

36 tasks

marcphilipp deleted the marc/242-discovery-issues branch March 18, 2025 18:13

marcphilipp mentioned this pull request Apr 11, 2025

Adopt JUnit 5.13's DiscoveryIssue-reporting mechanism cucumber/cucumber-jvm#2987

Closed

mpkorstanje mentioned this pull request Jun 10, 2025

[JUnit Platform] Use EngineDiscoveryRequestResolver cucumber/cucumber-jvm#2835

Merged

7 tasks


		void validate(IssueReporter issueReporter);

		interface IssueReporter {

Uh oh!

Draft API for reporting discovery issues #4380

Draft API for reporting discovery issues #4380

Uh oh!

Conversation

marcphilipp commented Mar 10, 2025

Overview

Definition of Done

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marcphilipp Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

leonard84 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mpkorstanje left a comment

Choose a reason for hiding this comment

Uh oh!

marcphilipp commented Mar 11, 2025

Gradle

Maven

IntelliJ

Run via Gradle

Eclipse

Uh oh!

leonard84 commented Mar 11, 2025

Uh oh!

sbrannen commented Mar 12, 2025

Uh oh!

marcphilipp commented Mar 12, 2025

Uh oh!

marcphilipp commented Mar 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

marcphilipp Mar 10, 2025 •

edited

Loading