Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r606194405
##########
File path:
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -501,6 +504,12 @@
@Parameter( property = "excludedGroups" )
private String excludedGroups;
+ @Parameter( property = "junitIncludeEngine" )
Review comment:
Both instance fileds are in wrong class. We have two plugins in this
project, so you have to generate getter and setter for these fields. Both must
be in `SurefirPlugin` class, see
[this](https://github.com/apache/maven-surefire/blob/master/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java#L99)
and also in
[here](https://github.com/apache/maven-surefire/blob/master/maven-failsafe-plugin/src/main/java/org/apache/maven/plugin/failsafe/IntegrationTestMojo.java#L116).
The reson is the prefix in the property. Due to AbstractSurefireMojo is an
abstract class, you have to generate getters and setters and use them instead
of fields in AbstractSurefireMojo.
Regarding the naming conventions pls start the name of fields and properties
with "include" and "exclude", and rename junit to "junit5" to make it clear
that we are not aiming for JUnit4 and 3.
Additionally both should be String[] because the history shows us that
people want to have more than one value in our config properties, pls see [the
tutorial for
MOJO](https://maven.apache.org/guides/plugin/guide-java-plugin-development.html#parameter-types-with-multiple-values).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
