-
Notifications
You must be signed in to change notification settings - Fork 3
Feature/junit5 #214
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
Feature/junit5 #214
Conversation
Codecov ReportBase: 69.15% // Head: 69.44% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #214 +/- ##
=============================================
+ Coverage 69.15% 69.44% +0.29%
- Complexity 1022 1047 +25
=============================================
Files 46 47 +1
Lines 5936 5937 +1
Branches 776 776
=============================================
+ Hits 4105 4123 +18
+ Misses 1498 1493 -5
+ Partials 333 321 -12
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
This reverts commit 17ee338.
SonarCloud Quality Gate failed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Out of curiosity, a lot of methods/classes changed from public
to package access, what was the reason?
Junit 5 added the ability to use package access. junit-team/junit-framework#679 |
Description
CLI version of the chain of PRs that ended with dockstore/dockstore#5301
There are still a couple tests that use PowerMock (seems dead, doesn't support Java 17 or JUnit 5, generally bad practice) that are more involved and will require a spin-offf PR before junit4 can be banned from the CLI as well. Also some tests for cwl launching.
One obnoxious test took forever, a reminder to check your assumptions. It was deterministically running differently on CircleCI as opposed to locally, but changing whether it failed and passed on unrelated changes. Looks like the plugin directory was being downloaded and/or cached, but only when some random tests (probably in completely other classes) had run before the test in question, whether due to distribution by the parallel tests script or some oddity due to CircleCI caching.
This is a long way of saying, check your assumptions about tests and how they're isolated from one another. Is the filesystem reset, have stdout and stderr been reset, etc. Oy.
Review Instructions
Number of tests should be the same (or more) before/after this PR
Hmmm, right now at time of comment, it looks like we go from 391 (insights) to 402 (manual count, may need wait for 24 hours for insights to update)
Issue
https://ucsc-cgl.atlassian.net/browse/SEAB-5133
Security
None known
Please make sure that you've checked the following before submitting your pull request. Thanks!
./mvnw clean install