From 220af5f6f74f056298614ba376a35259f6bdc985 Mon Sep 17 00:00:00 2001
From: Piotr Findeisen <piotr.findeisen@gmail.com>
Date: Tue, 14 Dec 2021 17:42:21 +0100
Subject: [PATCH] [SUREFIRE-1967] Fix parallel test ordering to prevent high
 resource consumption

Before the change, TestNG run from Surefire can execute `@BeforeClass`
on many different test classes/instances, without invoking `@AfterClass`
yet, leading to high resource utilization. This is not observed when
tests are invoked via a suite file. This is because `XmlClass.m_index`
field is used by TestNG to order test execution and this field used not
to be set by Surefire. This commit lets Surefire initialize `XmlClass`
object in a similar manner to how TestNG suite file XML parser does.
---
 .../maven/surefire/testng/TestNGExecutor.java | 43 ++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java b/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java
index 56fc9d5987..3667d90730 100644
--- a/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java
+++ b/surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java
@@ -72,11 +72,26 @@ final class TestNGExecutor
     private static final boolean HAS_TEST_ANNOTATION_ON_CLASSPATH =
             tryLoadClass( TestNGExecutor.class.getClassLoader(), "org.testng.annotations.Test" ) != null;
 
+    // Using reflection because XmlClass.m_index is not available in older versions of TestNG
+    private static final Method XML_CLASS_SET_INDEX = findXmlClassSetIndexMethod();
+
     private TestNGExecutor()
     {
         throw new IllegalStateException( "not instantiable constructor" );
     }
 
+    private static Method findXmlClassSetIndexMethod()
+    {
+        try
+        {
+            return XmlClass.class.getMethod( "setIndex", int.class );
+        }
+        catch ( NoSuchMethodException e )
+        {
+            return null;
+        }
+    }
+
     @SuppressWarnings( "checkstyle:parameternumbercheck" )
     static void run( Iterable<Class<?>> testClasses, String testSourceDirectory,
                             Map<String, String> options, // string,string because TestNGMapConfigurator#configure()
@@ -127,7 +142,7 @@ static void run( Iterable<Class<?>> testClasses, String testSourceDirectory,
                 suiteAndNamedTests.testNameToTest.put( metadata.testName, xmlTest );
             }
 
-            xmlTest.getXmlClasses().add( new XmlClass( testClass.getName() ) );
+            addXmlClass( xmlTest.getXmlClasses(), testClass.getName() );
         }
 
         testng.setXmlSuites( xmlSuites );
@@ -137,6 +152,32 @@ static void run( Iterable<Class<?>> testClasses, String testSourceDirectory,
         testng.run();
     }
 
+    private static void addXmlClass( List<XmlClass> xmlClasses, String testClassName )
+    {
+        int index = xmlClasses.size();
+        XmlClass xmlClass = new XmlClass( testClassName );
+        xmlClasses.add( xmlClass );
+        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, index );
+            }
+            catch ( ReflectiveOperationException e )
+            {
+                throw new RuntimeException( e );
+            }
+        }
+    }
+
     private static boolean isCliDebugOrShowErrors( List<CommandLineOption> mainCliOptions )
     {
         return mainCliOptions.contains( LOGGING_LEVEL_DEBUG ) || mainCliOptions.contains( SHOW_ERRORS );