findepi commented on a change in pull request #407:
URL: https://github.com/apache/maven-surefire/pull/407#discussion_r783424283



##########
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:
       @Tibor17 it's possible to select and invoke one of the constructors, but 
i don't think the added complexity is worth the effort (i am mostly concerned 
about future code readability and maintainability). Note that after all, we're 
solving a problem that exists in TestNG >= 6.3, and TestNG 5.x is not going to 
change.  The m_index field is used in TestNG 5.x  when `preserve-order` is set, 
but it's not possible to set it when invoking tests via surefire. More 
precisely, `preserve-order` seems to require a suite file, but then TestNG will 
populate the m_index, not us.
   




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