-
Notifications
You must be signed in to change notification settings - Fork 185
Migration to JUnit 5 - avoid using AbstractMojoTestCase #1574
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
base: master
Are you sure you want to change the base?
Conversation
sparsick
left a comment
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.
Also, please check if the following dependencies are still necessary:
<dependency>
<!-- support for JUnit 4 -->
<groupId>org.junit.vintage</groupId>
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13.2</version>
<scope>test</scope>
</dependency>
Should we rename the test classes, so that they have a Test suffix?
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.api.io.TempDir; | ||
|
|
||
| public class TestIncludeExcludeUnpackMojo extends AbstractDependencyMojoTestCase { |
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.
Can AbstractDependencyMojoTestCase be deleted? IMHO after this refactoring it should be possible.
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.
There are still test based on it ...
...java/org/apache/maven/plugins/dependency/fromConfiguration/TestIncludeExcludeUnpackMojo.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| public void assertMarkerFiles(Collection<ArtifactItem> items, boolean exist) { | ||
| private void assertMarkerFiles(UnpackMojo mojo, Collection<ArtifactItem> items, boolean exist) { |
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.
Is it the same private method like in TestIncludeExcludeUnpackMojo? If we have more such assert method, we should move them to a separate Assertion class.
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.
I wil check after finish
- Next part - bump plugin-testing to 3.5.0 - cleanups after parent # Conflicts: # pom.xml
e0c8273 to
99e5653
Compare
Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
mvn -Prun-its verify).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.