From 503ec1bb74a908863e6bdb1dd63f81a83537a7f3 Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Tue, 28 Nov 2023 20:56:58 +0530 Subject: [PATCH] Shared Threadpool for normal/datadriven tests. Closes #2019 --- CHANGES.txt | 1 + .../main/java/org/testng/xml/XmlSuite.java | 10 +++ .../main/java/org/testng/CommandLineArgs.java | 8 ++ .../src/main/java/org/testng/TestNG.java | 12 +++ .../java/org/testng/TestTaskExecutor.java | 29 ++++--- .../org/testng/internal/Configuration.java | 12 +++ .../org/testng/internal/IConfiguration.java | 4 + .../java/org/testng/internal/ObjectBag.java | 7 +- .../internal/invokers/MethodRunner.java | 26 +++--- .../org/testng/xml/TestNGContentHandler.java | 7 ++ testng-core/src/main/resources/testng-1.0.dtd | 3 - testng-core/src/main/resources/testng-1.1.dtd | 24 +++--- .../test/thread/SharedThreadPoolTest.java | 79 +++++++++++++++++++ .../thread/issue2019/TestClassSample.java | 34 ++++++++ .../2019_global_thread_pool_disabled.xml | 9 +++ .../2019_global_thread_pool_enabled.xml | 9 +++ .../2980_with_shared_threadpool_disabled.xml | 2 +- .../2980_with_shared_threadpool_enabled.xml | 2 +- testng-core/src/test/resources/testng.xml | 1 + 19 files changed, 243 insertions(+), 36 deletions(-) create mode 100644 testng-core/src/test/java/test/thread/SharedThreadPoolTest.java create mode 100644 testng-core/src/test/java/test/thread/issue2019/TestClassSample.java create mode 100644 testng-core/src/test/resources/2019_global_thread_pool_disabled.xml create mode 100644 testng-core/src/test/resources/2019_global_thread_pool_enabled.xml diff --git a/CHANGES.txt b/CHANGES.txt index 2214c1e2ca..9b2c930330 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-2019: Total thread count in testng parallel tests with dataproviders (Krishnan Mahadevan) Fixed: GITHUB-3006: ITestResult injected at @AfterMethod incorrect when a configuration method failed (Krishnan Mahadevan) Fixed: GITHUB-2980: Data Provider Threads configuration in the suite don't match the documentation (Krishnan Mahadevan) Fixed: GITHUB-3003: BeforeClass|AfterClass with inheritedGroups triggers cyclic dependencies (Krishnan Mahadevan) diff --git a/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java b/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java index 5c9f8864ac..4787323365 100644 --- a/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java +++ b/testng-core-api/src/main/java/org/testng/xml/XmlSuite.java @@ -145,6 +145,8 @@ public String toString() { private boolean shareThreadPoolForDataProviders = false; + private boolean useGlobalThreadPool = false; + /** The thread count. */ public static final Integer DEFAULT_THREAD_COUNT = 5; @@ -253,6 +255,14 @@ public void setShareThreadPoolForDataProviders(boolean shareThreadPoolForDataPro this.shareThreadPoolForDataProviders = shareThreadPoolForDataProviders; } + public boolean useGlobalThreadPool() { + return this.useGlobalThreadPool; + } + + public void shouldUseGlobalThreadPool(boolean flag) { + this.useGlobalThreadPool = flag; + } + public boolean isShareThreadPoolForDataProviders() { return shareThreadPoolForDataProviders; } diff --git a/testng-core/src/main/java/org/testng/CommandLineArgs.java b/testng-core/src/main/java/org/testng/CommandLineArgs.java index 1c1895a987..67d6386636 100644 --- a/testng-core/src/main/java/org/testng/CommandLineArgs.java +++ b/testng-core/src/main/java/org/testng/CommandLineArgs.java @@ -283,4 +283,12 @@ public class CommandLineArgs { description = "Should TestNG use a global Shared ThreadPool (At suite level) for running data providers.") public Boolean shareThreadPoolForDataProviders = false; + + public static final String USE_GLOBAL_THREAD_POOL = "-useGlobalThreadPool"; + + @Parameter( + names = USE_GLOBAL_THREAD_POOL, + description = + "Should TestNG use a global Shared ThreadPool (At suite level) for running regular and data driven tests.") + public Boolean useGlobalThreadPool = false; } diff --git a/testng-core/src/main/java/org/testng/TestNG.java b/testng-core/src/main/java/org/testng/TestNG.java index 4e69fa9f81..f91fa68653 100644 --- a/testng-core/src/main/java/org/testng/TestNG.java +++ b/testng-core/src/main/java/org/testng/TestNG.java @@ -616,6 +616,14 @@ public boolean isShareThreadPoolForDataProviders() { return this.m_configuration.isShareThreadPoolForDataProviders(); } + public boolean useGlobalThreadPool() { + return this.m_configuration.useGlobalThreadPool(); + } + + public void shouldUseGlobalThreadPool(boolean flag) { + this.m_configuration.shouldUseGlobalThreadPool(flag); + } + /** * Set the suites file names to be run by this TestNG object. This method tries to load and parse * the specified TestNG suite xml files. If a file is missing, it is ignored. @@ -1203,6 +1211,9 @@ public List runSuitesLocally() { if (m_configuration.isShareThreadPoolForDataProviders()) { xmlSuite.setShareThreadPoolForDataProviders(true); } + if (m_configuration.useGlobalThreadPool()) { + xmlSuite.shouldUseGlobalThreadPool(true); + } createSuiteRunners(suiteRunnerMap, xmlSuite); } @@ -1457,6 +1468,7 @@ public static TestNG privateMain(String[] argv, ITestListener listener) { * @param cla The command line parameters */ protected void configure(CommandLineArgs cla) { + Optional.ofNullable(cla.useGlobalThreadPool).ifPresent(this::shouldUseGlobalThreadPool); Optional.ofNullable(cla.shareThreadPoolForDataProviders) .ifPresent(this::shareThreadPoolForDataProviders); Optional.ofNullable(cla.propagateDataProviderFailureAsTestFailure) diff --git a/testng-core/src/main/java/org/testng/TestTaskExecutor.java b/testng-core/src/main/java/org/testng/TestTaskExecutor.java index 1bc2d24de2..110d8fe92e 100644 --- a/testng-core/src/main/java/org/testng/TestTaskExecutor.java +++ b/testng-core/src/main/java/org/testng/TestTaskExecutor.java @@ -5,7 +5,9 @@ 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; +import org.testng.internal.ObjectBag; import org.testng.internal.RuntimeBehavior; import org.testng.internal.Utils; import org.testng.internal.thread.TestNGThreadFactory; @@ -47,8 +49,7 @@ public TestTaskExecutor( public void execute() { String name = "test-" + xmlTest.getName(); - int threadCount = xmlTest.getThreadCount(); - threadCount = Math.max(threadCount, 1); + int threadCount = Math.max(xmlTest.getThreadCount(), 1); if (RuntimeBehavior.favourCustomThreadPoolExecutor()) { IExecutorFactory execFactory = configuration.getExecutorFactory(); ITestNGThreadPoolExecutor executor = @@ -65,14 +66,22 @@ public void execute() { executor.run(); service = executor; } else { - service = - new ThreadPoolExecutor( - threadCount, - threadCount, - 0, - TimeUnit.MILLISECONDS, - queue, - new TestNGThreadFactory(name)); + boolean reUse = xmlTest.getSuite().useGlobalThreadPool(); + Supplier supplier = + () -> + new ThreadPoolExecutor( + 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 executor = new GraphOrchestrator<>(service, factory, graph, comparator); executor.run(); diff --git a/testng-core/src/main/java/org/testng/internal/Configuration.java b/testng-core/src/main/java/org/testng/internal/Configuration.java index 03b41bd2b9..4dcabdf1f6 100644 --- a/testng-core/src/main/java/org/testng/internal/Configuration.java +++ b/testng-core/src/main/java/org/testng/internal/Configuration.java @@ -39,6 +39,8 @@ public class Configuration implements IConfiguration { private boolean propagateDataProviderFailureAsTestFailure; + private boolean useGlobalThreadPool = false; + public Configuration() { init(new JDK15AnnotationFinder(new DefaultAnnotationTransformer())); } @@ -180,4 +182,14 @@ public boolean isShareThreadPoolForDataProviders() { public void shareThreadPoolForDataProviders(boolean flag) { this.shareThreadPoolForDataProviders = flag; } + + @Override + public boolean useGlobalThreadPool() { + return this.useGlobalThreadPool; + } + + @Override + public void shouldUseGlobalThreadPool(boolean flag) { + this.useGlobalThreadPool = flag; + } } diff --git a/testng-core/src/main/java/org/testng/internal/IConfiguration.java b/testng-core/src/main/java/org/testng/internal/IConfiguration.java index b081ff6c50..26bed3c0f6 100644 --- a/testng-core/src/main/java/org/testng/internal/IConfiguration.java +++ b/testng-core/src/main/java/org/testng/internal/IConfiguration.java @@ -63,4 +63,8 @@ default boolean getReportAllDataDrivenTestsAsSkipped() { boolean isShareThreadPoolForDataProviders(); void shareThreadPoolForDataProviders(boolean flag); + + boolean useGlobalThreadPool(); + + void shouldUseGlobalThreadPool(boolean flag); } diff --git a/testng-core/src/main/java/org/testng/internal/ObjectBag.java b/testng-core/src/main/java/org/testng/internal/ObjectBag.java index 545610ca51..0e584b0c20 100644 --- a/testng-core/src/main/java/org/testng/internal/ObjectBag.java +++ b/testng-core/src/main/java/org/testng/internal/ObjectBag.java @@ -8,6 +8,7 @@ import java.util.function.Supplier; import org.testng.ISuite; import org.testng.log4testng.Logger; +import org.testng.xml.XmlSuite; /** * A simple bean bag that is intended to help share objects during the lifetime of TestNG without @@ -21,7 +22,11 @@ public final class ObjectBag { private static final Map instances = new ConcurrentHashMap<>(); public static ObjectBag getInstance(ISuite suite) { - return instances.computeIfAbsent(suite.getXmlSuite().SUITE_ID, k -> new ObjectBag()); + return getInstance(suite.getXmlSuite()); + } + + public static ObjectBag getInstance(XmlSuite xmlSuite) { + return instances.computeIfAbsent(xmlSuite.SUITE_ID, k -> new ObjectBag()); } public static void cleanup(ISuite suite) { diff --git a/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java b/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java index 47ccd06321..b792fc230b 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/MethodRunner.java @@ -12,6 +12,8 @@ import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; import java.util.stream.Collectors; import org.testng.ITestContext; import org.testng.ITestResult; @@ -21,7 +23,7 @@ import org.testng.internal.Parameters; import org.testng.internal.invokers.ITestInvoker.FailureContext; import org.testng.internal.invokers.TestMethodArguments.Builder; -import org.testng.internal.thread.ThreadUtil; +import org.testng.internal.thread.TestNGThreadFactory; import org.testng.xml.XmlSuite; public class MethodRunner implements IMethodRunner { @@ -107,9 +109,9 @@ public List runInParallel( XmlSuite suite = context.getSuite().getXmlSuite(); int parametersIndex = 0; ObjectBag objectBag = ObjectBag.getInstance(context.getSuite()); - boolean reUse = suite.isShareThreadPoolForDataProviders(); + boolean reUse = suite.isShareThreadPoolForDataProviders() || suite.useGlobalThreadPool(); - ExecutorService service = getOrCreate(reUse, suite.getDataProviderThreadCount(), objectBag); + ExecutorService service = getOrCreate(reUse, suite, objectBag); List>> all = new ArrayList<>(); for (Object[] next : CollectionUtils.asIterable(allParamValues)) { if (next == null) { @@ -163,18 +165,20 @@ public List runInParallel( return result; } - private static ExecutorService getOrCreate(boolean reUse, int count, ObjectBag objectBag) { + private static ExecutorService getOrCreate(boolean reUse, XmlSuite suite, ObjectBag objectBag) { + AtomicReference count = new AtomicReference<>(); + count.set(suite.getDataProviderThreadCount()); + Supplier supplier = () -> Executors.newFixedThreadPool(count.get(), threadFactory()); if (reUse) { - return (ExecutorService) - objectBag.createIfRequired( - ExecutorService.class, () -> Executors.newFixedThreadPool(count, threadFactory())); + if (suite.useGlobalThreadPool()) { + count.set(suite.getThreadCount()); + } + return (ExecutorService) objectBag.createIfRequired(ExecutorService.class, supplier); } - return Executors.newFixedThreadPool(count, threadFactory()); + return (ExecutorService) supplier.get(); } private static ThreadFactory threadFactory() { - AtomicInteger threadNumber = new AtomicInteger(0); - return r -> - new Thread(r, ThreadUtil.THREAD_NAME + "-PoolService-" + threadNumber.getAndIncrement()); + return new TestNGThreadFactory("PoolService"); } } diff --git a/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java b/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java index 29044e053d..c926b797aa 100644 --- a/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java +++ b/testng-core/src/main/java/org/testng/xml/TestNGContentHandler.java @@ -277,6 +277,13 @@ private void xmlSuite(boolean start, Attributes attributes) { m_currentSuite.setShareThreadPoolForDataProviders( Boolean.parseBoolean(shareThreadPoolForDataProviders))); + String useGlobalThreadPool = attributes.getValue("use-global-thread-pool"); + Optional.ofNullable(useGlobalThreadPool) + .ifPresent( + it -> + m_currentSuite.shouldUseGlobalThreadPool( + Boolean.parseBoolean(useGlobalThreadPool))); + String timeOut = attributes.getValue("time-out"); if (null != timeOut) { m_currentSuite.setTimeOut(timeOut); diff --git a/testng-core/src/main/resources/testng-1.0.dtd b/testng-core/src/main/resources/testng-1.0.dtd index 16bc4a7974..f7a49ce17b 100644 --- a/testng-core/src/main/resources/testng-1.0.dtd +++ b/testng-core/src/main/resources/testng-1.0.dtd @@ -56,8 +56,6 @@ Cedric Beust & Alexandru Popescu @attr skipfailedinvocationcounts Whether to skip failed invocations. @attr data-provider-thread-count An integer giving the size of the thread pool to use for parallel data providers. -@attr share-thread-pool-for-data-providers - Whether TestNG should use a common thread pool - for running parallel data providers. (Works only with TestNG versions 7.9.0 or higher) @attr object-factory A class that implements IObjectFactory that will be used to instantiate the test objects. @attr allow-return-values If true, tests that return a value will be run as well @@ -75,7 +73,6 @@ Cedric Beust & Alexandru Popescu time-out CDATA #IMPLIED skipfailedinvocationcounts (true | false) "false" data-provider-thread-count CDATA "10" - share-thread-pool-for-data-providers (true | false) "false" object-factory CDATA #IMPLIED group-by-instances (true | false) "false" preserve-order (true | false) "true" diff --git a/testng-core/src/main/resources/testng-1.1.dtd b/testng-core/src/main/resources/testng-1.1.dtd index 62c6fd0cb5..1e014acea9 100755 --- a/testng-core/src/main/resources/testng-1.1.dtd +++ b/testng-core/src/main/resources/testng-1.1.dtd @@ -54,13 +54,17 @@ Cedric Beust & Alexandru Popescu @attr time-out The time to wait in milliseconds before aborting the method (if parallel="methods") or the test (parallel="tests") @attr skipfailedinvocationcounts Whether to skip failed invocations. +@attr use-global-thread-pool - Whether TestNG should use a common thread pool +for running both regular and data driven tests in parallel. (Works only with TestNG versions 7.9.0 or higher) @attr data-provider-thread-count An integer giving the size of the thread pool to use for parallel data providers. +@attr share-thread-pool-for-data-providers - Whether TestNG should use a common thread pool + for running parallel data providers. (Works only with TestNG versions 7.9.0 or higher) @attr object-factory A class that implements IObjectFactory that will be used to instantiate the test objects. @attr allow-return-values If true, tests that return a value will be run as well --> - - + @@ -92,7 +98,7 @@ Parameters can be defined at the or at the level. Parameters defined at the level override parameters of the same name in Parameters are used to link Java method parameters to their actual value, defined here. --> - + @@ -103,12 +109,12 @@ They need to implement org.testng.IMethodSelector --> - + - + @@ -166,14 +172,14 @@ Defines additional groups ("groups of groups") and also which groups to include name CDATA #REQUIRED> - + - + @@ -182,7 +188,7 @@ Defines additional groups ("groups of groups") and also which groups to include - + @@ -208,6 +214,6 @@ Defines additional groups ("groups of groups") and also which groups to include - + diff --git a/testng-core/src/test/java/test/thread/SharedThreadPoolTest.java b/testng-core/src/test/java/test/thread/SharedThreadPoolTest.java new file mode 100644 index 0000000000..8351bb2272 --- /dev/null +++ b/testng-core/src/test/java/test/thread/SharedThreadPoolTest.java @@ -0,0 +1,79 @@ +package test.thread; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Collections; +import java.util.List; +import java.util.Queue; +import java.util.stream.Collectors; +import org.testng.TestNG; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.Test; +import org.testng.xml.XmlSuite; +import test.SimpleBaseTest; +import test.thread.issue2019.TestClassSample; + +public class SharedThreadPoolTest extends SimpleBaseTest { + + @AfterMethod + public void cleanup() { + TestClassSample.threads.clear(); + } + + @Test(description = "GITHUB-2019") + public void ensureCommonThreadPoolIsUsed() { + List list = runSimpleTest(true); + assertThat(list) + .withFailMessage( + "Expecting the regular and data driven tests to run at " + "max in 3 threads") + .hasSizeLessThanOrEqualTo(3); + } + + @Test(description = "GITHUB-2019") + public void ensureCommonThreadPoolIsNotUsed() { + List list = runSimpleTest(false); + assertThat(list) + .withFailMessage("Expecting the regular and data driven tests to run in at-least 3 threads") + .hasSizeGreaterThanOrEqualTo(3); + } + + private static List runSimpleTest(boolean useSharedGlobalThreadPool) { + TestNG testng = create(TestClassSample.class); + testng.shouldUseGlobalThreadPool(useSharedGlobalThreadPool); + testng.setThreadCount(3); + testng.setParallel(XmlSuite.ParallelMode.METHODS); + testng.run(); + Queue threads = TestClassSample.threads; + assertThat(threads).hasSize(10); + return threads.stream().distinct().collect(Collectors.toList()); + } + + @Test(description = "GITHUB-2019") + public void ensureCommonThreadPoolIsUsedWhenUsedWithSuiteFiles() { + String suite = "src/test/resources/2019_global_thread_pool_enabled.xml"; + List list = runSimpleTest(suite); + assertThat(list) + .withFailMessage( + "Expecting the regular and data driven tests to run at " + "max in 3 threads") + .hasSizeBetween(1, 3); + } + + @Test(description = "GITHUB-2019") + public void ensureCommonThreadPoolIsNotUsedWhenUsedWithSuiteFiles() { + String suite = "src/test/resources/2019_global_thread_pool_disabled.xml"; + List list = runSimpleTest(suite); + assertThat(list) + .withFailMessage( + "Expecting the regular and data driven tests to run at " + "max in 2 threads") + .hasSizeBetween(4, 10); + } + + private static List runSimpleTest(String suite) { + TestNG testng = create(); + testng.setTestSuites(Collections.singletonList(suite)); + testng.run(); + Queue threads = TestClassSample.threads; + assertThat(threads).hasSize(10); + return threads.stream().distinct().collect(Collectors.toList()); + } +} diff --git a/testng-core/src/test/java/test/thread/issue2019/TestClassSample.java b/testng-core/src/test/java/test/thread/issue2019/TestClassSample.java new file mode 100644 index 0000000000..7cccc685ce --- /dev/null +++ b/testng-core/src/test/java/test/thread/issue2019/TestClassSample.java @@ -0,0 +1,34 @@ +package test.thread.issue2019; + +import java.util.Queue; +import java.util.concurrent.ConcurrentLinkedQueue; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class TestClassSample { + + public static Queue threads = new ConcurrentLinkedQueue<>(); + + @Test + public void testA() {} + + @Test + public void testB() {} + + @Test(dataProvider = "getTestData") + public void testC(boolean ignored) {} + + @Test(dataProvider = "getTestData") + public void testD(boolean ignored) {} + + @AfterMethod + public void log() { + threads.add(Thread.currentThread().getId()); + } + + @DataProvider(name = "getTestData", parallel = true) + public Object[][] getTestData() { + return new Object[][] {{true}, {false}, {true}, {false}}; + } +} diff --git a/testng-core/src/test/resources/2019_global_thread_pool_disabled.xml b/testng-core/src/test/resources/2019_global_thread_pool_disabled.xml new file mode 100644 index 0000000000..a35348a1ef --- /dev/null +++ b/testng-core/src/test/resources/2019_global_thread_pool_disabled.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/testng-core/src/test/resources/2019_global_thread_pool_enabled.xml b/testng-core/src/test/resources/2019_global_thread_pool_enabled.xml new file mode 100644 index 0000000000..b4a8a2c51e --- /dev/null +++ b/testng-core/src/test/resources/2019_global_thread_pool_enabled.xml @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/testng-core/src/test/resources/2980_with_shared_threadpool_disabled.xml b/testng-core/src/test/resources/2980_with_shared_threadpool_disabled.xml index fd66a2c321..bca82f5cdc 100644 --- a/testng-core/src/test/resources/2980_with_shared_threadpool_disabled.xml +++ b/testng-core/src/test/resources/2980_with_shared_threadpool_disabled.xml @@ -1,5 +1,5 @@ - + diff --git a/testng-core/src/test/resources/2980_with_shared_threadpool_enabled.xml b/testng-core/src/test/resources/2980_with_shared_threadpool_enabled.xml index 767b47fbb1..4702e21cbf 100644 --- a/testng-core/src/test/resources/2980_with_shared_threadpool_enabled.xml +++ b/testng-core/src/test/resources/2980_with_shared_threadpool_enabled.xml @@ -1,5 +1,5 @@ - + diff --git a/testng-core/src/test/resources/testng.xml b/testng-core/src/test/resources/testng.xml index 4e3103d835..2bda7b165f 100644 --- a/testng-core/src/test/resources/testng.xml +++ b/testng-core/src/test/resources/testng.xml @@ -100,6 +100,7 @@ +