From 3457a35618d4948c8091d9d324f8c3c601648dce Mon Sep 17 00:00:00 2001 From: Krishnan Mahadevan Date: Thu, 2 Nov 2023 10:30:42 +0530 Subject: [PATCH] Restrict Group inheritance to Before|AfterGroups --- CHANGES.txt | 1 + .../testng/internal/TestNGMethodFinder.java | 14 +++++++------ .../test/configuration/ConfigurationTest.java | 11 ++++++++++ .../issue3003/BaseClassSample.java | 21 +++++++++++++++++++ .../issue3003/TestClassSample.java | 19 +++++++++++++++++ 5 files changed, 60 insertions(+), 6 deletions(-) create mode 100644 testng-core/src/test/java/test/configuration/issue3003/BaseClassSample.java create mode 100644 testng-core/src/test/java/test/configuration/issue3003/TestClassSample.java diff --git a/CHANGES.txt b/CHANGES.txt index 02d8d9a96..068e3c082 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ Current +Fixed: GITHUB-3003: BeforeClass|AfterClass with inheritedGroups triggers cyclic dependencies (Krishnan Mahadevan) New: Added @Inherited to the Listeners annotation, allowing it to be used in forming meta-annotations. (Pavlo Glushchenko) Fixed: GITHUB-2991: Suite attributes map should be thread safe (Krishnan Mahadevan) Fixed: GITHUB-2974: Command line arguments -groups and -excludegroups override defined groups in a suite xml file (dr29bart) diff --git a/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java b/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java index b1960c2cf..1f2446284 100644 --- a/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java +++ b/testng-core/src/main/java/org/testng/internal/TestNGMethodFinder.java @@ -172,15 +172,13 @@ private ITestNGMethod[] findConfiguration( case BEFORE_GROUPS: beforeGroups = configuration.getBeforeGroups(); create = - shouldCreateBeforeAfterGroup( - beforeGroups, annotationFinder, clazz, configuration.getInheritGroups()); + shouldCreateBeforeAfterGroup(beforeGroups, annotationFinder, clazz, configuration); isBeforeTestMethod = true; break; case AFTER_GROUPS: afterGroups = configuration.getAfterGroups(); create = - shouldCreateBeforeAfterGroup( - afterGroups, annotationFinder, clazz, configuration.getInheritGroups()); + shouldCreateBeforeAfterGroup(afterGroups, annotationFinder, clazz, configuration); isAfterTestMethod = true; break; default: @@ -220,10 +218,14 @@ private ITestNGMethod[] findConfiguration( } private static boolean shouldCreateBeforeAfterGroup( - String[] groups, IAnnotationFinder finder, Class clazz, boolean isInheritGroups) { - if (!isInheritGroups) { + String[] groups, IAnnotationFinder finder, Class clazz, IConfigurationAnnotation config) { + boolean isInheritGroups = config.getInheritGroups(); + boolean isBeforeAfterGroups = config.isBeforeGroups() || config.isAfterGroups(); + if (!isInheritGroups || !isBeforeAfterGroups) { return groups.length > 0; } + // Confine honouring of group inheritance ONLY to BeforeGroups|AfterGroups annotation + // This will ensure that we are backward compatible. ITestAnnotation test = AnnotationHelper.findTest(finder, clazz); if (test == null) { return groups.length > 0; diff --git a/testng-core/src/test/java/test/configuration/ConfigurationTest.java b/testng-core/src/test/java/test/configuration/ConfigurationTest.java index 797baadd0..c8d1e3d72 100644 --- a/testng-core/src/test/java/test/configuration/ConfigurationTest.java +++ b/testng-core/src/test/java/test/configuration/ConfigurationTest.java @@ -3,6 +3,7 @@ import static org.assertj.core.api.Assertions.assertThat; import java.util.Arrays; +import java.util.List; import org.testng.Assert; import org.testng.TestNG; import org.testng.annotations.DataProvider; @@ -65,6 +66,16 @@ public void ensureFirstTimeOnlyConfigsHaveProperTestStatuses(Class clazz) { assertThat(testng.getStatus()).isZero(); } + @Test(description = "GITHUB-3003") + public void ensureGroupInheritanceWorksForConfigMethods() { + TestNG testng = create(test.configuration.issue3003.TestClassSample.class); + testng.setVerbose(2); + testng.run(); + List expected = + Arrays.asList("setupMethod1", "setupMethod2", "setupMethod3", "testMethod1"); + assertThat(test.configuration.issue3003.TestClassSample.logs).containsAll(expected); + } + @DataProvider(name = "produceTestClasses") public Object[][] produceTestClasses() { return new Object[][] { diff --git a/testng-core/src/test/java/test/configuration/issue3003/BaseClassSample.java b/testng-core/src/test/java/test/configuration/issue3003/BaseClassSample.java new file mode 100644 index 000000000..88a34e072 --- /dev/null +++ b/testng-core/src/test/java/test/configuration/issue3003/BaseClassSample.java @@ -0,0 +1,21 @@ +package test.configuration.issue3003; + +import java.util.ArrayList; +import java.util.List; +import org.testng.annotations.BeforeClass; + +public class BaseClassSample { + public static final List logs = new ArrayList<>(); + + @BeforeClass(alwaysRun = true) + public void setupMethod1() { + logs.add("setupMethod1"); + } + + @BeforeClass( + alwaysRun = true, + dependsOnMethods = {"setupMethod1"}) + public void setupMethod2() { + logs.add("setupMethod2"); + } +} diff --git a/testng-core/src/test/java/test/configuration/issue3003/TestClassSample.java b/testng-core/src/test/java/test/configuration/issue3003/TestClassSample.java new file mode 100644 index 000000000..77007c27e --- /dev/null +++ b/testng-core/src/test/java/test/configuration/issue3003/TestClassSample.java @@ -0,0 +1,19 @@ +package test.configuration.issue3003; + +import org.testng.annotations.BeforeClass; +import org.testng.annotations.Test; + +@Test(groups = {"GROUP 1"}) +public class TestClassSample extends BaseClassSample { + + @BeforeClass( + alwaysRun = true, + dependsOnMethods = {"setupMethod2"}) + public void setupMethod3() { + logs.add("setupMethod3"); + } + + public void testMethod1() { + logs.add("testMethod1"); + } +}