Skip to content

Commit 62bc253

Browse files
krmahadevankrmahadevan
committed
Restore DependsOnGroups functionality
Closes testng-team#2664
1 parent 839e823 commit 62bc253

16 files changed

+222
-28
lines changed

‎CHANGES.txt

+1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
Current
2+
Fixed: GITHUB-2664: Order for DependsOnGroups has changed after TestNg 7.4.0 (Krishnan Mahadevan)
23
Fixed: GITHUB-2501: TestNG 7.4.0 throws an exception "sun.net.www.protocol.file.FileURLConnection cannot be cast to java.net.HttpURLConnection" when xml file contain "ENTITY SYSTEM" grammer (Krishnan Mahadevan)
34
Fixed: GITHUB-2693: TestNG ignores 'dataproviderthreadcount' CLA (Krishnan Mahadevan)
45
Fixed: GITHUB-2685: TestInvoker should clear Thread.interrupted flag before calling ITestListeners (Roman Morskyi)

‎testng-core-api/src/main/java/org/testng/xml/XmlTest.java

-19
Original file line numberDiff line numberDiff line change
@@ -663,23 +663,4 @@ public XmlGroups getXmlGroups() {
663663
public boolean nameMatchesAny(List<String> names) {
664664
return names.contains(getName());
665665
}
666-
667-
/**
668-
* @param xmlTest - The {@link XmlTest} that represents the current <code>test</code>
669-
* @return - <code>true</code> if group based selection was employed and false otherwise.
670-
*/
671-
public static boolean isGroupBasedExecution(XmlTest xmlTest) {
672-
if (xmlTest == null) {
673-
return false;
674-
}
675-
XmlGroups xmlGroups = xmlTest.getXmlGroups();
676-
if (xmlGroups == null) {
677-
return false;
678-
}
679-
XmlRun xmlRun = xmlGroups.getRun();
680-
if (xmlRun == null) {
681-
return false;
682-
}
683-
return !xmlRun.getIncludes().isEmpty();
684-
}
685666
}

‎testng-core/src/main/java/org/testng/internal/MethodHelper.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,10 @@ private static Graph<ITestNGMethod> topologicalSort(
289289
}
290290
predecessors.addAll(Arrays.asList(methodsNamed));
291291
}
292-
if (XmlTest.isGroupBasedExecution(xmlTest)) {
292+
boolean anyConfigExceptGroupConfigs =
293+
!(m.isBeforeGroupsConfiguration() || m.isAfterGroupsConfiguration());
294+
boolean isGroupAgnosticConfigMethod = !m.isTest() && anyConfigExceptGroupConfigs;
295+
if (isGroupAgnosticConfigMethod) {
293296
String[] groupsDependedUpon = m.getGroupsDependedUpon();
294297
if (groupsDependedUpon.length > 0) {
295298
for (String group : groupsDependedUpon) {

‎testng-core/src/main/java/org/testng/internal/MethodInheritance.java

+20-7
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
import org.testng.ITestNGMethod;
1111
import org.testng.collections.Lists;
1212
import org.testng.collections.Maps;
13-
import org.testng.xml.XmlTest;
1413

1514
public class MethodInheritance {
1615

@@ -135,12 +134,17 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean before)
135134
ITestNGMethod m1 = l.get(i);
136135
for (int j = i + 1; j < l.size(); j++) {
137136
ITestNGMethod m2 = l.get(j);
138-
boolean groupMode = XmlTest.isGroupBasedExecution(m2.getXmlTest());
139-
if (groupMode) {
140-
// Do not resort to adding implicit depends-on if there are groups
141-
continue;
142-
}
143-
if (!equalsEffectiveClass(m1, m2) && !dependencyExists(m1, m2, methods)) {
137+
boolean notEffectivelyEqual = !equalsEffectiveClass(m1, m2);
138+
boolean upstreamHierarchy = hasUpstreamHierarchy(m1, m2);
139+
boolean shouldConsider =
140+
before ? notEffectivelyEqual && upstreamHierarchy : notEffectivelyEqual;
141+
boolean hasGroupDependencies =
142+
m2.getGroupsDependedUpon().length == 0
143+
&& m1.getGroupsDependedUpon().length == 0;
144+
145+
if (shouldConsider
146+
&& !dependencyExists(m1, m2, methods)
147+
&& hasGroupDependencies) {
144148
Utils.log("MethodInheritance", 4, m2 + " DEPENDS ON " + m1);
145149
m2.addMethodDependedUpon(MethodHelper.calculateMethodCanonicalName(m1));
146150
}
@@ -149,6 +153,15 @@ public static void fixMethodInheritance(ITestNGMethod[] methods, boolean before)
149153
});
150154
}
151155

156+
private static boolean hasUpstreamHierarchy(ITestNGMethod m1, ITestNGMethod m2) {
157+
Class<?> c1 = m1.getRealClass();
158+
Class<?> c2 = m2.getRealClass();
159+
if (c1.equals(c2)) {
160+
return false;
161+
}
162+
return c1.isAssignableFrom(c2);
163+
}
164+
152165
private static boolean dependencyExists(
153166
ITestNGMethod m1, ITestNGMethod m2, ITestNGMethod[] methods) {
154167
return internalDependencyExists(m1, m2, methods) || internalDependencyExists(m2, m1, methods);

‎testng-core/src/test/java/test/configuration/ConfigurationGroupsTest.java

+23
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import org.testng.annotations.DataProvider;
77
import org.testng.annotations.Test;
88
import test.SimpleBaseTest;
9+
import test.configuration.issue2664.MethodInvocationListener;
910

1011
public class ConfigurationGroupsTest extends SimpleBaseTest {
1112

@@ -37,4 +38,26 @@ public Object[][] getTestData() {
3738
{ConfigurationGroups7SampleTest.class, "A"}
3839
};
3940
}
41+
42+
@Test(description = "GITHUB-2664", dataProvider = "getTestClasses")
43+
public void ensureDependsOnGroupsHonouredForConfigs(Class<?> clazz) {
44+
TestNG testng = create(clazz);
45+
testng.setVerbose(2);
46+
MethodInvocationListener listener = new MethodInvocationListener();
47+
testng.addListener(listener);
48+
testng.run();
49+
assertThat(listener.logs).containsExactly("s3", "s1", "s2", "test3");
50+
}
51+
52+
@DataProvider(name = "getTestClasses")
53+
public Object[][] getTestClasses() {
54+
return new Object[][] {
55+
{test.configuration.issue2664.suite.GroupDependenciesSample.class},
56+
{test.configuration.issue2664.suite.GroupDependenciesChildSample.class},
57+
{test.configuration.issue2664.test.GroupDependenciesSample.class},
58+
{test.configuration.issue2664.test.GroupDependenciesChildSample.class},
59+
{test.configuration.issue2664.cls.GroupDependenciesSample.class},
60+
{test.configuration.issue2664.cls.GroupDependenciesChildSample.class},
61+
};
62+
}
4063
}

‎testng-core/src/test/java/test/configuration/issue2432/IssueTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,9 @@ public void ensureNoImplicitDependencyIsAddedWhenGroupsAreInvolved() {
3434
List<String> expected =
3535
Arrays.asList(
3636
"prepareConfig",
37+
"prepareConfigForTest1",
3738
"uploadConfigToDatabase",
3839
"verifyConfigurationAfterInstall",
39-
"prepareConfigForTest1",
4040
"test1");
4141
assertThat(listener.getInvokedMethodNames()).containsExactlyElementsOf(expected);
4242
}

‎testng-core/src/test/java/test/configuration/issue2664/MethodInvocationListener.java

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package test.configuration.issue2664;
2+
3+
import java.util.ArrayList;
4+
import java.util.List;
5+
import org.testng.IInvokedMethod;
6+
import org.testng.IInvokedMethodListener;
7+
import org.testng.ITestResult;
8+
9+
public class MethodInvocationListener implements IInvokedMethodListener {
10+
11+
public final List<String> logs = new ArrayList<>();
12+
13+
@Override
14+
public void beforeInvocation(IInvokedMethod method, ITestResult testResult) {
15+
logs.add(method.getTestMethod().getMethodName());
16+
}
17+
18+
@Override
19+
public void afterInvocation(IInvokedMethod iInvokedMethod, ITestResult iTestResult) {}
20+
}

‎testng-core/src/test/java/test/configuration/issue2664/cls/GroupDependenciesBaseClass.java

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package test.configuration.issue2664.cls;
2+
3+
import org.testng.annotations.BeforeClass;
4+
5+
public class GroupDependenciesBaseClass {
6+
@BeforeClass(groups = {"g1"})
7+
public void s3() {}
8+
}

‎testng-core/src/test/java/test/configuration/issue2664/cls/GroupDependenciesChildSample.java

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package test.configuration.issue2664.cls;
2+
3+
import org.testng.annotations.BeforeClass;
4+
import org.testng.annotations.Test;
5+
6+
public class GroupDependenciesChildSample extends GroupDependenciesBaseClass {
7+
8+
@BeforeClass(
9+
groups = {"g2"},
10+
dependsOnGroups = "g1")
11+
public void s2() {}
12+
13+
@BeforeClass(
14+
groups = {"g2"},
15+
dependsOnGroups = "g1")
16+
public void s1() {}
17+
18+
@Test()
19+
public void test3() {}
20+
}

‎testng-core/src/test/java/test/configuration/issue2664/cls/GroupDependenciesSample.java

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package test.configuration.issue2664.cls;
2+
3+
import org.testng.annotations.BeforeClass;
4+
import org.testng.annotations.Test;
5+
6+
public class GroupDependenciesSample {
7+
8+
@BeforeClass(groups = {"g1"})
9+
public void s3() {}
10+
11+
@BeforeClass(
12+
groups = {"g2"},
13+
dependsOnGroups = "g1")
14+
public void s2() {}
15+
16+
@BeforeClass(
17+
groups = {"g2"},
18+
dependsOnGroups = "g1")
19+
public void s1() {}
20+
21+
@Test()
22+
public void test3() {}
23+
}

‎testng-core/src/test/java/test/configuration/issue2664/suite/GroupDependenciesBaseClass.java

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package test.configuration.issue2664.suite;
2+
3+
import org.testng.annotations.BeforeSuite;
4+
5+
public class GroupDependenciesBaseClass {
6+
@BeforeSuite(groups = {"g1"})
7+
public void s3() {}
8+
}

‎testng-core/src/test/java/test/configuration/issue2664/suite/GroupDependenciesChildSample.java

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package test.configuration.issue2664.suite;
2+
3+
import org.testng.annotations.BeforeSuite;
4+
import org.testng.annotations.Test;
5+
6+
public class GroupDependenciesChildSample extends GroupDependenciesBaseClass {
7+
8+
@BeforeSuite(
9+
groups = {"g2"},
10+
dependsOnGroups = "g1")
11+
public void s2() {}
12+
13+
@BeforeSuite(
14+
groups = {"g2"},
15+
dependsOnGroups = "g1")
16+
public void s1() {}
17+
18+
@Test()
19+
public void test3() {}
20+
}

‎testng-core/src/test/java/test/configuration/issue2664/suite/GroupDependenciesSample.java

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package test.configuration.issue2664.suite;
2+
3+
import org.testng.annotations.BeforeSuite;
4+
import org.testng.annotations.Test;
5+
6+
public class GroupDependenciesSample {
7+
8+
@BeforeSuite(groups = {"g1"})
9+
public void s3() {}
10+
11+
@BeforeSuite(
12+
groups = {"g2"},
13+
dependsOnGroups = "g1")
14+
public void s2() {}
15+
16+
@BeforeSuite(
17+
groups = {"g2"},
18+
dependsOnGroups = "g1")
19+
public void s1() {}
20+
21+
@Test()
22+
public void test3() {}
23+
}

‎testng-core/src/test/java/test/configuration/issue2664/test/GroupDependenciesBaseClass.java

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package test.configuration.issue2664.test;
2+
3+
import org.testng.annotations.BeforeTest;
4+
5+
public class GroupDependenciesBaseClass {
6+
@BeforeTest(groups = {"g1"})
7+
public void s3() {}
8+
}

‎testng-core/src/test/java/test/configuration/issue2664/test/GroupDependenciesChildSample.java

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package test.configuration.issue2664.test;
2+
3+
import org.testng.annotations.BeforeTest;
4+
import org.testng.annotations.Test;
5+
6+
public class GroupDependenciesChildSample extends GroupDependenciesBaseClass {
7+
8+
@BeforeTest(
9+
groups = {"g2"},
10+
dependsOnGroups = "g1")
11+
public void s2() {}
12+
13+
@BeforeTest(
14+
groups = {"g2"},
15+
dependsOnGroups = "g1")
16+
public void s1() {}
17+
18+
@Test()
19+
public void test3() {}
20+
}

‎testng-core/src/test/java/test/configuration/issue2664/test/GroupDependenciesSample.java

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
package test.configuration.issue2664.test;
2+
3+
import org.testng.annotations.BeforeTest;
4+
import org.testng.annotations.Test;
5+
6+
public class GroupDependenciesSample {
7+
8+
@BeforeTest(groups = {"g1"})
9+
public void s3() {}
10+
11+
@BeforeTest(
12+
groups = {"g2"},
13+
dependsOnGroups = "g1")
14+
public void s2() {}
15+
16+
@BeforeTest(
17+
groups = {"g2"},
18+
dependsOnGroups = "g1")
19+
public void s1() {}
20+
21+
@Test()
22+
public void test3() {}
23+
}

0 commit comments

Comments
 (0)