Skip to content

Commit

Permalink
Implement tests quarantining
Browse files Browse the repository at this point in the history
  • Loading branch information
nikita-tkachenko-datadog committed Jan 31, 2025
1 parent b24d153 commit af973a0
Show file tree
Hide file tree
Showing 54 changed files with 4,271 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class ExecutionSettings {
Collections.emptyMap(),
Collections.emptyList(),
null,
null,
LineDiff.EMPTY);

private final boolean itrEnabled;
Expand All @@ -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;
Expand All @@ -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) {
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -171,6 +181,7 @@ public int hashCode() {
itrCorrelationId,
skippableTests,
skippableTestsCoverage,
quarantinedTests,
flakyTests,
knownTests,
pullRequestDiff);
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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 =
Expand All @@ -251,6 +265,7 @@ public static ExecutionSettings deserialize(ByteBuffer buffer) {
itrCorrelationId,
skippableTests,
skippableTestsCoverage,
quarantinedTests,
flakyTests,
knownTests,
diff);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ class ExecutionSettingsTest extends Specification {
null,
[:],
[:],
new HashSet<>([]),
null,
new HashSet<>([]),
LineDiff.EMPTY),
Expand All @@ -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()])
Expand All @@ -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)]),
Expand All @@ -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)]),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
Loading

0 comments on commit af973a0

Please sign in to comment.