diff --git a/dd-java-agent/instrumentation/testng/testng-6.4/src/main/java/datadog/trace/instrumentation/testng6/TestNGClassListenerInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-6.4/src/main/java/datadog/trace/instrumentation/testng6/TestNGClassListenerInstrumentation.java index 3a0ca5f0da3..058073f1d6d 100644 --- a/dd-java-agent/instrumentation/testng/testng-6.4/src/main/java/datadog/trace/instrumentation/testng6/TestNGClassListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-6.4/src/main/java/datadog/trace/instrumentation/testng6/TestNGClassListenerInstrumentation.java @@ -59,7 +59,11 @@ public void methodAdvice(MethodTransformer transformer) { @Override public String[] helperClassNames() { return new String[] { - commonPackageName + ".TestNGUtils", commonPackageName + ".TestNGClassListener" + commonPackageName + ".TestEventsHandlerHolder", + commonPackageName + ".execution.RetryAnalyzer", + commonPackageName + ".execution.RetryAnnotationTransformer", + commonPackageName + ".TestNGUtils", + commonPackageName + ".TestNGClassListener" }; } diff --git a/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGClassListenerInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGClassListenerInstrumentation.java index 0989ed49195..20abdc9bace 100644 --- a/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGClassListenerInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGClassListenerInstrumentation.java @@ -54,7 +54,11 @@ public void methodAdvice(MethodTransformer transformer) { @Override public String[] helperClassNames() { return new String[] { - commonPackageName + ".TestNGUtils", commonPackageName + ".TestNGClassListener" + commonPackageName + ".TestEventsHandlerHolder", + commonPackageName + ".execution.RetryAnalyzer", + commonPackageName + ".execution.RetryAnnotationTransformer", + commonPackageName + ".TestNGUtils", + commonPackageName + ".TestNGClassListener" }; } diff --git a/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java index fd04267d368..db371a898f3 100644 --- a/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java @@ -57,6 +57,7 @@ public String[] helperClassNames() { commonPackageName + ".TestEventsHandlerHolder", commonPackageName + ".TestNGClassListener", commonPackageName + ".execution.RetryAnalyzer", + commonPackageName + ".execution.RetryAnnotationTransformer", commonPackageName + ".TracingListener", }; } @@ -118,7 +119,7 @@ public static void modifyStatus(@Advice.Argument(0) final ITestResult result) { RetryAnalyzer ddRetryAnalyzer = (RetryAnalyzer) retryAnalyzer; if (result.getStatus() == ITestResult.FAILURE - && ddRetryAnalyzer.getAndResetSuppressFailures()) { + && ddRetryAnalyzer.getAndResetSuppressFailures(result)) { // "failed but within success percentage" result.setStatus(ITestResult.SUCCESS_PERCENTAGE_FAILURE); } else if (result.isSuccess() && ddRetryAnalyzer.shouldPropagateFailure()) { diff --git a/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/order/TestNGOrderInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/order/TestNGOrderInstrumentation.java index 9aa927ca17e..79d8d726843 100644 --- a/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/order/TestNGOrderInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-7.0/src/main/java/datadog/trace/instrumentation/testng7/order/TestNGOrderInstrumentation.java @@ -58,6 +58,8 @@ public String[] helperClassNames() { parentPackageName + ".TestNGClassListener", parentPackageName + ".TestNGUtils", parentPackageName + ".TestEventsHandlerHolder", + parentPackageName + ".execution.RetryAnalyzer", + parentPackageName + ".execution.RetryAnnotationTransformer", packageName + ".FailFastOrderInterceptor", }; } diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java index a37da23e5d7..4d07a738c23 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGInstrumentation.java @@ -71,9 +71,7 @@ public static void addTracingListener(@Advice.This final TestNG testNG) { testNG.addListener((ITestNGListener) suiteListener); if (Config.get().isCiVisibilityExecutionPoliciesEnabled()) { - final RetryAnnotationTransformer transformer = - new RetryAnnotationTransformer(testNG.getAnnotationTransformer()); - testNG.addListener(transformer); + testNG.addListener(new RetryAnnotationTransformer(testNG.getAnnotationTransformer())); } } diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGMethodInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGMethodInstrumentation.java new file mode 100644 index 00000000000..a9d62eea483 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGMethodInstrumentation.java @@ -0,0 +1,43 @@ +package datadog.trace.instrumentation.testng; + +import static net.bytebuddy.matcher.ElementMatchers.isConstructor; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.Config; +import net.bytebuddy.asm.Advice; +import org.testng.ITestNGMethod; +import org.testng.internal.annotations.IAnnotationFinder; + +@AutoService(InstrumenterModule.class) +public class TestNGMethodInstrumentation extends InstrumenterModule.CiVisibility + implements Instrumenter.ForSingleType, Instrumenter.HasMethodAdvice { + public TestNGMethodInstrumentation() { + super("testng", "test-retry"); + } + + @Override + public boolean isEnabled() { + return super.isEnabled() && Config.get().isCiVisibilityExecutionPoliciesEnabled(); + } + + @Override + public String instrumentedType() { + return "org.testng.internal.TestNGMethod"; + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice(isConstructor(), getClass().getName() + "$ConfigureRetryAdvice"); + } + + public static class ConfigureRetryAdvice { + @Advice.OnMethodExit + public static void configure( + @Advice.This ITestNGMethod method, + @Advice.FieldValue("m_annotationFinder") IAnnotationFinder annotationFinder) { + TestNGUtils.configureRetryAnalyzer(method, annotationFinder); + } + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGSkipInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGSkipInstrumentation.java index d04d698e5ba..cb9c1f7e016 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGSkipInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGSkipInstrumentation.java @@ -10,12 +10,17 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.CIConstants; import datadog.trace.api.civisibility.config.TestIdentifier; +import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import java.lang.reflect.Method; import java.util.List; import net.bytebuddy.asm.Advice; +import org.testng.ITestNGMethod; +import org.testng.ITestResult; +import org.testng.Reporter; import org.testng.SkipException; import org.testng.annotations.DataProvider; +import org.testng.internal.ConstructorOrMethod; @AutoService(InstrumenterModule.class) public class TestNGSkipInstrumentation extends InstrumenterModule.CiVisibility @@ -57,14 +62,25 @@ public static void invokeMethod( @Advice.Argument(1) final Object instance, @Advice.Argument(2) final Object[] parameters) { TestIdentifier testIdentifier = TestNGUtils.toTestIdentifier(method, instance, parameters); + ITestResult invocation = Reporter.getCurrentTestResult(); + ITestNGMethod invocationMethod = invocation != null ? invocation.getMethod() : null; + ConstructorOrMethod constructorOrMethod = + invocationMethod != null ? invocationMethod.getConstructorOrMethod() : null; + List groups = + constructorOrMethod != null && method.equals(constructorOrMethod.getMethod()) + ? TestNGUtils.getGroups(invocation) + : TestNGUtils.getGroups(method); SkipReason skipReason = - TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(testIdentifier); + TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason( + testIdentifier, + new TestSourceData( + instance != null ? instance.getClass() : method.getDeclaringClass(), method), + groups); if (skipReason == null) { return; } if (skipReason == SkipReason.ITR) { - List groups = TestNGUtils.getGroups(method); if (groups.contains(CIConstants.Tags.ITR_UNSKIPPABLE_TAG)) { return; } diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGUtils.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGUtils.java index f643d6ffbbe..25b1edd53eb 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGUtils.java +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/TestNGUtils.java @@ -1,10 +1,14 @@ package datadog.trace.instrumentation.testng; +import static datadog.trace.api.civisibility.execution.TestAutomationExclusion.NO_AUTO_RETRY; + import datadog.json.JsonWriter; import datadog.trace.api.civisibility.config.LibraryCapability; import datadog.trace.api.civisibility.config.TestIdentifier; import datadog.trace.api.civisibility.config.TestSourceData; import datadog.trace.api.civisibility.events.TestSuiteDescriptor; +import datadog.trace.instrumentation.testng.execution.RetryAnalyzer; +import datadog.trace.instrumentation.testng.execution.RetryAnnotationTransformer; import datadog.trace.util.ComparableVersion; import java.io.InputStream; import java.lang.invoke.MethodHandle; @@ -26,9 +30,11 @@ import org.testng.ITestNGMethod; import org.testng.ITestResult; import org.testng.TestNG; +import org.testng.annotations.ITestAnnotation; import org.testng.annotations.Test; import org.testng.internal.ConstructorOrMethod; import org.testng.internal.ITestResultNotifier; +import org.testng.internal.annotations.IAnnotationFinder; import org.testng.xml.XmlTest; public abstract class TestNGUtils { @@ -48,6 +54,51 @@ public abstract class TestNGUtils { private static final MethodHandle TEST_METHOD_GET_RETRY_ANALYZER_LEGACY = METHOD_HANDLES.method(ITestNGMethod.class, "getRetryAnalyzer"); + private static final MethodHandle TEST_METHOD_GET_RETRY_ANALYZER_CLASS = + METHOD_HANDLES.method(ITestNGMethod.class, "getRetryAnalyzerClass"); + private static final MethodHandle TEST_METHOD_SET_RETRY_ANALYZER_CLASS = + METHOD_HANDLES.method(ITestNGMethod.class, "setRetryAnalyzerClass", Class.class); + private static final MethodHandle TEST_METHOD_SET_RETRY_ANALYZER_LEGACY = + METHOD_HANDLES.method(ITestNGMethod.class, "setRetryAnalyzer", IRetryAnalyzer.class); + + /** Apply Datadog policy after TestNG resolves annotations and native analyzer construction. */ + public static void configureRetryAnalyzer( + ITestNGMethod method, IAnnotationFinder annotationFinder) { + TestSourceData source = + new TestSourceData(method.getRealClass(), method.getConstructorOrMethod().getMethod()); + ITestAnnotation annotation = + (ITestAnnotation) + annotationFinder.findAnnotation(source.getTestMethod(), ITestAnnotation.class); + if (annotation == null) { + annotation = + (ITestAnnotation) + annotationFinder.findAnnotation( + source.getTestMethod().getDeclaringClass(), ITestAnnotation.class); + } + if (RetryAnnotationTransformer.hasFrameworkOverride(annotation)) { + return; + } + if (NO_AUTO_RETRY.isDeclared(source, Arrays.asList(method.getGroups()))) { + if (TEST_METHOD_GET_RETRY_ANALYZER_CLASS != null) { + Class nativeAnalyzer = + METHOD_HANDLES.invoke(TEST_METHOD_GET_RETRY_ANALYZER_CLASS, method); + if (nativeAnalyzer != null + && !nativeAnalyzer + .getName() + .equals("org.testng.internal.annotations.DisabledRetryAnalyzer")) { + return; + } + } else if (METHOD_HANDLES.invoke(TEST_METHOD_GET_RETRY_ANALYZER_LEGACY, method) != null) { + return; + } + } + if (TEST_METHOD_SET_RETRY_ANALYZER_CLASS != null) { + METHOD_HANDLES.invoke(TEST_METHOD_SET_RETRY_ANALYZER_CLASS, method, RetryAnalyzer.class); + } else { + METHOD_HANDLES.invoke(TEST_METHOD_SET_RETRY_ANALYZER_LEGACY, method, new RetryAnalyzer()); + } + } + private static final ComparableVersion testNGv75 = new ComparableVersion("7.5"); private static final ComparableVersion testNGv70 = new ComparableVersion("7.0"); @@ -233,11 +284,17 @@ public static IRetryAnalyzer getRetryAnalyzer(ITestResult result) { return null; } IRetryAnalyzer analyzer = METHOD_HANDLES.invoke(TEST_METHOD_GET_RETRY_ANALYZER, method, result); - if (analyzer != null) { - return analyzer; - } else { - return METHOD_HANDLES.invoke(TEST_METHOD_GET_RETRY_ANALYZER_LEGACY, method); + if (analyzer == null) { + analyzer = METHOD_HANDLES.invoke(TEST_METHOD_GET_RETRY_ANALYZER_LEGACY, method); + } + if (analyzer != null + && !(analyzer instanceof RetryAnalyzer) + && NO_AUTO_RETRY.isDeclared(toTestSourceData(result), getGroups(result))) { + // TestNG invokes its own analyzer. Keep a separate Datadog policy for reporting and + // quarantine. + return RetryAnalyzer.forFrameworkMethod(method, toTestIdentifier(result)); } + return analyzer; } @Nonnull diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java index c066a75f1fc..256e19f6a96 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java @@ -6,19 +6,40 @@ import datadog.trace.api.civisibility.execution.TestExecutionTracker; import datadog.trace.instrumentation.testng.TestEventsHandlerHolder; import datadog.trace.instrumentation.testng.TestNGUtils; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.WeakHashMap; import org.testng.IRetryAnalyzer; +import org.testng.ITestNGMethod; import org.testng.ITestResult; public class RetryAnalyzer implements IRetryAnalyzer { + private static final Map> FRAMEWORK_TRACKERS = + Collections.synchronizedMap( + new WeakHashMap>()); private volatile TestExecutionPolicy executionPolicy; - @SuppressFBWarnings( - value = "AT_STALE_THREAD_WRITE_OF_PRIMITIVE", - justification = "the field is confined to a single thread") - private boolean suppressFailures; + public static RetryAnalyzer forFrameworkMethod(ITestNGMethod method, TestIdentifier test) { + synchronized (FRAMEWORK_TRACKERS) { + Map trackers = FRAMEWORK_TRACKERS.get(method); + if (trackers == null) { + trackers = new HashMap<>(); + FRAMEWORK_TRACKERS.put(method, trackers); + } + RetryAnalyzer tracker = trackers.get(test); + if (tracker == null) { + tracker = new RetryAnalyzer(); + trackers.put(test, tracker); + } + return tracker; + } + } + + private static final String SUPPRESS_FAILURES_ATTRIBUTE = + RetryAnalyzer.class.getName() + ".suppressFailures"; public void createExecutionPolicy(ITestResult result) { if (executionPolicy == null) { @@ -46,17 +67,15 @@ public boolean retry(ITestResult result) { public void setSuppressFailures(ITestResult result) { createExecutionPolicy(result); - suppressFailures = executionPolicy.suppressFailures(); + result.setAttribute(SUPPRESS_FAILURES_ATTRIBUTE, executionPolicy.suppressFailures()); } public boolean shouldPropagateFailure() { return executionPolicy != null && executionPolicy.propagateFailure(); } - public boolean getAndResetSuppressFailures() { - boolean suppressFailures = this.suppressFailures; - this.suppressFailures = false; - return suppressFailures; + public boolean getAndResetSuppressFailures(ITestResult result) { + return Boolean.TRUE.equals(result.removeAttribute(SUPPRESS_FAILURES_ATTRIBUTE)); } public TestExecutionTracker getExecutionTracker() { diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnnotationTransformer.java b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnnotationTransformer.java index 471716729eb..264dbba148a 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnnotationTransformer.java +++ b/dd-java-agent/instrumentation/testng/testng-common/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnnotationTransformer.java @@ -1,23 +1,64 @@ package datadog.trace.instrumentation.testng.execution; +import datadog.trace.util.MethodHandles; +import java.lang.invoke.MethodHandle; import java.lang.reflect.Constructor; import java.lang.reflect.Method; +import java.util.Collections; +import java.util.Map; +import java.util.WeakHashMap; import org.testng.IAnnotationTransformer; +import org.testng.IRetryAnalyzer; +import org.testng.TestNG; import org.testng.annotations.ITestAnnotation; public class RetryAnnotationTransformer implements IAnnotationTransformer { + private static final MethodHandles METHOD_HANDLES = + new MethodHandles(TestNG.class.getClassLoader()); + private static final MethodHandle GET_RETRY_ANALYZER_CLASS = + METHOD_HANDLES.method(ITestAnnotation.class, "getRetryAnalyzerClass"); + private static final MethodHandle GET_RETRY_ANALYZER = + METHOD_HANDLES.method(ITestAnnotation.class, "getRetryAnalyzer"); + private static final Map FRAMEWORK_OVERRIDES = + Collections.synchronizedMap(new WeakHashMap()); private final IAnnotationTransformer delegate; public RetryAnnotationTransformer(IAnnotationTransformer delegate) { this.delegate = delegate; } + public static boolean hasFrameworkOverride(ITestAnnotation annotation) { + if (annotation == null) { + return false; + } + Boolean overridden = FRAMEWORK_OVERRIDES.get(annotation); + // A user-installed transformer can replace ours after TestNG construction. + return overridden == null || overridden; + } + + private static Class retryAnalyzerClass(ITestAnnotation annotation) { + if (GET_RETRY_ANALYZER_CLASS != null) { + return METHOD_HANDLES.invoke(GET_RETRY_ANALYZER_CLASS, annotation); + } + IRetryAnalyzer analyzer = METHOD_HANDLES.invoke(GET_RETRY_ANALYZER, annotation); + return analyzer != null ? analyzer.getClass() : null; + } + @Override public void transform( ITestAnnotation annotation, Class testClass, Constructor testConstructor, Method testMethod) { + FRAMEWORK_OVERRIDES.put(annotation, false); + Class nativeAnalyzer = retryAnalyzerClass(annotation); annotation.setRetryAnalyzer(RetryAnalyzer.class); if (delegate != null) { delegate.transform(annotation, testClass, testConstructor, testMethod); } + if (retryAnalyzerClass(annotation) != RetryAnalyzer.class) { + // Preserve the existing precedence of explicit user transformer overrides. + FRAMEWORK_OVERRIDES.put(annotation, true); + } else if (nativeAnalyzer != null) { + // Let TestNG resolve class inheritance and effective groups before selecting DD policy. + annotation.setRetryAnalyzer(nativeAnalyzer); + } } } diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy index 57f534def69..b31932671d6 100644 --- a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/groovy/datadog/trace/instrumentation/testng/TestNGTest.groovy @@ -7,6 +7,7 @@ import datadog.trace.civisibility.diff.LineDiff import org.example.* import org.junit.jupiter.api.Assumptions import org.testng.TestNG +import org.testng.IAnnotationTransformer import org.testng.xml.SuiteXmlParser import org.testng.xml.XmlSuite @@ -16,6 +17,225 @@ abstract class TestNGTest extends CiVisibilityInstrumentationTest { static currentTestNGVersion = TestNGUtils.getTestNGVersion() + def "test automation exclusions preserve a blocking failure and execute once"() { + given: + TestAutomationExcluded.executions = 0 + def id = new TestFQN("org.example.TestAutomationExcluded", "protected_failure") + givenDisabledTests([id]) + givenQuarantinedTests([id]) + givenAttemptToFixTests([id]) + givenFlakyRetryEnabled(true) + givenFlakyTests([id]) + givenEarlyFlakinessDetectionEnabled(true) + givenKnownTests([]) + + when: + runTests([TestAutomationExcluded], null, false) + + then: + TestAutomationExcluded.executions == 1 + def testSpan = TEST_WRITER.toList().flatten().find { it.getTag("test.name") == "protected_failure" } + testSpan != null + testSpan.getTag("test.test_management.no_auto_retry") == true + testSpan.getTag("test.test_management.prevent_failure_suppression") == true + testSpan.getTag("test.test_management.is_test_disabled") == true + testSpan.getTag("test.test_management.is_quarantined") == true + testSpan.getTag("test.test_management.is_attempt_to_fix") == true + testSpan.getTag("test.test_management.attempt_to_fix_passed") == null + } + + def "test automation exclusions preserve explicit framework retries"() { + given: + TestExplicitRetryExcluded.executions = 0 + givenFlakyRetryEnabled(true) + givenEarlyFlakinessDetectionEnabled(true) + givenKnownTests([]) + + when: + runTests([TestExplicitRetryExcluded], null, !currentTestNGVersion.startsWith("6.")) + + then: + TestExplicitRetryExcluded.executions == 2 + def testSpans = TEST_WRITER.toList().flatten().findAll { it.getTag("test.name") == "explicit_retry" } + testSpans.size() == 2 + testSpans.every { it.getTag("test.test_management.no_auto_retry") == true } + testSpans.every { it.getTag("test.is_retry") == null } + } + + def "test automation exclusions preserve class-level retry analyzers"() { + given: + TestClassRetryExcluded.executions = 0 + givenFlakyRetryEnabled(true) + givenKnownTests([]) + + when: + runTests([TestClassRetryExcluded], null, !currentTestNGVersion.startsWith("6.")) + + then: + TestClassRetryExcluded.executions == 2 + def spans = TEST_WRITER.toList().flatten().findAll { it.getTag("test.name") == "class_retry" } + spans.size() == 2 + spans.every { it.getTag("test.test_management.no_auto_retry") == true } + } + + def "test automation exclusions preserve object-factory retry analyzers"() { + given: + TestFactoryRetryExcluded.executions = 0 + givenFlakyRetryEnabled(true) + givenKnownTests([]) + def factoryType = Class.forName("org.testng.ITestObjectFactory", false, TestNG.classLoader) + Assumptions.assumeTrue(factoryType.methods.any { it.name == "newInstance" && it.parameterTypes[0] == Class }, + "Requires TestNG object factory API with class-based construction") + def factory = [newInstance: { Object type, Object[] arguments -> + if (type == TestFactoryRetryExcluded.ConfiguredRetry) { + return new TestFactoryRetryExcluded.ConfiguredRetry("configured") + } + type instanceof Class ? type.getDeclaredConstructor().newInstance() : type.newInstance(arguments) + }].asType(factoryType) + + when: + runTests([TestFactoryRetryExcluded], null, true, { TestNG testNG -> testNG.setObjectFactory(factory) }) + + then: + TestFactoryRetryExcluded.executions == 2 + def spans = TEST_WRITER.toList().flatten().findAll { it.getTag("test.name") == "factory_retry" } + spans.size() == 2 + spans.every { it.getTag("test.test_management.no_auto_retry") == true } + } + + def "test automation exclusions preserve quarantine with native retries"() { + given: + TestNativeQuarantinedExcluded.executions = 0 + def id = new TestFQN("org.example.TestNativeQuarantinedExcluded", "native_quarantined") + givenQuarantinedTests([id]) + givenAttemptToFixTests([id]) + givenFlakyRetryEnabled(true) + givenKnownTests([]) + + when: + runTests([TestNativeQuarantinedExcluded], null, TestNGUtils.isExceptionSuppressionSupported(currentTestNGVersion)) + + then: + TestNativeQuarantinedExcluded.executions == 2 + def spans = TEST_WRITER.toList().flatten().findAll { it.getTag("test.name") == "native_quarantined" } + spans.size() == 2 + spans.every { it.getTag("test.test_management.no_auto_retry") == true } + spans.every { it.getTag("test.test_management.is_quarantined") == true } + spans.every { it.getTag("test.test_management.attempt_to_fix_passed") == null } + } + + def "test automation exclusions isolate parallel native quarantine suppression"() { + given: + Assumptions.assumeTrue(TestNGUtils.isExceptionSuppressionSupported(currentTestNGVersion), + "Requires failure suppression support") + TestParallelNativeQuarantinedExcluded.reset() + givenQuarantinedTests([new TestFQN(TestParallelNativeQuarantinedExcluded.name, "parallel_native_quarantined")]) + givenFlakyRetryEnabled(true) + givenKnownTests([]) + + when: + runTests([TestParallelNativeQuarantinedExcluded], null, true, + { TestNG testNG -> testNG.addListener(new TestParallelNativeQuarantinedExcluded.Listener()) }) + + then: + TestParallelNativeQuarantinedExcluded.executions.get() == 2 + TestParallelNativeQuarantinedExcluded.suppressed.get() == 2 + TestParallelNativeQuarantinedExcluded.blocking.get() == 0 + def spans = TEST_WRITER.toList().flatten().findAll { it.getTag("test.name") == "parallel_native_quarantined" } + spans.size() == 2 + spans.every { it.getTag("test.test_management.no_auto_retry") == true } + spans.every { it.getTag("test.test_management.is_quarantined") == true } + } + + def "test automation exclusions preserve explicit disabled transformer overrides with #retryFeature"() { + given: + Assumptions.assumeTrue(TestNG.getResource("/org/testng/internal/annotations/DisabledRetryAnalyzer.class") != null, + "Requires class-based disabled retry analyzer") + Assumptions.assumeTrue(retryFeature != "EFD" || TestNGUtils.isEFDSupported(currentTestNGVersion), + "Requires EFD support") + TestTransformedDisabledRetry.executions = 0 + givenFlakyRetryEnabled(retryFeature == "ATR") + givenEarlyFlakinessDetectionEnabled(retryFeature == "EFD") + givenKnownTests([]) + def disabledAnalyzer = Class.forName("org.testng.internal.annotations.DisabledRetryAnalyzer", false, TestNG.classLoader) + def transformer = [transform: { annotation, testClass, constructor, method -> + if (method?.name == "transformed_disabled_retry") { + annotation.setRetryAnalyzer(disabledAnalyzer) + } + }] as IAnnotationTransformer + + when: + runTests([TestTransformedDisabledRetry], null, false, + { TestNG testNG -> testNG.addListener(transformer) }) + + then: + TestTransformedDisabledRetry.executions == 1 + def spans = TEST_WRITER.toList().flatten().findAll { it.getTag("test.name") == "transformed_disabled_retry" } + spans.size() == 1 + spans[0].getTag("test.is_retry") == null + spans[0].getTag("test.retry_reason") == null + + where: + retryFeature << ["ATR", "EFD"] + } + + def "test automation exclusions preserve unexcluded transformer retry overrides"() { + given: + TestTransformedNativeRetry.executions = 0 + givenFlakyRetryEnabled(false) + def transformer = [transform: { annotation, testClass, constructor, method -> + if (method?.name == "transformed_native_retry") { + annotation.setRetryAnalyzer(TestExplicitRetryExcluded.RetryOnce) + } + }] as IAnnotationTransformer + + when: + runTests([TestTransformedNativeRetry], null, !currentTestNGVersion.startsWith("6."), + { TestNG testNG -> testNG.addListener(transformer) }) + + then: + TestTransformedNativeRetry.executions == 2 + } + + def "test automation exclusions use effective inherited groups"() { + given: + TestInheritedGroupsExcluded.executions = 0 + def id = new TestFQN(TestInheritedGroupsExcluded.name, "inherited_groups") + givenDisabledTests([id]) + + when: + runTests([TestInheritedGroupsExcluded]) + + then: + TestInheritedGroupsExcluded.executions == 1 + def testSpan = TEST_WRITER.toList().flatten().find { it.getTag("test.name") == "inherited_groups" } + testSpan.getTag("test.status").toString() == "pass" + testSpan.getTag("test.test_management.prevent_failure_suppression") == true + testSpan.getTag("test.test_management.is_test_disabled") == true + } + + def "test automation exclusions use effective transformed groups"() { + given: + TestTransformedGroupsExcluded.executions = 0 + def id = new TestFQN(TestTransformedGroupsExcluded.name, "transformed_groups") + givenDisabledTests([id]) + def transformer = [transform: { annotation, testClass, constructor, method -> + if (method?.name == "transformed_groups") { + annotation.setGroups(["datadog_prevent_failure_suppression"] as String[]) + } + }] as IAnnotationTransformer + + when: + runTests([TestTransformedGroupsExcluded], null, true, { TestNG testNG -> testNG.addListener(transformer) }) + + then: + TestTransformedGroupsExcluded.executions == 1 + def testSpan = TEST_WRITER.toList().flatten().find { it.getTag("test.name") == "transformed_groups" } + testSpan.getTag("test.status").toString() == "pass" + testSpan.getTag("test.test_management.prevent_failure_suppression") == true + testSpan.getTag("test.test_management.is_test_disabled") == true + } + def "test #testcaseName"() { runTests(tests, null, success) @@ -281,13 +501,14 @@ abstract class TestNGTest extends CiVisibilityInstrumentationTest { "test-capabilities-ordering-efd-suppression" | TestNGUtils.capabilities("7.5") | (TestNGUtils.isTestOrderingSupported(currentTestNGVersion) && TestNGUtils.isEFDSupported(currentTestNGVersion) && TestNGUtils.isExceptionSuppressionSupported(currentTestNGVersion)) } - protected void runTests(List testClasses, String parallelMode = null, boolean expectSuccess = true) { + protected void runTests(List testClasses, String parallelMode = null, boolean expectSuccess = true, Closure configure = null) { TestEventsHandlerHolder.start() def testNG = new TestNG() testNG.setOutputDirectory(testOutputDir) Class[] testClassesArray = testClasses.toArray(new Class[0]) testNG.setTestClasses(testClassesArray) + configure?.call(testNG) if (parallelMode != null) { testNG.setParallel(parallelMode) } diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestAutomationExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestAutomationExcluded.java new file mode 100644 index 00000000000..710e5af424b --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestAutomationExcluded.java @@ -0,0 +1,17 @@ +package org.example; + +import datadog.trace.api.civisibility.NoAutoRetry; +import datadog.trace.api.civisibility.PreventFailureSuppression; +import org.testng.annotations.Test; + +public class TestAutomationExcluded { + public static int executions; + + @Test + @NoAutoRetry + @PreventFailureSuppression + public void protected_failure() { + executions++; + throw new AssertionError("protected failure"); + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestClassRetryExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestClassRetryExcluded.java new file mode 100644 index 00000000000..f22fe7d33f3 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestClassRetryExcluded.java @@ -0,0 +1,16 @@ +package org.example; + +import datadog.trace.api.civisibility.NoAutoRetry; +import org.testng.annotations.Test; + +@Test(retryAnalyzer = TestExplicitRetryExcluded.RetryOnce.class) +public class TestClassRetryExcluded { + public static int executions; + + @NoAutoRetry + public void class_retry() { + if (++executions == 1) { + throw new AssertionError("first framework attempt"); + } + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestExplicitRetryExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestExplicitRetryExcluded.java new file mode 100644 index 00000000000..667af018d15 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestExplicitRetryExcluded.java @@ -0,0 +1,28 @@ +package org.example; + +import datadog.trace.api.civisibility.NoAutoRetry; +import datadog.trace.api.civisibility.PreventFailureSuppression; +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; +import org.testng.annotations.Test; + +public class TestExplicitRetryExcluded { + public static int executions; + + @Test(retryAnalyzer = RetryOnce.class) + @NoAutoRetry + @PreventFailureSuppression + public void explicit_retry() { + if (++executions == 1) { + throw new AssertionError("first framework attempt"); + } + } + + public static class RetryOnce implements IRetryAnalyzer { + private int retries; + + public boolean retry(ITestResult result) { + return retries++ == 0; + } + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestFactoryRetryExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestFactoryRetryExcluded.java new file mode 100644 index 00000000000..187241bf1d7 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestFactoryRetryExcluded.java @@ -0,0 +1,34 @@ +package org.example; + +import datadog.trace.api.civisibility.NoAutoRetry; +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; +import org.testng.annotations.Test; + +public class TestFactoryRetryExcluded { + public static int executions; + + @Test(retryAnalyzer = ConfiguredRetry.class) + @NoAutoRetry + public void factory_retry() { + if (++executions == 1) { + throw new AssertionError("first framework attempt"); + } + } + + public static class ConfiguredRetry implements IRetryAnalyzer { + private final String configuration; + private int retries; + + public ConfiguredRetry(String configuration) { + this.configuration = configuration; + } + + public boolean retry(ITestResult result) { + if (!"configured".equals(configuration)) { + throw new AssertionError("missing object factory configuration"); + } + return retries++ == 0; + } + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestInheritedGroupsExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestInheritedGroupsExcluded.java new file mode 100644 index 00000000000..dfde9c7b5c6 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestInheritedGroupsExcluded.java @@ -0,0 +1,16 @@ +package org.example; + +import datadog.trace.api.civisibility.PreventFailureSuppression; +import org.testng.annotations.Test; + +@Test(groups = PreventFailureSuppression.TAG) +public class TestInheritedGroupsExcluded extends InheritedGroupsBase { + public static int executions; +} + +class InheritedGroupsBase { + @Test + public void inherited_groups() { + TestInheritedGroupsExcluded.executions++; + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestNativeQuarantinedExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestNativeQuarantinedExcluded.java new file mode 100644 index 00000000000..6be2c6f5e37 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestNativeQuarantinedExcluded.java @@ -0,0 +1,15 @@ +package org.example; + +import datadog.trace.api.civisibility.NoAutoRetry; +import org.testng.annotations.Test; + +public class TestNativeQuarantinedExcluded { + public static int executions; + + @Test(retryAnalyzer = TestExplicitRetryExcluded.RetryOnce.class) + @NoAutoRetry + public void native_quarantined() { + executions++; + throw new AssertionError("native failure remains quarantined"); + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestParallelNativeQuarantinedExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestParallelNativeQuarantinedExcluded.java new file mode 100644 index 00000000000..8d728256267 --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestParallelNativeQuarantinedExcluded.java @@ -0,0 +1,86 @@ +package org.example; + +import datadog.trace.api.civisibility.NoAutoRetry; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import org.testng.IRetryAnalyzer; +import org.testng.ITestResult; +import org.testng.TestListenerAdapter; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +public class TestParallelNativeQuarantinedExcluded { + public static final AtomicInteger executions = new AtomicInteger(); + public static final AtomicInteger suppressed = new AtomicInteger(); + public static final AtomicInteger blocking = new AtomicInteger(); + private static CountDownLatch bothRetrying; + private static CountDownLatch firstReported; + private static final AtomicInteger retryCalls = new AtomicInteger(); + + public static void reset() { + executions.set(0); + suppressed.set(0); + blocking.set(0); + retryCalls.set(0); + bothRetrying = new CountDownLatch(2); + firstReported = new CountDownLatch(1); + } + + @DataProvider(name = "colliding", parallel = true) + public Object[][] colliding() { + return new Object[][] {{new Parameter()}, {new Parameter()}}; + } + + public static class Parameter { + @Override + public String toString() { + return "same serialized parameter"; + } + } + + @Test(dataProvider = "colliding", retryAnalyzer = CoordinatedRetry.class) + @NoAutoRetry + public void parallel_native_quarantined(Parameter parameter) { + executions.incrementAndGet(); + throw new AssertionError("both parallel failures must remain quarantined"); + } + + public static class CoordinatedRetry implements IRetryAnalyzer { + @Override + public boolean retry(ITestResult result) { + int invocation = retryCalls.getAndIncrement(); + bothRetrying.countDown(); + await(bothRetrying); + if (invocation == 1) { + await(firstReported); + } + return false; + } + } + + public static class Listener extends TestListenerAdapter { + @Override + public void onTestFailedButWithinSuccessPercentage(ITestResult result) { + suppressed.incrementAndGet(); + firstReported.countDown(); + } + + @Override + public void onTestFailure(ITestResult result) { + blocking.incrementAndGet(); + firstReported.countDown(); + } + } + + private static void await(CountDownLatch latch) { + try { + if (!latch.await(10, TimeUnit.SECONDS)) { + throw new AssertionError("parallel native retry interleaving timed out"); + } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError(e); + } + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedDisabledRetry.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedDisabledRetry.java new file mode 100644 index 00000000000..64596464fad --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedDisabledRetry.java @@ -0,0 +1,13 @@ +package org.example; + +import org.testng.annotations.Test; + +public class TestTransformedDisabledRetry { + public static int executions; + + @Test + public void transformed_disabled_retry() { + ++executions; + throw new AssertionError("explicitly disabled framework retries"); + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedGroupsExcluded.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedGroupsExcluded.java new file mode 100644 index 00000000000..7e64f66c08a --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedGroupsExcluded.java @@ -0,0 +1,12 @@ +package org.example; + +import org.testng.annotations.Test; + +public class TestTransformedGroupsExcluded { + public static int executions; + + @Test + public void transformed_groups() { + executions++; + } +} diff --git a/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedNativeRetry.java b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedNativeRetry.java new file mode 100644 index 00000000000..c6b0f6acb8e --- /dev/null +++ b/dd-java-agent/instrumentation/testng/testng-common/src/testFixtures/java/org/example/TestTransformedNativeRetry.java @@ -0,0 +1,14 @@ +package org.example; + +import org.testng.annotations.Test; + +public class TestTransformedNativeRetry { + public static int executions; + + @Test + public void transformed_native_retry() { + if (++executions == 1) { + throw new AssertionError("first framework attempt"); + } + } +}