Tibor17 commented on code in PR #517:
URL: https://github.com/apache/maven-surefire/pull/517#discussion_r847792019


##########
surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java:
##########
@@ -37,33 +36,59 @@
  *
  * @since 3.0.0-M6
  */
-public class TestNG740Configurator extends TestNG60Configurator
+public class TestNG740Configurator
+    extends TestNG60Configurator
 {
     @Override
-    public void configure( XmlSuite suite, Map<String, String> options )
+    protected void configureThreadCount( XmlSuite suite, Map<String, String> 
options )
         throws TestSetFailedException
     {
-        String threadCountAsString = options.get( THREADCOUNT_PROP );
-        int threadCount = threadCountAsString == null ? 1 : parseInt( 
threadCountAsString );
-        suite.setThreadCount( threadCount );
-
-        String parallel = options.get( PARALLEL_PROP );
-        if ( parallel != null )
+        String threadCount = options.get( THREADCOUNT_PROP );
+        // if [threadcount] spec'd
+        if ( threadCount != null )
         {
-            if ( !"methods".equalsIgnoreCase( parallel ) && 
!"classes".equalsIgnoreCase( parallel ) )
+            try
             {
-                throw new TestSetFailedException( "Unsupported TestNG parallel 
setting: "
-                    + parallel + " ( only METHODS or CLASSES supported )" );
+                // convert and apply [threadcount] setting
+                suite.setThreadCount( Integer.parseInt( threadCount ) );
+            }
+            catch ( NumberFormatException e )
+            {
+                throw new TestSetFailedException( "Non-integer TestNG 
[threadcount] setting: " + threadCount, e );
             }
-            Class enumClass = tryLoadClass( XmlSuite.class.getClassLoader(), 
"org.testng.xml.XmlSuite$ParallelMode" );
-            Enum<?> parallelEnum = Enum.valueOf( enumClass, 
parallel.toUpperCase() );
-            invokeSetter( suite, "setParallel", enumClass, parallelEnum );
         }
-
-        String dataProviderThreadCount = options.get( 
"dataproviderthreadcount" );
-        if ( dataProviderThreadCount != null )
+    }
+    
+    @Override
+    protected void configureParallel( XmlSuite suite, Map<String, String> 
options )
+        throws TestSetFailedException
+    {
+        String parallel = options.get( PARALLEL_PROP );
+        // if [parallel] spec'd
+        if ( parallel != null )
         {
-            suite.setDataProviderThreadCount( Integer.parseInt( 
dataProviderThreadCount ) );
+            // try to load the [ParallelMode] enumeration
+            Class enumClass = tryLoadClass( XmlSuite.class.getClassLoader(), 
"org.testng.xml.XmlSuite$ParallelMode" );
+            // if enumeration loaded
+            if ( enumClass != null )
+            {
+                try
+                {
+                    // convert [parallel] option to corresponding constant
+                    Enum<?> parallelEnum = Enum.valueOf( enumClass, 
parallel.toUpperCase() );
+                    // set [XmlSuite] parallel mode to specified value
+                    invokeSetter( suite, "setParallel", enumClass, 
parallelEnum );

Review Comment:
   We are in the right version with this class. We do not expect null 
`enumClass`. The null check `if ( enumClass != null )` is hiding a future bug - 
we would not notice the bug. If we get NPE due to the guys in TestNg team broke 
backwards compatibility again, we would immediately notice it with NPE.



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

To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to