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


##########
surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/conf/TestNG740Configurator.java:
##########
@@ -37,33 +35,45 @@
  *
  * @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 )
         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 )
+        // if [threadcount] option in unspecified
+        if ( !options.containsKey( THREADCOUNT_PROP ) )
         {
-            if ( !"methods".equalsIgnoreCase( parallel ) && 
!"classes".equalsIgnoreCase( parallel ) )
-            {
-                throw new TestSetFailedException( "Unsupported TestNG parallel 
setting: "
-                    + parallel + " ( only METHODS or CLASSES supported )" );
-            }
-            Class enumClass = tryLoadClass( XmlSuite.class.getClassLoader(), 
"org.testng.xml.XmlSuite$ParallelMode" );
-            Enum<?> parallelEnum = Enum.valueOf( enumClass, 
parallel.toUpperCase() );
-            invokeSetter( suite, "setParallel", enumClass, parallelEnum );
+            // acquire default [threadcount] value to avoid superclass 
hardcoding to 1
+            options.put( THREADCOUNT_PROP, Integer.toString( 
suite.getThreadCount() ) );
         }
 
-        String dataProviderThreadCount = options.get( 
"dataproviderthreadcount" );
-        if ( dataProviderThreadCount != null )
+        // if [parallel] option is specified
+        if ( options.containsKey( PARALLEL_PROP ) )
         {
-            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 )
+            {
+                // extract [parallel] option
+                String parallel = options.remove( PARALLEL_PROP );
+                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 );
+                }
+                catch ( IllegalArgumentException e )
+                {
+                    throw new TestSetFailedException( "Unsupported TestNG 
parallel setting: " + parallel, e );
+                }
+            }
         }
+        
+        // invoke superclass handler
+        super.configure( suite, options );

Review Comment:
   I could do as you suggest and extract the logic to `setThreadCount` and 
setParallel` methods. My current approach was intended to limit the scope of 
revisions to this single class. Would you prefer that I refactor the 
implementation? This would reduce the complexity a bit while expanding the 
scope of this PR.



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