-
Notifications
You must be signed in to change notification settings - Fork 40
Improve JUnit5 extension, make similar to Maven 4 implementation #50
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
Improve JUnit5 extension, make similar to Maven 4 implementation #50
Conversation
maven-plugin-testing-harness/src/main/java/org/apache/maven/api/plugin/testing/InjectMojo.java
Show resolved
Hide resolved
|
This is really interesting. |
|
Hi @gnodet , So what's left open is the stubbing. I made a check and backporting all the stubs seems to be a bigger task. My current opinion is to NOT offer the stubs on maven 3 (which means maven-plugin-testing with stubs is only possible with old JUnit 4 style tests extending the AbstractMojoTestCase). What is your opinion there? |
|
Resolve #192 |
|
@aamotharald Same here :) |
c58431e to
4f7af5a
Compare
|
@gnodet @slachiewicz @aamotharald I have rebased and fix build I hope we can process it forward |
| import org.codehaus.plexus.util.xml.Xpp3DomBuilder; | ||
|
|
||
| /** | ||
| * TODO: add a way to use the plugin POM for the lookup so that the user doesn't have to provide the a:g:v:goal |
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.
That looks slightly weird to add a bunch of TODO and deprecate the class at the same time...
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 will remove TODO from deprecated classes .... 😄
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.
done
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
|
||
| @MojoTest | ||
| class Junit5Test { |
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.
Why removing this one ? Is it duplicating another test ?
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.
we have JUnit5 tests in maven-plugin-testing-harness/src/test/java/org/apache/maven/plugin/testing/ParametersMojoTest.java
# Conflicts: # maven-plugin-testing-harness/src/main/java/org/apache/maven/plugin/testing/AbstractMojoTestCase.java
cfe7813 to
a33ed8d
Compare
|
@gnodet @slachiewicz look ok for me .... I would like to process it I will change PR title as we improve JUnit 5 implementation, documentation will be fixed in next PR @aamotharald thanks for work |
| String basedir = AnnotationSupport.findAnnotation(context.getElement().get(), Basedir.class) | ||
| .map(Basedir::value) | ||
| .orElse(getBasedir()); | ||
| field.set(null, basedir); |
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.
this will be fixed with:
we should not override a static field
f46bdbe
into
apache:maven-plugin-testing-3.x
starting some documentation activities now and performing testing of the JUnit5 maven-plugin testing capabilities with existing open source maven plugins.
@gnodet I have made a small change in the MojoExtension as I think it's valid to have no configuration for your maven plugin at all. I'll just take the commit away if my understanding here is wrong