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


##########
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:
   @slawekjaranowski: The class header JavaDoc specifies the TestNG version 
(`7.4.0`). I suppose this comment could be a bit more explicit, however, 
regarding the necessity for this configurator class.
   @Tibor17: The inability to load the enumeration will trigger a 
clearly-explained failure: `Failed loading TestNG [ParallelMode] enumeration to 
convert [parallel] setting`. No need to crack open the code to determine the 
cause of the 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