Skip to content

Commit 2fb3b6f

Browse files
nikita-tkachenko-datadognikita-tkachenko-datadog
authored
Update TestNG instrumentation to differentiate between identical test cases executed concurrently (#5572)
1 parent 822ffc0 commit 2fb3b6f

File tree

7 files changed

+146
-23
lines changed

7 files changed

+146
-23
lines changed

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestDescriptor.java‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,31 @@
11
package datadog.trace.civisibility.events;
22

33
import java.util.Objects;
4+
import javax.annotation.Nullable;
45

56
final class TestDescriptor {
67
private final String testSuiteName;
78
private final Class<?> testClass;
89
private final String testName;
910
private final String testParameters;
1011

11-
TestDescriptor(String testSuiteName, Class<?> testClass, String testName, String testParameters) {
12+
/**
13+
* A test-framework-specific "tie-breaker" that helps to differentiate between tests that would
14+
* otherwise be considered identical.
15+
*/
16+
private final @Nullable Object testQualifier;
17+
18+
TestDescriptor(
19+
String testSuiteName,
20+
Class<?> testClass,
21+
String testName,
22+
String testParameters,
23+
@Nullable Object testQualifier) {
1224
this.testSuiteName = testSuiteName;
1325
this.testClass = testClass;
1426
this.testName = testName;
1527
this.testParameters = testParameters;
28+
this.testQualifier = testQualifier;
1629
}
1730

1831
@Override
@@ -27,11 +40,12 @@ public boolean equals(Object o) {
2740
return Objects.equals(testSuiteName, that.testSuiteName)
2841
&& Objects.equals(testClass, that.testClass)
2942
&& Objects.equals(testName, that.testName)
30-
&& Objects.equals(testParameters, that.testParameters);
43+
&& Objects.equals(testParameters, that.testParameters)
44+
&& Objects.equals(testQualifier, that.testQualifier);
3145
}
3246

3347
@Override
3448
public int hashCode() {
35-
return Objects.hash(testSuiteName, testClass, testName, testParameters);
49+
return Objects.hash(testSuiteName, testClass, testName, testParameters, testQualifier);
3650
}
3751
}

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ public void onTestSuiteFailure(
194194
public void onTestStart(
195195
final String testSuiteName,
196196
final String testName,
197+
final @Nullable Object testQualifier,
197198
final @Nullable String testFramework,
198199
final @Nullable String testFrameworkVersion,
199200
final @Nullable String testParameters,
@@ -240,7 +241,7 @@ public void onTestStart(
240241
}
241242

242243
TestDescriptor descriptor =
243-
new TestDescriptor(testSuiteName, testClass, testName, testParameters);
244+
new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier);
244245
inProgressTests.put(descriptor, test);
245246
}
246247

@@ -249,10 +250,11 @@ public void onTestSkip(
249250
String testSuiteName,
250251
Class<?> testClass,
251252
String testName,
253+
@Nullable Object testQualifier,
252254
@Nullable String testParameters,
253255
@Nullable String reason) {
254256
TestDescriptor descriptor =
255-
new TestDescriptor(testSuiteName, testClass, testName, testParameters);
257+
new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier);
256258
DDTest test = inProgressTests.get(descriptor);
257259
if (test == null) {
258260
log.debug(
@@ -270,10 +272,11 @@ public void onTestFailure(
270272
String testSuiteName,
271273
Class<?> testClass,
272274
String testName,
275+
@Nullable Object testQualifier,
273276
@Nullable String testParameters,
274277
@Nullable Throwable throwable) {
275278
TestDescriptor descriptor =
276-
new TestDescriptor(testSuiteName, testClass, testName, testParameters);
279+
new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier);
277280
DDTest test = inProgressTests.get(descriptor);
278281
if (test == null) {
279282
log.debug(
@@ -291,9 +294,10 @@ public void onTestFinish(
291294
final String testSuiteName,
292295
final Class<?> testClass,
293296
final String testName,
297+
final @Nullable Object testQualifier,
294298
final @Nullable String testParameters) {
295299
TestDescriptor descriptor =
296-
new TestDescriptor(testSuiteName, testClass, testName, testParameters);
300+
new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier);
297301
DDTest test = inProgressTests.remove(descriptor);
298302
if (test == null) {
299303
log.debug(
@@ -310,6 +314,7 @@ public void onTestFinish(
310314
public void onTestIgnore(
311315
final String testSuiteName,
312316
final String testName,
317+
final @Nullable Object testQualifier,
313318
final @Nullable String testFramework,
314319
final @Nullable String testFrameworkVersion,
315320
final @Nullable String testParameters,
@@ -320,14 +325,15 @@ public void onTestIgnore(
320325
onTestStart(
321326
testSuiteName,
322327
testName,
328+
testQualifier,
323329
testFramework,
324330
testFrameworkVersion,
325331
testParameters,
326332
categories,
327333
testClass,
328334
testMethod);
329-
onTestSkip(testSuiteName, testClass, testName, testParameters, reason);
330-
onTestFinish(testSuiteName, testClass, testName, testParameters);
335+
onTestSkip(testSuiteName, testClass, testName, testQualifier, testParameters, reason);
336+
onTestFinish(testSuiteName, testClass, testName, testQualifier, testParameters);
331337
}
332338

333339
private static boolean skipTrace(final Class<?> testClass) {

‎dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/TracingListener.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,15 @@ public void testStarted(final Description description) {
9898
List<String> categories = JUnit4Utils.getCategories(testClass, testMethod);
9999

100100
testEventsHandler.onTestStart(
101-
testSuiteName, testName, null, null, testParameters, categories, testClass, testMethod);
101+
testSuiteName,
102+
testName,
103+
null,
104+
null,
105+
null,
106+
testParameters,
107+
categories,
108+
testClass,
109+
testMethod);
102110
}
103111

104112
@Override
@@ -108,7 +116,7 @@ public void testFinished(final Description description) {
108116
Method testMethod = JUnit4Utils.getTestMethod(description);
109117
String testName = JUnit4Utils.getTestName(description, testMethod);
110118
String testParameters = JUnit4Utils.getParameters(description);
111-
testEventsHandler.onTestFinish(testSuiteName, testClass, testName, testParameters);
119+
testEventsHandler.onTestFinish(testSuiteName, testClass, testName, null, testParameters);
112120
}
113121

114122
// same callback is executed both for test cases and test suites (for setup/teardown errors)
@@ -126,7 +134,7 @@ public void testFailure(final Failure failure) {
126134
String testParameters = JUnit4Utils.getParameters(description);
127135
Throwable throwable = failure.getException();
128136
testEventsHandler.onTestFailure(
129-
testSuiteName, testClass, testName, testParameters, throwable);
137+
testSuiteName, testClass, testName, null, testParameters, throwable);
130138
}
131139
}
132140

@@ -156,7 +164,8 @@ public void testAssumptionFailure(final Failure failure) {
156164
String testName = JUnit4Utils.getTestName(description, testMethod);
157165
String testParameters = JUnit4Utils.getParameters(description);
158166

159-
testEventsHandler.onTestSkip(testSuiteName, testClass, testName, testParameters, reason);
167+
testEventsHandler.onTestSkip(
168+
testSuiteName, testClass, testName, null, testParameters, reason);
160169
}
161170
}
162171

@@ -201,6 +210,7 @@ private void testIgnored(Description description, Method testMethod, String reas
201210
testName,
202211
null,
203212
null,
213+
null,
204214
testParameters,
205215
categories,
206216
testClass,

‎dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TracingListener.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ private void testCaseExecutionStarted(final TestIdentifier testIdentifier) {
150150
testEventsHandler.onTestStart(
151151
testSuitName,
152152
testName,
153+
null,
153154
testFramework,
154155
testFrameworkVersion,
155156
testParameters,
@@ -178,14 +179,14 @@ private void testCaseExecutionFinished(
178179
if (throwable != null) {
179180
if (JUnit5Utils.isAssumptionFailure(throwable)) {
180181
testEventsHandler.onTestSkip(
181-
testSuiteName, testClass, testName, testParameters, throwable.getMessage());
182+
testSuiteName, testClass, testName, null, testParameters, throwable.getMessage());
182183
} else {
183184
testEventsHandler.onTestFailure(
184-
testSuiteName, testClass, testName, testParameters, throwable);
185+
testSuiteName, testClass, testName, null, testParameters, throwable);
185186
}
186187
}
187188

188-
testEventsHandler.onTestFinish(testSuiteName, testClass, testName, testParameters);
189+
testEventsHandler.onTestFinish(testSuiteName, testClass, testName, null, testParameters);
189190
}
190191

191192
@Override
@@ -245,6 +246,7 @@ private void testCaseExecutionSkipped(
245246
testEventsHandler.onTestIgnore(
246247
testSuiteName,
247248
testName,
249+
null,
248250
testFramework,
249251
testFrameworkVersion,
250252
testParameters,

‎dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/TracingListener.java‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,14 +83,23 @@ public void onTestStart(final ITestResult result) {
8383
String testSuiteName = result.getInstanceName();
8484
String testName =
8585
(result.getTestName() != null) ? result.getTestName() : result.getMethod().getMethodName();
86+
String testQualifier = result.getTestContext().getName();
8687
String testParameters = TestNGUtils.getParameters(result);
8788
List<String> groups = TestNGUtils.getGroups(result);
8889

8990
Class<?> testClass = TestNGUtils.getTestClass(result);
9091
Method testMethod = TestNGUtils.getTestMethod(result);
9192

9293
testEventsHandler.onTestStart(
93-
testSuiteName, testName, null, null, testParameters, groups, testClass, testMethod);
94+
testSuiteName,
95+
testName,
96+
testQualifier,
97+
null,
98+
null,
99+
testParameters,
100+
groups,
101+
testClass,
102+
testMethod);
94103
}
95104

96105
@Override
@@ -99,8 +108,10 @@ public void onTestSuccess(final ITestResult result) {
99108
final Class<?> testClass = TestNGUtils.getTestClass(result);
100109
String testName =
101110
(result.getTestName() != null) ? result.getTestName() : result.getMethod().getMethodName();
111+
String testQualifier = result.getTestContext().getName();
102112
String testParameters = TestNGUtils.getParameters(result);
103-
testEventsHandler.onTestFinish(testSuiteName, testClass, testName, testParameters);
113+
testEventsHandler.onTestFinish(
114+
testSuiteName, testClass, testName, testQualifier, testParameters);
104115
}
105116

106117
@Override
@@ -109,11 +120,14 @@ public void onTestFailure(final ITestResult result) {
109120
final Class<?> testClass = TestNGUtils.getTestClass(result);
110121
String testName =
111122
(result.getTestName() != null) ? result.getTestName() : result.getMethod().getMethodName();
123+
String testQualifier = result.getTestContext().getName();
112124
String testParameters = TestNGUtils.getParameters(result);
113125

114126
final Throwable throwable = result.getThrowable();
115-
testEventsHandler.onTestFailure(testSuiteName, testClass, testName, testParameters, throwable);
116-
testEventsHandler.onTestFinish(testSuiteName, testClass, testName, testParameters);
127+
testEventsHandler.onTestFailure(
128+
testSuiteName, testClass, testName, testQualifier, testParameters, throwable);
129+
testEventsHandler.onTestFinish(
130+
testSuiteName, testClass, testName, testQualifier, testParameters);
117131
}
118132

119133
@Override
@@ -127,12 +141,15 @@ public void onTestSkipped(final ITestResult result) {
127141
final Class<?> testClass = TestNGUtils.getTestClass(result);
128142
String testName =
129143
(result.getTestName() != null) ? result.getTestName() : result.getMethod().getMethodName();
144+
String testQualifier = result.getTestContext().getName();
130145
String testParameters = TestNGUtils.getParameters(result);
131146

132147
// Typically the way of skipping a TestNG test is throwing a SkipException
133148
Throwable throwable = result.getThrowable();
134149
String reason = throwable != null ? throwable.getMessage() : null;
135-
testEventsHandler.onTestSkip(testSuiteName, testClass, testName, testParameters, reason);
136-
testEventsHandler.onTestFinish(testSuiteName, testClass, testName, testParameters);
150+
testEventsHandler.onTestSkip(
151+
testSuiteName, testClass, testName, testQualifier, testParameters, reason);
152+
testEventsHandler.onTestFinish(
153+
testSuiteName, testClass, testName, testQualifier, testParameters);
137154
}
138155
}

‎dd-java-agent/instrumentation/testng/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,72 @@ abstract class TestNGTest extends CiVisibilityTest {
662662
})
663663
}
664664

665+
def "test successful test cases executed in parallel with TESTS parallel mode and same test case running concurrently"() {
666+
setup:
667+
def suiteXml = """
668+
<!DOCTYPE suite SYSTEM "https://testng.org/testng-1.0.dtd" >
669+
<suite name="API Test Suite" parallel="tests" configfailurepolicy="continue">
670+
<test name="Test A">
671+
<classes>
672+
<class name="org.example.TestSucceed">
673+
<methods>
674+
<include name="test_succeed"/>
675+
</methods>
676+
</class>
677+
</classes>
678+
</test>
679+
680+
<test name="Test B">
681+
<classes>
682+
<class name="org.example.TestSucceed">
683+
<methods>
684+
<include name="test_succeed"/>
685+
</methods>
686+
</class>
687+
</classes>
688+
</test>
689+
690+
<test name="Test C">
691+
<classes>
692+
<class name="org.example.TestSucceed">
693+
<methods>
694+
<include name="test_succeed"/>
695+
</methods>
696+
</class>
697+
</classes>
698+
</test>
699+
</suite>
700+
"""
701+
702+
def parser = new SuiteXmlParser()
703+
def xmlSuite = parser.parse("testng.xml", new ByteArrayInputStream(suiteXml.bytes), true)
704+
705+
def testNG = new TestNG()
706+
testNG.setOutputDirectory(testOutputDir)
707+
testNG.setParallel("tests")
708+
testNG.setXmlSuites(Collections.singletonList(xmlSuite))
709+
testNG.run()
710+
711+
expect:
712+
ListWriterAssert.assertTraces(TEST_WRITER, 4, false, SORT_TRACES_BY_DESC_SIZE_THEN_BY_NAMES, {
713+
long testModuleId
714+
long testSuiteId
715+
trace(2, true) {
716+
testModuleId = testModuleSpan(it, 0, CIConstants.TEST_PASS)
717+
testSuiteId = testSuiteSpan(it, 1, testModuleId, "org.example.TestSucceed", CIConstants.TEST_PASS)
718+
}
719+
trace(1) {
720+
testSpan(it, 0, testModuleId, testSuiteId, "org.example.TestSucceed", "test_succeed", CIConstants.TEST_PASS)
721+
}
722+
trace(1) {
723+
testSpan(it, 0, testModuleId, testSuiteId, "org.example.TestSucceed", "test_succeed", CIConstants.TEST_PASS)
724+
}
725+
trace(1) {
726+
testSpan(it, 0, testModuleId, testSuiteId, "org.example.TestSucceed", "test_succeed", CIConstants.TEST_PASS)
727+
}
728+
})
729+
}
730+
665731
def "test ITR skipping"() {
666732
setup:
667733
injectSysConfig(CiVisibilityConfig.CIVISIBILITY_SKIPPABLE_TESTS, SkippableTestsSerializer.serialize([

‎internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ void onTestSuiteStart(
2929
void onTestStart(
3030
String testSuiteName,
3131
String testName,
32+
@Nullable Object testQualifier,
3233
@Nullable String testFramework,
3334
@Nullable String testFrameworkVersion,
3435
@Nullable String testParameters,
@@ -40,22 +41,29 @@ void onTestSkip(
4041
String testSuiteName,
4142
Class<?> testClass,
4243
String testName,
44+
@Nullable Object testQualifier,
4345
@Nullable String testParameters,
4446
@Nullable String reason);
4547

4648
void onTestFailure(
4749
String testSuiteName,
4850
Class<?> testClass,
4951
String testName,
52+
@Nullable Object testQualifier,
5053
@Nullable String testParameters,
5154
@Nullable Throwable throwable);
5255

5356
void onTestFinish(
54-
String testSuiteName, Class<?> testClass, String testName, @Nullable String testParameters);
57+
String testSuiteName,
58+
Class<?> testClass,
59+
String testName,
60+
@Nullable Object testQualifier,
61+
@Nullable String testParameters);
5562

5663
void onTestIgnore(
5764
String testSuiteName,
5865
String testName,
66+
@Nullable Object testQualifier,
5967
@Nullable String testFramework,
6068
@Nullable String testFrameworkVersion,
6169
@Nullable String testParameters,

0 commit comments

Comments
 (0)