Skip to content
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

Fix reporting of @Ignored JUnit 3 test classes #3427

Merged
merged 7 commits into from
Sep 5, 2023
Merged

Fix reporting of @Ignored JUnit 3 test classes #3427

merged 7 commits into from
Sep 5, 2023

Conversation

pshevche
Copy link
Contributor

Overview

If JUnit3 test cases are annotated w/ @Ignore and run w/ JUnit Vintage engine, they will be reported twice: once as started and once as skipped.

This is caused by inconsistent checking for the annotation presence. DefensiveAllDefaultPossibilitiesBuilder will check for the annotation presence on the Java class to create an IgnoringRunnerDecorator (ref), while RunListenerAdapter will look for the annotation on the runner's Description to actually ignore the class (ref).

So, if the Java class is annotated w/ @Ignore, but the runner does no add it to the root Description, then the test class will be once reported as started by the RunListenerAdapter and once as skipped by IgnoringRunnerDecorator. And this is exactly the case for JUnit38ClassRunner which does not include class-level annotation in the runner's description (ref).

In this PR, I propose to explicilty add @Ignore annotation for the JUnit3 runner's description and warn creators of custom runners about the missing annotation. I open this PR as a draft proposal first to discuss the approach.


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


Definition of Done

@marcphilipp marcphilipp self-assigned this Aug 28, 2023
@marcphilipp marcphilipp self-requested a review August 28, 2023 12:31
@marcphilipp marcphilipp added this to the 5.10.1 milestone Sep 1, 2023
@marcphilipp marcphilipp marked this pull request as ready for review September 5, 2023 11:52
@marcphilipp
Copy link
Member

@pshevche Thanks for the PR! I had some qualms about decorating Description so went a slight different route. Instead of looking for the annotation in IgnoringRunnerDecorator that's now done in RunListenerAdapter.

Copy link
Contributor Author

@pshevche pshevche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marcphilipp , looks good from my side, thank you for getting it over the finish line 👍

@marcphilipp marcphilipp changed the title Fix wrong reporting of ignored JUnit3 test cases when using vintage engine Fix reporting of ignored JUnit 3 test classes Sep 5, 2023
@marcphilipp marcphilipp merged commit cb34452 into junit-team:main Sep 5, 2023
13 checks passed
sbrannen pushed a commit that referenced this pull request Sep 14, 2023
Prior to this commit, `@Ignore`-annotated JUnit 3 test classes where
reported as started and then as skipped due to their `Description` not
including class-level annotations in some case.

Co-authored-by: Marc Philipp <[email protected]>
@sbrannen sbrannen changed the title Fix reporting of ignored JUnit 3 test classes Fix reporting of @Ignored JUnit 3 test classes Sep 16, 2023
@pshevche pshevche deleted the pshevche/vintage-engine-wrong-reporting-ignored-junit3 branch November 16, 2023 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants