diff --git a/CHANGES.txt b/CHANGES.txt index f6810ede1..bcd59d2ed 100644 --- 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 new file mode 100644 index 000000000..5636e3790 --- /dev/null +++ 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 -threadpoolfactoryclass + * + */ +@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 workQueue, + ThreadFactory threadFactory); +} 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 index dd9872656..74f9e581f 100644 --- 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 - true if TestNG is to be using its custom implementation of {@link - * java.util.concurrent.ThreadPoolExecutor} for running concurrent tests. Defaults to - * false - */ - 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/SuiteRunner.java b/testng-core/src/main/java/org/testng/SuiteRunner.java index 7900b1663..bb6baffe1 100644 --- 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 index 8a3f3838f..194a66ede 100644 --- a/testng-core/src/main/java/org/testng/SuiteTaskExecutor.java +++ b/testng-core/src/main/java/org/testng/SuiteTaskExecutor.java @@ -2,16 +2,12 @@ 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 { @@ -41,33 +37,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; - } else { - service = - new ThreadPoolExecutor( - threadPoolSize, - threadPoolSize, - Integer.MAX_VALUE, - TimeUnit.MILLISECONDS, - queue, - new TestNGThreadFactory(name)); - GraphOrchestrator 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 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 index e02f7ef49..cf188cc26 100644 --- 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, 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 v7.9.0. */ - @Deprecated - public void setExecutorFactoryClass(String clazzName) { - this.m_executorFactory = createExecutorFactoryInstanceUsing(clazzName); + public void setExecutorServiceFactory(IExecutorServiceFactory factory) { + m_configuration.setExecutorServiceFactory( + Objects.requireNonNull(factory, "ExecutorServiceFactory cannot be null")); } 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 v7.9.0. */ - @Deprecated - public void setExecutorFactory(IExecutorFactory factory) { - this.m_executorFactory = factory; - } - - /** This method stands deprecated as of TestNG v7.9.0. */ - @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) { @@ -1217,11 +1188,9 @@ public List 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); @@ -1272,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. @@ -1514,9 +1476,13 @@ protected void configure(CommandLineArgs cla) { m_objectFactory.newInstance((Class) 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 index 110d8fe92..4818f0eec 100644 --- a/testng-core/src/main/java/org/testng/TestTaskExecutor.java +++ b/testng-core/src/main/java/org/testng/TestTaskExecutor.java @@ -3,18 +3,14 @@ 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; 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; 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; @@ -50,42 +46,27 @@ public TestTaskExecutor( 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; + boolean reUse = xmlTest.getSuite().useGlobalThreadPool(); + Supplier 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 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(); + service = (ExecutorService) supplier.get(); } + GraphOrchestrator executor = + new GraphOrchestrator<>(service, factory, graph, comparator); + executor.run(); } public void awaitCompletion() { 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 49940fdff..42fdd190c 100644 --- 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, 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 index 989388534..937dd8ac9 100644 --- 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 index a27e8fc38..f57c31791 100644 --- 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(); diff --git a/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java b/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java index d521aed39..54ffdd4e5 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/MethodInvocationHelper.java @@ -30,6 +30,7 @@ import org.testng.TestNGException; import org.testng.TestNotInvokedException; import org.testng.internal.ConstructorOrMethod; +import org.testng.internal.IConfiguration; import org.testng.internal.MethodHelper; import org.testng.internal.Utils; import org.testng.internal.annotations.IAnnotationFinder; @@ -63,12 +64,13 @@ protected static void invokeMethodConsideringTimeout( ConstructorOrMethod method, Object targetInstance, Object[] params, - ITestResult testResult) + ITestResult testResult, + IConfiguration config) throws Throwable { if (MethodHelper.calculateTimeOut(tm) <= 0) { MethodInvocationHelper.invokeMethod(method.getMethod(), targetInstance, params); } else { - MethodInvocationHelper.invokeWithTimeout(tm, targetInstance, params, testResult); + MethodInvocationHelper.invokeWithTimeout(config, tm, targetInstance, params, testResult); if (!testResult.isSuccess()) { // A time out happened Throwable ex = testResult.getThrowable(); @@ -281,12 +283,17 @@ public Object[] getParameters() { * as implementation an Executor and a CountDownLatch. */ protected static void invokeWithTimeout( - ITestNGMethod tm, Object instance, Object[] parameterValues, ITestResult testResult) + IConfiguration config, + ITestNGMethod tm, + Object instance, + Object[] parameterValues, + ITestResult testResult) throws InterruptedException, ThreadExecutionException { - invokeWithTimeout(tm, instance, parameterValues, testResult, null); + invokeWithTimeout(config, tm, instance, parameterValues, testResult, null); } protected static boolean invokeWithTimeout( + IConfiguration config, ITestNGMethod tm, Object instance, Object[] parameterValues, @@ -300,7 +307,8 @@ protected static boolean invokeWithTimeout( // lose the time out of the enclosing executor). return invokeWithTimeoutWithNoExecutor(tm, instance, parameterValues, testResult, hookable); } else { - return invokeWithTimeoutWithNewExecutor(tm, instance, parameterValues, testResult, hookable); + return invokeWithTimeoutWithNewExecutor( + config, tm, instance, parameterValues, testResult, hookable); } } @@ -372,13 +380,14 @@ private static boolean invokeWithTimeoutWithNoExecutor( } private static boolean invokeWithTimeoutWithNewExecutor( + IConfiguration configuration, ITestNGMethod tm, Object instance, Object[] parameterValues, ITestResult testResult, IHookable hookable) throws InterruptedException, ThreadExecutionException { - ExecutorService exec = ThreadUtil.createExecutor(1, tm.getMethodName()); + ExecutorService exec = ThreadUtil.createExecutor(configuration, 1, tm.getMethodName()); InvokeMethodRunnable imr = new InvokeMethodRunnable(tm, instance, parameterValues, hookable, testResult); diff --git a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java index ceef1db94..1d4adafe3 100644 --- a/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java +++ b/testng-core/src/main/java/org/testng/internal/invokers/TestInvoker.java @@ -377,7 +377,7 @@ private List runWorkers( long maxTimeOut = workers.parallelStream().map(IWorker::getTimeOut).max(Long::compare).orElse(-1L); - ThreadUtil.execute("methods", workers, threadPoolSize, maxTimeOut); + ThreadUtil.execute(m_configuration, "methods", workers, threadPoolSize, maxTimeOut); // // Collect all the TestResults @@ -673,6 +673,7 @@ private ITestResult invokeMethod( // Method with a timeout willfullyIgnored = !MethodInvocationHelper.invokeWithTimeout( + m_configuration, arguments.getTestMethod(), arguments.getInstance(), arguments.getParameterValues(), diff --git a/testng-core/src/main/java/org/testng/internal/thread/DefaultThreadPoolExecutorFactory.java b/testng-core/src/main/java/org/testng/internal/thread/DefaultThreadPoolExecutorFactory.java deleted file mode 100644 index 7be196a8f..000000000 --- a/testng-core/src/main/java/org/testng/internal/thread/DefaultThreadPoolExecutorFactory.java +++ /dev/null @@ -1,66 +0,0 @@ -package org.testng.internal.thread; - -import java.util.Comparator; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.TimeUnit; -import org.testng.IDynamicGraph; -import org.testng.ISuite; -import org.testng.ITestNGMethod; -import org.testng.internal.thread.graph.GraphThreadPoolExecutor; -import org.testng.thread.IExecutorFactory; -import org.testng.thread.ITestNGThreadPoolExecutor; -import org.testng.thread.IThreadWorkerFactory; - -/** - * @deprecated - This implementation stands deprecated as of TestNG v7.9.0. There are - * no alternatives for this implementation. - */ -@Deprecated -public class DefaultThreadPoolExecutorFactory implements IExecutorFactory { - - @Override - public ITestNGThreadPoolExecutor newSuiteExecutor( - String name, - IDynamicGraph graph, - IThreadWorkerFactory factory, - int corePoolSize, - int maximumPoolSize, - long keepAliveTime, - TimeUnit unit, - BlockingQueue workQueue, - Comparator comparator) { - return new GraphThreadPoolExecutor<>( - name, - graph, - factory, - corePoolSize, - maximumPoolSize, - keepAliveTime, - unit, - workQueue, - comparator); - } - - @Override - public ITestNGThreadPoolExecutor newTestMethodExecutor( - String name, - IDynamicGraph graph, - IThreadWorkerFactory factory, - int corePoolSize, - int maximumPoolSize, - long keepAliveTime, - TimeUnit unit, - BlockingQueue workQueue, - Comparator comparator) { - return new GraphThreadPoolExecutor<>( - name, - graph, - factory, - corePoolSize, - maximumPoolSize, - keepAliveTime, - unit, - workQueue, - comparator); - } -} diff --git a/testng-core/src/main/java/org/testng/internal/thread/ThreadUtil.java b/testng-core/src/main/java/org/testng/internal/thread/ThreadUtil.java index 9dc21f3fe..27dfa599c 100644 --- a/testng-core/src/main/java/org/testng/internal/thread/ThreadUtil.java +++ b/testng-core/src/main/java/org/testng/internal/thread/ThreadUtil.java @@ -5,9 +5,9 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; -import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; import org.testng.collections.Lists; +import org.testng.internal.IConfiguration; import org.testng.internal.Utils; import org.testng.log4testng.Logger; @@ -30,7 +30,11 @@ public static boolean isTestNGThread() { * @param timeout a maximum timeout to wait for tasks finalization */ public static void execute( - String name, List tasks, int threadPoolSize, long timeout) { + IConfiguration configuration, + String name, + List tasks, + int threadPoolSize, + long timeout) { Utils.log( "ThreadUtil", @@ -43,13 +47,15 @@ public static void execute( + " threadPoolSize:" + threadPoolSize); ExecutorService pooledExecutor = - new ThreadPoolExecutor( - threadPoolSize, - threadPoolSize, - timeout, - TimeUnit.MILLISECONDS, - new LinkedBlockingQueue<>(), - new TestNGThreadFactory(name)); + configuration + .getExecutorServiceFactory() + .create( + threadPoolSize, + threadPoolSize, + timeout, + TimeUnit.MILLISECONDS, + new LinkedBlockingQueue<>(), + new TestNGThreadFactory(name)); List> callables = Lists.newArrayList(); for (final Runnable task : tasks) { @@ -79,9 +85,12 @@ public static String currentThreadInfo() { return thread.getName() + "@" + thread.hashCode(); } - public static ExecutorService createExecutor(int threadCount, String threadFactoryName) { + public static ExecutorService createExecutor( + IConfiguration config, int threadCount, String threadFactoryName) { ThreadFactory tf = new TestNGThreadFactory("method=" + threadFactoryName); - return new ThreadPoolExecutor( - threadCount, threadCount, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), tf); + return config + .getExecutorServiceFactory() + .create( + threadCount, threadCount, 0L, TimeUnit.MILLISECONDS, new LinkedBlockingQueue<>(), tf); } } diff --git a/testng-core/src/main/java/org/testng/thread/IExecutorFactory.java b/testng-core/src/main/java/org/testng/thread/IExecutorFactory.java deleted file mode 100644 index 4829fbba3..000000000 --- a/testng-core/src/main/java/org/testng/thread/IExecutorFactory.java +++ /dev/null @@ -1,74 +0,0 @@ -package org.testng.thread; - -import java.util.Comparator; -import java.util.concurrent.BlockingQueue; -import java.util.concurrent.TimeUnit; -import org.testng.IDynamicGraph; -import org.testng.ISuite; -import org.testng.ITestNGMethod; - -/** - * Represents the capabilities to be possessed by any implementation that can be plugged into TestNG - * to execute nodes from a {@link org.testng.IDynamicGraph} object. - * - * @deprecated - This interface stands deprecated as of TestNG v7.9.0. - */ -@Deprecated -public interface IExecutorFactory { - - /** - * @param name - The name to be used as a prefix for all created threads. - * @param graph - A {@link org.testng.IDynamicGraph} object that represents the graph of methods - * and the hierarchy of execution. - * @param factory - A {@link IThreadWorkerFactory} factory to create threads. - * @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 comparator - A {@link Comparator} to order nodes internally. - * @return - A new {@link ITestNGThreadPoolExecutor} that is capable of running suites in - * parallel. - */ - ITestNGThreadPoolExecutor newSuiteExecutor( - String name, - IDynamicGraph graph, - IThreadWorkerFactory factory, - int corePoolSize, - int maximumPoolSize, - long keepAliveTime, - TimeUnit unit, - BlockingQueue workQueue, - Comparator comparator); - - /** - * @param name - The name to be used as a prefix for all created threads. - * @param graph - A {@link IDynamicGraph} object that represents the graph of methods and the - * hierarchy of execution. - * @param factory - A {@link IThreadWorkerFactory} factory to create threads. - * @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 comparator - A {@link Comparator} to order nodes internally. - * @return - A new {@link ITestNGThreadPoolExecutor} that is capable of running test methods in - * parallel. - */ - ITestNGThreadPoolExecutor newTestMethodExecutor( - String name, - IDynamicGraph graph, - IThreadWorkerFactory factory, - int corePoolSize, - int maximumPoolSize, - long keepAliveTime, - TimeUnit unit, - BlockingQueue workQueue, - Comparator comparator); -} diff --git a/testng-core/src/test/java/test/dataprovider/DataProviderTest.java b/testng-core/src/test/java/test/dataprovider/DataProviderTest.java index aaec7b8b2..63a2c626e 100644 --- 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, Integer> pair) { diff --git a/testng-core/src/test/java/test/thread/CustomExecutorServiceFactoryTest.java b/testng-core/src/test/java/test/thread/CustomExecutorServiceFactoryTest.java new file mode 100644 index 000000000..b371166b8 --- /dev/null +++ b/testng-core/src/test/java/test/thread/CustomExecutorServiceFactoryTest.java @@ -0,0 +1,95 @@ +package test.thread; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Stream; +import org.testng.TestNG; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.Test; +import org.testng.collections.Lists; +import org.testng.xml.XmlSuite; +import test.SimpleBaseTest; +import test.thread.issue3066.Issue3066ExecutorServiceFactory; +import test.thread.issue3066.Issue3066ThreadPoolExecutor; +import test.thread.issue3066.TestClassSample; + +public class CustomExecutorServiceFactoryTest extends SimpleBaseTest { + + @Test(description = "GITHUB-3066") + public void ensureCanWireInCustomExecutorServiceWhenEnabledViaConfigParam() { + String[] args = { + "-testclass", + TestClassSample.class.getName(), + "-threadpoolfactoryclass", + Issue3066ExecutorServiceFactory.class.getName(), + "-parallel", + "methods" + }; + TestNG.privateMain(args, null); + assertThat(Issue3066ThreadPoolExecutor.isInvoked()).isTrue(); + } + + @Test(description = "GITHUB-3066") + public void ensureCanWireInCustomExecutorServiceWhenEnabledViaAPI() { + TestNG testng = create(TestClassSample.class); + testng.setExecutorServiceFactory(new Issue3066ExecutorServiceFactory()); + testng.setParallel(XmlSuite.ParallelMode.METHODS); + testng.run(); + assertThat(Issue3066ThreadPoolExecutor.isInvoked()).isTrue(); + } + + @Test(description = "GITHUB-3066") + public void ensureCanWireInCustomExecutorServiceWhenEnabledViaAPIForMultipleSuites() { + XmlSuite xmlSuite1 = createXmlSuite("suite1", "test1", TestClassSample.class); + XmlSuite xmlSuite2 = createXmlSuite("suite2", "test2", TestClassSample.class); + TestNG testng = create(); + testng.setXmlSuites(List.of(xmlSuite1, xmlSuite2)); + testng.setSuiteThreadPoolSize(2); + testng.setExecutorServiceFactory(new Issue3066ExecutorServiceFactory()); + testng.run(); + assertThat(Issue3066ThreadPoolExecutor.isInvoked()).isTrue(); + } + + @Test(description = "GITHUB-3066") + public void ensureCanWireInCustomExecutorServiceWhenEnabledViaConfigForMultipleSuites() { + AtomicInteger counter = new AtomicInteger(1); + List suites = new ArrayList<>(); + File dir = createDirInTempDir("suites"); + Stream.of(TestClassSample.class, TestClassSample.class) + .map( + it -> createXmlSuite("suite-" + counter.get(), "test-" + counter.getAndIncrement(), it)) + .map(XmlSuite::toXml) + .forEach( + it -> { + Path s1 = Paths.get(dir.getAbsolutePath(), UUID.randomUUID() + "-suite.xml"); + try { + Files.writeString(s1, it); + suites.add(s1.toFile().getAbsolutePath()); + } catch (IOException ignored) { + } + }); + + List args = + List.of( + "-threadpoolfactoryclass", + Issue3066ExecutorServiceFactory.class.getName(), + "-suitethreadpoolsize", + "2"); + TestNG.privateMain(Lists.merge(suites, args).toArray(String[]::new), null); + assertThat(Issue3066ThreadPoolExecutor.isInvoked()).isTrue(); + } + + @AfterMethod + public void resetState() { + Issue3066ThreadPoolExecutor.resetInvokedState(); + } +} diff --git a/testng-core/src/test/java/test/thread/SharedThreadPoolTest.java b/testng-core/src/test/java/test/thread/SharedThreadPoolTest.java index 5a31845fc..8351bb227 100644 --- 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 runSimpleTest(boolean useSharedGlobalThreadPool) { TestNG testng = create(TestClassSample.class); testng.shouldUseGlobalThreadPool(useSharedGlobalThreadPool); diff --git a/testng-core/src/test/java/test/thread/issue3066/Issue3066ExecutorServiceFactory.java b/testng-core/src/test/java/test/thread/issue3066/Issue3066ExecutorServiceFactory.java new file mode 100644 index 000000000..c5bfe0307 --- /dev/null +++ b/testng-core/src/test/java/test/thread/issue3066/Issue3066ExecutorServiceFactory.java @@ -0,0 +1,21 @@ +package test.thread.issue3066; + +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import org.testng.IExecutorServiceFactory; + +public class Issue3066ExecutorServiceFactory implements IExecutorServiceFactory { + @Override + public ExecutorService create( + int corePoolSize, + int maximumPoolSize, + long keepAliveTime, + TimeUnit unit, + BlockingQueue workQueue, + ThreadFactory threadFactory) { + return new Issue3066ThreadPoolExecutor( + corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue, threadFactory); + } +} diff --git a/testng-core/src/test/java/test/thread/issue3066/Issue3066ThreadPoolExecutor.java b/testng-core/src/test/java/test/thread/issue3066/Issue3066ThreadPoolExecutor.java new file mode 100644 index 000000000..0c625cf68 --- /dev/null +++ b/testng-core/src/test/java/test/thread/issue3066/Issue3066ThreadPoolExecutor.java @@ -0,0 +1,35 @@ +package test.thread.issue3066; + +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import org.jetbrains.annotations.NotNull; + +public class Issue3066ThreadPoolExecutor extends ThreadPoolExecutor { + + private static boolean invoked = false; + + public Issue3066ThreadPoolExecutor( + int corePoolSize, + int maximumPoolSize, + long keepAliveTime, + @NotNull TimeUnit unit, + @NotNull BlockingQueue workQueue, + @NotNull ThreadFactory threadFactory) { + super(corePoolSize, maximumPoolSize, keepAliveTime, unit, workQueue, threadFactory); + setInvoked(); + } + + private static void setInvoked() { + Issue3066ThreadPoolExecutor.invoked = true; + } + + public static boolean isInvoked() { + return invoked; + } + + public static void resetInvokedState() { + invoked = false; + } +} diff --git a/testng-core/src/test/java/test/thread/issue3066/TestClassSample.java b/testng-core/src/test/java/test/thread/issue3066/TestClassSample.java new file mode 100644 index 000000000..15bde069f --- /dev/null +++ b/testng-core/src/test/java/test/thread/issue3066/TestClassSample.java @@ -0,0 +1,12 @@ +package test.thread.issue3066; + +import org.testng.annotations.Test; + +public class TestClassSample { + + @Test + public void testMethod1() {} + + @Test + public void testMethod2() {} +} diff --git a/testng-core/src/test/resources/testng.xml b/testng-core/src/test/resources/testng.xml index 57ad2c64a..5df51a1d5 100644 --- a/testng-core/src/test/resources/testng.xml +++ b/testng-core/src/test/resources/testng.xml @@ -99,6 +99,7 @@ +