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

Re-enable JUnit 4 and JUnit 3 unit tests #3273

Conversation

csaboka
Copy link
Contributor

@csaboka csaboka commented Feb 25, 2024

Looks like this was broken in #3233. With the addition of JUnit 5, the existing unit tests got ignored (note the sharp drop in overall coverage in the Codecov report). This PR should restore those old tests to full functionality.

Fixes #3270

When JUnit 5 support was added to the project, the "JUnit Vintage Engine"
artifact wasn't added. Without this artifact, the JUnit 5 engine ignores
any tests written for JUnit 4 or JUnit 3, i.e. the majority of the tests.
Copy link

codecov bot commented Feb 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.45%. Comparing base (bc18c36) to head (6f7c5ee).

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #3273       +/-   ##
=============================================
+ Coverage      1.78%   55.45%   +53.67%     
- Complexity      100     5506     +5406     
=============================================
  Files           721      721               
  Lines         29534    29534               
  Branches       3840     3840               
=============================================
+ Hits            526    16378    +15852     
+ Misses        28970    11630    -17340     
- Partials         38     1526     +1488     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@kwin kwin left a comment

Choose a reason for hiding this comment

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

Looks good in general, just some minor remarks regarding dependency scopes/management.

pom.xml Outdated Show resolved Hide resolved
bundle/pom.xml Show resolved Hide resolved
- Pull JUnit BOM instead of individual artifacts
- Make JUnit engines dependencies of the surefire plugin
Pull only the necessary artifacts to the test classpath.
@csaboka
Copy link
Contributor Author

csaboka commented Feb 26, 2024

I have no idea why the Mac test failed or why the codeclimate check is stuck. Is there a way to redo the checks? Should I push a dummy commit?

@kwin
Copy link
Contributor

kwin commented Feb 26, 2024

I have no idea why the Mac test failed or why the codeclimate check is stuck

The first seems to be a race condition happening from time to time. Only happens on Mac CI, not related to your commit. No clue about the second one. Nothing to worry about though, this doesn't prevent the merge. I will wait till the end of the week before merging to give @davidjgonzalez to have a look as well.

@kwin kwin merged commit 12facd8 into Adobe-Consulting-Services:master Mar 1, 2024
18 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only JUnit5 tests executed by m-surefire-p
2 participants