Fix test status propagation in headless mode (#9478)

Author: nikita-tkachenko-datadog

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/SpanTagsPropagator.java

@@ -136,23 +136,19 @@ private void unsafePropagateStatus(AgentSpan childSpan) {
       return;
     }
 
+    Boolean childFailureSuppressed = (Boolean) childSpan.getTag(Tags.TEST_FAILURE_SUPPRESSED);
     TestStatus parentStatus = (TestStatus) parentSpan.getTag(Tags.TEST_STATUS);
-    switch (childStatus) {
-      case pass:
-        if (parentStatus == null || TestStatus.skip.equals(parentStatus)) {
-          parentSpan.setTag(Tags.TEST_STATUS, TestStatus.pass);
-        }
-        break;
-      case fail:
-        parentSpan.setTag(Tags.TEST_STATUS, TestStatus.fail);
-        break;
-      case skip:
-        if (parentStatus == null) {
-          parentSpan.setTag(Tags.TEST_STATUS, TestStatus.skip);
-        }
-        break;
-      default:
-        break;
+    if (childStatus == TestStatus.pass
+        || (childFailureSuppressed != null && childFailureSuppressed)) {
+      if (parentStatus == null || TestStatus.skip.equals(parentStatus)) {
+        parentSpan.setTag(Tags.TEST_STATUS, TestStatus.pass);
+      }
+    } else if (childStatus == TestStatus.fail) {
+      parentSpan.setTag(Tags.TEST_STATUS, TestStatus.fail);
+    } else if (childStatus == TestStatus.skip) {
+      if (parentStatus == null) {
+        parentSpan.setTag(Tags.TEST_STATUS, TestStatus.skip);
+      }
     }
   }
 

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

@@ -58,8 +58,7 @@ public void onTestStart(
       @Nullable String testParameters,
       @Nullable Collection<String> categories,
       @Nonnull TestSourceData testSourceData,
-      @Nullable Long startTime,
-      @Nullable TestExecutionHistory testExecutionHistory) {
+      @Nullable Long startTime) {
     // do nothing
   }
 

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

@@ -141,8 +141,7 @@ public void onTestStart(
       final @Nullable String testParameters,
       final @Nullable Collection<String> categories,
       final @Nonnull TestSourceData testSourceData,
-      final @Nullable Long startTime,
-      final @Nullable TestExecutionHistory testExecutionHistory) {
+      final @Nullable Long startTime) {
     if (skipTrace(testSourceData.getTestClass())) {
       return;
     }
@@ -180,14 +179,6 @@ public void onTestStart(
       test.setTag(Tags.TEST_TEST_MANAGEMENT_IS_ATTEMPT_TO_FIX, 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) {
@@ -254,20 +245,28 @@ public void onTestFinish(
       return;
     }
 
-    TestIdentifier thisTest = test.getIdentifier();
     if (testExecutionHistory != null) {
       TestStatus testStatus = test.getStatus();
-      testExecutionHistory.registerExecution(
-          testStatus != null ? testStatus : TestStatus.skip, test.getDuration(endTime));
+      TestExecutionHistory.ExecutionOutcome outcome =
+          testExecutionHistory.registerExecution(
+              testStatus != null ? testStatus : TestStatus.skip, test.getDuration(endTime));
 
-      if (testExecutionHistory.hasFailedAllRetries()) {
+      RetryReason retryReason = outcome.retryReason();
+      if (retryReason != null) {
+        test.setTag(Tags.TEST_IS_RETRY, true);
+        test.setTag(Tags.TEST_RETRY_REASON, retryReason);
+      }
+
+      if (outcome.failedAllRetries()) {
         test.setTag(Tags.TEST_HAS_FAILED_ALL_RETRIES, true);
       }
 
-      if (testExecutionHistory.wasLastExecution() && testModule.isAttemptToFix(thisTest)) {
-        test.setTag(
-            Tags.TEST_TEST_MANAGEMENT_ATTEMPT_TO_FIX_PASSED,
-            testExecutionHistory.hasSucceededAllRetries());
+      if (outcome.lastExecution() && testModule.isAttemptToFix(test.getIdentifier())) {
+        test.setTag(Tags.TEST_TEST_MANAGEMENT_ATTEMPT_TO_FIX_PASSED, outcome.succeededAllRetries());
+      }
+
+      if (outcome.failureSuppressed()) {
+        test.setTag(Tags.TEST_FAILURE_SUPPRESSED, true);
       }
     }
 
@@ -295,8 +294,7 @@ public void onTestIgnore(
         testParameters,
         categories,
         testSourceData,
-        null,
-        testExecutionHistory);
+        null);
     onTestSkip(testDescriptor, reason);
     onTestFinish(testDescriptor, null, testExecutionHistory);
   }

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

@@ -0,0 +1,53 @@
+package datadog.trace.civisibility.execution;
+
+import datadog.trace.api.civisibility.execution.TestExecutionHistory;
+import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
+import org.jetbrains.annotations.Nullable;
+
+class ExecutionOutcomeImpl implements TestExecutionHistory.ExecutionOutcome {
+
+  private final boolean failureSuppressed;
+  private final boolean lastExecution;
+  private final boolean failedAllRetries;
+  private final boolean succeededAllRetries;
+  private final RetryReason retryReason;
+
+  ExecutionOutcomeImpl(
+      boolean failureSuppressed,
+      boolean lastExecution,
+      boolean failedAllRetries,
+      boolean succeededAllRetries,
+      RetryReason retryReason) {
+    this.failureSuppressed = failureSuppressed;
+    this.lastExecution = lastExecution;
+    this.failedAllRetries = failedAllRetries;
+    this.succeededAllRetries = succeededAllRetries;
+    this.retryReason = retryReason;
+  }
+
+  @Override
+  public boolean failureSuppressed() {
+    return failureSuppressed;
+  }
+
+  @Override
+  public boolean lastExecution() {
+    return lastExecution;
+  }
+
+  @Override
+  public boolean failedAllRetries() {
+    return failedAllRetries;
+  }
+
+  @Override
+  public boolean succeededAllRetries() {
+    return succeededAllRetries;
+  }
+
+  @Nullable
+  @Override
+  public RetryReason retryReason() {
+    return retryReason;
+  }
+}

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

@@ -1,5 +1,6 @@
 package datadog.trace.civisibility.execution;
 
+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.tag.RetryReason;
@@ -13,11 +14,8 @@ public class Regular implements TestExecutionPolicy {
   private Regular() {}
 
   @Override
-  public void registerExecution(TestStatus status, long durationMillis) {}
-
-  @Override
-  public boolean wasLastExecution() {
-    return true;
+  public ExecutionOutcome registerExecution(TestStatus status, long durationMillis) {
+    return RegularExecutionOutcome.INSTANCE;
   }
 
   @Override
@@ -30,19 +28,37 @@ public boolean suppressFailures() {
     return false;
   }
 
-  @Nullable
-  @Override
-  public RetryReason currentExecutionRetryReason() {
-    return null;
-  }
+  private static final class RegularExecutionOutcome
+      implements TestExecutionHistory.ExecutionOutcome {
 
-  @Override
-  public boolean hasFailedAllRetries() {
-    return false;
-  }
+    static final TestExecutionHistory.ExecutionOutcome INSTANCE = new RegularExecutionOutcome();
 
-  @Override
-  public boolean hasSucceededAllRetries() {
-    return false;
+    private RegularExecutionOutcome() {}
+
+    @Override
+    public boolean failureSuppressed() {
+      return false;
+    }
+
+    @Override
+    public boolean lastExecution() {
+      return false;
+    }
+
+    @Override
+    public boolean failedAllRetries() {
+      return false;
+    }
+
+    @Override
+    public boolean succeededAllRetries() {
+      return false;
+    }
+
+    @Nullable
+    @Override
+    public RetryReason retryReason() {
+      return null;
+    }
   }
 }

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

@@ -4,7 +4,6 @@
 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;
 
 /** Retries a test case if it failed, up to a maximum number of times. */
 public class RetryUntilSuccessful implements TestExecutionPolicy {
@@ -26,54 +25,38 @@ public RetryUntilSuccessful(
   }
 
   @Override
-  public void registerExecution(TestStatus status, long durationMillis) {
+  public ExecutionOutcome registerExecution(TestStatus status, long durationMillis) {
     ++executions;
     successfulExecutionSeen |= (status != TestStatus.fail);
     if (executions > 1) {
       totalRetryCount.incrementAndGet();
     }
-  }
 
-  @Override
-  public boolean wasLastExecution() {
-    return successfulExecutionSeen || executions == maxExecutions;
+    boolean lastExecution = !retriesLeft();
+    boolean retry = executions > 1; // first execution is not a retry
+    return new ExecutionOutcomeImpl(
+        status == TestStatus.fail && (!lastExecution || suppressFailures),
+        lastExecution,
+        lastExecution && !successfulExecutionSeen,
+        false,
+        retry ? RetryReason.atr : null);
   }
 
-  private boolean currentExecutionIsLast() {
-    return executions == maxExecutions - 1;
+  private boolean retriesLeft() {
+    return !successfulExecutionSeen && executions < maxExecutions;
   }
 
   @Override
   public boolean applicable() {
     // executions must always be registered, therefore consider it applicable as long as there are
     // retries left
-    return !wasLastExecution();
+    return retriesLeft();
   }
 
   @Override
   public boolean suppressFailures() {
-    // do not suppress failures for last execution
-    // (unless flag to suppress all failures is set)
-    return !currentExecutionIsLast() || suppressFailures;
-  }
-
-  @Nullable
-  @Override
-  public RetryReason currentExecutionRetryReason() {
-    return currentExecutionIsRetry() ? RetryReason.atr : null;
-  }
-
-  private boolean currentExecutionIsRetry() {
-    return executions > 0;
-  }
-
-  @Override
-  public boolean hasFailedAllRetries() {
-    return wasLastExecution() && !successfulExecutionSeen;
-  }
-
-  @Override
-  public boolean hasSucceededAllRetries() {
-    return false;
+    // do not suppress failures for last execution (unless flag to suppress all failures is set);
+    // the +1 is because this method is called _before_ subsequent execution is registered
+    return executions + 1 < maxExecutions || suppressFailures;
   }
 }

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

@@ -5,7 +5,6 @@
 import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
 import datadog.trace.civisibility.config.ExecutionsByDuration;
 import java.util.List;
-import javax.annotation.Nullable;
 
 /** Runs a test case N times (N depends on test duration) regardless of success or failure. */
 public class RunNTimes implements TestExecutionPolicy {
@@ -31,27 +30,35 @@ public RunNTimes(
   }
 
   @Override
-  public void registerExecution(TestStatus status, long durationMillis) {
+  public ExecutionOutcome registerExecution(TestStatus status, long durationMillis) {
     lastStatus = status;
     ++executions;
     if (status != TestStatus.fail) {
       ++successfulExecutionsSeen;
     }
     int maxExecutionsForGivenDuration = getExecutions(durationMillis);
     maxExecutions = Math.min(maxExecutions, maxExecutionsForGivenDuration);
+
+    boolean lastExecution = !retriesLeft();
+    boolean retry = executions > 1; // first execution is not a retry
+    return new ExecutionOutcomeImpl(
+        status == TestStatus.fail && suppressFailures(),
+        lastExecution,
+        lastExecution && successfulExecutionsSeen == 0,
+        lastExecution && successfulExecutionsSeen == executions,
+        retry ? retryReason : null);
   }
 
-  @Override
-  public boolean wasLastExecution() {
+  private boolean retriesLeft() {
     // skipped tests (either by the framework or DD) should not be retried
-    return lastStatus == TestStatus.skip || executions >= maxExecutions;
+    return lastStatus != TestStatus.skip && executions < maxExecutions;
   }
 
   @Override
   public boolean applicable() {
     // executions must always be registered, therefore consider it applicable as long as there are
     // retries left
-    return !wasLastExecution();
+    return retriesLeft();
   }
 
   @Override
@@ -67,24 +74,4 @@ private int getExecutions(long durationMillis) {
     }
     return 0;
   }
-
-  @Nullable
-  @Override
-  public RetryReason currentExecutionRetryReason() {
-    return currentExecutionIsRetry() ? retryReason : null;
-  }
-
-  private boolean currentExecutionIsRetry() {
-    return executions > 0;
-  }
-
-  @Override
-  public boolean hasFailedAllRetries() {
-    return wasLastExecution() && successfulExecutionsSeen == 0;
-  }
-
-  @Override
-  public boolean hasSucceededAllRetries() {
-    return wasLastExecution() && successfulExecutionsSeen == executions;
-  }
 }