Implement tests quarantining (#8320)

Author: nikita-tkachenko-datadog

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettings.java

@@ -30,6 +30,7 @@ public class ExecutionSettings {
           Collections.emptyMap(),
           Collections.emptyList(),
           null,
+          null,
           LineDiff.EMPTY);
 
   private final boolean itrEnabled;
@@ -41,6 +42,7 @@ public class ExecutionSettings {
   @Nullable private final String itrCorrelationId;
   @Nonnull private final Map<TestIdentifier, TestMetadata> skippableTests;
   @Nonnull private final Map<String, BitSet> skippableTestsCoverage;
+  @Nonnull private final Collection<TestIdentifier> quarantinedTests;
   @Nullable private final Collection<TestIdentifier> flakyTests;
   @Nullable private final Collection<TestIdentifier> knownTests;
   @Nonnull private final Diff pullRequestDiff;
@@ -55,6 +57,7 @@ public ExecutionSettings(
       @Nullable String itrCorrelationId,
       @Nonnull Map<TestIdentifier, TestMetadata> skippableTests,
       @Nonnull Map<String, BitSet> skippableTestsCoverage,
+      @Nonnull Collection<TestIdentifier> quarantinedTests,
       @Nullable Collection<TestIdentifier> flakyTests,
       @Nullable Collection<TestIdentifier> knownTests,
       @Nonnull Diff pullRequestDiff) {
@@ -67,6 +70,7 @@ public ExecutionSettings(
     this.itrCorrelationId = itrCorrelationId;
     this.skippableTests = skippableTests;
     this.skippableTestsCoverage = skippableTestsCoverage;
+    this.quarantinedTests = quarantinedTests;
     this.flakyTests = flakyTests;
     this.knownTests = knownTests;
     this.pullRequestDiff = pullRequestDiff;
@@ -117,6 +121,11 @@ public Map<TestIdentifier, TestMetadata> getSkippableTests() {
     return skippableTests;
   }
 
+  @Nonnull
+  public Collection<TestIdentifier> getQuarantinedTests() {
+    return quarantinedTests;
+  }
+
   /**
    * @return the list of known tests for the given module (can be empty), or {@code null} if known
    *     tests could not be obtained
@@ -156,6 +165,7 @@ public boolean equals(Object o) {
         && Objects.equals(itrCorrelationId, that.itrCorrelationId)
         && Objects.equals(skippableTests, that.skippableTests)
         && Objects.equals(skippableTestsCoverage, that.skippableTestsCoverage)
+        && Objects.equals(quarantinedTests, that.quarantinedTests)
         && Objects.equals(flakyTests, that.flakyTests)
         && Objects.equals(knownTests, that.knownTests)
         && Objects.equals(pullRequestDiff, that.pullRequestDiff);
@@ -171,6 +181,7 @@ public int hashCode() {
         itrCorrelationId,
         skippableTests,
         skippableTestsCoverage,
+        quarantinedTests,
         flakyTests,
         knownTests,
         pullRequestDiff);
@@ -207,6 +218,7 @@ public static ByteBuffer serialize(ExecutionSettings settings) {
           TestMetadataSerializer::serialize);
 
       s.write(settings.skippableTestsCoverage, Serializer::write, Serializer::write);
+      s.write(settings.quarantinedTests, TestIdentifierSerializer::serialize);
       s.write(settings.flakyTests, TestIdentifierSerializer::serialize);
       s.write(settings.knownTests, TestIdentifierSerializer::serialize);
 
@@ -234,6 +246,8 @@ public static ExecutionSettings deserialize(ByteBuffer buffer) {
 
       Map<String, BitSet> skippableTestsCoverage =
           Serializer.readMap(buffer, Serializer::readString, Serializer::readBitSet);
+      Collection<TestIdentifier> quarantinedTests =
+          Serializer.readSet(buffer, TestIdentifierSerializer::deserialize);
       Collection<TestIdentifier> flakyTests =
           Serializer.readSet(buffer, TestIdentifierSerializer::deserialize);
       Collection<TestIdentifier> knownTests =
@@ -251,6 +265,7 @@ public static ExecutionSettings deserialize(ByteBuffer buffer) {
           itrCorrelationId,
           skippableTests,
           skippableTestsCoverage,
+          quarantinedTests,
           flakyTests,
           knownTests,
           diff);

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java

@@ -233,6 +233,7 @@ private Map<String, ExecutionSettings> doCreate(
                   .getIdentifiersByModule()
                   .getOrDefault(moduleName, Collections.emptyMap()),
               skippableTests.getCoveredLinesByRelativeSourcePath(),
+              Collections.emptyList(), // FIXME implement retrieving quarantined tests
               flakyTestsByModule != null
                   ? flakyTestsByModule.getOrDefault(moduleName, Collections.emptyList())
                   : null,

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RetryUntilSuccessful.java

@@ -9,29 +9,34 @@
 public class RetryUntilSuccessful implements TestExecutionPolicy {
 
   private final int maxExecutions;
+  private final boolean suppressFailures;
   private int executions;
 
   /** Total execution counter that is shared by all retry policies */
   private final AtomicInteger totalExecutions;
 
-  public RetryUntilSuccessful(int maxExecutions, AtomicInteger totalExecutions) {
+  public RetryUntilSuccessful(
+      int maxExecutions, boolean suppressFailures, AtomicInteger totalExecutions) {
     this.maxExecutions = maxExecutions;
+    this.suppressFailures = suppressFailures;
     this.totalExecutions = totalExecutions;
     this.executions = 0;
   }
 
   @Override
   public boolean applicable() {
-    // the last execution is not altered by the policy
-    // (no retries, no exceptions suppressing)
-    return executions < maxExecutions - 1;
+    return currentExecutionIsNotLast() || suppressFailures;
   }
 
   @Override
   public boolean suppressFailures() {
-    // if this isn't the last execution,
-    // possible failures should be suppressed
-    return applicable();
+    // do not suppress failures for last execution
+    // (unless flag to suppress all failures is set)
+    return currentExecutionIsNotLast() || suppressFailures;
+  }
+
+  private boolean currentExecutionIsNotLast() {
+    return executions < maxExecutions - 1;
   }
 
   @Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java

@@ -9,25 +9,30 @@
 public class RunNTimes implements TestExecutionPolicy {
 
   private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
+  private final boolean suppressFailures;
   private int executions;
   private int maxExecutions;
 
-  public RunNTimes(EarlyFlakeDetectionSettings earlyFlakeDetectionSettings) {
+  public RunNTimes(
+      EarlyFlakeDetectionSettings earlyFlakeDetectionSettings, boolean suppressFailures) {
     this.earlyFlakeDetectionSettings = earlyFlakeDetectionSettings;
+    this.suppressFailures = suppressFailures;
     this.executions = 0;
     this.maxExecutions = earlyFlakeDetectionSettings.getExecutions(0);
   }
 
   @Override
   public boolean applicable() {
-    // the last execution is not altered by the policy
-    // (no retries, no exceptions suppressing)
+    return currentExecutionIsNotLast() || suppressFailures();
+  }
+
+  private boolean currentExecutionIsNotLast() {
     return executions < maxExecutions - 1;
   }
 
   @Override
   public boolean suppressFailures() {
-    return false;
+    return suppressFailures;
   }
 
   @Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunOnceIgnoreOutcome.java

@@ -0,0 +1,36 @@
+package datadog.trace.civisibility.execution;
+
+import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
+import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Runs a test case once. If it fails - suppresses the failure so that the build status is not
+ * affected.
+ */
+public class RunOnceIgnoreOutcome implements TestExecutionPolicy {
+
+  private boolean testExecuted;
+
+  @Override
+  public boolean applicable() {
+    return !testExecuted;
+  }
+
+  @Override
+  public boolean suppressFailures() {
+    return true;
+  }
+
+  @Override
+  public boolean retry(boolean successful, long durationMillis) {
+    testExecuted = true;
+    return false;
+  }
+
+  @Nullable
+  @Override
+  public RetryReason currentExecutionRetryReason() {
+    return null;
+  }
+}

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java

@@ -11,6 +11,7 @@
 import datadog.trace.civisibility.execution.Regular;
 import datadog.trace.civisibility.execution.RetryUntilSuccessful;
 import datadog.trace.civisibility.execution.RunNTimes;
+import datadog.trace.civisibility.execution.RunOnceIgnoreOutcome;
 import datadog.trace.civisibility.source.LinesResolver;
 import datadog.trace.civisibility.source.SourcePathResolver;
 import java.lang.reflect.Method;
@@ -60,6 +61,11 @@ public boolean isFlaky(TestIdentifier test) {
     return flakyTests != null && flakyTests.contains(test.withoutParameters());
   }
 
+  public boolean isQuarantined(TestIdentifier test) {
+    Collection<TestIdentifier> quarantinedTests = executionSettings.getQuarantinedTests();
+    return quarantinedTests.contains(test.withoutParameters());
+  }
+
   @Nullable
   public SkipReason skipReason(TestIdentifier test) {
     if (test == null) {
@@ -85,29 +91,44 @@ public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData t
       return Regular.INSTANCE;
     }
 
-    EarlyFlakeDetectionSettings efdSettings = executionSettings.getEarlyFlakeDetectionSettings();
-    if (efdSettings.isEnabled() && !isEFDLimitReached()) {
-      if (isNew(test) || isModified(testSource)) {
-        // check-then-act with "earlyFlakeDetectionsUsed" is not atomic here,
-        // but we don't care if we go "a bit" over the limit, it does not have to be precise
-        earlyFlakeDetectionsUsed.incrementAndGet();
-        return new RunNTimes(efdSettings);
-      }
+    if (isEFDApplicable(test, testSource)) {
+      // check-then-act with "earlyFlakeDetectionsUsed" is not atomic here,
+      // but we don't care if we go "a bit" over the limit, it does not have to be precise
+      earlyFlakeDetectionsUsed.incrementAndGet();
+      return new RunNTimes(executionSettings.getEarlyFlakeDetectionSettings(), isQuarantined(test));
     }
 
-    if (executionSettings.isFlakyTestRetriesEnabled()) {
-      Collection<TestIdentifier> flakyTests = executionSettings.getFlakyTests();
-      if ((flakyTests == null || flakyTests.contains(test.withoutParameters()))
-          && autoRetriesUsed.get() < config.getCiVisibilityTotalFlakyRetryCount()) {
-        // check-then-act with "autoRetriesUsed" is not atomic here,
-        // but we don't care if we go "a bit" over the limit, it does not have to be precise
-        return new RetryUntilSuccessful(config.getCiVisibilityFlakyRetryCount(), autoRetriesUsed);
-      }
+    if (isAutoRetryApplicable(test)) {
+      // check-then-act with "autoRetriesUsed" is not atomic here,
+      // but we don't care if we go "a bit" over the limit, it does not have to be precise
+      return new RetryUntilSuccessful(
+          config.getCiVisibilityFlakyRetryCount(), isQuarantined(test), autoRetriesUsed);
+    }
+
+    if (isQuarantined(test)) {
+      return new RunOnceIgnoreOutcome();
     }
 
     return Regular.INSTANCE;
   }
 
+  private boolean isAutoRetryApplicable(TestIdentifier test) {
+    if (!executionSettings.isFlakyTestRetriesEnabled()) {
+      return false;
+    }
+
+    Collection<TestIdentifier> flakyTests = executionSettings.getFlakyTests();
+    return (flakyTests == null || flakyTests.contains(test.withoutParameters()))
+        && autoRetriesUsed.get() < config.getCiVisibilityTotalFlakyRetryCount();
+  }
+
+  private boolean isEFDApplicable(TestIdentifier test, TestSourceData testSource) {
+    EarlyFlakeDetectionSettings efdSettings = executionSettings.getEarlyFlakeDetectionSettings();
+    return efdSettings.isEnabled()
+        && !isEFDLimitReached()
+        && (isNew(test) || isModified(testSource));
+  }
+
   public boolean isEFDLimitReached() {
     Collection<TestIdentifier> knownTests = executionSettings.getKnownTests();
     if (knownTests == null) {

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/config/ExecutionSettingsTest.groovy

@@ -29,6 +29,7 @@ class ExecutionSettingsTest extends Specification {
       null,
       [:],
       [:],
+      new HashSet<>([]),
       null,
       new HashSet<>([]),
       LineDiff.EMPTY),
@@ -41,8 +42,9 @@ class ExecutionSettingsTest extends Specification {
       true,
       new EarlyFlakeDetectionSettings(true, [], 10),
       "",
-      [new TestIdentifier("bc", "def", "g"): new TestMetadata(true), new TestIdentifier("de", "f", null): new TestMetadata(false)],
+      [(new TestIdentifier("bc", "def", "g")): new TestMetadata(true), (new TestIdentifier("de", "f", null)): new TestMetadata(false)],
       [:],
+      new HashSet<>([new TestIdentifier("suite", "quarantined", null)]),
       new HashSet<>([new TestIdentifier("name", null, null)]),
       new HashSet<>([new TestIdentifier("b", "c", "g")]),
       new LineDiff(["path": lines()])
@@ -62,6 +64,7 @@ class ExecutionSettingsTest extends Specification {
         }), "cov2": BitSet.valueOf(new byte[]{
           4, 5, 6
         })],
+      new HashSet<>([new TestIdentifier("suite", "quarantined", null), new TestIdentifier("another", "another-quarantined", null)]),
       new HashSet<>([new TestIdentifier("name", null, "g"), new TestIdentifier("b", "c", null)]),
       new HashSet<>([new TestIdentifier("b", "c", null), new TestIdentifier("bb", "cc", null)]),
       new LineDiff(["path": lines(1, 2, 3)]),
@@ -75,12 +78,13 @@ class ExecutionSettingsTest extends Specification {
       true,
       new EarlyFlakeDetectionSettings(true, [new EarlyFlakeDetectionSettings.ExecutionsByDuration(10, 20), new EarlyFlakeDetectionSettings.ExecutionsByDuration(30, 40)], 10),
       "itrCorrelationId",
-      [new TestIdentifier("bc", "def", null): new TestMetadata(true), new TestIdentifier("de", "f", null): new TestMetadata(true)],
+      [(new TestIdentifier("bc", "def", null)): new TestMetadata(true), (new TestIdentifier("de", "f", null)): new TestMetadata(true)],
       ["cov"    : BitSet.valueOf(new byte[]{
           1, 2, 3
         }), "cov2": BitSet.valueOf(new byte[]{
           4, 5, 6
         })],
+      new HashSet<>([new TestIdentifier("suite", "quarantined", null), new TestIdentifier("another", "another-quarantined", null)]),
       new HashSet<>([]),
       new HashSet<>([new TestIdentifier("b", "c", null), new TestIdentifier("bb", "cc", "g")]),
       new LineDiff(["path": lines(1, 2, 3), "path-b": lines(1, 2, 128, 257, 999)]),

dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy

@@ -71,6 +71,7 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
   private static Path agentKeyFile
 
   private static final List<TestIdentifier> skippableTests = new ArrayList<>()
+  private static final List<TestIdentifier> quarantinedTests = new ArrayList<>()
   private static final List<TestIdentifier> flakyTests = new ArrayList<>()
   private static final List<TestIdentifier> knownTests = new ArrayList<>()
   private static volatile Diff diff = LineDiff.EMPTY
@@ -79,6 +80,7 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
   private static volatile boolean flakyRetryEnabled = false
   private static volatile boolean earlyFlakinessDetectionEnabled = false
   private static volatile boolean impactedTestsDetectionEnabled = false
+  private static volatile boolean quarantineEnabled = false
 
   public static final int SLOW_TEST_THRESHOLD_MILLIS = 1000
   public static final int VERY_SLOW_TEST_THRESHOLD_MILLIS = 2000
@@ -130,6 +132,7 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
         itrEnabled ? "itrCorrelationId" : null,
         skippableTestsWithMetadata,
         [:],
+        quarantinedTests,
         flakyTests,
         earlyFlakinessDetectionEnabled || CIConstants.FAIL_FAST_TEST_ORDER.equalsIgnoreCase(Config.get().ciVisibilityTestOrder) ? knownTests : null,
         diff)
@@ -251,10 +254,12 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
   @Override
   void setup() {
     skippableTests.clear()
+    quarantinedTests.clear()
     flakyTests.clear()
     knownTests.clear()
     diff = LineDiff.EMPTY
     itrEnabled = false
+    quarantineEnabled = false
     flakyRetryEnabled = false
     earlyFlakinessDetectionEnabled = false
     impactedTestsDetectionEnabled = false
@@ -275,10 +280,18 @@ abstract class CiVisibilityInstrumentationTest extends AgentTestRunner {
     knownTests.addAll(tests)
   }
 
+  def givenQuarantinedTests(List<TestIdentifier> tests) {
+    quarantinedTests.addAll(tests)
+  }
+
   def givenDiff(Diff diff) {
     this.diff = diff
   }
 
+  def givenQuarantineEnabled(boolean quarantineEnabled) {
+    this.quarantineEnabled = quarantineEnabled
+  }
+
   def givenFlakyRetryEnabled(boolean flakyRetryEnabled) {
     this.flakyRetryEnabled = flakyRetryEnabled
   }