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