[GitHub] [maven-surefire] adam11grafik opened a new pull request #344: SUREFIRE-1854 Support include/exclude junit test engine

[GitBox]

adam11grafik opened a new pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344


   Support include/exclude junit test engine


--
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:
[hidden email]

[GitBox]

adam11grafik commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811354712


   Hi @Tibor17, can you help me with build issue?
   maven-checkstyle-plugin throw a lot of error s but any formatter was not triggered for my branch  when I was building project locally.
   So how can I keep all those standards?
   BR,
   Adam


--
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:
[hidden email]

[GitBox]

Tibor17 commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811555475


   yes, you have modified the biggest class AbstractSurefireMojo. The problem is that I am not able to recognize real changes and the code format. I guess you pressed CRTL+L in your IDE and reformatted the code. Pls do not do that. That;s the reason why the build is not successful. You should place only chages without reformating all the code around.


--
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:
[hidden email]

[GitBox]

Tibor17 edited a comment on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811555475


   yes, you have modified the biggest class AbstractSurefireMojo. The problem is that I am not able to recognize real changes within the code format    you changed. I guess you pressed CRTL+L in your IDE and reformatted the code. Pls do not do that. That;s the reason why the build is not successful. You should place only chages without reformating all the code around.


--
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:
[hidden email]

[GitBox]

Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r605305237



##########
File path: surefire-providers/surefire-junit-platform/src/main/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProvider.java
##########
@@ -60,10 +60,7 @@
 import org.apache.maven.surefire.shared.utils.StringUtils;
 import org.junit.platform.engine.DiscoverySelector;
 import org.junit.platform.engine.Filter;
-import org.junit.platform.launcher.Launcher;
-import org.junit.platform.launcher.LauncherDiscoveryRequest;
-import org.junit.platform.launcher.TagFilter;
-import org.junit.platform.launcher.TestIdentifier;
+import org.junit.platform.launcher.*;

Review comment:
       star is not legal




--
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:
[hidden email]

[GitBox]

Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r605305509



##########
File path: surefire-providers/surefire-junit-platform/src/test/java/org/apache/maven/surefire/junitplatform/JUnitPlatformProviderTest.java
##########
@@ -22,8 +22,8 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
 import static java.util.stream.Collectors.toSet;
-import static org.apache.maven.surefire.api.booter.ProviderParameterNames.TESTNG_EXCLUDEDGROUPS_PROP;
-import static org.apache.maven.surefire.api.booter.ProviderParameterNames.TESTNG_GROUPS_PROP;
+import static org.apache.maven.surefire.api.booter.ProviderParameterNames.*;

Review comment:
       again star here




--
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:
[hidden email]

[GitBox]

adam11grafik closed pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344


   


--
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:
[hidden email]

[GitBox]

adam11grafik commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811720519


   >
   >
   > yes, you have modified the biggest class AbstractSurefireMojo. The problem is that I am not able to recognize real changes within the code format you changed. I guess you pressed CRTL+L in your IDE and reformatted the code. Pls do not do that. That;s the reason why the build is not successful. You should place only chages without reformating all the code around.
   
   


--
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:
[hidden email]

[GitBox]

adam11grafik commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811722247


   @Tibor17, hmm why maven repository does not use maven formatter plugin? :)
   Should I fix this on the same branch and create new PR?
   I do not know why PR was closed :/
   Or should I create new branch?
   BR,
   Adam


--
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:
[hidden email]

[GitBox]

Tibor17 commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811727484


   I can see that zou closed your PR but I have reopened it.
   Please continue here.
   


--
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:
[hidden email]

[GitBox]

Tibor17 edited a comment on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811727484


   I can see that you closed your PR but I have reopened it.
   Please continue here.
   


--
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:
[hidden email]

[GitBox]

adam11grafik commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811729840


   Ahh sorry, maybe I click wrong button :)


--
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:
[hidden email]

[GitBox]

Tibor17 commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811729884


   Do you use IntelliJ IDEA?
   You can configure it so that the IDE will be according to the checkstyle plugin, see this:
   https://maven.apache.org/developers/conventions/code.html
   ```
   Download maven-idea-codestyle.xml and import it into IDEA using File > Settings > Editor > Code Style > Gear icon > Import Scheme > IntelliJ IDEA Code Style XML
   ```


--
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:
[hidden email]

[GitBox]

Tibor17 commented on pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#issuecomment-811738125


   My advice for you is to copy the class and revert it. Then apply only code changes.


--
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:
[hidden email]

[GitBox]

adam11grafik closed pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344


   


--
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:
[hidden email]

[GitBox]

Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r605481968



##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -1560,6 +1566,18 @@ private void convertGroupParameters()
         }
     }
 
+    private void convertJunitEngineParameters()
+    {
+        if ( this.getJunitIncludeEngine() != null )

Review comment:
       Pls use `getJunitIncludeEngine()` instead of `this.getJunitIncludeEngine()`.
   Remove "this".




--
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:
[hidden email]

[GitBox]

Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r605482069



##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -1560,6 +1566,18 @@ private void convertGroupParameters()
         }
     }
 
+    private void convertJunitEngineParameters()
+    {
+        if ( this.getJunitIncludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_INCLUDE_ENGINE_PROP, this.getJunitIncludeEngine() );

Review comment:
       Remove "this".

##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -1560,6 +1566,18 @@ private void convertGroupParameters()
         }
     }
 
+    private void convertJunitEngineParameters()
+    {
+        if ( this.getJunitIncludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_INCLUDE_ENGINE_PROP, this.getJunitIncludeEngine() );
+        }
+        if ( this.getJunitExcludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_EXCLUDE_ENGINE_PROP, this.getJunitExcludeEngine() );

Review comment:
       Remove "this".




--
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:
[hidden email]

[GitBox]

Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r605484013



##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -1560,6 +1566,18 @@ private void convertGroupParameters()
         }
     }
 
+    private void convertJunitEngineParameters()
+    {
+        if ( this.getJunitIncludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_INCLUDE_ENGINE_PROP, this.getJunitIncludeEngine() );

Review comment:
       Here can be the static import and the line would become shorter.

##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -1560,6 +1566,18 @@ private void convertGroupParameters()
         }
     }
 
+    private void convertJunitEngineParameters()
+    {
+        if ( this.getJunitIncludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_INCLUDE_ENGINE_PROP, this.getJunitIncludeEngine() );
+        }
+        if ( this.getJunitExcludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_EXCLUDE_ENGINE_PROP, this.getJunitExcludeEngine() );

Review comment:
       Here can be the static import and the line would become shorter.




--
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:
[hidden email]

[GitBox]

Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r605484013



##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -1560,6 +1566,18 @@ private void convertGroupParameters()
         }
     }
 
+    private void convertJunitEngineParameters()
+    {
+        if ( this.getJunitIncludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_INCLUDE_ENGINE_PROP, this.getJunitIncludeEngine() );

Review comment:
       Here can be the static import for JUNIT_EXCLUDE_ENGINE_PROP and the line would become shorter.




--
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:
[hidden email]

[GitBox]

Tibor17 commented on a change in pull request #344:
URL: https://github.com/apache/maven-surefire/pull/344#discussion_r605484013



##########
File path: maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/AbstractSurefireMojo.java
##########
@@ -1560,6 +1566,18 @@ private void convertGroupParameters()
         }
     }
 
+    private void convertJunitEngineParameters()
+    {
+        if ( this.getJunitIncludeEngine() != null )
+        {
+            getProperties().setProperty( ProviderParameterNames.JUNIT_INCLUDE_ENGINE_PROP, this.getJunitIncludeEngine() );

Review comment:
       Here can be the static import for JUNIT_INCLUDE_ENGINE_PROP and the line would become shorter.




--
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:
[hidden email]