Skip to content

Implement early exit for retry mechanisms #10604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daniel-mohedano wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Implement early exit for retry mechanisms #10604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0043a80
feat: implement early exit policy
daniel-mohedano Feb 17, 2026
26dafe3
chore: update test fixtures removing additional retries
daniel-mohedano Feb 17, 2026
7cec3c3
chore: missed final status updates
daniel-mohedano Feb 17, 2026
c4bbae6
fix: only tag "test.has_failed_all_retries" if test retried > 0
daniel-mohedano Feb 18, 2026
8350acb
chore: lint
daniel-mohedano Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ public void onTestFinish(
if (outcome.lastExecution()) {
test.setTag(Tags.TEST_FINAL_STATUS, outcome.finalStatus());

if (outcome.failedAllRetries()) {
if (retryReason != null && outcome.failedAllRetries()) {
Copy link
Contributor

@nikita-tkachenko-datadog nikita-tkachenko-datadog Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, what's the reason for this change?

Copy link
Contributor Author

@daniel-mohedano daniel-mohedano Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to avoid tagging tests with @test.has_failed_all_retries when they haven't actually been retried. This specific path could be triggered for an Attempt to Fix test that fails on the first run and exits early out of being retried, causing its only execution (a failure) to be the final one

Copy link
Contributor Author

@daniel-mohedano daniel-mohedano Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at it now, it might be worth considering renaming the Outcome attributes to succeededAllExecutions and failedAllExecutions given that it is more closely aligned with what the variables actually hold (as for event tag, there's a similar case to be made 😢 )

test.setTag(Tags.TEST_HAS_FAILED_ALL_RETRIES, true);
}

Expand Down
27 changes: 19 additions & 8 deletions ...ent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,29 +4,37 @@
import datadog.trace.api.civisibility.execution.TestStatus;
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
import datadog.trace.civisibility.config.ExecutionsByDuration;
import datadog.trace.civisibility.execution.exit.EarlyExitPolicy;
import java.util.List;

/** Runs a test case N times (N depends on test duration) regardless of success or failure. */
/**
* Runs a test case N times (N depends on test duration) regardless of success or failure. The
* execution can also be terminated early if its ExitPolicy evaluates to {@code true}.
*/
public class RunNTimes implements TestExecutionPolicy {

private final boolean suppressFailures;
private final List<ExecutionsByDuration> executionsByDuration;
private int executions;
private int maxExecutions;
private int successfulExecutionsSeen;
private int failedExecutionsSeen;
private final RetryReason retryReason;
private TestStatus lastStatus;
private final EarlyExitPolicy exitPolicy;

public RunNTimes(
List<ExecutionsByDuration> executionsByDuration,
boolean suppressFailures,
RetryReason retryReason) {
RetryReason retryReason,
EarlyExitPolicy exitPolicy) {
this.suppressFailures = suppressFailures;
this.executionsByDuration = executionsByDuration;
this.executions = 0;
this.maxExecutions = getExecutions(0);
this.successfulExecutionsSeen = 0;
this.retryReason = retryReason;
this.exitPolicy = exitPolicy;
}

@Override
Expand All @@ -35,20 +43,21 @@ public ExecutionOutcome registerExecution(TestStatus status, long durationMillis
++executions;
if (status != TestStatus.fail) {
++successfulExecutionsSeen;
} else {
++failedExecutionsSeen;
}
int maxExecutionsForGivenDuration = getExecutions(durationMillis);
maxExecutions = Math.min(maxExecutions, maxExecutionsForGivenDuration);

boolean lastExecution = !retriesLeft();
boolean retry = executions > 1; // first execution is not a retry
boolean failureSuppressed = status == TestStatus.fail && suppressFailures();
boolean succeededAllRetries = lastExecution && successfulExecutionsSeen == executions;

boolean succeededAllExecutions = successfulExecutionsSeen == executions;
TestStatus finalStatus = null;
if (lastExecution) {
// final status will only be "pass" if all retries pass (or the failures were suppressed)
// also, the `suppressFailures()` call works because its value cannot change between retries
if (succeededAllRetries || suppressFailures()) {
if (succeededAllExecutions || suppressFailures()) {
finalStatus = TestStatus.pass;
} else {
finalStatus = TestStatus.fail;
Expand All @@ -58,15 +67,17 @@ public ExecutionOutcome registerExecution(TestStatus status, long durationMillis
return new ExecutionOutcomeImpl(
failureSuppressed,
lastExecution,
lastExecution && successfulExecutionsSeen == 0,
succeededAllRetries,
lastExecution && failedExecutionsSeen == executions,
Copy link
Contributor

@nikita-tkachenko-datadog nikita-tkachenko-datadog Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a skipped test will be considered as "failed all retries" with this update, is this intended?

Copy link
Contributor Author

@daniel-mohedano daniel-mohedano Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a test is skipped, lastExecution == true but failedExecutionsSeen should stay at 0 compared to the executions being incremented to 1, so the check for failedAllRetries should be false. Am I missing anything?

lastExecution && succeededAllExecutions,
retry ? retryReason : null,
finalStatus);
}

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
&& !exitPolicy.evaluate(failedExecutionsSeen != 0, successfulExecutionsSeen != 0);
}

@Override
Expand Down
9 changes: 9 additions & 0 deletions ...i-visibility/src/main/java/datadog/trace/civisibility/execution/exit/EarlyExitPolicy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package datadog.trace.civisibility.execution.exit;

public interface EarlyExitPolicy {

/**
* @return {@code true} if the policy indicates that the test should not be retried anymore.
*/
boolean evaluate(boolean hasFailedExecutions, boolean hasPassedExecutions);
}
13 changes: 13 additions & 0 deletions ...-ci-visibility/src/main/java/datadog/trace/civisibility/execution/exit/ExitOnFailure.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package datadog.trace.civisibility.execution.exit;

/** Policy to avoid retries once a test fails once. Used for Attempt to Fix. */
public class ExitOnFailure implements EarlyExitPolicy {
public static final EarlyExitPolicy INSTANCE = new ExitOnFailure();

private ExitOnFailure() {}

@Override
public boolean evaluate(boolean hasFailedExecutions, boolean hasPassedExecutions) {
return hasFailedExecutions;
}
}
16 changes: 16 additions & 0 deletions ...nt-ci-visibility/src/main/java/datadog/trace/civisibility/execution/exit/ExitOnFlake.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package datadog.trace.civisibility.execution.exit;

/**
* Policy to avoid retries once a test flakes (has at least one failed and passed execution). Used
* for Early Flake Detection.
*/
public class ExitOnFlake implements EarlyExitPolicy {
public static final EarlyExitPolicy INSTANCE = new ExitOnFlake();

private ExitOnFlake() {}

@Override
public boolean evaluate(boolean hasFailedExecutions, boolean hasPassedExecutions) {
return hasFailedExecutions && hasPassedExecutions;
}
}
13 changes: 13 additions & 0 deletions ...t/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/exit/NoExit.java
View file
Open in desktop
Copy link
Contributor

@nikita-tkachenko-datadog nikita-tkachenko-datadog Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class doesn't seem to be used anywhere, do we need it?

Copy link
Contributor Author

@daniel-mohedano daniel-mohedano Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created it for testing purposes to maintain the old test cases, but in hindsight I can totally remove it and align the test cases with the only two possible policies currently (ExitOnFlake and ExitOnFailure)

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package datadog.trace.civisibility.execution.exit;

/** Policy that ignores early exit on retries. Currently only used in testing. */
public class NoExit implements EarlyExitPolicy {
Copy link
Contributor

@nikita-tkachenko-datadog nikita-tkachenko-datadog Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could rename it to NoEarlyExit so it sounds less dramatic :)

public static final EarlyExitPolicy INSTANCE = new NoExit();

private NoExit() {}

@Override
public boolean evaluate(boolean hasFailedExecutions, boolean hasPassedExecutions) {
return false;
}
}
8 changes: 6 additions & 2 deletions .../agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import datadog.trace.civisibility.execution.RetryUntilSuccessful;
import datadog.trace.civisibility.execution.RunNTimes;
import datadog.trace.civisibility.execution.RunOnceIgnoreOutcome;
import datadog.trace.civisibility.execution.exit.ExitOnFailure;
import datadog.trace.civisibility.execution.exit.ExitOnFlake;
import datadog.trace.civisibility.source.LinesResolver;
import datadog.trace.civisibility.source.SourcePathResolver;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -129,7 +131,8 @@ public TestExecutionPolicy executionPolicy(
return new RunNTimes(
executionSettings.getTestManagementSettings().getAttemptToFixExecutions(),
isQuarantined(test) || isDisabled(test),
RetryReason.attemptToFix);
RetryReason.attemptToFix,
ExitOnFailure.INSTANCE);
}

if (isEFDApplicable(test, testSource, testTags)) {
Expand All @@ -139,7 +142,8 @@ public TestExecutionPolicy executionPolicy(
return new RunNTimes(
executionSettings.getEarlyFlakeDetectionSettings().getExecutionsByDuration(),
isQuarantined(test),
RetryReason.efd);
RetryReason.efd,
ExitOnFlake.INSTANCE);
}

if (isAutoRetryApplicable(test)) {
Expand Down
46 changes: 40 additions & 6 deletions ...t-ci-visibility/src/test/groovy/datadog/trace/civisibility/execution/RunNTimesTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ package datadog.trace.civisibility.execution
import datadog.trace.api.civisibility.execution.TestStatus
import datadog.trace.api.civisibility.telemetry.tag.RetryReason
import datadog.trace.civisibility.config.ExecutionsByDuration
import datadog.trace.civisibility.execution.exit.ExitOnFailure
import datadog.trace.civisibility.execution.exit.ExitOnFlake
import datadog.trace.civisibility.execution.exit.NoExit
import spock.lang.Specification

class RunNTimesTest extends Specification {

def "test run N times"() {
setup:
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.efd)
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.efd, NoExit.INSTANCE)

when:
def outcome = executionPolicy.registerExecution(TestStatus.fail, 0)
Expand Down Expand Up @@ -47,7 +50,7 @@ class RunNTimesTest extends Specification {

def "test failed all retries"() {
setup:
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.efd)
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.efd, NoExit.INSTANCE)

when:
def outcome = executionPolicy.registerExecution(TestStatus.fail, 0)
Expand Down Expand Up @@ -85,7 +88,7 @@ class RunNTimesTest extends Specification {

def "test succeeded all retries"() {
setup:
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.efd)
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.efd, NoExit.INSTANCE)

when:
def outcome = executionPolicy.registerExecution(TestStatus.pass, 0)
Expand Down Expand Up @@ -123,7 +126,7 @@ class RunNTimesTest extends Specification {

def "test suppress failures"() {
setup:
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], true, RetryReason.attemptToFix)
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], true, RetryReason.attemptToFix, NoExit.INSTANCE)

when:
def outcome = executionPolicy.registerExecution(TestStatus.fail, 0)
Expand Down Expand Up @@ -159,17 +162,48 @@ class RunNTimesTest extends Specification {
outcome3.finalStatus() == TestStatus.pass
}

def "test exits on flake"(){
setup:
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.efd, ExitOnFlake.INSTANCE)

when:
def outcome = executionPolicy.registerExecution(TestStatus.fail, 0)

then:
!outcome.lastExecution()
outcome.finalStatus() == null

when:
def outcome2 = executionPolicy.registerExecution(TestStatus.pass, 0)

then:
outcome2.lastExecution()
outcome2.finalStatus() == TestStatus.fail
}

def "test exits on failure"() {
setup:
def executionPolicy = new RunNTimes([new ExecutionsByDuration(Long.MAX_VALUE, 3)], false, RetryReason.attemptToFix, ExitOnFailure.INSTANCE)

when:
def outcome = executionPolicy.registerExecution(TestStatus.fail, 0)

then:
outcome.lastExecution()
outcome.finalStatus() == TestStatus.fail
}

def "test adaptive retry count"() {
when:
def executionPolicy = new RunNTimes([new ExecutionsByDuration(100, 3), new ExecutionsByDuration(Long.MAX_VALUE, 1)], true, RetryReason.efd)
def executionPolicy = new RunNTimes([new ExecutionsByDuration(100, 3), new ExecutionsByDuration(Long.MAX_VALUE, 1)], true, RetryReason.efd, NoExit.INSTANCE)

then:
!executionPolicy.registerExecution(TestStatus.fail, 0).lastExecution()
!executionPolicy.registerExecution(TestStatus.fail, 0).lastExecution()
executionPolicy.registerExecution(TestStatus.fail, 0).lastExecution()

when:
executionPolicy = new RunNTimes([new ExecutionsByDuration(100, 3), new ExecutionsByDuration(Long.MAX_VALUE, 1)], true, RetryReason.efd)
executionPolicy = new RunNTimes([new ExecutionsByDuration(100, 3), new ExecutionsByDuration(Long.MAX_VALUE, 1)], true, RetryReason.efd, NoExit.INSTANCE)

then:
!executionPolicy.registerExecution(TestStatus.fail, 0).lastExecution()
Expand Down
28 changes: 0 additions & 28 deletions ...junit-4-cucumber-5.4/src/test/resources/test-attempt-to-fix-disabled-failed/coverages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,4 @@
"span_id" : ${content_span_id},
"test_session_id" : ${content_test_session_id},
"test_suite_id" : ${content_test_suite_id}
}, {
"files" : [ {
"filename" : "org/example/cucumber/calculator/basic_arithmetic_failed.feature"
} ],
"span_id" : ${content_span_id_2},
"test_session_id" : ${content_test_session_id},
"test_suite_id" : ${content_test_suite_id}
}, {
"files" : [ {
"filename" : "org/example/cucumber/calculator/basic_arithmetic_failed.feature"
} ],
"span_id" : ${content_span_id_3},
"test_session_id" : ${content_test_session_id},
"test_suite_id" : ${content_test_suite_id}
}, {
"files" : [ {
"filename" : "org/example/cucumber/calculator/basic_arithmetic_failed.feature"
} ],
"span_id" : ${content_span_id_4},
"test_session_id" : ${content_test_session_id},
"test_suite_id" : ${content_test_suite_id}
}, {
"files" : [ {
"filename" : "org/example/cucumber/calculator/basic_arithmetic_failed.feature"
} ],
"span_id" : ${content_span_id_5},
"test_session_id" : ${content_test_session_id},
"test_suite_id" : ${content_test_suite_id}
} ]
Loading