[SUREFIRE-1967] Fix parallel test ordering to prevent high resource c…

…onsumption

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.

surefire-its/src/test/java/org/apache/maven/surefire/its/jiras/Surefire1967CheckTestNgMethodParallelOrderingIT.java

@@ -0,0 +1,41 @@
+package org.apache.maven.surefire.its.jiras;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.apache.maven.surefire.its.fixture.SurefireJUnit4IntegrationTestCase;
+import org.junit.Test;
+
+/**
+ * Test TestNG setup and teardown ordering with parallelism
+ *
+ * @author findepi
+ */
+public class Surefire1967CheckTestNgMethodParallelOrderingIT
+    extends SurefireJUnit4IntegrationTestCase
+{
+    @Test
+    public void testNgParallelOrdering()
+    {
+        unpack( "surefire-1967-testng-method-parallel-ordering" )
+                .sysProp( "testNgVersion", "7.3.0" )
+                .executeTest()
+                .verifyErrorFree( 12 );
+    }
+}

surefire-its/src/test/resources/surefire-1967-testng-method-parallel-ordering/pom.xml

@@ -0,0 +1,82 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Licensed to the Apache Software Foundation (ASF) under one
+  ~ or more contributor license agreements.  See the NOTICE file
+  ~ distributed with this work for additional information
+  ~ regarding copyright ownership.  The ASF licenses this file
+  ~ to you under the Apache License, Version 2.0 (the
+  ~ "License"); you may not use this file except in compliance
+  ~ with the License.  You may obtain a copy of the License at
+  ~
+  ~     http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing,
+  ~ software distributed under the License is distributed on an
+  ~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  ~ KIND, either express or implied.  See the License for the
+  ~ specific language governing permissions and limitations
+  ~ under the License.
+  -->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+
+  <parent>
+    <groupId>org.apache.maven.surefire</groupId>
+    <artifactId>it-parent</artifactId>
+    <version>1.0</version>
+    <relativePath>../pom.xml</relativePath>
+  </parent>
+
+  <groupId>org.apache.maven.plugins.surefire</groupId>
+  <artifactId>surefire-1967-testng-method-parallel-ordering</artifactId>
+  <version>1.0-SNAPSHOT</version>
+  <name>TestNG parallel ordering</name>
+
+  <profiles>
+    <profile>
+      <id>testng-old</id>
+      <activation>
+        <property><name>testNgClassifier</name></property>
+      </activation>
+      <dependencies>
+        <dependency>
+          <groupId>org.testng</groupId>
+          <artifactId>testng</artifactId>
+          <version>${testNgVersion}</version>
+          <classifier>${testNgClassifier}</classifier>
+        </dependency>
+      </dependencies>
+    </profile>
+    <profile>
+      <id>testng-new</id>
+      <activation>
+        <property><name>!testNgClassifier</name></property>
+      </activation>
+      <dependencies>
+        <dependency>
+          <groupId>org.testng</groupId>
+          <artifactId>testng</artifactId>
+          <version>${testNgVersion}</version>
+        </dependency>
+      </dependencies>
+    </profile>
+  </profiles>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-surefire-plugin</artifactId>
+        <version>${surefire.version}</version>
+        <configuration>
+          <threadCount>2</threadCount>
+          <parallel>methods</parallel>
+        </configuration>
+      </plugin>
+    </plugins>
+  </build>
+
+</project>

surefire-its/src/test/resources/surefire-1967-testng-method-parallel-ordering/src/test/java/testng/parallelOrdering/Base.java

@@ -0,0 +1,59 @@
+package testng.parallelOrdering;
+
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import java.util.concurrent.ThreadLocalRandom;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public abstract class Base
+{
+    private static final AtomicInteger resources = new AtomicInteger();
+
+    // This simulates resource allocation
+    @BeforeClass
+    public void setupAllocateResources()
+    {
+        int concurrentResources = resources.incrementAndGet();
+        if (concurrentResources > 2) {
+            throw new IllegalStateException("Tests execute in two threads, so there should be at most 2 resources allocated, got: " + concurrentResources);
+        }
+    }
+
+    // This simulates freeing resources
+    @AfterClass(alwaysRun = true)
+    public void tearDownReleaseResources()
+    {
+        resources.decrementAndGet();
+    }
+
+    @Test
+    public void test1()
+    {
+        sleepShortly();
+    }
+
+    @Test
+    public void test2()
+    {
+        sleepShortly();
+    }
+
+    @Test
+    public void test3()
+    {
+        sleepShortly();
+    }
+
+    // Sleep random time to let tests interleave. Keep sleep short not to extend tests duration too much.
+    private void sleepShortly()
+    {
+        try {
+            Thread.sleep(ThreadLocalRandom.current().nextInt(3));
+        }
+        catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+    }
+}

surefire-its/src/test/resources/surefire-1967-testng-method-parallel-ordering/src/test/java/testng/parallelOrdering/TestClass1.java

@@ -0,0 +1,3 @@
+package testng.parallelOrdering;
+
+public class TestClass1 extends Base {}

surefire-its/src/test/resources/surefire-1967-testng-method-parallel-ordering/src/test/java/testng/parallelOrdering/TestClass2.java

@@ -0,0 +1,3 @@
+package testng.parallelOrdering;
+
+public class TestClass2 extends Base {}

surefire-its/src/test/resources/surefire-1967-testng-method-parallel-ordering/src/test/java/testng/parallelOrdering/TestClass3.java

@@ -0,0 +1,3 @@
+package testng.parallelOrdering;
+
+public class TestClass3 extends Base {}

surefire-its/src/test/resources/surefire-1967-testng-method-parallel-ordering/src/test/java/testng/parallelOrdering/TestClass4.java

@@ -0,0 +1,3 @@
+package testng.parallelOrdering;
+
+public class TestClass4 extends Base {}

surefire-providers/surefire-testng/src/main/java/org/apache/maven/surefire/testng/TestNGExecutor.java

@@ -72,11 +72,28 @@ 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.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.
+    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 +144,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 +154,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 );
+            }
+        }
+        xmlClasses.add( xmlClass );
+    }
+
     private static boolean isCliDebugOrShowErrors( List<CommandLineOption> mainCliOptions )
     {
         return mainCliOptions.contains( LOGGING_LEVEL_DEBUG ) || mainCliOptions.contains( SHOW_ERRORS );