From 42f120ad1ee87fc020d75d48784ede15c439e39e Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Mon, 7 Sep 2020 23:55:30 +0530 Subject: [PATCH] =?UTF-8?q?Streamline=20honoring=20of=20=E2=80=9CsingleThr?= =?UTF-8?q?eaded=E2=80=9D=20attribute?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #2361 --- CHANGES.txt | 1 + .../internal/ClassBasedParallelWorker.java | 22 ++++++--- .../issue2361/AnotherChildClassExample.java | 8 ++++ .../issue2361/BaseTestClassExample.java | 45 +++++++++++++++++++ .../thread/issue2361/ChildClassExample.java | 8 ++++ .../test/thread/issue2361/FactorySample.java | 15 +++++++ .../java/test/thread/issue2361/IssueTest.java | 32 +++++++++++++ src/test/resources/testng.xml | 1 + 8 files changed, 126 insertions(+), 6 deletions(-) create mode 100644 src/test/java/test/thread/issue2361/AnotherChildClassExample.java create mode 100644 src/test/java/test/thread/issue2361/BaseTestClassExample.java create mode 100644 src/test/java/test/thread/issue2361/ChildClassExample.java create mode 100644 src/test/java/test/thread/issue2361/FactorySample.java create mode 100644 src/test/java/test/thread/issue2361/IssueTest.java diff --git a/CHANGES.txt b/CHANGES.txt index 57a7047ef3..1ade934486 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-2361: No way to enforce @Test(singleThreaded = true) when test defined in base class (Krishnan Mahadevan) Fixed: GITHUB-2343: Injectors are not reused when they share the same set of modules (Krishnan Mahadevan) Fixed: GITHUB-2346: ITestResult attributes are null when retrieved by Listener onTestStart if test fails at BeforeMethod (Krishnan Mahadevan) Fixed: GITHUB-2357: TestNG 7.3.0 transitive dependencies diff --git a/src/main/java/org/testng/internal/ClassBasedParallelWorker.java b/src/main/java/org/testng/internal/ClassBasedParallelWorker.java index 97bad10555..8b415c04d3 100644 --- a/src/main/java/org/testng/internal/ClassBasedParallelWorker.java +++ b/src/main/java/org/testng/internal/ClassBasedParallelWorker.java @@ -17,11 +17,7 @@ class ClassBasedParallelWorker extends AbstractParallelWorker { - @Override - public List> createWorkers(Arguments arguments) { - List> result = Lists.newArrayList(); - // Methods that belong to classes with a sequential=true or parallel=classes - // attribute must all be run in the same worker + private static Set> gatherClassesThatShouldRunSequentially(Arguments arguments) { Set> sequentialClasses = Sets.newHashSet(); for (ITestNGMethod m : arguments.getMethods()) { Class cls = m.getRealClass(); @@ -33,6 +29,15 @@ public List> createWorkers(Arguments arguments) { sequentialClasses.add(cls); } } + return sequentialClasses; + } + + @Override + public List> createWorkers(Arguments arguments) { + List> result = Lists.newArrayList(); + // Methods that belong to classes with a sequential=true or parallel=classes + // attribute must all be run in the same worker + Set> sequentialClasses = gatherClassesThatShouldRunSequentially(arguments); List methodInstances = Lists.newArrayList(); for (ITestNGMethod tm : arguments.getMethods()) { @@ -49,7 +54,7 @@ public List> createWorkers(Arguments arguments) { params = getParameters(im); prevClass = c; } - if (sequentialClasses.contains(c)) { + if (shouldRunSequentially(c, sequentialClasses)) { if (!processedClasses.contains(c)) { processedClasses.add(c); // Sequential class: all methods in one worker @@ -67,6 +72,11 @@ public List> createWorkers(Arguments arguments) { return result; } + private static boolean shouldRunSequentially(Class c, Set> sequentialClasses) { + return sequentialClasses.contains(c) || + sequentialClasses.stream().anyMatch(each -> each.isAssignableFrom(c)); + } + private static List findClasses( List methodInstances, Class c) { return methodInstances diff --git a/src/test/java/test/thread/issue2361/AnotherChildClassExample.java b/src/test/java/test/thread/issue2361/AnotherChildClassExample.java new file mode 100644 index 0000000000..b8fcdc82ab --- /dev/null +++ b/src/test/java/test/thread/issue2361/AnotherChildClassExample.java @@ -0,0 +1,8 @@ +package test.thread.issue2361; + +import org.testng.annotations.Test; + +@Test(singleThreaded = true) +public class AnotherChildClassExample extends BaseTestClassExample { + +} diff --git a/src/test/java/test/thread/issue2361/BaseTestClassExample.java b/src/test/java/test/thread/issue2361/BaseTestClassExample.java new file mode 100644 index 0000000000..f1ce547024 --- /dev/null +++ b/src/test/java/test/thread/issue2361/BaseTestClassExample.java @@ -0,0 +1,45 @@ +package test.thread.issue2361; + +import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static org.testng.Assert.assertEquals; + +import java.util.concurrent.atomic.AtomicInteger; +import org.testng.annotations.Test; + +@Test(singleThreaded = true) +public class BaseTestClassExample { + + private final AtomicInteger currentTests = new AtomicInteger(); + + protected void test() { + int currentTests = this.currentTests.incrementAndGet(); + try { + assertEquals(currentTests, 1); + MILLISECONDS.sleep(10); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } finally { + this.currentTests.decrementAndGet(); + } + } + + @Test + public void test1() { + test(); + } + + @Test + public void test2() { + test(); + } + + @Test + public void test3() { + test(); + } + + @Test + public void test4() { + test(); + } +} diff --git a/src/test/java/test/thread/issue2361/ChildClassExample.java b/src/test/java/test/thread/issue2361/ChildClassExample.java new file mode 100644 index 0000000000..41fe8334f7 --- /dev/null +++ b/src/test/java/test/thread/issue2361/ChildClassExample.java @@ -0,0 +1,8 @@ +package test.thread.issue2361; + +import org.testng.annotations.Test; + +@Test(singleThreaded = true) +public class ChildClassExample extends BaseTestClassExample { + +} diff --git a/src/test/java/test/thread/issue2361/FactorySample.java b/src/test/java/test/thread/issue2361/FactorySample.java new file mode 100644 index 0000000000..f749dbb4f5 --- /dev/null +++ b/src/test/java/test/thread/issue2361/FactorySample.java @@ -0,0 +1,15 @@ +package test.thread.issue2361; + +import org.testng.annotations.Factory; + +public class FactorySample { + + @Factory + public static Object[] newInstances() { + return new Object[] { + new ChildClassExample(), + new AnotherChildClassExample() + }; + } + +} diff --git a/src/test/java/test/thread/issue2361/IssueTest.java b/src/test/java/test/thread/issue2361/IssueTest.java new file mode 100644 index 0000000000..9d66f4a73a --- /dev/null +++ b/src/test/java/test/thread/issue2361/IssueTest.java @@ -0,0 +1,32 @@ +package test.thread.issue2361; + +import org.assertj.core.api.Assertions; +import org.testng.TestNG; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import org.testng.xml.XmlSuite; +import org.testng.xml.XmlSuite.ParallelMode; +import org.testng.xml.XmlTest; +import test.SimpleBaseTest; + +public class IssueTest extends SimpleBaseTest { + + @Test(dataProvider = "dp") + public void ensureClassLevelSingleThreadedNatureGetsHonoured(Class cls) { + XmlSuite suite = createXmlSuite("Sample_Suite"); + suite.setParallel(ParallelMode.METHODS); + XmlTest xmlTest = createXmlTest(suite, "Sample_Test", cls); + xmlTest.setParallel(ParallelMode.METHODS); + TestNG testng = create(suite); + testng.run(); + Assertions.assertThat(testng.getStatus()).isEqualTo(0); + } + + @DataProvider(name = "dp") + public Object[][] getTestData() { + return new Object[][] { + {ChildClassExample.class}, + {FactorySample.class} + }; + } +} diff --git a/src/test/resources/testng.xml b/src/test/resources/testng.xml index eb7fa1c968..756d83dbce 100644 --- a/src/test/resources/testng.xml +++ b/src/test/resources/testng.xml @@ -99,6 +99,7 @@ +