Implement quarantined tests tagging (#8326)

DataDog · Feb 3, 2025 · 38b8211 · 38b8211
1 parent 9af5347
commit 38b8211
Show file tree

Hide file tree

Showing 182 changed files with 7,908 additions and 1,763 deletions.
diff --git a/...nt-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java b/...nt-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java
@@ -31,6 +31,8 @@ TestSuiteImpl testSuiteStart(
 
   boolean isModified(TestSourceData testSourceData);
 
+  boolean isQuarantined(TestIdentifier test);
+
   /**
    * Returns the reason for skipping a test, IF it can be skipped.
    *

diff --git a/...a-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/...a-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java
@@ -18,8 +18,10 @@
 import datadog.trace.api.civisibility.telemetry.TagValue;
 import datadog.trace.api.civisibility.telemetry.tag.BrowserDriver;
 import datadog.trace.api.civisibility.telemetry.tag.EventType;
+import datadog.trace.api.civisibility.telemetry.tag.HasFailedAllRetries;
 import datadog.trace.api.civisibility.telemetry.tag.IsModified;
 import datadog.trace.api.civisibility.telemetry.tag.IsNew;
+import datadog.trace.api.civisibility.telemetry.tag.IsQuarantined;
 import datadog.trace.api.civisibility.telemetry.tag.IsRetry;
 import datadog.trace.api.civisibility.telemetry.tag.IsRum;
 import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
@@ -190,6 +192,10 @@ public TestIdentifier getIdentifier() {
     return identifier;
   }
 
+  public boolean hasFailed() {
+    return span.isError();
+  }
+
   @Override
   public void setTag(String key, Object value) {
     span.setTag(key, value);
@@ -265,13 +271,15 @@ public void end(@Nullable Long endTime) {
 
     Object retryReason = span.getTag(Tags.TEST_RETRY_REASON);
     metricCollector.add(
-        CiVisibilityCountMetric.EVENT_FINISHED,
+        CiVisibilityCountMetric.TEST_EVENT_FINISHED,
         1,
         instrumentation,
         EventType.TEST,
         span.getTag(Tags.TEST_IS_NEW) != null ? IsNew.TRUE : null,
         span.getTag(Tags.TEST_IS_MODIFIED) != null ? IsModified.TRUE : null,
+        span.getTag(Tags.TEST_MANAGEMENT_IS_QUARANTINED) != null ? IsQuarantined.TRUE : null,
         span.getTag(Tags.TEST_IS_RETRY) != null ? IsRetry.TRUE : null,
+        span.getTag(Tags.TEST_HAS_FAILED_ALL_RETRIES) != null ? HasFailedAllRetries.TRUE : null,
         retryReason instanceof TagValue ? (TagValue) retryReason : null,
         span.getTag(Tags.TEST_IS_RUM_ACTIVE) != null ? IsRum.TRUE : null,
         CIConstants.SELENIUM_BROWSER_DRIVER.equals(span.getTag(Tags.TEST_BROWSER_DRIVER))

diff --git a/...sibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java b/...sibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java
@@ -103,6 +103,11 @@ public boolean isModified(TestSourceData testSourceData) {
     return executionStrategy.isModified(testSourceData);
   }
 
+  @Override
+  public boolean isQuarantined(TestIdentifier test) {
+    return executionStrategy.isQuarantined(test);
+  }
+
   @Nullable
   @Override
   public SkipReason skipReason(TestIdentifier test) {

diff --git a/...sibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java b/...sibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java
@@ -88,6 +88,11 @@ public boolean isModified(TestSourceData testSourceData) {
     return executionStrategy.isModified(testSourceData);
   }
 
+  @Override
+  public boolean isQuarantined(TestIdentifier test) {
+    return executionStrategy.isQuarantined(test);
+  }
+
   @Nullable
   @Override
   public SkipReason skipReason(TestIdentifier test) {

diff --git a/...-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java b/...-ci-visibility/src/main/java/datadog/trace/civisibility/events/NoOpTestEventsHandler.java
@@ -5,8 +5,8 @@
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.config.TestSourceData;
 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.telemetry.tag.RetryReason;
 import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.ContextStore;
@@ -58,8 +58,8 @@ public void onTestStart(
       @Nullable String testParameters,
       @Nullable Collection<String> categories,
       @Nonnull TestSourceData testSourceData,
-      RetryReason retryReason,
-      @Nullable Long startTime) {
+      @Nullable Long startTime,
+      @Nullable TestExecutionHistory testExecutionHistory) {
     // do nothing
   }
 
@@ -74,7 +74,8 @@ public void onTestFailure(TestKey descriptor, @Nullable Throwable throwable) {
   }
 
   @Override
-  public void onTestFinish(TestKey descriptor, @Nullable Long endTime) {
+  public void onTestFinish(
+      TestKey descriptor, @Nullable Long endTime, @Nullable TestExecutionHistory executionHistory) {
     // do nothing
   }
 

diff --git a/...-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java b/...-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java
@@ -8,6 +8,7 @@
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.config.TestSourceData;
 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.telemetry.CiVisibilityCountMetric;
 import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
@@ -139,8 +140,8 @@ public void onTestStart(
       final @Nullable String testParameters,
       final @Nullable Collection<String> categories,
       final @Nonnull TestSourceData testSourceData,
-      final @Nullable RetryReason retryReason,
-      final @Nullable Long startTime) {
+      final @Nullable Long startTime,
+      final @Nullable TestExecutionHistory testExecutionHistory) {
     if (skipTrace(testSourceData.getTestClass())) {
       return;
     }
@@ -166,6 +167,18 @@ public void onTestStart(
       test.setTag(Tags.TEST_IS_MODIFIED, true);
     }
 
+    if (testModule.isQuarantined(thisTest)) {
+      test.setTag(Tags.TEST_MANAGEMENT_IS_QUARANTINED, true);
+    }
+
+    if (testExecutionHistory != null) {
+      RetryReason retryReason = testExecutionHistory.currentExecutionRetryReason();
+      if (retryReason != null) {
+        test.setTag(Tags.TEST_IS_RETRY, true);
+        test.setTag(Tags.TEST_RETRY_REASON, retryReason);
+      }
+    }
+
     if (testFramework != null) {
       test.setTag(Tags.TEST_FRAMEWORK, testFramework);
       if (testFrameworkVersion != null) {
@@ -198,11 +211,6 @@ public void onTestStart(
       }
     }
 
-    if (retryReason != null) {
-      test.setTag(Tags.TEST_IS_RETRY, true);
-      test.setTag(Tags.TEST_RETRY_REASON, retryReason);
-    }
-
     inProgressTests.put(descriptor, test);
   }
 
@@ -227,12 +235,22 @@ public void onTestFailure(TestKey descriptor, @Nullable Throwable throwable) {
   }
 
   @Override
-  public void onTestFinish(TestKey descriptor, @Nullable Long endTime) {
+  public void onTestFinish(
+      TestKey descriptor,
+      @Nullable Long endTime,
+      @Nullable TestExecutionHistory testExecutionHistory) {
     TestImpl test = inProgressTests.remove(descriptor);
     if (test == null) {
       log.debug("Ignoring finish event, could not find test {}", descriptor);
       return;
     }
+
+    if (testExecutionHistory != null) {
+      if (test.hasFailed() && testExecutionHistory.hasFailedAllRetries()) {
+        test.setTag(Tags.TEST_HAS_FAILED_ALL_RETRIES, true);
+      }
+    }
+
     test.end(endTime);
   }
 
@@ -259,7 +277,7 @@ public void onTestIgnore(
         null,
         null);
     onTestSkip(testDescriptor, reason);
-    onTestFinish(testDescriptor, null);
+    onTestFinish(testDescriptor, null, null);
   }
 
   @Override

diff --git a/...agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/Regular.java b/...agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/Regular.java
@@ -31,4 +31,9 @@ public boolean retry(boolean successful, long durationMillis) {
   public RetryReason currentExecutionRetryReason() {
     return null;
   }
+
+  @Override
+  public boolean hasFailedAllRetries() {
+    return false;
+  }
 }
diff --git a/...i-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java b/...i-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java
@@ -11,6 +11,7 @@ public class RetryUntilSuccessful implements TestExecutionPolicy {
   private final int maxExecutions;
   private final boolean suppressFailures;
   private int executions;
+  private boolean successfulExecutionSeen;
 
   /** Total execution counter that is shared by all retry policies */
   private final AtomicInteger totalExecutions;
@@ -25,22 +26,23 @@ public RetryUntilSuccessful(
 
   @Override
   public boolean applicable() {
-    return currentExecutionIsNotLast() || suppressFailures;
+    return !currentExecutionIsLast() || suppressFailures;
   }
 
   @Override
   public boolean suppressFailures() {
     // do not suppress failures for last execution
     // (unless flag to suppress all failures is set)
-    return currentExecutionIsNotLast() || suppressFailures;
+    return !currentExecutionIsLast() || suppressFailures;
   }
 
-  private boolean currentExecutionIsNotLast() {
-    return executions < maxExecutions - 1;
+  private boolean currentExecutionIsLast() {
+    return executions == maxExecutions - 1;
   }
 
   @Override
   public boolean retry(boolean successful, long durationMillis) {
+    successfulExecutionSeen |= successful;
     if (!successful && ++executions < maxExecutions) {
       totalExecutions.incrementAndGet();
       return true;
@@ -58,4 +60,9 @@ public RetryReason currentExecutionRetryReason() {
   private boolean currentExecutionIsRetry() {
     return executions > 0;
   }
+
+  @Override
+  public boolean hasFailedAllRetries() {
+    return currentExecutionIsLast() && !successfulExecutionSeen;
+  }
 }
diff --git a/...ent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java b/...ent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java
@@ -12,6 +12,7 @@ public class RunNTimes implements TestExecutionPolicy {
   private final boolean suppressFailures;
   private int executions;
   private int maxExecutions;
+  private boolean successfulExecutionSeen;
 
   public RunNTimes(
       EarlyFlakeDetectionSettings earlyFlakeDetectionSettings, boolean suppressFailures) {
@@ -23,11 +24,11 @@ public RunNTimes(
 
   @Override
   public boolean applicable() {
-    return currentExecutionIsNotLast() || suppressFailures();
+    return !currentExecutionIsLast() || suppressFailures();
   }
 
-  private boolean currentExecutionIsNotLast() {
-    return executions < maxExecutions - 1;
+  private boolean currentExecutionIsLast() {
+    return executions == maxExecutions - 1;
   }
 
   @Override
@@ -37,6 +38,7 @@ public boolean suppressFailures() {
 
   @Override
   public boolean retry(boolean successful, long durationMillis) {
+    successfulExecutionSeen |= successful;
     // adjust maximum retries based on the now known test duration
     int maxExecutionsForGivenDuration = earlyFlakeDetectionSettings.getExecutions(durationMillis);
     maxExecutions = Math.min(maxExecutions, maxExecutionsForGivenDuration);
@@ -52,4 +54,9 @@ public RetryReason currentExecutionRetryReason() {
   private boolean currentExecutionIsRetry() {
     return executions > 0;
   }
+
+  @Override
+  public boolean hasFailedAllRetries() {
+    return currentExecutionIsLast() && !successfulExecutionSeen;
+  }
 }
diff --git a/...i-visibility/src/main/java/datadog/trace/civisibility/execution/RunOnceIgnoreOutcome.java b/...i-visibility/src/main/java/datadog/trace/civisibility/execution/RunOnceIgnoreOutcome.java
@@ -33,4 +33,9 @@ public boolean retry(boolean successful, long durationMillis) {
   public RetryReason currentExecutionRetryReason() {
     return null;
   }
+
+  @Override
+  public boolean hasFailedAllRetries() {
+    return false;
+  }
 }
diff --git a/...r-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java b/...r-junit-4/src/main/java/datadog/trace/instrumentation/junit4/CucumberTracingListener.java
@@ -4,7 +4,7 @@
 import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge;
 import datadog.trace.api.civisibility.events.TestDescriptor;
 import datadog.trace.api.civisibility.events.TestSuiteDescriptor;
-import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
+import datadog.trace.api.civisibility.execution.TestExecutionHistory;
 import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
 import datadog.trace.bootstrap.ContextStore;
 import io.cucumber.core.gherkin.Pickle;
@@ -29,13 +29,13 @@ public class CucumberTracingListener extends TracingListener {
   public static final String FRAMEWORK_NAME = "cucumber";
   public static final String FRAMEWORK_VERSION = CucumberUtils.getVersion();
 
-  private final ContextStore<Description, TestExecutionPolicy> retryPolicies;
+  private final ContextStore<Description, TestExecutionHistory> executionHistories;
   private final Map<Object, Pickle> pickleById;
 
   public CucumberTracingListener(
-      ContextStore<Description, TestExecutionPolicy> retryPolicies,
+      ContextStore<Description, TestExecutionHistory> executionHistories,
       List<ParentRunner<?>> featureRunners) {
-    this.retryPolicies = retryPolicies;
+    this.executionHistories = executionHistories;
     pickleById = CucumberUtils.getPicklesById(featureRunners);
   }
 
@@ -71,7 +71,6 @@ public void testStarted(final Description description) {
     String testName = CucumberUtils.getTestNameForScenario(description);
     List<String> categories = getCategories(description);
 
-    TestExecutionPolicy retryPolicy = retryPolicies.get(description);
     TestEventsHandlerHolder.TEST_EVENTS_HANDLER.onTestStart(
         new TestSuiteDescriptor(testSuiteName, null),
         CucumberUtils.toTestDescriptor(description),
@@ -81,8 +80,8 @@ public void testStarted(final Description description) {
         null,
         categories,
         TestSourceData.UNKNOWN,
-        retryPolicy != null ? retryPolicy.currentExecutionRetryReason() : null,
-        null);
+        null,
+        executionHistories.get(description));
 
     recordFeatureFileCodeCoverage(description);
   }
@@ -100,7 +99,9 @@ private static void recordFeatureFileCodeCoverage(Description scenarioDescriptio
   @Override
   public void testFinished(final Description description) {
     TestDescriptor testDescriptor = CucumberUtils.toTestDescriptor(description);
-    TestEventsHandlerHolder.TEST_EVENTS_HANDLER.onTestFinish(testDescriptor, null);
+    TestExecutionHistory executionHistory = executionHistories.get(description);
+    TestEventsHandlerHolder.TEST_EVENTS_HANDLER.onTestFinish(
+        testDescriptor, null, executionHistory);
   }
 
   // same callback is executed both for test cases and test suites (for setup/teardown errors)

diff --git a/...t-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java b/...t-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java
@@ -7,7 +7,7 @@
 import datadog.trace.agent.tooling.Instrumenter;
 import datadog.trace.agent.tooling.InstrumenterModule;
 import datadog.trace.agent.tooling.muzzle.Reference;
-import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
+import datadog.trace.api.civisibility.execution.TestExecutionHistory;
 import datadog.trace.bootstrap.InstrumentationContext;
 import java.util.Collections;
 import java.util.List;
@@ -46,7 +46,7 @@ public String[] helperClassNames() {
   @Override
   public Map<String, String> contextStore() {
     return Collections.singletonMap(
-        "org.junit.runner.Description", TestExecutionPolicy.class.getName());
+        "org.junit.runner.Description", TestExecutionHistory.class.getName());
   }
 
   @Override
@@ -84,7 +84,7 @@ public static void addTracingListener(
 
       replacedNotifier.addListener(
           new CucumberTracingListener(
-              InstrumentationContext.get(Description.class, TestExecutionPolicy.class), children));
+              InstrumentationContext.get(Description.class, TestExecutionHistory.class), children));
       runNotifier = replacedNotifier;
     }
   }
-Original file line number
+Diff line change
@@ Expand Up / @@ -31,6 +31,8 @@ TestSuiteImpl testSuiteStart( @@
       boolean isModified(TestSourceData testSourceData);
+      boolean isQuarantined(TestIdentifier test);
       /**
        * Returns the reason for skipping a test, IF it can be skipped.
        *
@@ Expand Down @@