oehme commented on a change in pull request #333:
URL: https://github.com/apache/maven-surefire/pull/333#discussion_r568468631



##########
File path: 
maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
##########
@@ -880,4 +900,20 @@ protected final ForkNodeFactory getForkNode()
     {
         return forkNode;
     }
+
+    @Override
+    boolean verifyParameters() throws MojoFailureException, 
MojoExecutionException

Review comment:
       SurefirePlugin and VerifyMojo do not have a common base class. 

##########
File path: 
maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
##########
@@ -880,4 +900,20 @@ protected final ForkNodeFactory getForkNode()
     {
         return forkNode;
     }
+
+    @Override
+    boolean verifyParameters() throws MojoFailureException, 
MojoExecutionException
+    {
+        if ( failOnFlakeCount < 0 )

Review comment:
       See above, this wouldn't make sense because IntegrationTestMojo does not 
have `failOnFlakeCount`. That's in the VerifyMojo, which doesn't have the same 
base class.

##########
File path: 
maven-surefire-common/src/main/java/org/apache/maven/plugin/surefire/SurefireHelper.java
##########
@@ -286,7 +289,15 @@ private static String createErrorMessage( 
SurefireReportParameters reportParamet
         }
         else
         {
-            msg.append( "There are test failures.\n\nPlease refer to " )
+            if ( result.getFailures() > 0 )
+            {
+                msg.append( "There are test failures." );
+            }
+            else
+            {
+                msg.append( "There are too many flakes." );

Review comment:
       Done, both messages are reported now if both conditions are true. Also, 
the number of flakes and failOnFlakeCount are reported now.

##########
File path: 
maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
##########
@@ -880,4 +900,20 @@ protected final ForkNodeFactory getForkNode()
     {
         return forkNode;
     }
+
+    @Override
+    boolean verifyParameters() throws MojoFailureException, 
MojoExecutionException

Review comment:
       But why would the super class contain logic that only applies to a 
single subclass?

##########
File path: 
maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
##########
@@ -880,4 +900,20 @@ protected final ForkNodeFactory getForkNode()
     {
         return forkNode;
     }
+
+    @Override
+    boolean verifyParameters() throws MojoFailureException, 
MojoExecutionException

Review comment:
       Nevermind, I see the if-else in that method now. 

##########
File path: 
maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
##########
@@ -880,4 +900,20 @@ protected final ForkNodeFactory getForkNode()
     {
         return forkNode;
     }
+
+    @Override
+    boolean verifyParameters() throws MojoFailureException, 
MojoExecutionException

Review comment:
       Nevermind, I see the if-else in that method now. Moved call up to the 
base class.




----------------------------------------------------------------
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


Reply via email to