Skip to content

Commit deab2cd

Browse files
daniel-mohedanodaniel-mohedano
authored
Refactor execution policies (#10676)
refactor: execution policy and history fix: spotbugs Merge branch 'master' into daniel.mohedano/refactor-execution-policies Co-authored-by: daniel.mohedano <daniel.mohedano@datadoghq.com>
1 parent b977fe3 commit deab2cd

File tree

49 files changed

+568
-600
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+568
-600
lines changed

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22

33
import com.squareup.moshi.FromJson;
44
import java.nio.ByteBuffer;
5-
import java.util.Collections;
6-
import java.util.List;
75
import java.util.Map;
86
import java.util.Objects;
97

@@ -27,10 +25,6 @@ public int getAttemptToFixRetries() {
2725
return attemptToFixRetries;
2826
}
2927

30-
public List<ExecutionsByDuration> getAttemptToFixExecutions() {
31-
return Collections.singletonList(new ExecutionsByDuration(Long.MAX_VALUE, attemptToFixRetries));
32-
}
33-
3428
@Override
3529
public boolean equals(Object o) {
3630
if (this == o) {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
import datadog.trace.api.civisibility.config.TestIdentifier;
77
import datadog.trace.api.civisibility.config.TestSourceData;
88
import datadog.trace.api.civisibility.events.TestEventsHandler;
9-
import datadog.trace.api.civisibility.execution.TestExecutionHistory;
109
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
10+
import datadog.trace.api.civisibility.execution.TestExecutionTracker;
1111
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
1212
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1313
import datadog.trace.bootstrap.ContextStore;
@@ -59,7 +59,7 @@ public void onTestStart(
5959
@Nullable Collection<String> categories,
6060
@Nonnull TestSourceData testSourceData,
6161
@Nullable Long startTime,
62-
@Nullable TestExecutionHistory testExecutionHistory) {
62+
@Nullable TestExecutionTracker testExecutionTracker) {
6363
// do nothing
6464
}
6565

@@ -75,7 +75,7 @@ public void onTestFailure(TestKey descriptor, @Nullable Throwable throwable) {
7575

7676
@Override
7777
public void onTestFinish(
78-
TestKey descriptor, @Nullable Long endTime, @Nullable TestExecutionHistory executionHistory) {
78+
TestKey descriptor, @Nullable Long endTime, @Nullable TestExecutionTracker executionTracker) {
7979
// do nothing
8080
}
8181

@@ -90,7 +90,7 @@ public void onTestIgnore(
9090
@Nullable Collection<String> categories,
9191
@Nonnull TestSourceData testSourceData,
9292
@Nullable String reason,
93-
@Nullable TestExecutionHistory testExecutionHistory) {
93+
@Nullable TestExecutionTracker testExecutionTracker) {
9494
// do nothing
9595
}
9696

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

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@
88
import datadog.trace.api.civisibility.config.TestIdentifier;
99
import datadog.trace.api.civisibility.config.TestSourceData;
1010
import datadog.trace.api.civisibility.events.TestEventsHandler;
11-
import datadog.trace.api.civisibility.execution.TestExecutionHistory;
11+
import datadog.trace.api.civisibility.execution.ExecutionAggregation;
1212
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
13+
import datadog.trace.api.civisibility.execution.TestExecutionTracker;
1314
import datadog.trace.api.civisibility.execution.TestStatus;
1415
import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
1516
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
@@ -142,7 +143,7 @@ public void onTestStart(
142143
final @Nullable Collection<String> categories,
143144
final @Nonnull TestSourceData testSourceData,
144145
final @Nullable Long startTime,
145-
final @Nullable TestExecutionHistory testExecutionHistory) {
146+
final @Nullable TestExecutionTracker testExecutionTracker) {
146147
if (skipTrace(testSourceData.getTestClass())) {
147148
return;
148149
}
@@ -180,8 +181,8 @@ public void onTestStart(
180181
test.setTag(Tags.TEST_TEST_MANAGEMENT_IS_ATTEMPT_TO_FIX, true);
181182
}
182183

183-
if (testExecutionHistory != null) {
184-
test.getContext().set(TestExecutionHistory.class, testExecutionHistory);
184+
if (testExecutionTracker != null) {
185+
test.getContext().set(TestExecutionTracker.class, testExecutionTracker);
185186
}
186187

187188
if (testFramework != null) {
@@ -243,7 +244,7 @@ public void onTestFailure(TestKey descriptor, @Nullable Throwable throwable) {
243244
public void onTestFinish(
244245
TestKey descriptor,
245246
@Nullable Long endTime,
246-
@Nullable TestExecutionHistory testExecutionHistory) {
247+
@Nullable TestExecutionTracker testExecutionTracker) {
247248
TestImpl test = inProgressTests.remove(descriptor);
248249
if (test == null) {
249250
log.debug("Ignoring finish event, could not find test {}", descriptor);
@@ -252,9 +253,9 @@ public void onTestFinish(
252253

253254
TestStatus testStatus = test.getStatus() != null ? test.getStatus() : TestStatus.skip;
254255

255-
if (testExecutionHistory != null) {
256-
TestExecutionHistory.ExecutionOutcome outcome =
257-
testExecutionHistory.registerExecution(testStatus, test.getDuration(endTime));
256+
if (testExecutionTracker != null) {
257+
TestExecutionTracker.ExecutionOutcome outcome =
258+
testExecutionTracker.registerExecution(testStatus, test.getDuration(endTime));
258259

259260
RetryReason retryReason = outcome.retryReason();
260261
if (retryReason != null) {
@@ -269,13 +270,14 @@ public void onTestFinish(
269270
if (outcome.lastExecution()) {
270271
test.setTag(Tags.TEST_FINAL_STATUS, outcome.finalStatus());
271272

272-
if (retryReason != null && outcome.failedAllRetries()) {
273+
if (retryReason != null && outcome.aggregation() == ExecutionAggregation.ONLY_FAILED) {
273274
test.setTag(Tags.TEST_HAS_FAILED_ALL_RETRIES, true);
274275
}
275276

276277
if (testModule.isAttemptToFix(test.getIdentifier())) {
277278
test.setTag(
278-
Tags.TEST_TEST_MANAGEMENT_ATTEMPT_TO_FIX_PASSED, outcome.succeededAllRetries());
279+
Tags.TEST_TEST_MANAGEMENT_ATTEMPT_TO_FIX_PASSED,
280+
outcome.aggregation() == ExecutionAggregation.ONLY_PASSED);
279281
}
280282
}
281283
} else {
@@ -296,7 +298,7 @@ public void onTestIgnore(
296298
final @Nullable Collection<String> categories,
297299
@Nonnull TestSourceData testSourceData,
298300
final @Nullable String reason,
299-
@Nullable TestExecutionHistory testExecutionHistory) {
301+
@Nullable TestExecutionTracker testExecutionTracker) {
300302
onTestStart(
301303
suiteDescriptor,
302304
testDescriptor,
@@ -307,9 +309,9 @@ public void onTestIgnore(
307309
categories,
308310
testSourceData,
309311
null,
310-
testExecutionHistory);
312+
testExecutionTracker);
311313
onTestSkip(testDescriptor, reason);
312-
onTestFinish(testDescriptor, null, testExecutionHistory);
314+
onTestFinish(testDescriptor, null, testExecutionTracker);
313315
}
314316

315317
@Override

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package datadog.trace.civisibility.execution;
2+
3+
import datadog.trace.api.civisibility.execution.ExecutionAggregation;
4+
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
5+
import datadog.trace.api.civisibility.execution.TestStatus;
6+
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
7+
8+
/**
9+
* Execution policy for the Attempt to Fix feature. Runs a test case up to N times. Stops retrying
10+
* as soon as a failure is observed, since a single failure proves the fix did not work.
11+
*/
12+
public class AttemptToFix implements TestExecutionPolicy {
13+
14+
private final int maxExecutions;
15+
private final boolean suppressFailures;
16+
private int executions;
17+
private ExecutionAggregation results;
18+
private TestStatus lastStatus;
19+
20+
public AttemptToFix(int maxExecutions, boolean suppressFailures) {
21+
this.maxExecutions = maxExecutions;
22+
this.suppressFailures = suppressFailures;
23+
this.executions = 0;
24+
this.results = ExecutionAggregation.NONE;
25+
}
26+
27+
@Override
28+
public ExecutionOutcome registerExecution(TestStatus status, long durationMillis) {
29+
lastStatus = status;
30+
++executions;
31+
results = results.withExecution(status);
32+
33+
boolean lastExecution = !retriesLeft();
34+
boolean retry = executions > 1;
35+
boolean failureSuppressed = status == TestStatus.fail && suppressFailures;
36+
TestStatus finalStatus = null;
37+
if (lastExecution) {
38+
if (results == ExecutionAggregation.ONLY_PASSED || suppressFailures) {
39+
finalStatus = TestStatus.pass;
40+
} else {
41+
finalStatus = TestStatus.fail;
42+
}
43+
}
44+
45+
return new ExecutionOutcomeImpl(
46+
failureSuppressed,
47+
lastExecution,
48+
results,
49+
retry ? RetryReason.attemptToFix : null,
50+
finalStatus);
51+
}
52+
53+
private boolean retriesLeft() {
54+
// stop retrying if the test was skipped, max executions reached,
55+
// or a failure was observed (the fix didn't work)
56+
return lastStatus != TestStatus.skip
57+
&& executions < maxExecutions
58+
&& results != ExecutionAggregation.ONLY_FAILED
59+
&& results != ExecutionAggregation.MIXED;
60+
}
61+
62+
@Override
63+
public boolean applicable() {
64+
return retriesLeft();
65+
}
66+
67+
@Override
68+
public boolean suppressFailures() {
69+
return suppressFailures;
70+
}
71+
72+
@Override
73+
public boolean failedTestReplayApplicable() {
74+
return false;
75+
}
76+
}

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

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,42 @@
11
package datadog.trace.civisibility.execution;
22

3+
import datadog.trace.api.civisibility.execution.ExecutionAggregation;
34
import datadog.trace.api.civisibility.execution.TestExecutionPolicy;
45
import datadog.trace.api.civisibility.execution.TestStatus;
56
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
67
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
78
import java.util.concurrent.atomic.AtomicInteger;
89

9-
/** Retries a test case if it failed, up to a maximum number of times. */
10+
/**
11+
* Execution policy for Auto Test Retries (ATR). Retries a test case if it failed, up to a maximum
12+
* number of times. Stops retrying as soon as the test passes.
13+
*/
1014
@SuppressFBWarnings(
11-
value = {"AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE", "AT_STALE_THREAD_WRITE_OF_PRIMITIVE"},
15+
value = {"AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE"},
1216
justification =
1317
"TestExecutionPolicy instances are confined to a single thread and are not meant to be thread-safe")
14-
public class RetryUntilSuccessful implements TestExecutionPolicy {
18+
public class AutoTestRetry implements TestExecutionPolicy {
1519

1620
private final int maxExecutions;
1721
private final boolean suppressFailures;
1822
private int executions;
19-
private boolean successfulExecutionSeen;
23+
private ExecutionAggregation results;
2024

21-
/** Total retry counter that is shared by all retry until successful policies (currently ATR) */
25+
/** Total retry counter that is shared by all auto test retry policies */
2226
private final AtomicInteger totalRetryCount;
2327

24-
public RetryUntilSuccessful(
25-
int maxExecutions, boolean suppressFailures, AtomicInteger totalRetryCount) {
28+
public AutoTestRetry(int maxExecutions, boolean suppressFailures, AtomicInteger totalRetryCount) {
2629
this.maxExecutions = maxExecutions;
2730
this.suppressFailures = suppressFailures;
2831
this.totalRetryCount = totalRetryCount;
2932
this.executions = 0;
33+
this.results = ExecutionAggregation.NONE;
3034
}
3135

3236
@Override
3337
public ExecutionOutcome registerExecution(TestStatus status, long durationMillis) {
3438
++executions;
35-
successfulExecutionSeen |= (status != TestStatus.fail);
39+
results = results.withExecution(status);
3640
if (executions > 1) {
3741
totalRetryCount.incrementAndGet();
3842
}
@@ -47,22 +51,17 @@ public ExecutionOutcome registerExecution(TestStatus status, long durationMillis
4751
}
4852

4953
return new ExecutionOutcomeImpl(
50-
failureSuppressed,
51-
lastExecution,
52-
lastExecution && !successfulExecutionSeen,
53-
false,
54-
retry ? RetryReason.atr : null,
55-
finalStatus);
54+
failureSuppressed, lastExecution, results, retry ? RetryReason.atr : null, finalStatus);
5655
}
5756

5857
private boolean retriesLeft() {
59-
return !successfulExecutionSeen && executions < maxExecutions;
58+
return executions < maxExecutions
59+
&& results != ExecutionAggregation.ONLY_PASSED
60+
&& results != ExecutionAggregation.MIXED;
6061
}
6162

6263
@Override
6364
public boolean applicable() {
64-
// executions must always be registered, therefore consider it applicable as long as there are
65-
// retries left
6665
return retriesLeft();
6766
}
6867

0 commit comments

Comments
 (0)