findepi commented on a change in pull request #407:
URL: https://github.com/apache/maven-surefire/pull/407#discussion_r783434179
##########
File path:
surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java
##########
@@ -137,6 +152,31 @@ static void run( Iterable<Class<?>> testClasses, String
testSourceDirectory,
testng.run();
}
+ private static void addXmlClass( List<XmlClass> xmlClasses, String
testClassName )
+ {
+ XmlClass xmlClass = new XmlClass( testClassName );
+ if ( XML_CLASS_SET_INDEX != null )
+ {
+ try
+ {
+ // In case of parallel test execution with parallel="methods",
TestNG orders test execution
+ // by XmlClass.m_index field. When unset (equal for all
XmlClass instances), TestNG can
+ // invoke `@BeforeClass` setup methods on many instances,
without invoking `@AfterClass`
+ // tearDown methods, thus leading to high resource
consumptions when test instances
+ // allocate resources.
+ // With index set, order of setup, test and tearDown methods
is reasonable, with approximately
+ // #thread-count many test classes being initialized at given
point in time.
+ // Note: XmlClass.m_index field is set automatically by TestNG
when it reads a suite file.
+ XML_CLASS_SET_INDEX.invoke( xmlClass, xmlClasses.size() );
+ }
+ catch ( ReflectiveOperationException e )
+ {
+ throw new RuntimeException( e );
+ }
+ }
Review comment:
> Pls add a line with a comment regarding TestNG 5.10. It was very good
point.
You mean this one:
_The code is compiled against TestNG 5.10 and that version doesn't have the
`index` field in `XmlClass`, hence use of reflection._ ?
I wouldn't want to put exact TestNG 5.10 version in the source, as it's
subject to change, but i added explanatory comment above XML_CLASS_SET_INDEX
constant:
```
// Using reflection because XmlClass.setIndex is available since TestNG 6.3
// XmlClass.m_index field is available since TestNG 5.13, but prior to 6.3
required invoking constructor
// and constructor XmlClass constructor signatures evolved over time.
```
i tried to make it convey same meaning: that we have to use the reflection.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
