Fixing review comments

testng-team · Feb 21, 2024 · 1e943fc · 1e943fc
1 parent b78ef23
commit 1e943fc
Show file tree

Hide file tree

Showing 6 changed files with 33 additions and 92 deletions.
diff --git a/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java b/testng-core-api/src/main/java/org/testng/internal/RuntimeBehavior.java
@@ -29,15 +29,6 @@ public static boolean ignoreCallbackInvocationSkips() {
     return Boolean.getBoolean(IGNORE_CALLBACK_INVOCATION_SKIPS);
   }
 
-  /**
-   * @return - <code>true</code> if TestNG is to be using its custom implementation of {@link
-   *     java.util.concurrent.ThreadPoolExecutor} for running concurrent tests. Defaults to <code>
-   *     false</code>
-   */
-  public static boolean favourCustomThreadPoolExecutor() {
-    return Boolean.getBoolean(FAVOR_CUSTOM_THREAD_POOL_EXECUTOR);
-  }
-
   /**
    * @return - A comma separated list of packages that represent special listeners which users will
    *     expect to be executed after executing the regular listeners. Here special listeners can be

diff --git a/testng-core/src/main/java/org/testng/SuiteTaskExecutor.java b/testng-core/src/main/java/org/testng/SuiteTaskExecutor.java
@@ -4,7 +4,6 @@
 import java.util.concurrent.ExecutorService;
 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;
@@ -38,22 +37,18 @@ public SuiteTaskExecutor(
 
   public void execute() {
     String name = "suites-";
-    if (RuntimeBehavior.favourCustomThreadPoolExecutor()) {
-      throw new UnsupportedOperationException("This is NO LONGER Supported in TestNG");
-    } else {
-      service =
-          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();
-    }
+    service =
+        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();
   }
 
   public void awaitCompletion() {

diff --git a/testng-core/src/main/java/org/testng/TestNG.java b/testng-core/src/main/java/org/testng/TestNG.java
@@ -842,8 +842,8 @@ public void setVerbose(int verbose) {
   }
 
   public void setExecutorServiceFactory(IExecutorServiceFactory factory) {
-    Objects.requireNonNull(factory);
-    m_configuration.setExecutorServiceFactory(factory);
+    m_configuration.setExecutorServiceFactory(
+        Objects.requireNonNull(factory, "ExecutorServiceFactory cannot be null"));
   }
 
   public void setListenerFactory(ITestNGListenerFactory factory) {
@@ -1188,11 +1188,9 @@ public List<ISuite> runSuitesLocally() {
     // Create a map with XmlSuite as key and corresponding SuiteRunner as value
     for (XmlSuite xmlSuite : m_suites) {
       if (m_configuration.isShareThreadPoolForDataProviders()) {
-        abortIfUsingGraphThreadPoolExecutor("Shared thread-pool for data providers");
         xmlSuite.setShareThreadPoolForDataProviders(true);
       }
       if (m_configuration.useGlobalThreadPool()) {
-        abortIfUsingGraphThreadPoolExecutor("Global thread-pool");
         xmlSuite.shouldUseGlobalThreadPool(true);
       }
       createSuiteRunners(suiteRunnerMap, xmlSuite);
@@ -1243,13 +1241,6 @@ private static void error(String s) {
     LOGGER.error(s);
   }
 
-  private static void abortIfUsingGraphThreadPoolExecutor(String prefix) {
-    if (RuntimeBehavior.favourCustomThreadPoolExecutor()) {
-      throw new UnsupportedOperationException(
-          prefix + " is NOT COMPATIBLE with TestNG's custom thread pool executor");
-    }
-  }
-
   /**
    * @return the verbose level, checking in order: the verbose level on the suite, the verbose level
    *     on the TestNG object, or 1.

diff --git a/testng-core/src/main/java/org/testng/TestTaskExecutor.java b/testng-core/src/main/java/org/testng/TestTaskExecutor.java
@@ -7,7 +7,6 @@
 import java.util.function.Supplier;
 import org.testng.internal.IConfiguration;
 import org.testng.internal.ObjectBag;
-import org.testng.internal.RuntimeBehavior;
 import org.testng.internal.Utils;
 import org.testng.internal.thread.TestNGThreadFactory;
 import org.testng.internal.thread.graph.GraphOrchestrator;
@@ -47,32 +46,27 @@ public TestTaskExecutor(
   public void execute() {
     String name = "test-" + xmlTest.getName();
     int threadCount = Math.max(xmlTest.getThreadCount(), 1);
-    if (RuntimeBehavior.favourCustomThreadPoolExecutor()) {
-      throw new UnsupportedOperationException("This is NO LONGER Supported in TestNG");
-
+    boolean reUse = xmlTest.getSuite().useGlobalThreadPool();
+    Supplier<Object> supplier =
+        () ->
+            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);
     } else {
-      boolean reUse = xmlTest.getSuite().useGlobalThreadPool();
-      Supplier<Object> supplier =
-          () ->
-              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);
-      } else {
-        service = (ExecutorService) supplier.get();
-      }
-      GraphOrchestrator<ITestNGMethod> executor =
-          new GraphOrchestrator<>(service, factory, graph, comparator);
-      executor.run();
+      service = (ExecutorService) supplier.get();
     }
+    GraphOrchestrator<ITestNGMethod> executor =
+        new GraphOrchestrator<>(service, factory, graph, comparator);
+    executor.run();
   }
 
   public void awaitCompletion() {

diff --git a/testng-core/src/test/java/test/dataprovider/DataProviderTest.java b/testng-core/src/test/java/test/dataprovider/DataProviderTest.java
@@ -18,7 +18,6 @@
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import org.testng.collections.Lists;
-import org.testng.internal.RuntimeBehavior;
 import org.testng.internal.collections.Pair;
 import org.testng.internal.reflect.MethodMatcherException;
 import org.testng.xml.XmlClass;
@@ -615,20 +614,6 @@ public void ensureSequentialDataProviderWithRetryAnalyserWorks() {
     runTest(false);
   }
 
-  @Test(
-      description = "GITHUB-2980",
-      expectedExceptions = UnsupportedOperationException.class,
-      expectedExceptionsMessageRegExp =
-          "Shared thread-pool for data providers is NOT COMPATIBLE with TestNG's custom thread pool executor")
-  public void ensureErrorShownWhenUsedWithGraphThreadPoolExecutor() {
-    try {
-      System.setProperty(RuntimeBehavior.FAVOR_CUSTOM_THREAD_POOL_EXECUTOR, "true");
-      runDataProviderTest(true);
-    } finally {
-      System.setProperty(RuntimeBehavior.FAVOR_CUSTOM_THREAD_POOL_EXECUTOR, "false");
-    }
-  }
-
   @Test(description = "GITHUB-2980", dataProvider = "dataProviderForIssue2980")
   public void ensureWeCanShareThreadPoolForDataProviders(
       boolean flag, Pair<List<String>, Integer> pair) {

diff --git a/testng-core/src/test/java/test/thread/SharedThreadPoolTest.java b/testng-core/src/test/java/test/thread/SharedThreadPoolTest.java
@@ -9,7 +9,6 @@
 import org.testng.TestNG;
 import org.testng.annotations.AfterMethod;
 import org.testng.annotations.Test;
-import org.testng.internal.RuntimeBehavior;
 import org.testng.xml.XmlSuite;
 import test.SimpleBaseTest;
 import test.thread.issue2019.TestClassSample;
@@ -38,20 +37,6 @@ public void ensureCommonThreadPoolIsNotUsed() {
         .hasSizeGreaterThanOrEqualTo(3);
   }
 
-  @Test(
-      description = "GITHUB-2019",
-      expectedExceptions = UnsupportedOperationException.class,
-      expectedExceptionsMessageRegExp =
-          "Global thread-pool is NOT COMPATIBLE with TestNG's custom thread pool executor")
-  public void ensureErrorShownWhenUsedWithGraphThreadPoolExecutor() {
-    try {
-      System.setProperty(RuntimeBehavior.FAVOR_CUSTOM_THREAD_POOL_EXECUTOR, "true");
-      runSimpleTest(true);
-    } finally {
-      System.setProperty(RuntimeBehavior.FAVOR_CUSTOM_THREAD_POOL_EXECUTOR, "false");
-    }
-  }
-
   private static List<Long> runSimpleTest(boolean useSharedGlobalThreadPool) {
     TestNG testng = create(TestClassSample.class);
     testng.shouldUseGlobalThreadPool(useSharedGlobalThreadPool);