sbabcoc commented on code in PR #517: URL: https://github.com/apache/maven-surefire/pull/517#discussion_r846668357
########## 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: Look closer... The value of the [parallel] option is being **removed** from the map. I'm processing the option here, and the removal of the option avoids double-processing (with the resultant failure) in the superclass implementation. As I pointed out in my comments, skipping the invocation of the superclass implementation has the result that all other specified options get ignored. -- 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