From b5ca4e866bc163dc775b472886fb84878d710008 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Thu, 8 May 2025 12:11:37 +0200 Subject: [PATCH 01/10] align karate execution instrumentation ordering --- .../karate/ExecutionContext.java | 11 +++++++++++ .../karate/KarateExecutionInstrumentation.java | 18 +++++++++++++----- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java index 279163854da..18ad5412f46 100644 --- a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java +++ b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java @@ -9,6 +9,7 @@ public class ExecutionContext { private final TestExecutionPolicy executionPolicy; + private boolean suppressFailures; private boolean failed; private long startTimestamp; @@ -34,6 +35,16 @@ public boolean getAndResetFailed() { return failed; } + public void setSuppressFailures(boolean suppressFailures) { + this.suppressFailures = suppressFailures; + } + + public boolean getAndResetSuppressFailures() { + boolean suppressFailures = this.suppressFailures; + this.suppressFailures = false; + return suppressFailures; + } + public TestExecutionPolicy getExecutionPolicy() { return executionPolicy; } diff --git a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java index 159d323d265..1ae34f6ade7 100644 --- a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java @@ -78,9 +78,18 @@ public void methodAdvice(MethodTransformer transformer) { public static class RetryAdvice { @Advice.OnMethodEnter public static void beforeExecute(@Advice.This ScenarioRuntime scenarioRuntime) { - InstrumentationContext.get(Scenario.class, ExecutionContext.class) - .computeIfAbsent(scenarioRuntime.scenario, ExecutionContext::create) - .setStartTimestamp(System.currentTimeMillis()); + ExecutionContext executionContext = + InstrumentationContext.get(Scenario.class, ExecutionContext.class) + .computeIfAbsent(scenarioRuntime.scenario, ExecutionContext::create); + executionContext.setStartTimestamp(System.currentTimeMillis()); + + // Indicate beforehand if the failures should be suppressed. This aligns the ordering with the + // rest of the frameworks + TestExecutionPolicy executionPolicy = executionContext.getExecutionPolicy(); + executionContext.setSuppressFailures(executionPolicy.suppressFailures()); + + scenarioRuntime.magicVariables.putIfAbsent( + KarateUtils.EXECUTION_HISTORY_MAGICVARIABLE, executionPolicy); } @Advice.OnMethodExit @@ -137,8 +146,7 @@ public static void onAddingStepResult( executionContext.setFailed(true); - TestExecutionPolicy retryPolicy = executionContext.getExecutionPolicy(); - if (retryPolicy.suppressFailures()) { + if (executionContext.getAndResetSuppressFailures()) { stepResult = new StepResult(stepResult.getStep(), KarateUtils.abortedResult()); stepResult.setFailedReason(result.getError()); stepResult.setErrorIgnored(true); From 72fa9900c671fd8c549b20f417c4b303aa0e4dd1 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Thu, 8 May 2025 12:23:25 +0200 Subject: [PATCH 02/10] align scalatest execution instrumentation ordering --- .../instrumentation/scalatest/DatadogReporter.java | 8 ++++---- .../trace/instrumentation/scalatest/RunContext.java | 12 ------------ 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java index 5b8a5e1d6a3..16f74422963 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java @@ -189,7 +189,7 @@ private static void onTestSuccess(TestSucceeded event) { new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier); TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); TestEventsHandler eventHandler = context.getEventHandler(); eventHandler.onTestFinish(testDescriptor, null, executionHistory); @@ -212,7 +212,7 @@ private static void onTestFailure(TestFailed event) { new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier); TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); TestEventsHandler eventHandler = context.getEventHandler(); eventHandler.onTestFailure(testDescriptor, throwable); @@ -274,7 +274,7 @@ private static void onTestCancel(TestCanceled event) { } TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); eventHandler.onTestFinish(testDescriptor, null, executionHistory); } @@ -299,7 +299,7 @@ private static void onTestPending(TestPending event) { eventHandler.onTestSkip(testDescriptor, reason); TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); eventHandler.onTestFinish(testDescriptor, null, executionHistory); } diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java index b200dd3ef84..338d251b2e9 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java @@ -162,18 +162,6 @@ public TestExecutionHistory getExecutionHistory(TestIdentifier testIdentifier) { return executionPolicies.get(testIdentifier); } - @Nullable - public TestExecutionHistory popExecutionHistory(TestIdentifier testIdentifier) { - TestExecutionPolicy[] holder = new TestExecutionPolicy[1]; - executionPolicies.computeIfPresent( - testIdentifier, - (ti, policy) -> { - holder[0] = policy; - return policy.applicable() ? policy : null; - }); - return holder[0]; - } - public void destroy() { eventHandler.close(); } From 64034a79369cc1801fd23873efd860189457a459 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Thu, 8 May 2025 14:13:38 +0200 Subject: [PATCH 03/10] align testng execution instrumentation ordering --- .../testng/execution/RetryAnalyzer.java | 26 ++++++++++++---- .../TestNGExecutionInstrumentation.java | 30 ++++++++++++++++++- 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java index c4df845cc88..0716a8ca2b5 100644 --- a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java +++ b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java @@ -13,12 +13,9 @@ public class RetryAnalyzer implements IRetryAnalyzer { private volatile TestExecutionPolicy executionPolicy; + private boolean suppressFailures; - @Override - public boolean retry(ITestResult result) { - if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER == null) { - return false; - } + private void setExecutionPolicy(ITestResult result) { if (executionPolicy == null) { synchronized (this) { if (executionPolicy == null) { @@ -31,10 +28,29 @@ public boolean retry(ITestResult result) { } } } + } + + @Override + public boolean retry(ITestResult result) { + if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER == null) { + return false; + } + setExecutionPolicy(result); return executionPolicy.retry( result.isSuccess(), result.getEndMillis() - result.getStartMillis()); } + public void setSuppressFailures(ITestResult result) { + setExecutionPolicy(result); + suppressFailures = executionPolicy.suppressFailures(); + } + + public boolean getAndResetSuppressFailures() { + boolean suppressFailures = this.suppressFailures; + this.suppressFailures = false; + return suppressFailures; + } + public TestExecutionHistory getExecutionHistory() { return executionPolicy; } diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java index 1a415f7afed..a04f3909ab9 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java @@ -8,13 +8,18 @@ import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.api.Config; import datadog.trace.instrumentation.testng.TestNGUtils; +import datadog.trace.instrumentation.testng.TracingListener; import datadog.trace.instrumentation.testng.execution.RetryAnalyzer; import datadog.trace.util.Strings; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.util.Collections; import java.util.Set; import net.bytebuddy.asm.Advice; import org.testng.IRetryAnalyzer; +import org.testng.ITestListener; import org.testng.ITestResult; +import org.testng.internal.ITestResultNotifier; +import org.testng.internal.TestListenerHelper; @AutoService(InstrumenterModule.class) public class TestNGExecutionInstrumentation extends InstrumenterModule.CiVisibility @@ -59,6 +64,29 @@ public String[] helperClassNames() { } public static class ExecutionAdvice { + @Advice.OnMethodEnter + public static void alignBeforeRetry( + @Advice.FieldValue(value = "m_notifier") final ITestResultNotifier resultNotifier, + @Advice.Argument(1) final ITestResult result) { + IRetryAnalyzer retryAnalyzer = TestNGUtils.getRetryAnalyzer(result); + if (retryAnalyzer instanceof RetryAnalyzer) { + // If DD's retry analyzer is used, report the test result beforehand to the tracing listener + // to align execution ordering with other frameworks (reports before `retry` method is + // called). Also avoids TestNG marking retried tests as skipped + ITestListener tracingListener = null; + for (ITestListener listener : resultNotifier.getTestListeners()) { + if (listener instanceof TracingListener) { + tracingListener = listener; + } + } + + TestListenerHelper.runTestListeners(result, Collections.singletonList(tracingListener)); + + // Also set suppress failures beforehand to align execution ordering. + ((RetryAnalyzer) retryAnalyzer).setSuppressFailures(result); + } + } + @SuppressWarnings("bytebuddy-exception-suppression") @SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL") @Advice.OnMethodExit @@ -89,7 +117,7 @@ public static void suppressFailures(@Advice.Argument(0) final ITestResult result return; } RetryAnalyzer ddRetryAnalyzer = (RetryAnalyzer) retryAnalyzer; - if (ddRetryAnalyzer.suppressFailures()) { + if (ddRetryAnalyzer.getAndResetSuppressFailures()) { // "failed but within success percentage" result.setStatus(ITestResult.SUCCESS_PERCENTAGE_FAILURE); } From f8cff99396aeef6fb73c957fe3208c038b4b027d Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Fri, 9 May 2025 17:14:26 +0200 Subject: [PATCH 04/10] refactor execution history logic after alignment --- .../domain/AbstractTestModule.java | 1 + .../domain/AbstractTestSession.java | 1 + .../trace/civisibility/domain/TestImpl.java | 24 +++++++++++ .../civisibility/domain/TestSuiteImpl.java | 1 + .../events/BuildEventsHandlerImpl.java | 2 +- .../events/NoOpTestEventsHandler.java | 3 +- .../events/TestEventsHandlerImpl.java | 16 +++---- .../trace/civisibility/execution/Regular.java | 12 ++++-- .../execution/RetryUntilSuccessful.java | 32 +++++++------- .../civisibility/execution/RunNTimes.java | 42 +++++++++++-------- .../execution/RunOnceIgnoreOutcome.java | 17 +++++--- .../trace/civisibility/utils/SpanUtils.java | 2 +- .../test/ExecutionStrategyTest.groovy | 4 +- .../junit4/CucumberTracingListener.java | 7 +++- .../Cucumber4ExecutionInstrumentation.java | 10 +---- .../junit4/MUnitTracingListener.java | 5 ++- .../MUnitExecutionInstrumentation.java | 10 +---- .../junit4/JUnit4TracingListener.java | 3 +- .../execution/FailureSuppressingNotifier.java | 10 ----- .../JUnit4ExecutionInstrumentation.java | 10 +---- .../junit5/CucumberTracingListener.java | 3 +- .../junit5/SpockTracingListener.java | 3 +- .../junit5/TracingListener.java | 3 +- .../JUnit5ExecutionInstrumentation.java | 8 +--- .../src/test/groovy/JUnit5Test.groovy | 16 +++---- .../karate/ExecutionContext.java | 20 --------- .../KarateExecutionInstrumentation.java | 6 +-- .../karate/KarateTracingHook.java | 6 ++- .../scalatest/DatadogReporter.java | 3 +- .../ScalatestExecutionInstrumentation.java | 2 +- .../execution/TestExecutionWrapper.java | 12 +----- .../testng/execution/RetryAnalyzer.java | 7 +--- .../events/TestEventsHandler.java | 3 +- .../execution/TestExecutionHistory.java | 12 ++++++ .../execution/TestExecutionPolicy.java | 14 +++---- .../civisibility/execution}/TestStatus.java | 2 +- 36 files changed, 167 insertions(+), 165 deletions(-) rename {dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain => internal-api/src/main/java/datadog/trace/api/civisibility/execution}/TestStatus.java (50%) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java index 5e99ea598f2..e87807d0ed0 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestModule.java @@ -3,6 +3,7 @@ import static datadog.trace.civisibility.Constants.CI_VISIBILITY_INSTRUMENTATION_NAME; import datadog.trace.api.Config; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.civisibility.telemetry.tag.EventType; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java index 51d2b1ceec9..51e9392732b 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/AbstractTestSession.java @@ -7,6 +7,7 @@ import datadog.trace.api.DDTraceId; import datadog.trace.api.IdGenerationStrategy; import datadog.trace.api.civisibility.CIConstants; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.civisibility.telemetry.TagValue; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index 9cb06e65b60..8bd64818847 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -14,6 +14,7 @@ import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge; import datadog.trace.api.civisibility.coverage.CoverageStore; import datadog.trace.api.civisibility.domain.TestContext; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.civisibility.telemetry.TagValue; @@ -30,6 +31,7 @@ import datadog.trace.api.civisibility.telemetry.tag.SkipReason; import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation; import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.time.SystemTimeSource; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext; import datadog.trace.bootstrap.instrumentation.api.AgentTracer; @@ -45,6 +47,7 @@ import java.lang.reflect.Method; import java.util.Collection; import java.util.Collections; +import java.util.concurrent.TimeUnit; import java.util.function.Consumer; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -65,6 +68,9 @@ public class TestImpl implements DDTest { private final TestContext context; private final TestIdentifier identifier; + private final Long startMicros; + private TestStatus status; + public TestImpl( AgentSpanContext moduleSpanContext, long suiteId, @@ -111,7 +117,10 @@ public TestImpl( .withRequestContextData(RequestContextSlot.CI_VISIBILITY, context); if (startTime != null) { + startMicros = startTime; spanBuilder = spanBuilder.withStartTimestamp(startTime); + } else { + startMicros = SystemTimeSource.INSTANCE.getCurrentTimeMicros(); } span = spanBuilder.start(); @@ -130,6 +139,7 @@ public TestImpl( span.setTag(Tags.TEST_MODULE_ID, moduleSpanContext.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, moduleSpanContext.getTraceId()); + status = TestStatus.pass; span.setTag(Tags.TEST_STATUS, TestStatus.pass); if (testClass != null && !testClass.getName().equals(testSuiteName)) { @@ -214,11 +224,14 @@ public void setTag(String key, Object value) { public void setErrorInfo(Throwable error) { span.setError(true); span.addThrowable(error); + status = TestStatus.fail; span.setTag(Tags.TEST_STATUS, TestStatus.fail); } @Override public void setSkipReason(String skipReason) { + status = TestStatus.skip; + span.setTag(Tags.TEST_STATUS, TestStatus.skip); if (skipReason != null) { span.setTag(Tags.TEST_SKIP_REASON, skipReason); @@ -231,6 +244,17 @@ public void setSkipReason(String skipReason) { } } + public TestStatus getStatus() { + return status; + } + + public long getDuration(@Nullable Long endMicros) { + if (endMicros == null) { + endMicros = SystemTimeSource.INSTANCE.getCurrentTimeMicros(); + } + return TimeUnit.MICROSECONDS.convert(endMicros - startMicros, TimeUnit.MILLISECONDS); + } + @Override public void end(@Nullable Long endTime) { closeOutstandingSpans(); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java index 732fd1493c6..24319318968 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java @@ -8,6 +8,7 @@ import datadog.trace.api.civisibility.DDTestSuite; import datadog.trace.api.civisibility.config.LibraryCapability; import datadog.trace.api.civisibility.coverage.CoverageStore; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.civisibility.telemetry.tag.EventType; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java index 705002bedbb..0af3e26f0d1 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java @@ -11,7 +11,7 @@ import datadog.trace.civisibility.config.JvmInfoFactory; import datadog.trace.civisibility.domain.BuildSystemModule; import datadog.trace.civisibility.domain.BuildSystemSession; -import datadog.trace.civisibility.domain.TestStatus; +import datadog.trace.api.civisibility.execution.TestStatus; import java.nio.file.Path; import java.util.Collection; import java.util.Map; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java index b6852d4ffba..0660bff05f2 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java @@ -89,7 +89,8 @@ public void onTestIgnore( @Nullable String testParameters, @Nullable Collection categories, @Nonnull TestSourceData testSourceData, - @Nullable String reason) { + @Nullable String reason, + @Nullable TestExecutionHistory testExecutionHistory) { // do nothing } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java index 58487a15459..0fa3894da6e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java @@ -255,11 +255,12 @@ public void onTestFinish( TestIdentifier thisTest = test.getIdentifier(); if (testExecutionHistory != null) { - if (test.hasFailed() && testExecutionHistory.hasFailedAllRetries()) { + testExecutionHistory.registerExecution(test.getStatus(), test.getDuration(endTime)); + + if (testExecutionHistory.hasFailedAllRetries()) { test.setTag(Tags.TEST_HAS_FAILED_ALL_RETRIES, true); - } else if (!test.hasFailed() - && testModule.isAttemptToFix(thisTest) - && testExecutionHistory.hasSucceededAllRetries()) { + } else if (testExecutionHistory.hasSucceededAllRetries() + && testModule.isAttemptToFix(thisTest)) { test.setTag(Tags.TEST_TEST_MANAGEMENT_ATTEMPT_TO_FIX_PASSED, true); } } @@ -277,7 +278,8 @@ public void onTestIgnore( final @Nullable String testParameters, final @Nullable Collection categories, @Nonnull TestSourceData testSourceData, - final @Nullable String reason) { + final @Nullable String reason, + @Nullable TestExecutionHistory testExecutionHistory) { onTestStart( suiteDescriptor, testDescriptor, @@ -288,9 +290,9 @@ public void onTestIgnore( categories, testSourceData, null, - null); + testExecutionHistory); onTestSkip(testDescriptor, reason); - onTestFinish(testDescriptor, null, null); + onTestFinish(testDescriptor, null, testExecutionHistory); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/Regular.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/Regular.java index 4aedac03bdd..434892bc98f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/Regular.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/Regular.java @@ -1,6 +1,7 @@ package datadog.trace.civisibility.execution; import datadog.trace.api.civisibility.execution.TestExecutionPolicy; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.tag.RetryReason; import javax.annotation.Nullable; @@ -12,17 +13,20 @@ public class Regular implements TestExecutionPolicy { private Regular() {} @Override - public boolean applicable() { - return false; + public void registerExecution(TestStatus status, long durationMillis) {} + + @Override + public boolean wasLastExecution() { + return true; } @Override - public boolean suppressFailures() { + public boolean applicable() { return false; } @Override - public boolean retry(boolean successful, long durationMillis) { + public boolean suppressFailures() { return false; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java index c2ee70faf47..dc367751a6c 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java @@ -1,6 +1,7 @@ package datadog.trace.civisibility.execution; import datadog.trace.api.civisibility.execution.TestExecutionPolicy; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.tag.RetryReason; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nullable; @@ -25,15 +26,15 @@ public RetryUntilSuccessful( } @Override - public boolean applicable() { - return !currentExecutionIsLast() || suppressFailures; + public void registerExecution(TestStatus status, long durationMillis) { + ++executions; + successfulExecutionSeen |= (status != TestStatus.fail); + totalExecutions.incrementAndGet(); } @Override - public boolean suppressFailures() { - // do not suppress failures for last execution - // (unless flag to suppress all failures is set) - return !currentExecutionIsLast() || suppressFailures; + public boolean wasLastExecution() { + return successfulExecutionSeen || executions == maxExecutions; } private boolean currentExecutionIsLast() { @@ -41,14 +42,15 @@ private boolean currentExecutionIsLast() { } @Override - public boolean retry(boolean successful, long durationMillis) { - successfulExecutionSeen |= successful; - if (!successful && ++executions < maxExecutions) { - totalExecutions.incrementAndGet(); - return true; - } else { - return false; - } + public boolean applicable() { + return !currentExecutionIsLast() || suppressFailures; + } + + @Override + public boolean suppressFailures() { + // do not suppress failures for last execution + // (unless flag to suppress all failures is set) + return !currentExecutionIsLast() || suppressFailures; } @Nullable @@ -63,7 +65,7 @@ private boolean currentExecutionIsRetry() { @Override public boolean hasFailedAllRetries() { - return currentExecutionIsLast() && !successfulExecutionSeen; + return wasLastExecution() && !successfulExecutionSeen; } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java index 574ecf88f0d..19e43deb76e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java @@ -1,6 +1,7 @@ package datadog.trace.civisibility.execution; import datadog.trace.api.civisibility.execution.TestExecutionPolicy; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.tag.RetryReason; import datadog.trace.civisibility.config.ExecutionsByDuration; import java.util.List; @@ -13,9 +14,9 @@ public class RunNTimes implements TestExecutionPolicy { private final List executionsByDuration; private int executions; private int maxExecutions; - private int totalExecutionsSeen; private int successfulExecutionsSeen; private final RetryReason retryReason; + private TestStatus lastStatus; public RunNTimes( List executionsByDuration, @@ -25,20 +26,38 @@ public RunNTimes( this.executionsByDuration = executionsByDuration; this.executions = 0; this.maxExecutions = getExecutions(0); - this.totalExecutionsSeen = 0; this.successfulExecutionsSeen = 0; this.retryReason = retryReason; } @Override - public boolean applicable() { - return !currentExecutionIsLast() || suppressFailures(); + public void registerExecution(TestStatus status, long durationMillis) { + lastStatus = status; + ++executions; + if (status != TestStatus.fail) { + ++successfulExecutionsSeen; + } + int maxExecutionsForGivenDuration = getExecutions(durationMillis); + maxExecutions = Math.min(maxExecutions, maxExecutionsForGivenDuration); + } + + @Override + public boolean wasLastExecution() { + // skipped tests (either by the framework or DD) should not be retried + return lastStatus == TestStatus.skip || executions >= maxExecutions; } private boolean currentExecutionIsLast() { + // this could give false negatives if maxExecutions is updated after the current execution is + // registered return executions == maxExecutions - 1; } + @Override + public boolean applicable() { + return !currentExecutionIsLast() || suppressFailures(); + } + @Override public boolean suppressFailures() { return suppressFailures; @@ -53,17 +72,6 @@ private int getExecutions(long durationMillis) { return 0; } - @Override - public boolean retry(boolean successful, long durationMillis) { - ++totalExecutionsSeen; - if (successful) { - ++successfulExecutionsSeen; - } - int maxExecutionsForGivenDuration = getExecutions(durationMillis); - maxExecutions = Math.min(maxExecutions, maxExecutionsForGivenDuration); - return ++executions < maxExecutions; - } - @Nullable @Override public RetryReason currentExecutionRetryReason() { @@ -76,11 +84,11 @@ private boolean currentExecutionIsRetry() { @Override public boolean hasFailedAllRetries() { - return currentExecutionIsLast() && successfulExecutionsSeen == 0; + return wasLastExecution() && successfulExecutionsSeen == 0; } @Override public boolean hasSucceededAllRetries() { - return currentExecutionIsLast() && successfulExecutionsSeen == totalExecutionsSeen; + return wasLastExecution() && successfulExecutionsSeen == executions; } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunOnceIgnoreOutcome.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunOnceIgnoreOutcome.java index 5e1fb1b6cc0..83c25e98514 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunOnceIgnoreOutcome.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunOnceIgnoreOutcome.java @@ -1,6 +1,7 @@ package datadog.trace.civisibility.execution; import datadog.trace.api.civisibility.execution.TestExecutionPolicy; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.tag.RetryReason; import javax.annotation.Nullable; @@ -12,6 +13,16 @@ public class RunOnceIgnoreOutcome implements TestExecutionPolicy { private boolean testExecuted; + @Override + public void registerExecution(TestStatus status, long durationMillis) { + testExecuted = true; + } + + @Override + public boolean wasLastExecution() { + return testExecuted; + } + @Override public boolean applicable() { return !testExecuted; @@ -22,12 +33,6 @@ public boolean suppressFailures() { return true; } - @Override - public boolean retry(boolean successful, long durationMillis) { - testExecuted = true; - return false; - } - @Nullable @Override public RetryReason currentExecutionRetryReason() { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java index 7e47ec79bfe..b665d114de2 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java @@ -2,7 +2,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.Tags; -import datadog.trace.civisibility.domain.TestStatus; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.civisibility.ipc.TestFramework; import java.util.ArrayList; import java.util.Collection; diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy index aabcb79d2ff..7d9b6106103 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy @@ -100,8 +100,8 @@ class ExecutionStrategyTest extends Specification { def strategy = givenAnExecutionStrategy(executionSettings) def policy = strategy.executionPolicy(testID, TestSourceData.UNKNOWN, []) - // retry once to get the retry reason - policy.retry(true, 0) + // register one execution to get the retry reason + policy.registerExecution(true, 0,) expect: policy.class == RunNTimes diff --git a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java index ed1e175f5e1..072c7db79d2 100644 --- a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java +++ b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java @@ -182,19 +182,22 @@ public void testIgnored(final Description description) { } else { String testSuiteName = CucumberUtils.getTestSuiteNameForScenario(description); String testName = CucumberUtils.getTestNameForScenario(description); + TestDescriptor testDescriptor = CucumberUtils.toTestDescriptor(description); List categories = getCategories(description); + TestEventsHandlerHolder.HANDLERS .get(TestFrameworkInstrumentation.CUCUMBER) .onTestIgnore( new TestSuiteDescriptor(testSuiteName, null), - CucumberUtils.toTestDescriptor(description), + testDescriptor, testName, FRAMEWORK_NAME, FRAMEWORK_VERSION, null, categories, TestSourceData.UNKNOWN, - reason); + reason, + executionHistories.get(description)); } } diff --git a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/Cucumber4ExecutionInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/Cucumber4ExecutionInstrumentation.java index 4f738e274b2..3cec47ee1af 100644 --- a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/Cucumber4ExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/Cucumber4ExecutionInstrumentation.java @@ -113,18 +113,12 @@ public static Boolean execute( FailureSuppressingNotifier failureSuppressingNotifier = new FailureSuppressingNotifier(executionPolicy, notifier); - long duration; - boolean testFailed; do { - long startTimestamp = System.currentTimeMillis(); try { runPickle.invokeWithArguments(featureRunner, pickleRunner, failureSuppressingNotifier); - testFailed = failureSuppressingNotifier.getAndResetFailedFlag(); - } catch (Throwable throwable) { - testFailed = true; + } catch (Throwable ignored) { } - duration = System.currentTimeMillis() - startTimestamp; - } while (executionPolicy.retry(!testFailed, duration)); + } while (!executionPolicy.wasLastExecution()); // skip original method return Boolean.TRUE; diff --git a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitTracingListener.java b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitTracingListener.java index 3c56443de34..7e8b8651ca9 100644 --- a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitTracingListener.java +++ b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/MUnitTracingListener.java @@ -182,7 +182,7 @@ public void testIgnored(final Description description) { .onTestSkip(testDescriptor, null); TestEventsHandlerHolder.HANDLERS .get(TestFrameworkInstrumentation.MUNIT) - .onTestFinish(testDescriptor, null, null); + .onTestFinish(testDescriptor, null, executionHistories.get(description)); } else if (testClass != null) { TestSuiteDescriptor suiteDescriptor = MUnitUtils.toSuiteDescriptor(description); @@ -246,7 +246,8 @@ private void testCaseIgnored(final Description description) { null, categories, JUnit4Utils.toTestSourceData(description), - null); + null, + executionHistories.get(description)); } private static boolean isSuiteContainingChildren(final Description description) { diff --git a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/MUnitExecutionInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/MUnitExecutionInstrumentation.java index 29dfb8e3874..a722e20b3cb 100644 --- a/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/MUnitExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/munit-junit-4/src/main/java/datadog/trace/instrumentation/junit4/execution/MUnitExecutionInstrumentation.java @@ -109,19 +109,13 @@ public static Future apply( FailureSuppressingNotifier failureSuppressingNotifier = new FailureSuppressingNotifier(executionPolicy, notifier); - long duration; - boolean testFailed; do { - long startTimestamp = System.currentTimeMillis(); try { runTest.setAccessible(true); result = (Future) runTest.invoke(runner, failureSuppressingNotifier, test); - testFailed = failureSuppressingNotifier.getAndResetFailedFlag(); - } catch (Throwable throwable) { - testFailed = true; + } catch (Throwable ignored) { } - duration = System.currentTimeMillis() - startTimestamp; - } while (executionPolicy.retry(!testFailed, duration)); + } while (!executionPolicy.wasLastExecution()); // skip original method return result; diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java index 5adf3795e45..d50f4fadf4c 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/JUnit4TracingListener.java @@ -235,6 +235,7 @@ private void testIgnored(Description description, Method testMethod, String reas testParameters, categories, testSourceData, - reason); + reason, + executionHistories.get(description)); } } diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/FailureSuppressingNotifier.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/FailureSuppressingNotifier.java index d09c5878e3a..4002cbd5016 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/FailureSuppressingNotifier.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/FailureSuppressingNotifier.java @@ -12,8 +12,6 @@ public class FailureSuppressingNotifier extends RunNotifier { private final TestExecutionPolicy executionPolicy; - private boolean failed; - public FailureSuppressingNotifier(TestExecutionPolicy executionPolicy, RunNotifier notifier) { this.executionPolicy = executionPolicy; @@ -25,8 +23,6 @@ public FailureSuppressingNotifier(TestExecutionPolicy executionPolicy, RunNotifi @Override public void fireTestFailure(Failure failure) { - this.failed = true; - if (!executionPolicy.suppressFailures()) { super.fireTestFailure(failure); return; @@ -44,10 +40,4 @@ public void fireTestFailure(Failure failure) { } } } - - public boolean getAndResetFailedFlag() { - boolean failed = this.failed; - this.failed = false; - return failed; - } } diff --git a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/JUnit4ExecutionInstrumentation.java b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/JUnit4ExecutionInstrumentation.java index 3e656415f4b..2f356a2d7b9 100644 --- a/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/JUnit4ExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-4.10/src/main/java/datadog/trace/instrumentation/junit4/execution/JUnit4ExecutionInstrumentation.java @@ -118,19 +118,13 @@ public static Boolean apply( FailureSuppressingNotifier failureSuppressingNotifier = new FailureSuppressingNotifier(executionPolicy, notifier); - long duration; - boolean testFailed; do { - long startTimestamp = System.currentTimeMillis(); try { runTest.setAccessible(true); runTest.invoke(runner, statement, description, failureSuppressingNotifier); - testFailed = failureSuppressingNotifier.getAndResetFailedFlag(); - } catch (Throwable throwable) { - testFailed = true; + } catch (Throwable ignored) { } - duration = System.currentTimeMillis() - startTimestamp; - } while (executionPolicy.retry(!testFailed, duration)); + } while (!executionPolicy.wasLastExecution()); // skip original method return Boolean.TRUE; diff --git a/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/CucumberTracingListener.java b/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/CucumberTracingListener.java index f2bb1e7ee3c..ce8daeb90e4 100644 --- a/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/CucumberTracingListener.java +++ b/dd-java-agent/instrumentation/junit-5.3/cucumber-junit-5/src/main/java/datadog/trace/instrumentation/junit5/CucumberTracingListener.java @@ -196,6 +196,7 @@ private void testResourceExecutionSkipped( null, tags, TestSourceData.UNKNOWN, - reason); + reason, + TestEventsHandlerHolder.getExecutionHistory(testDescriptor)); } } diff --git a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockTracingListener.java b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockTracingListener.java index 4288c03917c..f88cf5c5852 100644 --- a/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockTracingListener.java +++ b/dd-java-agent/instrumentation/junit-5.3/spock-junit-5/src/main/java/datadog/trace/instrumentation/junit5/SpockTracingListener.java @@ -237,6 +237,7 @@ private void testMethodExecutionSkipped( testParameters, tags, testSourceData, - reason); + reason, + TestEventsHandlerHolder.getExecutionHistory(testDescriptor)); } } diff --git a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TracingListener.java b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TracingListener.java index e1b4cb8d3c0..8f4f693fc7e 100644 --- a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TracingListener.java +++ b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/TracingListener.java @@ -240,6 +240,7 @@ private void testMethodExecutionSkipped( testParameters, tags, testSourceData, - reason); + reason, + TestEventsHandlerHolder.getExecutionHistory(testDescriptor)); } } diff --git a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java index c464c299f0b..3f22d470a9c 100644 --- a/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/junit-5.3/src/main/java/datadog/trace/instrumentation/junit5/execution/JUnit5ExecutionInstrumentation.java @@ -160,22 +160,16 @@ public static Boolean execute(@Advice.This HierarchicalTestExecutorService.TestT TestDescriptorHandle descriptorHandle = new TestDescriptorHandle(testDescriptor); int retryAttemptIdx = 0; - boolean retry; while (true) { factory.setSuppressFailures(executionPolicy.suppressFailures()); - long startTimestamp = System.currentTimeMillis(); CallDepthThreadLocalMap.incrementCallDepth(HierarchicalTestExecutorService.TestTask.class); testTask.execute(); CallDepthThreadLocalMap.decrementCallDepth(HierarchicalTestExecutorService.TestTask.class); - long duration = System.currentTimeMillis() - startTimestamp; - Throwable error = factory.getCollector().getThrowable(); factory.setSuppressFailures(false); // restore default behavior - boolean success = error == null || JUnitPlatformUtils.isAssumptionFailure(error); - retry = executionPolicy.retry(success, duration); - if (!retry) { + if (executionPolicy.wasLastExecution()) { break; } diff --git a/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy b/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy index 20e5f45aa4d..356bcf9537b 100644 --- a/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy +++ b/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy @@ -122,15 +122,15 @@ class JUnit5Test extends CiVisibilityInstrumentationTest { where: testcaseName | success | tests | retriedTests - "test-failed" | false | [TestFailed] | [] - "test-retry-failed" | false | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] - "test-failed-then-succeed" | true | [TestFailedThenSucceed] | [new TestFQN("org.example.TestFailedThenSucceed", "test_failed_then_succeed")] - "test-retry-template" | false | [TestFailedTemplate] | [new TestFQN("org.example.TestFailedTemplate", "test_template")] - "test-retry-factory" | false | [TestFailedFactory] | [new TestFQN("org.example.TestFailedFactory", "test_factory")] - "test-assumption-is-not-retried" | true | [TestAssumption] | [new TestFQN("org.example.TestAssumption", "test_fail_assumption")] + //"test-failed" | false | [TestFailed] | [] + //"test-retry-failed" | false | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] + //"test-failed-then-succeed" | true | [TestFailedThenSucceed] | [new TestFQN("org.example.TestFailedThenSucceed", "test_failed_then_succeed")] + //"test-retry-template" | false | [TestFailedTemplate] | [new TestFQN("org.example.TestFailedTemplate", "test_template")] + //"test-retry-factory" | false | [TestFailedFactory] | [new TestFQN("org.example.TestFailedFactory", "test_factory")] + //"test-assumption-is-not-retried" | true | [TestAssumption] | [new TestFQN("org.example.TestAssumption", "test_fail_assumption")] "test-skipped-is-not-retried" | true | [TestSkipped] | [new TestFQN("org.example.TestSkipped", "test_skipped")] - "test-retry-parameterized" | false | [TestFailedParameterized] | [new TestFQN("org.example.TestFailedParameterized", "test_failed_parameterized")] - "test-expected-exception-is-not-retried" | true | [TestSucceedExpectedException] | [new TestFQN("org.example.TestSucceedExpectedException", "test_succeed")] + //"test-retry-parameterized" | false | [TestFailedParameterized] | [new TestFQN("org.example.TestFailedParameterized", "test_failed_parameterized")] + //"test-expected-exception-is-not-retried" | true | [TestSucceedExpectedException] | [new TestFQN("org.example.TestSucceedExpectedException", "test_succeed")] } def "test early flakiness detection #testcaseName"() { diff --git a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java index 18ad5412f46..fbcc6c37d90 100644 --- a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java +++ b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/ExecutionContext.java @@ -10,31 +10,11 @@ public class ExecutionContext { private final TestExecutionPolicy executionPolicy; private boolean suppressFailures; - private boolean failed; - private long startTimestamp; public ExecutionContext(TestExecutionPolicy executionPolicy) { this.executionPolicy = executionPolicy; } - public void setStartTimestamp(long startTimestamp) { - this.startTimestamp = startTimestamp; - } - - public long getStartTimestamp() { - return startTimestamp; - } - - public void setFailed(boolean failed) { - this.failed = failed; - } - - public boolean getAndResetFailed() { - boolean failed = this.failed; - this.failed = false; - return failed; - } - public void setSuppressFailures(boolean suppressFailures) { this.suppressFailures = suppressFailures; } diff --git a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java index 1ae34f6ade7..83656a22f8c 100644 --- a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateExecutionInstrumentation.java @@ -81,7 +81,6 @@ public static void beforeExecute(@Advice.This ScenarioRuntime scenarioRuntime) { ExecutionContext executionContext = InstrumentationContext.get(Scenario.class, ExecutionContext.class) .computeIfAbsent(scenarioRuntime.scenario, ExecutionContext::create); - executionContext.setStartTimestamp(System.currentTimeMillis()); // Indicate beforehand if the failures should be suppressed. This aligns the ordering with the // rest of the frameworks @@ -109,8 +108,7 @@ public static void afterExecute(@Advice.This ScenarioRuntime scenarioRuntime) { ScenarioResult finalResult = scenarioRuntime.result; TestExecutionPolicy executionPolicy = context.getExecutionPolicy(); - long duration = System.currentTimeMillis() - context.getStartTimestamp(); - while (executionPolicy.retry(!context.getAndResetFailed(), duration)) { + while (!executionPolicy.wasLastExecution()) { ScenarioRuntime retry = new ScenarioRuntime(scenarioRuntime.featureRuntime, scenarioRuntime.scenario); retry.magicVariables.put(KarateUtils.EXECUTION_HISTORY_MAGICVARIABLE, executionPolicy); @@ -144,8 +142,6 @@ public static void onAddingStepResult( return; } - executionContext.setFailed(true); - if (executionContext.getAndResetSuppressFailures()) { stepResult = new StepResult(stepResult.getStep(), KarateUtils.abortedResult()); stepResult.setFailedReason(result.getError()); diff --git a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java index d0bbfec9372..f400c2a8c0f 100644 --- a/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java +++ b/dd-java-agent/instrumentation/karate/src/main/java/datadog/trace/instrumentation/karate/KarateTracingHook.java @@ -120,6 +120,9 @@ public boolean beforeScenario(ScenarioRuntime sr) { if (skipReason != null && !(skipReason == SkipReason.ITR && categories.contains(CIConstants.Tags.ITR_UNSKIPPABLE_TAG))) { + TestExecutionHistory executionHistory = + (TestExecutionHistory) + sr.magicVariables.get(KarateUtils.EXECUTION_HISTORY_MAGICVARIABLE); TestEventsHandlerHolder.TEST_EVENTS_HANDLER.onTestIgnore( suiteDescriptor, testDescriptor, @@ -129,7 +132,8 @@ public boolean beforeScenario(ScenarioRuntime sr) { parameters, categories, TestSourceData.UNKNOWN, - skipReason.getDescription()); + skipReason.getDescription(), + executionHistory); return false; } } diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java index 16f74422963..d198a8f154e 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java @@ -246,7 +246,8 @@ private static void onTestIgnore(TestIgnored event) { testParameters, categories, new TestSourceData(testClass, null, null), - reason != null ? reason.getDescription() : null); + reason != null ? reason.getDescription() : null, + context.getExecutionHistory(skippableTest)); } private static void onTestCancel(TestCanceled event) { diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/ScalatestExecutionInstrumentation.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/ScalatestExecutionInstrumentation.java index 2c662b378e2..187caa69b2d 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/ScalatestExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/ScalatestExecutionInstrumentation.java @@ -107,7 +107,7 @@ public static void afterTest( @Advice.Return(readOnly = false) Status status) throws Throwable { TestExecutionWrapper invokeWrapper = (TestExecutionWrapper) invokeWithFixture; - if (invokeWrapper.retry()) { + if (!invokeWrapper.wasLastExecution()) { status = (Status) runTest.invokeWithArguments( diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/TestExecutionWrapper.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/TestExecutionWrapper.java index 3741f5aa2e3..c2aeb2839af 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/TestExecutionWrapper.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/execution/TestExecutionWrapper.java @@ -10,9 +10,6 @@ public class TestExecutionWrapper implements scala.Function1.Test private final scala.Function1.TestLeaf, Outcome> delegate; private final TestExecutionPolicy executionPolicy; - private boolean executionFailed; - private long duration; - public TestExecutionWrapper( Function1.TestLeaf, Outcome> delegate, TestExecutionPolicy executionPolicy) { this.delegate = delegate; @@ -21,14 +18,9 @@ public TestExecutionWrapper( @Override public Outcome apply(SuperEngine.TestLeaf testLeaf) { - executionFailed = false; - - long startTimestamp = System.currentTimeMillis(); Outcome outcome = delegate.apply(testLeaf); - duration = System.currentTimeMillis() - startTimestamp; if (outcome.isFailed()) { - executionFailed = true; if (executionPolicy.suppressFailures()) { Throwable t = outcome.toOption().get(); return Canceled.apply( @@ -39,7 +31,7 @@ public Outcome apply(SuperEngine.TestLeaf testLeaf) { return outcome; } - public boolean retry() { - return executionPolicy.retry(!executionFailed, duration); + public boolean wasLastExecution() { + return executionPolicy.wasLastExecution(); } } diff --git a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java index 0716a8ca2b5..24f0021451e 100644 --- a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java +++ b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java @@ -36,8 +36,7 @@ public boolean retry(ITestResult result) { return false; } setExecutionPolicy(result); - return executionPolicy.retry( - result.isSuccess(), result.getEndMillis() - result.getStartMillis()); + return !executionPolicy.wasLastExecution(); } public void setSuppressFailures(ITestResult result) { @@ -54,8 +53,4 @@ public boolean getAndResetSuppressFailures() { public TestExecutionHistory getExecutionHistory() { return executionPolicy; } - - public boolean suppressFailures() { - return executionPolicy != null && executionPolicy.suppressFailures(); - } } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java index 4709fa25a05..869b8f6cc9b 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/events/TestEventsHandler.java @@ -86,7 +86,8 @@ void onTestIgnore( @Nullable String testParameters, @Nullable Collection categories, @Nonnull TestSourceData testSourceData, - @Nullable String reason); + @Nullable String reason, + @Nullable TestExecutionHistory testExecutionHistory); @Nonnull TestExecutionPolicy executionPolicy( diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java index bd48195d0c7..71e4328ab3b 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java @@ -5,6 +5,18 @@ public interface TestExecutionHistory { + /** + * @param status result of the execution: pass, fail or skip + * @param durationMillis duration of current test execution in milliseconds + */ + void registerExecution(TestStatus status, long durationMillis); + + /** + * @return {@code true} if the last execution registered was the last one (only for policies that + * allow multiple retries) + */ + boolean wasLastExecution(); + /** * @return retry reason for current test execution ({@code null} if current execution is not a * retry) diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionPolicy.java b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionPolicy.java index 5489a809548..00de8d306e4 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionPolicy.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionPolicy.java @@ -3,6 +3,9 @@ public interface TestExecutionPolicy extends TestExecutionHistory { /** + * Must be called before the execution is registered by {@link + * TestExecutionHistory#registerExecution(TestStatus, long)}. + * * @return {@code true} if the next execution of the test will be altered in any way. This method * is used to avoid unnecessary performance penalty: if it returns {@code false}, * instrumentation code needed to alter test behavior will be skipped. @@ -10,16 +13,11 @@ public interface TestExecutionPolicy extends TestExecutionHistory { boolean applicable(); /** + * Must be called before the execution is registered by {@link + * TestExecutionHistory#registerExecution(TestStatus, long)}. + * * @return {@code true} if failure of the next execution of the test should not affect build * result */ boolean suppressFailures(); - - /** - * @param successful {@code true} if current test execution passed or was skipped, {@code false} - * otherwise - * @param durationMillis duration of current test execution in milliseconds - * @return {@code true} if another execution of the same test should be done - */ - boolean retry(boolean successful, long durationMillis); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestStatus.java b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestStatus.java similarity index 50% rename from dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestStatus.java rename to internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestStatus.java index 57e985f5da3..00fb5ddf904 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestStatus.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestStatus.java @@ -1,4 +1,4 @@ -package datadog.trace.civisibility.domain; +package datadog.trace.api.civisibility.execution; public enum TestStatus { pass, From ad337c0ba46c4491dac24467e492ca5622f3f1c0 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 12 May 2025 09:29:33 +0200 Subject: [PATCH 05/10] fix test duration calculation --- .../trace/civisibility/domain/TestImpl.java | 4 ++-- .../events/BuildEventsHandlerImpl.java | 2 +- .../trace/civisibility/utils/SpanUtils.java | 2 +- .../src/test/groovy/JUnit413Test.groovy | 2 +- .../junit-5.8/src/test/groovy/JUnit58Test.groovy | 2 +- .../junit-5.3/src/test/groovy/JUnit5Test.groovy | 16 ++++++++-------- .../execution/TestExecutionHistory.java | 2 +- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index 8bd64818847..fbf54602007 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -68,7 +68,7 @@ public class TestImpl implements DDTest { private final TestContext context; private final TestIdentifier identifier; - private final Long startMicros; + private Long startMicros; private TestStatus status; public TestImpl( @@ -252,7 +252,7 @@ public long getDuration(@Nullable Long endMicros) { if (endMicros == null) { endMicros = SystemTimeSource.INSTANCE.getCurrentTimeMicros(); } - return TimeUnit.MICROSECONDS.convert(endMicros - startMicros, TimeUnit.MILLISECONDS); + return TimeUnit.MICROSECONDS.toMillis(endMicros - startMicros); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java index 0af3e26f0d1..75ea9a4291b 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/BuildEventsHandlerImpl.java @@ -5,13 +5,13 @@ import datadog.trace.api.civisibility.domain.BuildSessionSettings; import datadog.trace.api.civisibility.domain.JavaAgent; import datadog.trace.api.civisibility.events.BuildEventsHandler; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.civisibility.config.JvmInfo; import datadog.trace.civisibility.config.JvmInfoFactory; import datadog.trace.civisibility.domain.BuildSystemModule; import datadog.trace.civisibility.domain.BuildSystemSession; -import datadog.trace.api.civisibility.execution.TestStatus; import java.nio.file.Path; import java.util.Collection; import java.util.Map; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java index b665d114de2..64d154292b3 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/utils/SpanUtils.java @@ -1,8 +1,8 @@ package datadog.trace.civisibility.utils; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.Tags; -import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.civisibility.ipc.TestFramework; import java.util.ArrayList; import java.util.Collection; diff --git a/dd-java-agent/instrumentation/junit-4.10/junit-4.13/src/test/groovy/JUnit413Test.groovy b/dd-java-agent/instrumentation/junit-4.10/junit-4.13/src/test/groovy/JUnit413Test.groovy index 993b48576e0..6b140020064 100644 --- a/dd-java-agent/instrumentation/junit-4.10/junit-4.13/src/test/groovy/JUnit413Test.groovy +++ b/dd-java-agent/instrumentation/junit-4.10/junit-4.13/src/test/groovy/JUnit413Test.groovy @@ -24,7 +24,7 @@ class JUnit413Test extends CiVisibilityInstrumentationTest { def runner = new JUnitCore() - def "test #testcaseName"() { + def "test setup teardown methods #testcaseName"() { runTests(tests, success) assertSpansData(testcaseName) diff --git a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy index 46cc26e1d76..f2add590963 100644 --- a/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy +++ b/dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/test/groovy/JUnit58Test.groovy @@ -23,7 +23,7 @@ import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass @DisableTestTrace(reason = "avoid self-tracing") class JUnit58Test extends CiVisibilityInstrumentationTest { - def "test #testcaseName"() { + def "test setup teardown methods #testcaseName"() { runTests(tests, success) assertSpansData(testcaseName) diff --git a/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy b/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy index 356bcf9537b..20e5f45aa4d 100644 --- a/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy +++ b/dd-java-agent/instrumentation/junit-5.3/src/test/groovy/JUnit5Test.groovy @@ -122,15 +122,15 @@ class JUnit5Test extends CiVisibilityInstrumentationTest { where: testcaseName | success | tests | retriedTests - //"test-failed" | false | [TestFailed] | [] - //"test-retry-failed" | false | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] - //"test-failed-then-succeed" | true | [TestFailedThenSucceed] | [new TestFQN("org.example.TestFailedThenSucceed", "test_failed_then_succeed")] - //"test-retry-template" | false | [TestFailedTemplate] | [new TestFQN("org.example.TestFailedTemplate", "test_template")] - //"test-retry-factory" | false | [TestFailedFactory] | [new TestFQN("org.example.TestFailedFactory", "test_factory")] - //"test-assumption-is-not-retried" | true | [TestAssumption] | [new TestFQN("org.example.TestAssumption", "test_fail_assumption")] + "test-failed" | false | [TestFailed] | [] + "test-retry-failed" | false | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] + "test-failed-then-succeed" | true | [TestFailedThenSucceed] | [new TestFQN("org.example.TestFailedThenSucceed", "test_failed_then_succeed")] + "test-retry-template" | false | [TestFailedTemplate] | [new TestFQN("org.example.TestFailedTemplate", "test_template")] + "test-retry-factory" | false | [TestFailedFactory] | [new TestFQN("org.example.TestFailedFactory", "test_factory")] + "test-assumption-is-not-retried" | true | [TestAssumption] | [new TestFQN("org.example.TestAssumption", "test_fail_assumption")] "test-skipped-is-not-retried" | true | [TestSkipped] | [new TestFQN("org.example.TestSkipped", "test_skipped")] - //"test-retry-parameterized" | false | [TestFailedParameterized] | [new TestFQN("org.example.TestFailedParameterized", "test_failed_parameterized")] - //"test-expected-exception-is-not-retried" | true | [TestSucceedExpectedException] | [new TestFQN("org.example.TestSucceedExpectedException", "test_succeed")] + "test-retry-parameterized" | false | [TestFailedParameterized] | [new TestFQN("org.example.TestFailedParameterized", "test_failed_parameterized")] + "test-expected-exception-is-not-retried" | true | [TestSucceedExpectedException] | [new TestFQN("org.example.TestSucceedExpectedException", "test_succeed")] } def "test early flakiness detection #testcaseName"() { diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java index 71e4328ab3b..e97aa2b458c 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/execution/TestExecutionHistory.java @@ -6,7 +6,7 @@ public interface TestExecutionHistory { /** - * @param status result of the execution: pass, fail or skip + * @param status result of the execution: pass, fail or skip * @param durationMillis duration of current test execution in milliseconds */ void registerExecution(TestStatus status, long durationMillis); From a5087e146c1fe3f9f0cef861ec34d5a5b0119344 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 12 May 2025 11:20:43 +0200 Subject: [PATCH 06/10] fix testng execution history creation and fixtures --- .../testng/execution/RetryAnalyzer.java | 6 ++--- .../TestNGExecutionInstrumentation.java | 11 +++++---- .../events.ftl | 2 +- .../events.ftl | 23 ++++++++++--------- .../test-attempt-to-fix-failed/events.ftl | 2 +- .../events.ftl | 2 +- .../events.ftl | 23 ++++++++++--------- .../test-attempt-to-fix-succeeded/events.ftl | 23 ++++++++++--------- .../events.ftl | 10 ++++---- .../events.ftl | 8 +++---- .../events.ftl | 22 +++++++++--------- .../test-efd-new-slow-test/events.ftl | 10 ++++---- .../resources/test-efd-new-test/events.ftl | 14 +++++------ .../test-quarantined-failed-atr/events.ftl | 2 +- .../test-quarantined-failed-efd/events.ftl | 2 +- .../resources/test-retry-error/events.ftl | 2 +- .../resources/test-retry-failed/events.ftl | 2 +- .../test-retry-parameterized/events.ftl | 2 +- 18 files changed, 86 insertions(+), 80 deletions(-) diff --git a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java index 24f0021451e..b8459ad36db 100644 --- a/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java +++ b/dd-java-agent/instrumentation/testng/src/main/java/datadog/trace/instrumentation/testng/execution/RetryAnalyzer.java @@ -15,7 +15,7 @@ public class RetryAnalyzer implements IRetryAnalyzer { private volatile TestExecutionPolicy executionPolicy; private boolean suppressFailures; - private void setExecutionPolicy(ITestResult result) { + public void createExecutionPolicy(ITestResult result) { if (executionPolicy == null) { synchronized (this) { if (executionPolicy == null) { @@ -35,12 +35,12 @@ public boolean retry(ITestResult result) { if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER == null) { return false; } - setExecutionPolicy(result); + createExecutionPolicy(result); return !executionPolicy.wasLastExecution(); } public void setSuppressFailures(ITestResult result) { - setExecutionPolicy(result); + createExecutionPolicy(result); suppressFailures = executionPolicy.suppressFailures(); } diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java index a04f3909ab9..9232c93b834 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java @@ -70,9 +70,12 @@ public static void alignBeforeRetry( @Advice.Argument(1) final ITestResult result) { IRetryAnalyzer retryAnalyzer = TestNGUtils.getRetryAnalyzer(result); if (retryAnalyzer instanceof RetryAnalyzer) { - // If DD's retry analyzer is used, report the test result beforehand to the tracing listener - // to align execution ordering with other frameworks (reports before `retry` method is - // called). Also avoids TestNG marking retried tests as skipped + // If DD's retry analyzer is used, create the execution history and report the test result + // beforehand to the tracing listener to align execution ordering with other frameworks. + // Also avoids TestNG marking retried tests as skipped + RetryAnalyzer ddRetryAnalyzer = (RetryAnalyzer) retryAnalyzer; + ddRetryAnalyzer.createExecutionPolicy(result); + ITestListener tracingListener = null; for (ITestListener listener : resultNotifier.getTestListeners()) { if (listener instanceof TracingListener) { @@ -83,7 +86,7 @@ public static void alignBeforeRetry( TestListenerHelper.runTestListeners(result, Collections.singletonList(tracingListener)); // Also set suppress failures beforehand to align execution ordering. - ((RetryAnalyzer) retryAnalyzer).setSuppressFailures(result); + ddRetryAnalyzer.setSuppressFailures(result); } } diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-failed/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-failed/events.ftl index 13e664ad760..dfd78d2e054 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-failed/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-failed/events.ftl @@ -214,7 +214,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", @@ -269,6 +268,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-succeeded/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-succeeded/events.ftl index bc64b19b217..248c5daad4c 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-succeeded/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-disabled-succeeded/events.ftl @@ -14,7 +14,7 @@ "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", "test.source.file" : "dummy_source_path", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -55,7 +55,7 @@ "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_test_disabled" : "true", @@ -86,7 +86,7 @@ }, { "content" : { "duration" : ${content_duration_3}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -106,7 +106,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_test_disabled" : "true", @@ -137,7 +137,7 @@ }, { "content" : { "duration" : ${content_duration_4}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -157,7 +157,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_test_disabled" : "true", @@ -188,7 +188,7 @@ }, { "content" : { "duration" : ${content_duration_5}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -208,7 +208,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_test_disabled" : "true", @@ -261,6 +261,7 @@ "test.source.method" : "test_succeed()V", "test.status" : "pass", "test.suite" : "org.example.TestSucceed", + "test.test_management.attempt_to_fix_passed" : "true", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_test_disabled" : "true", "test.type" : "test", @@ -305,7 +306,7 @@ "test.command" : "testng-7", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.status" : "fail", + "test.status" : "pass", "test.test_management.enabled" : "true", "test.type" : "test", "test_session.name" : "session-name" @@ -338,7 +339,7 @@ "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", - "test.status" : "fail", + "test.status" : "pass", "test.test_management.enabled" : "true", "test.type" : "test", "test_session.name" : "session-name" diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-failed/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-failed/events.ftl index c7ed3b42378..fdf7aa3959d 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-failed/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-failed/events.ftl @@ -211,7 +211,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", @@ -265,6 +264,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-failed/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-failed/events.ftl index 2ab6a803578..1b12a1db556 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-failed/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-failed/events.ftl @@ -214,7 +214,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", @@ -269,6 +268,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-succeeded/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-succeeded/events.ftl index 9d33a0c7173..76cd43fdfc0 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-succeeded/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-quarantined-succeeded/events.ftl @@ -14,7 +14,7 @@ "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", "test.source.file" : "dummy_source_path", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -55,7 +55,7 @@ "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_quarantined" : "true", @@ -86,7 +86,7 @@ }, { "content" : { "duration" : ${content_duration_3}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -106,7 +106,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_quarantined" : "true", @@ -137,7 +137,7 @@ }, { "content" : { "duration" : ${content_duration_4}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -157,7 +157,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_quarantined" : "true", @@ -188,7 +188,7 @@ }, { "content" : { "duration" : ${content_duration_5}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -208,7 +208,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_quarantined" : "true", @@ -261,6 +261,7 @@ "test.source.method" : "test_succeed()V", "test.status" : "pass", "test.suite" : "org.example.TestSucceed", + "test.test_management.attempt_to_fix_passed" : "true", "test.test_management.is_attempt_to_fix" : "true", "test.test_management.is_quarantined" : "true", "test.type" : "test", @@ -305,7 +306,7 @@ "test.command" : "testng-7", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.status" : "fail", + "test.status" : "pass", "test.test_management.enabled" : "true", "test.type" : "test", "test_session.name" : "session-name" @@ -338,7 +339,7 @@ "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", - "test.status" : "fail", + "test.status" : "pass", "test.test_management.enabled" : "true", "test.type" : "test", "test_session.name" : "session-name" diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-succeeded/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-succeeded/events.ftl index c2c9ef8fa63..a462d4d9390 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-succeeded/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-attempt-to-fix-succeeded/events.ftl @@ -14,7 +14,7 @@ "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", "test.source.file" : "dummy_source_path", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -55,7 +55,7 @@ "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.type" : "test", @@ -85,7 +85,7 @@ }, { "content" : { "duration" : ${content_duration_3}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -105,7 +105,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.type" : "test", @@ -135,7 +135,7 @@ }, { "content" : { "duration" : ${content_duration_4}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -155,7 +155,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.type" : "test", @@ -185,7 +185,7 @@ }, { "content" : { "duration" : ${content_duration_5}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -205,7 +205,7 @@ "test.retry_reason" : "attempt_to_fix", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.test_management.is_attempt_to_fix" : "true", "test.type" : "test", @@ -257,6 +257,7 @@ "test.source.method" : "test_succeed()V", "test.status" : "pass", "test.suite" : "org.example.TestSucceed", + "test.test_management.attempt_to_fix_passed" : "true", "test.test_management.is_attempt_to_fix" : "true", "test.type" : "test", "test_session.name" : "session-name" @@ -300,7 +301,7 @@ "test.command" : "testng-7", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.status" : "fail", + "test.status" : "pass", "test.test_management.enabled" : "true", "test.type" : "test", "test_session.name" : "session-name" @@ -333,7 +334,7 @@ "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", - "test.status" : "fail", + "test.status" : "pass", "test.test_management.enabled" : "true", "test.type" : "test", "test_session.name" : "session-name" diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-faulty-session-threshold/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-faulty-session-threshold/events.ftl index 84f9cc8d492..6759513e589 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-faulty-session-threshold/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-faulty-session-threshold/events.ftl @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -56,7 +56,7 @@ "test.name" : "test_another_succeed", "test.source.file" : "dummy_source_path", "test.source.method" : "test_another_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestFailedAndSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -85,7 +85,7 @@ }, { "content" : { "duration" : ${content_duration_3}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -106,7 +106,7 @@ "test.retry_reason" : "early_flake_detection", "test.source.file" : "dummy_source_path", "test.source.method" : "test_another_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestFailedAndSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -253,7 +253,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_new" : "true", "test.is_retry" : "true", "test.module" : "testng-7", @@ -307,6 +306,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_new" : "true", "test.is_retry" : "true", "test.module" : "testng-7", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-known-tests-and-new-test/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-known-tests-and-new-test/events.ftl index a75b977ae37..145ee78c010 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-known-tests-and-new-test/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-known-tests-and-new-test/events.ftl @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -56,7 +56,7 @@ "test.name" : "test_another_succeed", "test.source.file" : "dummy_source_path", "test.source.method" : "test_another_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestFailedAndSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -85,7 +85,7 @@ }, { "content" : { "duration" : ${content_duration_3}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -106,7 +106,7 @@ "test.retry_reason" : "early_flake_detection", "test.source.file" : "dummy_source_path", "test.source.method" : "test_another_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestFailedAndSucceed", "test.type" : "test", "test_session.name" : "session-name" diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-parameterized-test/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-parameterized-test/events.ftl index 08d2ff98cef..26c43529215 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-parameterized-test/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-parameterized-test/events.ftl @@ -14,7 +14,7 @@ "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", "test.source.file" : "dummy_source_path", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestParameterized", "test.type" : "test", "test_session.name" : "session-name" @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -57,7 +57,7 @@ "test.parameters" : "{\"arguments\":{\"0\":\"hello\",\"1\":\"true\"}}", "test.source.file" : "dummy_source_path", "test.source.method" : "parameterized_test_succeed(Ljava/lang/String;Z)V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestParameterized", "test.type" : "test", "test_session.name" : "session-name" @@ -86,7 +86,7 @@ }, { "content" : { "duration" : ${content_duration_3}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -108,7 +108,7 @@ "test.retry_reason" : "early_flake_detection", "test.source.file" : "dummy_source_path", "test.source.method" : "parameterized_test_succeed(Ljava/lang/String;Z)V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestParameterized", "test.type" : "test", "test_session.name" : "session-name" @@ -188,7 +188,7 @@ }, { "content" : { "duration" : ${content_duration_5}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -208,7 +208,7 @@ "test.parameters" : "{\"arguments\":{\"0\":\"\\\"goodbye\\\"\",\"1\":\"false\"}}", "test.source.file" : "dummy_source_path", "test.source.method" : "parameterized_test_succeed(Ljava/lang/String;Z)V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestParameterized", "test.type" : "test", "test_session.name" : "session-name" @@ -237,7 +237,7 @@ }, { "content" : { "duration" : ${content_duration_6}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -259,7 +259,7 @@ "test.retry_reason" : "early_flake_detection", "test.source.file" : "dummy_source_path", "test.source.method" : "parameterized_test_succeed(Ljava/lang/String;Z)V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestParameterized", "test.type" : "test", "test_session.name" : "session-name" @@ -356,7 +356,7 @@ "test.early_flake.enabled" : "true", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.status" : "fail", + "test.status" : "pass", "test.type" : "test", "test_session.name" : "session-name" }, @@ -390,7 +390,7 @@ "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", - "test.status" : "fail", + "test.status" : "pass", "test.type" : "test", "test_session.name" : "session-name" }, diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-slow-test/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-slow-test/events.ftl index aab28578694..7cc5b33cce3 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-slow-test/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-slow-test/events.ftl @@ -14,7 +14,7 @@ "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", "test.source.file" : "dummy_source_path", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceedSlow", "test.type" : "test", "test_session.name" : "session-name" @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -56,7 +56,7 @@ "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceedSlow", "test.type" : "test", "test_session.name" : "session-name" @@ -151,7 +151,7 @@ "test.early_flake.enabled" : "true", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.status" : "fail", + "test.status" : "pass", "test.type" : "test", "test_session.name" : "session-name" }, @@ -184,7 +184,7 @@ "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", - "test.status" : "fail", + "test.status" : "pass", "test.type" : "test", "test_session.name" : "session-name" }, diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-test/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-test/events.ftl index ab51201b7d2..7a0cceef770 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-test/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-efd-new-test/events.ftl @@ -14,7 +14,7 @@ "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", "test.source.file" : "dummy_source_path", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -37,7 +37,7 @@ }, { "content" : { "duration" : ${content_duration_2}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -56,7 +56,7 @@ "test.name" : "test_succeed", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -85,7 +85,7 @@ }, { "content" : { "duration" : ${content_duration_3}, - "error" : 1, + "error" : 0, "meta" : { "_dd.profiling.ctx" : "test", "_dd.tracer_host" : ${content_meta__dd_tracer_host}, @@ -106,7 +106,7 @@ "test.retry_reason" : "early_flake_detection", "test.source.file" : "dummy_source_path", "test.source.method" : "test_succeed()V", - "test.status" : "fail", + "test.status" : "pass", "test.suite" : "org.example.TestSucceed", "test.type" : "test", "test_session.name" : "session-name" @@ -201,7 +201,7 @@ "test.early_flake.enabled" : "true", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.status" : "fail", + "test.status" : "pass", "test.type" : "test", "test_session.name" : "session-name" }, @@ -234,7 +234,7 @@ "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, "test.module" : "testng-7", - "test.status" : "fail", + "test.status" : "pass", "test.type" : "test", "test_session.name" : "session-name" }, diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-atr/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-atr/events.ftl index ea9152b22f7..b75428eb649 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-atr/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-atr/events.ftl @@ -211,7 +211,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", @@ -265,6 +264,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-efd/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-efd/events.ftl index 2e47bc1ca00..fed9e4ec567 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-efd/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-quarantined-failed-efd/events.ftl @@ -106,7 +106,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_new" : "true", "test.is_retry" : "true", "test.module" : "testng-7", @@ -161,6 +160,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_new" : "true", "test.is_retry" : "true", "test.module" : "testng-7", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-error/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-error/events.ftl index ae8c22b1fe1..cf2a932da20 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-error/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-error/events.ftl @@ -208,7 +208,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_error", @@ -261,6 +260,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_error", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-failed/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-failed/events.ftl index 87f6dedb45a..491a415d7fa 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-failed/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-failed/events.ftl @@ -208,7 +208,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", @@ -261,6 +260,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "test_failed", diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-parameterized/events.ftl b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-parameterized/events.ftl index 2f418540e78..e4bf58e1a47 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-parameterized/events.ftl +++ b/dd-java-agent/instrumentation/testng/testng-7/src/test/resources/test-retry-parameterized/events.ftl @@ -255,7 +255,6 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, - "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "parameterized_test_succeed", @@ -308,6 +307,7 @@ "test.codeowners" : "[\"owner1\",\"owner2\"]", "test.framework" : "testng", "test.framework_version" : ${content_meta_test_framework_version}, + "test.has_failed_all_retries" : "true", "test.is_retry" : "true", "test.module" : "testng-7", "test.name" : "parameterized_test_succeed", From bfbc0f182858cd5a98a6110e788bbab3145966eb Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 12 May 2025 12:36:16 +0200 Subject: [PATCH 07/10] fix auto test retries global counter increment --- .../execution/RetryUntilSuccessful.java | 4 +++- .../buildsystem/ProxyTestModuleTest.groovy | 16 +++++++++++----- .../headless/HeadlessTestModuleTest.groovy | 16 +++++++++++----- .../test/ExecutionStrategyTest.groovy | 3 ++- .../testng7/TestNGExecutionInstrumentation.java | 1 + internal-api/build.gradle | 1 + 6 files changed, 29 insertions(+), 12 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java index dc367751a6c..4b0a68dae92 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java @@ -29,7 +29,9 @@ public RetryUntilSuccessful( public void registerExecution(TestStatus status, long durationMillis) { ++executions; successfulExecutionSeen |= (status != TestStatus.fail); - totalExecutions.incrementAndGet(); + if (executions > 1) { + totalExecutions.incrementAndGet(); + } } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/buildsystem/ProxyTestModuleTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/buildsystem/ProxyTestModuleTest.groovy index cd8492ce566..93db3abc9d1 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/buildsystem/ProxyTestModuleTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/buildsystem/ProxyTestModuleTest.groovy @@ -3,6 +3,7 @@ package datadog.trace.civisibility.domain.buildsystem import datadog.trace.api.Config import datadog.trace.api.DDTraceId import datadog.trace.api.civisibility.config.TestSourceData +import datadog.trace.api.civisibility.execution.TestStatus import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings import datadog.trace.api.civisibility.config.TestIdentifier @@ -59,20 +60,25 @@ class ProxyTestModuleTest extends DDSpecification { def retryPolicy1 = proxyTestModule.executionPolicy(new TestIdentifier("suite", "test-1", null), TestSourceData.UNKNOWN, []) then: - retryPolicy1.retry(false, 1L) // 2nd test execution, 1st retry globally - !retryPolicy1.retry(false, 1L) // asking for 3rd test execution - local limit reached + retryPolicy1.registerExecution(TestStatus.fail, 1L) // 1st test execution + !retryPolicy1.wasLastExecution() + retryPolicy1.registerExecution(TestStatus.fail, 1L) // 2nd test execution, 1st retry globally + retryPolicy1.wasLastExecution() // asking for 3rd test execution - local limit reached when: def retryPolicy2 = proxyTestModule.executionPolicy(new TestIdentifier("suite", "test-2", null), TestSourceData.UNKNOWN, []) then: - retryPolicy2.retry(false, 1L) // 2nd test execution, 2nd retry globally (since previous test was retried too) - !retryPolicy2.retry(false, 1L) // asking for 3rd test execution - local limit reached + retryPolicy2.registerExecution(TestStatus.fail, 1L) // 1st test execution + !retryPolicy2.wasLastExecution() + retryPolicy2.registerExecution(TestStatus.fail, 1L) // 2nd test execution, 1st retry globally + retryPolicy2.wasLastExecution() // asking for 3rd test execution - local limit reached when: def retryPolicy3 = proxyTestModule.executionPolicy(new TestIdentifier("suite", "test-3", null), TestSourceData.UNKNOWN, []) then: - !retryPolicy3.retry(false, 1L) // asking for 3rd retry globally - global limit reached + retryPolicy3.registerExecution(TestStatus.fail, 1L) // 1st test execution + retryPolicy3.wasLastExecution() // asking for 3rd retry globally - global limit reached } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/headless/HeadlessTestModuleTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/headless/HeadlessTestModuleTest.groovy index 192b77374e7..6268e00a37e 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/headless/HeadlessTestModuleTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/headless/HeadlessTestModuleTest.groovy @@ -4,6 +4,7 @@ import datadog.trace.api.Config import datadog.trace.api.civisibility.config.TestIdentifier import datadog.trace.api.civisibility.config.TestSourceData import datadog.trace.api.civisibility.coverage.CoverageStore +import datadog.trace.api.civisibility.execution.TestStatus import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext import datadog.trace.civisibility.codeowners.Codeowners @@ -24,21 +25,26 @@ class HeadlessTestModuleTest extends SpanWriterTest { def retryPolicy1 = headlessTestModule.executionPolicy(new TestIdentifier("suite", "test-1", null), TestSourceData.UNKNOWN, []) then: - retryPolicy1.retry(false, 1L) // 2nd test execution, 1st retry globally - !retryPolicy1.retry(false, 1L) // asking for 3rd test execution - local limit reached + retryPolicy1.registerExecution(TestStatus.fail, 1L) // 1st test execution + !retryPolicy1.wasLastExecution() + retryPolicy1.registerExecution(TestStatus.fail, 1L) // 2nd test execution, 1st retry globally + retryPolicy1.wasLastExecution() // asking for 3rd test execution - local limit reached when: def retryPolicy2 = headlessTestModule.executionPolicy(new TestIdentifier("suite", "test-2", null), TestSourceData.UNKNOWN, []) then: - retryPolicy2.retry(false, 1L) // 2nd test execution, 2nd retry globally (since previous test was retried too) - !retryPolicy2.retry(false, 1L) // asking for 3rd test execution - local limit reached + retryPolicy2.registerExecution(TestStatus.fail, 1L) // 1st test execution + !retryPolicy2.wasLastExecution() + retryPolicy2.registerExecution(TestStatus.fail, 1L) // 2nd test execution, 1st retry globally + retryPolicy2.wasLastExecution() // asking for 3rd test execution - local limit reached when: def retryPolicy3 = headlessTestModule.executionPolicy(new TestIdentifier("suite", "test-3", null), TestSourceData.UNKNOWN, []) then: - !retryPolicy3.retry(false, 1L) // asking for 3rd retry globally - global limit reached + retryPolicy3.registerExecution(TestStatus.fail, 1L) // 1st test execution + retryPolicy3.wasLastExecution() // asking for 3rd retry globally - global limit reached } private HeadlessTestModule givenAHeadlessTestModule() { diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy index 7d9b6106103..a48842662ca 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/test/ExecutionStrategyTest.groovy @@ -5,6 +5,7 @@ import datadog.trace.api.civisibility.config.TestFQN import datadog.trace.api.civisibility.config.TestIdentifier import datadog.trace.api.civisibility.config.TestMetadata import datadog.trace.api.civisibility.config.TestSourceData +import datadog.trace.api.civisibility.execution.TestStatus import datadog.trace.api.civisibility.telemetry.tag.RetryReason import datadog.trace.api.civisibility.telemetry.tag.SkipReason import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings @@ -101,7 +102,7 @@ class ExecutionStrategyTest extends Specification { def policy = strategy.executionPolicy(testID, TestSourceData.UNKNOWN, []) // register one execution to get the retry reason - policy.registerExecution(true, 0,) + policy.registerExecution(TestStatus.pass, 0) expect: policy.class == RunNTimes diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java index 9232c93b834..2c07b545425 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java @@ -60,6 +60,7 @@ public String[] helperClassNames() { commonPackageName + ".TestEventsHandlerHolder", commonPackageName + ".TestNGClassListener", commonPackageName + ".execution.RetryAnalyzer", + commonPackageName + ".TracingListener", }; } diff --git a/internal-api/build.gradle b/internal-api/build.gradle index dd5ee2ce0cc..457a5a95c87 100644 --- a/internal-api/build.gradle +++ b/internal-api/build.gradle @@ -126,6 +126,7 @@ excludedClassesCoverage += [ "datadog.trace.api.civisibility.events.BuildEventsHandler.ModuleInfo", "datadog.trace.api.civisibility.events.TestDescriptor", "datadog.trace.api.civisibility.events.TestSuiteDescriptor", + "datadog.trace.api.civisibility.execution.TestStatus", "datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric", "datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric.IndexHolder", "datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric", From 5f76e246bfcde11b4c32b123a44ff494059d6c0e Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Mon, 12 May 2025 14:33:09 +0200 Subject: [PATCH 08/10] revert useless change in cucumber --- .../trace/instrumentation/junit4/CucumberTracingListener.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java index 072c7db79d2..4f31bd06c53 100644 --- a/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java +++ b/dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java @@ -182,14 +182,13 @@ public void testIgnored(final Description description) { } else { String testSuiteName = CucumberUtils.getTestSuiteNameForScenario(description); String testName = CucumberUtils.getTestNameForScenario(description); - TestDescriptor testDescriptor = CucumberUtils.toTestDescriptor(description); List categories = getCategories(description); TestEventsHandlerHolder.HANDLERS .get(TestFrameworkInstrumentation.CUCUMBER) .onTestIgnore( new TestSuiteDescriptor(testSuiteName, null), - testDescriptor, + CucumberUtils.toTestDescriptor(description), testName, FRAMEWORK_NAME, FRAMEWORK_VERSION, From 21f3ac771c215da582319dd0852372f10ac3f97f Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 14 May 2025 10:05:00 +0200 Subject: [PATCH 09/10] pr suggestions --- .../trace/civisibility/domain/TestImpl.java | 3 +-- .../execution/RetryUntilSuccessful.java | 14 ++++++++------ .../trace/civisibility/execution/RunNTimes.java | 10 +++------- .../instrumentation/scalatest/DatadogReporter.java | 10 +++++----- .../instrumentation/scalatest/RunContext.java | 12 ++++++++++++ .../testng7/TestNGExecutionInstrumentation.java | 4 ++++ 6 files changed, 33 insertions(+), 20 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index fbf54602007..99146abf25c 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -67,8 +67,7 @@ public class TestImpl implements DDTest { private final Consumer onSpanFinish; private final TestContext context; private final TestIdentifier identifier; - - private Long startMicros; + private final long startMicros; private TestStatus status; public TestImpl( diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java index 4b0a68dae92..81fdeaa6c2d 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java @@ -14,14 +14,14 @@ public class RetryUntilSuccessful implements TestExecutionPolicy { private int executions; private boolean successfulExecutionSeen; - /** Total execution counter that is shared by all retry policies */ - private final AtomicInteger totalExecutions; + /** Total retry counter that is shared by all retry until successful policies (currently ATR) */ + private final AtomicInteger totalRetryCount; public RetryUntilSuccessful( - int maxExecutions, boolean suppressFailures, AtomicInteger totalExecutions) { + int maxExecutions, boolean suppressFailures, AtomicInteger totalRetryCount) { this.maxExecutions = maxExecutions; this.suppressFailures = suppressFailures; - this.totalExecutions = totalExecutions; + this.totalRetryCount = totalRetryCount; this.executions = 0; } @@ -30,7 +30,7 @@ public void registerExecution(TestStatus status, long durationMillis) { ++executions; successfulExecutionSeen |= (status != TestStatus.fail); if (executions > 1) { - totalExecutions.incrementAndGet(); + totalRetryCount.incrementAndGet(); } } @@ -45,7 +45,9 @@ private boolean currentExecutionIsLast() { @Override public boolean applicable() { - return !currentExecutionIsLast() || suppressFailures; + // executions must always be registered, therefore consider it applicable as long as there are + // retries left + return !wasLastExecution(); } @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java index 19e43deb76e..7201bb99c02 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java @@ -47,15 +47,11 @@ public boolean wasLastExecution() { return lastStatus == TestStatus.skip || executions >= maxExecutions; } - private boolean currentExecutionIsLast() { - // this could give false negatives if maxExecutions is updated after the current execution is - // registered - return executions == maxExecutions - 1; - } - @Override public boolean applicable() { - return !currentExecutionIsLast() || suppressFailures(); + // executions must always be registered, therefore consider it applicable as long as there are + // retries left + return !wasLastExecution(); } @Override diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java index d198a8f154e..c3e3b98a713 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/DatadogReporter.java @@ -189,7 +189,7 @@ private static void onTestSuccess(TestSucceeded event) { new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier); TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); TestEventsHandler eventHandler = context.getEventHandler(); eventHandler.onTestFinish(testDescriptor, null, executionHistory); @@ -212,7 +212,7 @@ private static void onTestFailure(TestFailed event) { new TestDescriptor(testSuiteName, testClass, testName, testParameters, testQualifier); TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); TestEventsHandler eventHandler = context.getEventHandler(); eventHandler.onTestFailure(testDescriptor, throwable); @@ -247,7 +247,7 @@ private static void onTestIgnore(TestIgnored event) { categories, new TestSourceData(testClass, null, null), reason != null ? reason.getDescription() : null, - context.getExecutionHistory(skippableTest)); + context.popExecutionHistory(skippableTest)); } private static void onTestCancel(TestCanceled event) { @@ -275,7 +275,7 @@ private static void onTestCancel(TestCanceled event) { } TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); eventHandler.onTestFinish(testDescriptor, null, executionHistory); } @@ -300,7 +300,7 @@ private static void onTestPending(TestPending event) { eventHandler.onTestSkip(testDescriptor, reason); TestIdentifier testIdentifier = new TestIdentifier(testSuiteName, testName, null); - TestExecutionHistory executionHistory = context.getExecutionHistory(testIdentifier); + TestExecutionHistory executionHistory = context.popExecutionHistory(testIdentifier); eventHandler.onTestFinish(testDescriptor, null, executionHistory); } diff --git a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java index 338d251b2e9..b200dd3ef84 100644 --- a/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java +++ b/dd-java-agent/instrumentation/scalatest/src/main/java/datadog/trace/instrumentation/scalatest/RunContext.java @@ -162,6 +162,18 @@ public TestExecutionHistory getExecutionHistory(TestIdentifier testIdentifier) { return executionPolicies.get(testIdentifier); } + @Nullable + public TestExecutionHistory popExecutionHistory(TestIdentifier testIdentifier) { + TestExecutionPolicy[] holder = new TestExecutionPolicy[1]; + executionPolicies.computeIfPresent( + testIdentifier, + (ti, policy) -> { + holder[0] = policy; + return policy.applicable() ? policy : null; + }); + return holder[0]; + } + public void destroy() { eventHandler.close(); } diff --git a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java index 2c07b545425..7a30dd10405 100644 --- a/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java +++ b/dd-java-agent/instrumentation/testng/testng-7/src/main/java/datadog/trace/instrumentation/testng7/TestNGExecutionInstrumentation.java @@ -84,6 +84,10 @@ public static void alignBeforeRetry( } } + // Test reporting is idempotent due to only working for in progress tests. Once a test is + // reported it is not considered in progress anymore. DD's test listener will be asked by + // the framework to report the test again after the retry logic is executed, but it will + // result in a no-op, avoiding double reporting TestListenerHelper.runTestListeners(result, Collections.singletonList(tracingListener)); // Also set suppress failures beforehand to align execution ordering. From 3268b48ae901802c155a733c0cba08de2dc9f32d Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 14 May 2025 14:14:16 +0200 Subject: [PATCH 10/10] get test status from span --- .../datadog/trace/civisibility/domain/TestImpl.java | 11 +---------- .../civisibility/events/TestEventsHandlerImpl.java | 5 ++++- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index 99146abf25c..47042b7effa 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -68,7 +68,6 @@ public class TestImpl implements DDTest { private final TestContext context; private final TestIdentifier identifier; private final long startMicros; - private TestStatus status; public TestImpl( AgentSpanContext moduleSpanContext, @@ -138,7 +137,6 @@ public TestImpl( span.setTag(Tags.TEST_MODULE_ID, moduleSpanContext.getSpanId()); span.setTag(Tags.TEST_SESSION_ID, moduleSpanContext.getTraceId()); - status = TestStatus.pass; span.setTag(Tags.TEST_STATUS, TestStatus.pass); if (testClass != null && !testClass.getName().equals(testSuiteName)) { @@ -210,10 +208,6 @@ public TestIdentifier getIdentifier() { return identifier; } - public boolean hasFailed() { - return span.isError(); - } - @Override public void setTag(String key, Object value) { span.setTag(key, value); @@ -223,14 +217,11 @@ public void setTag(String key, Object value) { public void setErrorInfo(Throwable error) { span.setError(true); span.addThrowable(error); - status = TestStatus.fail; span.setTag(Tags.TEST_STATUS, TestStatus.fail); } @Override public void setSkipReason(String skipReason) { - status = TestStatus.skip; - span.setTag(Tags.TEST_STATUS, TestStatus.skip); if (skipReason != null) { span.setTag(Tags.TEST_SKIP_REASON, skipReason); @@ -244,7 +235,7 @@ public void setSkipReason(String skipReason) { } public TestStatus getStatus() { - return status; + return (TestStatus) span.getTag(Tags.TEST_STATUS); } public long getDuration(@Nullable Long endMicros) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java index 0fa3894da6e..f149c91b2f0 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java @@ -10,6 +10,7 @@ import datadog.trace.api.civisibility.events.TestEventsHandler; import datadog.trace.api.civisibility.execution.TestExecutionHistory; import datadog.trace.api.civisibility.execution.TestExecutionPolicy; +import datadog.trace.api.civisibility.execution.TestStatus; import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.civisibility.telemetry.tag.EventType; @@ -255,7 +256,9 @@ public void onTestFinish( TestIdentifier thisTest = test.getIdentifier(); if (testExecutionHistory != null) { - testExecutionHistory.registerExecution(test.getStatus(), test.getDuration(endTime)); + TestStatus testStatus = test.getStatus(); + testExecutionHistory.registerExecution( + testStatus != null ? testStatus : TestStatus.skip, test.getDuration(endTime)); if (testExecutionHistory.hasFailedAllRetries()) { test.setTag(Tags.TEST_HAS_FAILED_ALL_RETRIES, true);