Skip to content
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.

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

Generalize TestRetryPolicy to TestExecutionPolicy #8302

Merged
merged 1 commit into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import datadog.trace.api.civisibility.config.TestIdentifier;
import datadog.trace.api.civisibility.config.TestSourceData;
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
import javax.annotation.Nonnull;
Expand Down Expand Up @@ -41,7 +41,7 @@ TestSuiteImpl testSuiteStart(
SkipReason skipReason(TestIdentifier test);

@Nonnull
TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource);
TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData testSource);

void end(Long startTime);
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
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.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
Expand Down Expand Up @@ -111,8 +111,8 @@ public SkipReason skipReason(TestIdentifier test) {

@Override
@Nonnull
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) {
return executionStrategy.retryPolicy(test, testSource);
public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData testSource) {
return executionStrategy.executionPolicy(test, testSource);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
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.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
Expand Down Expand Up @@ -96,8 +96,8 @@ public SkipReason skipReason(TestIdentifier test) {

@Override
@Nonnull
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) {
return executionStrategy.retryPolicy(test, testSource);
public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData testSource) {
return executionStrategy.executionPolicy(test, testSource);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
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.retry.TestRetryPolicy;
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;
import datadog.trace.civisibility.retry.NeverRetry;
import datadog.trace.civisibility.execution.Regular;
import java.util.Collection;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -99,8 +99,8 @@ public SkipReason skipReason(TestIdentifier test) {

@NotNull
@Override
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData source) {
return NeverRetry.INSTANCE;
public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData source) {
return Regular.INSTANCE;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +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.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
import datadog.trace.api.civisibility.telemetry.tag.EventType;
Expand Down Expand Up @@ -264,8 +264,8 @@ public void onTestIgnore(

@Override
@Nonnull
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) {
return testModule.retryPolicy(test, testSource);
public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData testSource) {
return testModule.executionPolicy(test, testSource);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
package datadog.trace.civisibility.retry;
package datadog.trace.civisibility.execution;

import datadog.trace.api.civisibility.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
import org.jetbrains.annotations.Nullable;

public class NeverRetry implements TestRetryPolicy {
/** Regular test case execution with no alterations. */
public class Regular implements TestExecutionPolicy {

public static final TestRetryPolicy INSTANCE = new NeverRetry();
public static final TestExecutionPolicy INSTANCE = new Regular();

private NeverRetry() {}
private Regular() {}

@Override
public boolean retriesLeft() {
public boolean applicable() {
return false;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,35 +1,37 @@
package datadog.trace.civisibility.retry;
package datadog.trace.civisibility.execution;

import datadog.trace.api.civisibility.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
import java.util.concurrent.atomic.AtomicInteger;
import org.jetbrains.annotations.Nullable;

/** Retries a test case if it failed, up to a maximum number of times. */
public class RetryIfFailed implements TestRetryPolicy {
public class RetryUntilSuccessful implements TestExecutionPolicy {

private final int maxExecutions;
private int executions;

/** Total execution counter that is shared by all retry policies */
private final AtomicInteger totalExecutions;

public RetryIfFailed(int maxExecutions, AtomicInteger totalExecutions) {
public RetryUntilSuccessful(int maxExecutions, AtomicInteger totalExecutions) {
this.maxExecutions = maxExecutions;
this.totalExecutions = totalExecutions;
this.executions = 0;
}

@Override
public boolean retriesLeft() {
public boolean applicable() {
// the last execution is not altered by the policy
// (no retries, no exceptions suppressing)
return executions < maxExecutions - 1;
}

@Override
public boolean suppressFailures() {
// if this isn't the last attempt,
// if this isn't the last execution,
// possible failures should be suppressed
return retriesLeft();
return applicable();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
package datadog.trace.civisibility.retry;
package datadog.trace.civisibility.execution;

import datadog.trace.api.civisibility.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings;
import org.jetbrains.annotations.Nullable;

/** Retries a test case N times (N depends on test duration) regardless of success or failure. */
public class RetryNTimes implements TestRetryPolicy {
/** Runs a test case N times (N depends on test duration) regardless of success or failure. */
public class RunNTimes implements TestExecutionPolicy {

private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
private int executions;
private int maxExecutions;

public RetryNTimes(EarlyFlakeDetectionSettings earlyFlakeDetectionSettings) {
public RunNTimes(EarlyFlakeDetectionSettings earlyFlakeDetectionSettings) {
this.earlyFlakeDetectionSettings = earlyFlakeDetectionSettings;
this.executions = 0;
this.maxExecutions = earlyFlakeDetectionSettings.getExecutions(0);
}

@Override
public boolean retriesLeft() {
public boolean applicable() {
// the last execution is not altered by the policy
// (no retries, no exceptions suppressing)
return executions < maxExecutions - 1;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
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.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings;
import datadog.trace.civisibility.config.ExecutionSettings;
import datadog.trace.civisibility.retry.NeverRetry;
import datadog.trace.civisibility.retry.RetryIfFailed;
import datadog.trace.civisibility.retry.RetryNTimes;
import datadog.trace.civisibility.execution.Regular;
import datadog.trace.civisibility.execution.RetryUntilSuccessful;
import datadog.trace.civisibility.execution.RunNTimes;
import datadog.trace.civisibility.source.LinesResolver;
import datadog.trace.civisibility.source.SourcePathResolver;
import java.lang.reflect.Method;
Expand Down Expand Up @@ -80,9 +80,9 @@ public SkipReason skipReason(TestIdentifier test) {
}

@Nonnull
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) {
public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData testSource) {
if (test == null) {
return NeverRetry.INSTANCE;
return Regular.INSTANCE;
}

EarlyFlakeDetectionSettings efdSettings = executionSettings.getEarlyFlakeDetectionSettings();
Expand All @@ -91,7 +91,7 @@ public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSourc
// 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 RetryNTimes(efdSettings);
return new RunNTimes(efdSettings);
}
}

Expand All @@ -101,10 +101,11 @@ public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSourc
&& 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 RetryIfFailed(config.getCiVisibilityFlakyRetryCount(), autoRetriesUsed);
return new RetryUntilSuccessful(config.getCiVisibilityFlakyRetryCount(), autoRetriesUsed);
}
}
return NeverRetry.INSTANCE;

return Regular.INSTANCE;
}

public boolean isEFDLimitReached() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,21 +56,21 @@ class ProxyTestModuleTest extends DDSpecification {
)

when:
def retryPolicy1 = proxyTestModule.retryPolicy(new TestIdentifier("suite", "test-1", null), TestSourceData.UNKNOWN)
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

when:
def retryPolicy2 = proxyTestModule.retryPolicy(new TestIdentifier("suite", "test-2", null), TestSourceData.UNKNOWN)
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

when:
def retryPolicy3 = proxyTestModule.retryPolicy(new TestIdentifier("suite", "test-3", null), TestSourceData.UNKNOWN)
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,21 @@ class HeadlessTestModuleTest extends DDSpecification {
)

when:
def retryPolicy1 = headlessTestModule.retryPolicy(new TestIdentifier("suite", "test-1", null), TestSourceData.UNKNOWN)
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

when:
def retryPolicy2 = headlessTestModule.retryPolicy(new TestIdentifier("suite", "test-2", null), TestSourceData.UNKNOWN)
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

when:
def retryPolicy3 = headlessTestModule.retryPolicy(new TestIdentifier("suite", "test-3", null), TestSourceData.UNKNOWN)
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
import datadog.trace.bootstrap.ContextStore;
import io.cucumber.core.gherkin.Pickle;
Expand All @@ -29,11 +29,11 @@ public class CucumberTracingListener extends TracingListener {
public static final String FRAMEWORK_NAME = "cucumber";
public static final String FRAMEWORK_VERSION = CucumberUtils.getVersion();

private final ContextStore<Description, TestRetryPolicy> retryPolicies;
private final ContextStore<Description, TestExecutionPolicy> retryPolicies;
private final Map<Object, Pickle> pickleById;

public CucumberTracingListener(
ContextStore<Description, TestRetryPolicy> retryPolicies,
ContextStore<Description, TestExecutionPolicy> retryPolicies,
List<ParentRunner<?>> featureRunners) {
this.retryPolicies = retryPolicies;
pickleById = CucumberUtils.getPicklesById(featureRunners);
Expand Down Expand Up @@ -71,7 +71,7 @@ public void testStarted(final Description description) {
String testName = CucumberUtils.getTestNameForScenario(description);
List<String> categories = getCategories(description);

TestRetryPolicy retryPolicy = retryPolicies.get(description);
TestExecutionPolicy retryPolicy = retryPolicies.get(description);
TestEventsHandlerHolder.TEST_EVENTS_HANDLER.onTestStart(
new TestSuiteDescriptor(testSuiteName, null),
CucumberUtils.toTestDescriptor(description),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.retry.TestRetryPolicy;
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
import datadog.trace.bootstrap.InstrumentationContext;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -46,7 +46,7 @@ public String[] helperClassNames() {
@Override
public Map<String, String> contextStore() {
return Collections.singletonMap(
"org.junit.runner.Description", TestRetryPolicy.class.getName());
"org.junit.runner.Description", TestExecutionPolicy.class.getName());
}

@Override
Expand Down Expand Up @@ -84,7 +84,7 @@ public static void addTracingListener(

replacedNotifier.addListener(
new CucumberTracingListener(
InstrumentationContext.get(Description.class, TestRetryPolicy.class), children));
InstrumentationContext.get(Description.class, TestExecutionPolicy.class), children));
runNotifier = replacedNotifier;
}
}
Expand Down
Loading
Loading