Restrict Group inheritance to Before|AfterGroups

testng-team · Nov 3, 2023 · 3457a35 · 3457a35
1 parent 4e09c18
commit 3457a35
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 6 deletions.
diff --git 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
@@ -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
@@ -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<String> 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
@@ -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<String> 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
@@ -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");
+  }
+}