Allow custom thread pool executors to be wired in.

Closes #3066 Following should be done. * Implement `org.testng.IExecutorServiceFactory` * plugin the fully qualified class name of the above implementation via the configuration parameter "-threadpoolfactoryclass" If using Maven surefire plugin then it can be wired in as below: <configuration> <properties> <property> <name>threadpoolfactoryclass</name> <value>test.thread.MyExecutorServiceFactory</value> </property> </properties> </configuration> If using TestNG API, then it can be wired in as below: TestNG testng = new TestNG(); testng.setExecutorServiceFactory(new MyExecutorServiceFactory());
testng-team · Feb 20, 2024 · 1efd809 · 1efd809
1 parent ee22dc0
commit 1efd809
Show file tree

Hide file tree

Showing 19 changed files with 288 additions and 257 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,5 +1,6 @@
 Current (7.10.0)
-New:   GITHUB-2916: Allow users to define ordering for TestNG listeners (Krishnan Mahadevan)
+Fixed: GITHUB-3066: How to dynamically adjust the number of TestNG threads after IExecutorFactory is deprecated? (Krishnan Mahadevan)
+New:   GITHUB-2874: Allow users to define ordering for TestNG listeners (Krishnan Mahadevan)
 Fixed: GITHUB-3033: Moved ant support under own repository https://github.com/testng-team/testng-ant (Julien Herr)
 Fixed: GITHUB-3064: TestResult lost if failure creating RetryAnalyzer (Krishnan Mahadevan)
 Fixed: GITHUB-3048: ConcurrentModificationException when injecting values (Krishnan Mahadevan)

diff --git a/testng-core-api/src/main/java/org/testng/IExecutorServiceFactory.java b/testng-core-api/src/main/java/org/testng/IExecutorServiceFactory.java
@@ -0,0 +1,35 @@
+package org.testng;
+
+import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * Represents the capability to create a custom {@link ExecutorService} by downstream consumers. The
+ * implementation can be plugged in via the configuration parameter <code>-threadpoolfactoryclass
+ * </code>
+ */
+@FunctionalInterface
+public interface IExecutorServiceFactory {
+
+  /**
+   * @param corePoolSize the number of threads to keep in the pool, even if they are idle, unless
+   *     {@code allowCoreThreadTimeOut} is set
+   * @param maximumPoolSize the maximum number of threads to allow in the pool
+   * @param keepAliveTime when the number of threads is greater than the core, this is the maximum
+   *     time that excess idle threads will wait for new tasks before terminating.
+   * @param unit the time unit for the {@code keepAliveTime} argument
+   * @param workQueue the queue to use for holding tasks before they are executed. This queue will
+   *     hold only the {@code Runnable} tasks submitted by the {@code execute} method.
+   * @param threadFactory the factory to use when the executor creates a new thread *
+   * @return - An implementation of {@link ExecutorService}
+   */
+  ExecutorService create(
+      int corePoolSize,
+      int maximumPoolSize,
+      long keepAliveTime,
+      TimeUnit unit,
+      BlockingQueue<Runnable> workQueue,
+      ThreadFactory threadFactory);
+}
diff --git a/testng-core/src/main/java/org/testng/SuiteRunner.java b/testng-core/src/main/java/org/testng/SuiteRunner.java
@@ -439,7 +439,11 @@ private void runInParallelTestMode() {
     }
 
     ThreadUtil.execute(
-        "tests", tasks, xmlSuite.getThreadCount(), xmlSuite.getTimeOut(XmlTest.DEFAULT_TIMEOUT_MS));
+        configuration,
+        "tests",
+        tasks,
+        xmlSuite.getThreadCount(),
+        xmlSuite.getTimeOut(XmlTest.DEFAULT_TIMEOUT_MS));
   }
 
   private class SuiteWorker implements Runnable {

diff --git a/testng-core/src/main/java/org/testng/SuiteTaskExecutor.java b/testng-core/src/main/java/org/testng/SuiteTaskExecutor.java
@@ -2,16 +2,13 @@
 
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import org.testng.internal.IConfiguration;
 import org.testng.internal.RuntimeBehavior;
 import org.testng.internal.Utils;
 import org.testng.internal.thread.TestNGThreadFactory;
 import org.testng.internal.thread.graph.GraphOrchestrator;
 import org.testng.log4testng.Logger;
-import org.testng.thread.IExecutorFactory;
-import org.testng.thread.ITestNGThreadPoolExecutor;
 import org.testng.thread.IThreadWorkerFactory;
 
 class SuiteTaskExecutor {
@@ -42,29 +39,18 @@ public SuiteTaskExecutor(
   public void execute() {
     String name = "suites-";
     if (RuntimeBehavior.favourCustomThreadPoolExecutor()) {
-      IExecutorFactory execFactory = configuration.getExecutorFactory();
-      ITestNGThreadPoolExecutor executor =
-          execFactory.newSuiteExecutor(
-              name,
-              graph,
-              factory,
-              threadPoolSize,
-              threadPoolSize,
-              Integer.MAX_VALUE,
-              TimeUnit.MILLISECONDS,
-              queue,
-              null);
-      executor.run();
-      service = executor;
+      throw new UnsupportedOperationException("This is NO LONGER Supported in TestNG");
     } else {
       service =
-          new ThreadPoolExecutor(
-              threadPoolSize,
-              threadPoolSize,
-              Integer.MAX_VALUE,
-              TimeUnit.MILLISECONDS,
-              queue,
-              new TestNGThreadFactory(name));
+          this.configuration
+              .getExecutorServiceFactory()
+              .create(
+                  threadPoolSize,
+                  threadPoolSize,
+                  Integer.MAX_VALUE,
+                  TimeUnit.MILLISECONDS,
+                  queue,
+                  new TestNGThreadFactory(name));
       GraphOrchestrator<ISuite> executor = new GraphOrchestrator<>(service, factory, graph, null);
       executor.run();
     }

diff --git a/testng-core/src/main/java/org/testng/TestNG.java b/testng-core/src/main/java/org/testng/TestNG.java
@@ -16,6 +16,7 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.ServiceLoader;
 import java.util.Set;
@@ -58,7 +59,6 @@
 import org.testng.reporters.VerboseReporter;
 import org.testng.reporters.XMLReporter;
 import org.testng.reporters.jq.Main;
-import org.testng.thread.IExecutorFactory;
 import org.testng.thread.IThreadWorkerFactory;
 import org.testng.util.Strings;
 import org.testng.xml.IPostProcessor;
@@ -151,8 +151,6 @@ public class TestNG {
   private final Map<Class<? extends IDataProviderInterceptor>, IDataProviderInterceptor>
       m_dataProviderInterceptors = Maps.newLinkedHashMap();
 
-  private IExecutorFactory m_executorFactory = null;
-
   public static final Integer DEFAULT_VERBOSE = 1;
 
   // Command line suite parameters
@@ -843,10 +841,9 @@ public void setVerbose(int verbose) {
     m_verbose = verbose;
   }
 
-  /** This method stands deprecated as of TestNG <code>v7.9.0</code>. */
-  @Deprecated
-  public void setExecutorFactoryClass(String clazzName) {
-    this.m_executorFactory = createExecutorFactoryInstanceUsing(clazzName);
+  public void setExecutorServiceFactory(IExecutorServiceFactory factory) {
+    Objects.requireNonNull(factory);
+    m_configuration.setExecutorServiceFactory(factory);
   }
 
   public void setListenerFactory(ITestNGListenerFactory factory) {
@@ -857,31 +854,6 @@ public void setGenerateResultsPerSuite(boolean generateResultsPerSuite) {
     this.m_generateResultsPerSuite = generateResultsPerSuite;
   }
 
-  private IExecutorFactory createExecutorFactoryInstanceUsing(String clazzName) {
-    Class<?> cls = ClassHelper.forName(clazzName);
-    Object instance = m_objectFactory.newInstance(cls);
-    if (instance instanceof IExecutorFactory) {
-      return (IExecutorFactory) instance;
-    }
-    throw new IllegalArgumentException(
-        clazzName + " does not implement " + IExecutorFactory.class.getName());
-  }
-
-  /** This method stands deprecated as of TestNG <code>v7.9.0</code>. */
-  @Deprecated
-  public void setExecutorFactory(IExecutorFactory factory) {
-    this.m_executorFactory = factory;
-  }
-
-  /** This method stands deprecated as of TestNG <code>v7.9.0</code>. */
-  @Deprecated
-  public IExecutorFactory getExecutorFactory() {
-    if (this.m_executorFactory == null) {
-      this.m_executorFactory = createExecutorFactoryInstanceUsing(DEFAULT_THREADPOOL_FACTORY);
-    }
-    return this.m_executorFactory;
-  }
-
   private void initializeCommandLineSuites() {
     if (m_commandLineTestClasses != null || m_commandLineMethods != null) {
       if (null != m_commandLineMethods) {
@@ -1018,7 +990,6 @@ private void initializeConfiguration() {
     m_configuration.setConfigurable(m_configurable);
     m_configuration.setObjectFactory(factory);
     m_configuration.setAlwaysRunListeners(this.m_alwaysRun);
-    m_configuration.setExecutorFactory(getExecutorFactory());
   }
 
   private void addListeners(XmlSuite s) {
@@ -1514,9 +1485,13 @@ protected void configure(CommandLineArgs cla) {
             m_objectFactory.newInstance((Class<IInjectorFactory>) clazz));
       }
     }
-    if (cla.threadPoolFactoryClass != null) {
-      setExecutorFactoryClass(cla.threadPoolFactoryClass);
-    }
+    Optional.ofNullable(cla.threadPoolFactoryClass)
+        .map(ClassHelper::forName)
+        .filter(IExecutorServiceFactory.class::isAssignableFrom)
+        .map(it -> m_objectFactory.newInstance(it))
+        .map(it -> (IExecutorServiceFactory) it)
+        .ifPresent(this::setExecutorServiceFactory);
+
     setOutputDirectory(cla.outputDirectory);
 
     String testClasses = cla.testClass;

diff --git a/testng-core/src/main/java/org/testng/TestTaskExecutor.java b/testng-core/src/main/java/org/testng/TestTaskExecutor.java
@@ -3,7 +3,6 @@
 import java.util.Comparator;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutorService;
-import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
 import org.testng.internal.IConfiguration;
@@ -13,8 +12,6 @@
 import org.testng.internal.thread.TestNGThreadFactory;
 import org.testng.internal.thread.graph.GraphOrchestrator;
 import org.testng.log4testng.Logger;
-import org.testng.thread.IExecutorFactory;
-import org.testng.thread.ITestNGThreadPoolExecutor;
 import org.testng.thread.IThreadWorkerFactory;
 import org.testng.xml.XmlTest;
 
@@ -51,31 +48,21 @@ public void execute() {
     String name = "test-" + xmlTest.getName();
     int threadCount = Math.max(xmlTest.getThreadCount(), 1);
     if (RuntimeBehavior.favourCustomThreadPoolExecutor()) {
-      IExecutorFactory execFactory = configuration.getExecutorFactory();
-      ITestNGThreadPoolExecutor executor =
-          execFactory.newTestMethodExecutor(
-              name,
-              graph,
-              factory,
-              threadCount,
-              threadCount,
-              0,
-              TimeUnit.MILLISECONDS,
-              queue,
-              comparator);
-      executor.run();
-      service = executor;
+      throw new UnsupportedOperationException("This is NO LONGER Supported in TestNG");
+
     } else {
       boolean reUse = xmlTest.getSuite().useGlobalThreadPool();
       Supplier<Object> supplier =
           () ->
-              new ThreadPoolExecutor(
-                  threadCount,
-                  threadCount,
-                  0,
-                  TimeUnit.MILLISECONDS,
-                  queue,
-                  new TestNGThreadFactory(name));
+              configuration
+                  .getExecutorServiceFactory()
+                  .create(
+                      threadCount,
+                      threadCount,
+                      0,
+                      TimeUnit.MILLISECONDS,
+                      queue,
+                      new TestNGThreadFactory(name));
       if (reUse) {
         ObjectBag bag = ObjectBag.getInstance(xmlTest.getSuite());
         service = (ExecutorService) bag.createIfRequired(ExecutorService.class, supplier);

diff --git a/testng-core/src/main/java/org/testng/internal/Configuration.java b/testng-core/src/main/java/org/testng/internal/Configuration.java
@@ -2,9 +2,12 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ThreadPoolExecutor;
 import org.testng.IConfigurable;
 import org.testng.IConfigurationListener;
 import org.testng.IExecutionListener;
+import org.testng.IExecutorServiceFactory;
 import org.testng.IHookable;
 import org.testng.IInjectorFactory;
 import org.testng.ITestNGListenerFactory;
@@ -16,8 +19,6 @@
 import org.testng.internal.annotations.IAnnotationFinder;
 import org.testng.internal.annotations.JDK15AnnotationFinder;
 import org.testng.internal.objects.GuiceBackedInjectorFactory;
-import org.testng.internal.thread.DefaultThreadPoolExecutorFactory;
-import org.testng.thread.IExecutorFactory;
 
 public class Configuration implements IConfiguration {
 
@@ -34,7 +35,7 @@ public class Configuration implements IConfiguration {
   private final Map<Class<? extends IConfigurationListener>, IConfigurationListener>
       m_configurationListeners = Maps.newLinkedHashMap();
   private boolean alwaysRunListeners = true;
-  private IExecutorFactory m_executorFactory = new DefaultThreadPoolExecutorFactory();
+  private IExecutorServiceFactory executorServiceFactory = ThreadPoolExecutor::new;
 
   private IInjectorFactory injectorFactory = new GuiceBackedInjectorFactory();
 
@@ -145,13 +146,13 @@ public void setAlwaysRunListeners(boolean alwaysRunListeners) {
   }
 
   @Override
-  public void setExecutorFactory(IExecutorFactory factory) {
-    this.m_executorFactory = factory;
+  public void setExecutorServiceFactory(IExecutorServiceFactory executorServiceFactory) {
+    this.executorServiceFactory = Objects.requireNonNull(executorServiceFactory);
   }
 
   @Override
-  public IExecutorFactory getExecutorFactory() {
-    return this.m_executorFactory;
+  public IExecutorServiceFactory getExecutorServiceFactory() {
+    return executorServiceFactory;
   }
 
   @Override

diff --git a/testng-core/src/main/java/org/testng/internal/IConfiguration.java b/testng-core/src/main/java/org/testng/internal/IConfiguration.java
@@ -3,7 +3,6 @@
 import java.util.List;
 import org.testng.*;
 import org.testng.internal.annotations.IAnnotationFinder;
-import org.testng.thread.IExecutorFactory;
 
 public interface IConfiguration {
   IAnnotationFinder getAnnotationFinder();
@@ -46,11 +45,11 @@ default boolean addExecutionListenerIfAbsent(IExecutionListener l) {
 
   void setAlwaysRunListeners(boolean alwaysRun);
 
-  void setExecutorFactory(IExecutorFactory factory);
+  IInjectorFactory getInjectorFactory();
 
-  IExecutorFactory getExecutorFactory();
+  IExecutorServiceFactory getExecutorServiceFactory();
 
-  IInjectorFactory getInjectorFactory();
+  void setExecutorServiceFactory(IExecutorServiceFactory factory);
 
   void setInjectorFactory(IInjectorFactory factory);
 

diff --git a/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/ConfigInvoker.java
@@ -397,7 +397,7 @@ private void invokeConfigurationMethod(
                 targetInstance, params, configurableInstance, method.getMethod(), testResult);
       } else {
         MethodInvocationHelper.invokeMethodConsideringTimeout(
-            tm, method, targetInstance, params, testResult);
+            tm, method, targetInstance, params, testResult, m_configuration);
       }
       boolean testStatusRemainedUnchanged = testResult.isNotRunning();
       boolean throwException = !RuntimeBehavior.ignoreCallbackInvocationSkips();