Introduce error handling mechanism for validation and discovery errors

[marcphilipp]

Overview

The JUnit Platform currently provides no means to report warnings and errors during test discovery. Thus, malformed test cases (for example, Jupiter test methods with a non-void return type) get silently ignored which poses a severe risk for downstream projects: something that looks like a test in a pull request diff might not actually get executed. Therefore, we should introduce a mechanism to collect and report warnings and errors and use it for the Jupiter and Vintage engines. In addition, we should reach out to third-party test engine open source projects and ask them to adopt the new mechanism.

Validation errors (e.g., for invalid @BeforeEach method declarations) should not abort the entire discovery phase. Instead the discovery phase should continue, with the error tracked and reported during the execution phase.

Areas of Applicability

• exceptions thrown by test engines, as discussed in DefaultLauncher should catch exceptions thrown by test engines #750
• unresolvable unique IDs, as discussed in Handle unparseable UniqueID consistently #210
  • note, however, that a TestEngine should not report a unique ID as unresolvable if the unique ID does not apply to the particular TestEngine (see JupiterTestEngine attempts to resolve unique IDs that belong to other engines #1026 ).
• invalid @BeforeAll, @AfterAll, @BeforeEach, and @AfterEach method declarations
• Competing method-level annotations (e.g. @Test and @TestTemplate on the same method)
• invalid @Test, @TestFactory, @RepeatedTest, and @ParameterizedTest method declarations (see IsTestableMethod silently ignores @Test annotated methods that return a value #2244)
• invalid @Suite class declarations
• invalid @Nested test class declarations (see Improve diagnostics when Vintage test engine is missing #1223 and Abstract classes with @Nested are not ignored / not handled properly #2717)
• non-static test class declaration (see Better diagnostics for orphaned selectClass() on a nested class #2311)
• missing @Nested annotation (see Warn about missing @Nested annotations #1736)
• blank display names supplied by the user, as discussed in Display names must not be allowed to be blank #743
• @Order annotations on test classes and methods when class/method orderer is not OrderAnnotation

Proposals

1. Allow engines to track errors by creating a special type of TestDescriptor such as an AlwaysFailingTestDescriptor, DeadOnArrivalTestDescriptor, or ErrorReportingTestDescriptor.
   • Tracked errors for such a corresponding TestDescriptor could then be thrown as an exception during the execution phase instead of executing the corresponding container or test.
2. Introduce a new property in TestDescriptor that signals an error that was encountered during the discovery phase.
3. Pass a reporting object from the Launcher into each TestEngine to report errors.
   • See Introduce error handling mechanism for validation and discovery errors #242 (comment)

Related Issues

• Introduce validation rules for tests #121
• Handle unparseable UniqueID consistently #210
• Display names must not be allowed to be blank #743
• DefaultLauncher should catch exceptions thrown by test engines #750
• Forbid return values from test and lifecycle methods #835
• Improve error message if multiple "test" annotations are present on a method #876
• Static @BeforeEach method causes entire test plan to abort #971
• JupiterTestEngine attempts to resolve unique IDs that belong to other engines #1026
• Complain when @Test is applied to an inappropriate method #1074
• Improve diagnostics when Vintage test engine is missing #1223
• Unsupported or missing version of JUnit 4 should fail the build #1944
• IsTestableMethod silently ignores @Test annotated methods that return a value #2244
• Better diagnostics for orphaned selectClass() on a nested class #2311
• Abstract classes with @Nested are not ignored / not handled properly #2717
• Tests within inner classes that are children of an abstract class are not executed despite @Nested #4125

Deliverables

• Add initial Platform support for reporting discovery issues #4385
• Report problematic selector resolution results as discovery issues #4389
• Introduce EngineTestKit support for test discovery #4393
• Report discovery issues for Jupiter lifecycle methods #4398
• Revisit the changes made in Static @BeforeEach method causes entire test plan to abort #971 and determine if it makes sense to move the look-up of lifecycle methods back to the constructor of ClassTestDescriptor.
• Report discovery issue for competing test method annotations #4401
• Report discovery issues for invalid suite lifecycle methods #4405
• Report discovery issues for invalid params invocation lifecycle methods #4408
• Report discovery errors for ineffectual lifecycle methods #4415
• Report discovery issues for invalid testable methods #4420
• Revisit the results of Forbid return values from test and lifecycle methods #835 and update the code accordingly by tracking such errors instead of just ignoring such incorrect usage.
• Make critical discovery issue severity configurable #4423
• Move @TestFactory return type validation to IsTestFactoryMethod #949
• Report discovery issues for @Suite classes #4429
• Report discovery issues for invalid test classes #4448
• Warn about missing @Nested annotations #1736
• Introduce JFR events for discovery issues #4453
• Report discovery issues for invalid tag syntax #4456
• Report discovery issues for blank display names #4458
• Report discovery issues for blank @SentenceFragments #4459
• Use the new mechanism to replace the current ad hoc use of logging and transparent defaulting as a work-around.
  • Search for TODO [#242] within the code base.
  • See DefaultLauncher should catch exceptions thrown by test engines #750 (comment)
• Determine where else the new mechanism can be used and apply it.
• Report discovery issues for class and method ordering #4463
• Document discovery issue reporting #4465
• Reach out to third-party test engine authors and ask them to adopt the new mechanism
  • Consider adopting JUnit 5.13's DiscoveryIssue-reporting mechanism spockframework/spock#2145
  • Consider adopting JUnit 5.13's DiscoveryIssue-reporting mechanism cucumber/cucumber-jvm#2987

[sbrannen]

The same is true for invalid custom display names supplied by the user, as described in #743.

I will therefore update the description of this issue to reflect the broader scope.

[sbrannen]

FYI: I have introduced a minimum set of Deliverables.