Streamline working of shared thread pools

CHANGES.txt

@@ -1,5 +1,6 @@
 Current (7.12.0)
-Fixed: GITHUB-2765: Test timeouts using existing Executor now propagate the stack trace to the ThreadTimeoutException
+Fixed: GITHUB-3179: ClassCastException when use shouldUseGlobalThreadPool(true) property (Krishnan Mahadevan)
+Fixed: GITHUB-2765: Test timeouts using existing Executor now propagate the stack trace to the ThreadTimeoutException (Charlie Hayes)
 
 7.11.0
 Fixed: GITHUB-3180: TestNG testng-failed.xml 'invocation-numbers' values are not calculated correctly with retry and dataproviders (Krishnan Mahadevan)

testng-core/src/main/java/org/testng/TestRunner.java

@@ -662,11 +662,6 @@ private static Comparator<ITestNGMethod> newComparator(boolean needPrioritySort)
     return needPrioritySort ? new TestMethodComparator() : null;
   }
 
-  private boolean sortOnPriority(ITestNGMethod[] interceptedOrder) {
-    return m_methodInterceptors.size() > 1
-        || Arrays.stream(interceptedOrder).anyMatch(m -> m.getPriority() != 0);
-  }
-
   // If any of the test methods specify a priority other than the default, we'll need to be able to
   // sort them.
   private static BlockingQueue<Runnable> newQueue(boolean needPrioritySort) {
@@ -710,15 +705,23 @@ private void privateRun(XmlTest xmlTest) {
     // In some cases, additional sorting is needed to make sure tests run in the appropriate order.
     // If the user specified a method interceptor, or if we have any methods that have a non-default
     // priority on them, we need to sort.
-    boolean needPrioritySort = sortOnPriority(interceptedOrder);
+    boolean hasMultipleInterceptors = m_methodInterceptors.size() > 1;
+    boolean hasNonZeroPriorityMethods =
+        Arrays.stream(interceptedOrder).anyMatch(m -> m.getPriority() != 0);
+    boolean needPrioritySort = hasMultipleInterceptors || hasNonZeroPriorityMethods;
     Comparator<ITestNGMethod> methodComparator = newComparator(needPrioritySort);
     if (parallel) {
       if (graph.getNodeCount() <= 0) {
         return;
       }
       TestTaskExecutor taskExecutor =
           new TestTaskExecutor(
-              m_configuration, xmlTest, this, newQueue(needPrioritySort), graph, methodComparator);
+              m_configuration,
+              xmlTest,
+              this,
+              newQueue(hasNonZeroPriorityMethods),
+              graph,
+              methodComparator);
       taskExecutor.execute();
       taskExecutor.awaitCompletion();
       return;

testng-core/src/test/java/test/thread/SharedThreadPoolTest.java

@@ -17,6 +17,8 @@
 import test.thread.issue3028.AnotherDataDrivenTestSample;
 import test.thread.issue3028.DataDrivenTestSample;
 import test.thread.issue3028.FactoryPoweredDataDrivenTestSample;
+import test.thread.issue3179.DummyMethodInterceptor;
+import test.thread.issue3179.SampleSuiteAlteringListener;
 
 public class SharedThreadPoolTest extends SimpleBaseTest {
 
@@ -83,6 +85,15 @@ public void ensureDeadLocksAreDetectedForDataDrivenTestsRunningInParallel(
     testng.run();
   }
 
+  @Test(description = "GITHUB-3179")
+  public void ensurePriorityAgnosticInterceptorsDontCauseExceptions() {
+    TestNG testng = create(test.thread.issue3179.TestClassSample.class);
+    testng.addListener(new SampleSuiteAlteringListener());
+    testng.addListener(new DummyMethodInterceptor());
+    testng.run();
+    assertThat(testng.getStatus()).isZero();
+  }
+
   @DataProvider(name = "modes")
   public Object[][] parallelModes() {
     return new Object[][] {

testng-core/src/test/java/test/thread/issue3179/DummyMethodInterceptor.java

@@ -0,0 +1,13 @@
+package test.thread.issue3179;
+
+import java.util.List;
+import org.testng.IMethodInstance;
+import org.testng.IMethodInterceptor;
+import org.testng.ITestContext;
+
+public class DummyMethodInterceptor implements IMethodInterceptor {
+  @Override
+  public List<IMethodInstance> intercept(List<IMethodInstance> methods, ITestContext context) {
+    return methods;
+  }
+}

testng-core/src/test/java/test/thread/issue3179/SampleSuiteAlteringListener.java

@@ -0,0 +1,14 @@
+package test.thread.issue3179;
+
+import java.util.List;
+import org.testng.IAlterSuiteListener;
+import org.testng.xml.XmlSuite;
+
+public class SampleSuiteAlteringListener implements IAlterSuiteListener {
+  @Override
+  public void alter(List<XmlSuite> suites) {
+    suites.get(0).shouldUseGlobalThreadPool(true);
+    suites.get(0).setThreadCount(3);
+    suites.get(0).setParallel(XmlSuite.ParallelMode.METHODS);
+  }
+}

testng-core/src/test/java/test/thread/issue3179/TestClassSample.java

@@ -0,0 +1,23 @@
+package test.thread.issue3179;
+
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+public class TestClassSample {
+  @DataProvider(parallel = true)
+  public Object[] dp() {
+    return new Object[] {1};
+  }
+
+  @Test(dataProvider = "dp")
+  public void test1(int dp) {}
+
+  @Test(dataProvider = "dp")
+  public void test3(int dp) {}
+
+  @Test
+  public void test2() {}
+
+  @Test
+  public void test4() {}
+}