Skip to content

Commit e442a04

Browse files
daniel-mohedanodaniel-mohedano
authored
Honor attempt to fix failures (#11244)
feat: honor attempt to fix failures fix: fixtures fix: smoke tests Co-authored-by: daniel.mohedano <daniel.mohedano@datadoghq.com>
1 parent 840f009 commit e442a04

35 files changed

Lines changed: 456 additions & 203 deletions

File tree

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

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,20 @@
88
/**
99
* Execution policy for the Attempt to Fix feature. Runs a test case up to N times. Stops retrying
1010
* as soon as a failure is observed, since a single failure proves the fix did not work.
11+
*
12+
* <p>Failures are never suppressed, even when the test is also quarantined or disabled — the whole
13+
* point of an attempt-to-fix is to verify that the fix worked, so a failing run must surface as a
14+
* failing run.
1115
*/
1216
public class AttemptToFix implements TestExecutionPolicy {
1317

1418
private final int maxExecutions;
15-
private final boolean suppressFailures;
1619
private int executions;
1720
private ExecutionAggregation results;
1821
private TestStatus lastStatus;
1922

20-
public AttemptToFix(int maxExecutions, boolean suppressFailures) {
23+
public AttemptToFix(int maxExecutions) {
2124
this.maxExecutions = maxExecutions;
22-
this.suppressFailures = suppressFailures;
2325
this.executions = 0;
2426
this.results = ExecutionAggregation.NONE;
2527
}
@@ -32,22 +34,13 @@ public ExecutionOutcome registerExecution(TestStatus status, long durationMillis
3234

3335
boolean lastExecution = !retriesLeft();
3436
boolean retry = executions > 1;
35-
boolean failureSuppressed = status == TestStatus.fail && suppressFailures;
3637
TestStatus finalStatus = null;
3738
if (lastExecution) {
38-
if (results == ExecutionAggregation.ONLY_PASSED || suppressFailures) {
39-
finalStatus = TestStatus.pass;
40-
} else {
41-
finalStatus = TestStatus.fail;
42-
}
39+
finalStatus = results == ExecutionAggregation.ONLY_PASSED ? TestStatus.pass : TestStatus.fail;
4340
}
4441

4542
return new ExecutionOutcomeImpl(
46-
failureSuppressed,
47-
lastExecution,
48-
results,
49-
retry ? RetryReason.attemptToFix : null,
50-
finalStatus);
43+
false, lastExecution, results, retry ? RetryReason.attemptToFix : null, finalStatus);
5144
}
5245

5346
private boolean retriesLeft() {
@@ -66,7 +59,7 @@ public boolean applicable() {
6659

6760
@Override
6861
public boolean suppressFailures() {
69-
return suppressFailures;
62+
return false;
7063
}
7164

7265
@Override

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,7 @@ public TestExecutionPolicy executionPolicy(
127127

128128
if (isAttemptToFix(test)) {
129129
return new AttemptToFix(
130-
executionSettings.getTestManagementSettings().getAttemptToFixRetries(),
131-
isQuarantined(test) || isDisabled(test));
130+
executionSettings.getTestManagementSettings().getAttemptToFixRetries());
132131
}
133132

134133
if (isEFDApplicable(test, testSource, testTags)) {

‎dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/execution/AttemptToFixTest.groovy‎

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class AttemptToFixTest extends Specification {
99

1010
def "test attempt to fix exits on failure"() {
1111
setup:
12-
def executionPolicy = new AttemptToFix(3, false)
12+
def executionPolicy = new AttemptToFix(3)
1313

1414
when:
1515
def outcome = executionPolicy.registerExecution(TestStatus.fail, 0)
@@ -24,7 +24,7 @@ class AttemptToFixTest extends Specification {
2424

2525
def "test attempt to fix succeeded all executions"() {
2626
setup:
27-
def executionPolicy = new AttemptToFix(3, false)
27+
def executionPolicy = new AttemptToFix(3)
2828

2929
when:
3030
def outcome = executionPolicy.registerExecution(TestStatus.pass, 0)
@@ -56,19 +56,4 @@ class AttemptToFixTest extends Specification {
5656
outcome3.aggregation() == ExecutionAggregation.ONLY_PASSED
5757
outcome3.finalStatus() == TestStatus.pass
5858
}
59-
60-
def "test attempt to fix suppresses failures when quarantined or disabled"() {
61-
setup:
62-
def executionPolicy = new AttemptToFix(3, true)
63-
64-
when:
65-
def outcome = executionPolicy.registerExecution(TestStatus.fail, 0)
66-
67-
then:
68-
outcome.retryReason() == null
69-
outcome.lastExecution()
70-
outcome.failureSuppressed()
71-
outcome.aggregation() == ExecutionAggregation.ONLY_FAILED
72-
outcome.finalStatus() == TestStatus.pass
73-
}
7459
}

‎dd-java-agent/instrumentation/junit/junit-4/junit-4-cucumber-5.4/src/test/groovy/CucumberTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ class CucumberTest extends CiVisibilityInstrumentationTest {
181181
"test-attempt-to-fix-succeeded" | true | ["org/example/cucumber/calculator/basic_arithmetic.feature"] | [
182182
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic.feature:Basic Arithmetic", "Addition")
183183
] | [] | []
184-
"test-attempt-to-fix-quarantined-failed" | true | ["org/example/cucumber/calculator/basic_arithmetic_failed.feature"] | [
184+
"test-attempt-to-fix-quarantined-failed" | false | ["org/example/cucumber/calculator/basic_arithmetic_failed.feature"] | [
185185
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic_failed.feature:Basic Arithmetic", "Addition")
186186
] | [
187187
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic_failed.feature:Basic Arithmetic", "Addition")
@@ -191,7 +191,7 @@ class CucumberTest extends CiVisibilityInstrumentationTest {
191191
] | [
192192
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic.feature:Basic Arithmetic", "Addition")
193193
] | []
194-
"test-attempt-to-fix-disabled-failed" | true | ["org/example/cucumber/calculator/basic_arithmetic_failed.feature"] | [
194+
"test-attempt-to-fix-disabled-failed" | false | ["org/example/cucumber/calculator/basic_arithmetic_failed.feature"] | [
195195
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic_failed.feature:Basic Arithmetic", "Addition")
196196
] | [] | [
197197
new TestFQN("classpath:org/example/cucumber/calculator/basic_arithmetic_failed.feature:Basic Arithmetic", "Addition")

‎dd-java-agent/instrumentation/junit/junit-4/junit-4-cucumber-5.4/src/test/resources/test-attempt-to-fix-disabled-failed/events.ftl‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"test.framework" : "cucumber",
1313
"test.framework_version" : ${content_meta_test_framework_version},
1414
"test.module" : "junit-4-cucumber-5.4",
15-
"test.status" : "pass",
15+
"test.status" : "fail",
1616
"test.suite" : "classpath:org/example/cucumber/calculator/basic_arithmetic_failed.feature:Basic Arithmetic",
1717
"test.type" : "test",
1818
"test_session.name" : "session-name"
@@ -47,8 +47,7 @@
4747
"library_version" : ${content_meta_library_version},
4848
"runtime-id" : ${content_meta_runtime_id},
4949
"span.kind" : "test",
50-
"test.failure_suppressed" : "true",
51-
"test.final_status" : "pass",
50+
"test.final_status" : "fail",
5251
"test.framework" : "cucumber",
5352
"test.framework_version" : ${content_meta_test_framework_version},
5453
"test.module" : "junit-4-cucumber-5.4",
@@ -99,7 +98,7 @@
9998
"test.command" : "junit-4-cucumber-5.4",
10099
"test.framework" : "cucumber",
101100
"test.framework_version" : ${content_meta_test_framework_version},
102-
"test.status" : "pass",
101+
"test.status" : "fail",
103102
"test.test_management.enabled" : "true",
104103
"test.type" : "test",
105104
"test_session.name" : "session-name"
@@ -132,7 +131,7 @@
132131
"test.framework" : "cucumber",
133132
"test.framework_version" : ${content_meta_test_framework_version},
134133
"test.module" : "junit-4-cucumber-5.4",
135-
"test.status" : "pass",
134+
"test.status" : "fail",
136135
"test.test_management.enabled" : "true",
137136
"test.type" : "test",
138137
"test_session.name" : "session-name"

‎dd-java-agent/instrumentation/junit/junit-4/junit-4-cucumber-5.4/src/test/resources/test-attempt-to-fix-quarantined-failed/events.ftl‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
"test.framework" : "cucumber",
1313
"test.framework_version" : ${content_meta_test_framework_version},
1414
"test.module" : "junit-4-cucumber-5.4",
15-
"test.status" : "pass",
15+
"test.status" : "fail",
1616
"test.suite" : "classpath:org/example/cucumber/calculator/basic_arithmetic_failed.feature:Basic Arithmetic",
1717
"test.type" : "test",
1818
"test_session.name" : "session-name"
@@ -47,8 +47,7 @@
4747
"library_version" : ${content_meta_library_version},
4848
"runtime-id" : ${content_meta_runtime_id},
4949
"span.kind" : "test",
50-
"test.failure_suppressed" : "true",
51-
"test.final_status" : "pass",
50+
"test.final_status" : "fail",
5251
"test.framework" : "cucumber",
5352
"test.framework_version" : ${content_meta_test_framework_version},
5453
"test.module" : "junit-4-cucumber-5.4",
@@ -99,7 +98,7 @@
9998
"test.command" : "junit-4-cucumber-5.4",
10099
"test.framework" : "cucumber",
101100
"test.framework_version" : ${content_meta_test_framework_version},
102-
"test.status" : "pass",
101+
"test.status" : "fail",
103102
"test.test_management.enabled" : "true",
104103
"test.type" : "test",
105104
"test_session.name" : "session-name"
@@ -132,7 +131,7 @@
132131
"test.framework" : "cucumber",
133132
"test.framework_version" : ${content_meta_test_framework_version},
134133
"test.module" : "junit-4-cucumber-5.4",
135-
"test.status" : "pass",
134+
"test.status" : "fail",
136135
"test.test_management.enabled" : "true",
137136
"test.type" : "test",
138137
"test_session.name" : "session-name"

‎dd-java-agent/instrumentation/junit/junit-4/junit-4-munit-0.7.28/src/test/groovy/MUnitTest.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,9 @@ class MUnitTest extends CiVisibilityInstrumentationTest {
138138
testcaseName | success | tests | attemptToFix | quarantined | disabled
139139
"test-attempt-to-fix-failed" | false | [TestFailedMUnit] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")] | [] | []
140140
"test-attempt-to-fix-succeeded" | true | [TestSucceedMUnit] | [new TestFQN("org.example.TestSucceedMUnit", "Calculator.add")] | [] | []
141-
"test-attempt-to-fix-quarantined-failed" | true | [TestFailedMUnit] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")] | []
141+
"test-attempt-to-fix-quarantined-failed" | false | [TestFailedMUnit] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")] | []
142142
"test-attempt-to-fix-quarantined-succeeded" | true | [TestSucceedMUnit] | [new TestFQN("org.example.TestSucceedMUnit", "Calculator.add")] | [new TestFQN("org.example.TestSucceedMUnit", "Calculator.add")] | []
143-
"test-attempt-to-fix-disabled-failed" | true | [TestFailedMUnit] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")] | [] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")]
143+
"test-attempt-to-fix-disabled-failed" | false | [TestFailedMUnit] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")] | [] | [new TestFQN("org.example.TestFailedMUnit", "Calculator.add")]
144144
"test-attempt-to-fix-disabled-succeeded" | true | [TestSucceedMUnit] | [new TestFQN("org.example.TestSucceedMUnit", "Calculator.add")] | [] | [new TestFQN("org.example.TestSucceedMUnit", "Calculator.add")]
145145
}
146146

‎dd-java-agent/instrumentation/junit/junit-4/junit-4-munit-0.7.28/src/test/resources/test-attempt-to-fix-disabled-failed/events.ftl‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"test.command" : "junit-4-munit-0.7.28",
1717
"test.framework" : "munit",
1818
"test.framework_version" : ${content_meta_test_framework_version},
19-
"test.status" : "pass",
19+
"test.status" : "fail",
2020
"test.test_management.enabled" : "true",
2121
"test.type" : "test",
2222
"test_session.name" : "session-name"
@@ -49,7 +49,7 @@
4949
"test.framework" : "munit",
5050
"test.framework_version" : ${content_meta_test_framework_version},
5151
"test.module" : "junit-4-munit-0.7.28",
52-
"test.status" : "pass",
52+
"test.status" : "fail",
5353
"test.test_management.enabled" : "true",
5454
"test.type" : "test",
5555
"test_session.name" : "session-name"
@@ -82,7 +82,7 @@
8282
"test.framework_version" : ${content_meta_test_framework_version},
8383
"test.module" : "junit-4-munit-0.7.28",
8484
"test.source.file" : "dummy_source_path",
85-
"test.status" : "pass",
85+
"test.status" : "fail",
8686
"test.suite" : "org.example.TestFailedMUnit",
8787
"test.type" : "test",
8888
"test_session.name" : "session-name"
@@ -120,8 +120,7 @@
120120
"runtime-id" : ${content_meta_runtime_id},
121121
"span.kind" : "test",
122122
"test.codeowners" : "[\"owner1\",\"owner2\"]",
123-
"test.failure_suppressed" : "true",
124-
"test.final_status" : "pass",
123+
"test.final_status" : "fail",
125124
"test.framework" : "munit",
126125
"test.framework_version" : ${content_meta_test_framework_version},
127126
"test.module" : "junit-4-munit-0.7.28",

‎dd-java-agent/instrumentation/junit/junit-4/junit-4-munit-0.7.28/src/test/resources/test-attempt-to-fix-quarantined-failed/events.ftl‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
"test.command" : "junit-4-munit-0.7.28",
1717
"test.framework" : "munit",
1818
"test.framework_version" : ${content_meta_test_framework_version},
19-
"test.status" : "pass",
19+
"test.status" : "fail",
2020
"test.test_management.enabled" : "true",
2121
"test.type" : "test",
2222
"test_session.name" : "session-name"
@@ -49,7 +49,7 @@
4949
"test.framework" : "munit",
5050
"test.framework_version" : ${content_meta_test_framework_version},
5151
"test.module" : "junit-4-munit-0.7.28",
52-
"test.status" : "pass",
52+
"test.status" : "fail",
5353
"test.test_management.enabled" : "true",
5454
"test.type" : "test",
5555
"test_session.name" : "session-name"
@@ -82,7 +82,7 @@
8282
"test.framework_version" : ${content_meta_test_framework_version},
8383
"test.module" : "junit-4-munit-0.7.28",
8484
"test.source.file" : "dummy_source_path",
85-
"test.status" : "pass",
85+
"test.status" : "fail",
8686
"test.suite" : "org.example.TestFailedMUnit",
8787
"test.type" : "test",
8888
"test_session.name" : "session-name"
@@ -120,8 +120,7 @@
120120
"runtime-id" : ${content_meta_runtime_id},
121121
"span.kind" : "test",
122122
"test.codeowners" : "[\"owner1\",\"owner2\"]",
123-
"test.failure_suppressed" : "true",
124-
"test.final_status" : "pass",
123+
"test.final_status" : "fail",
125124
"test.framework" : "munit",
126125
"test.framework_version" : ${content_meta_test_framework_version},
127126
"test.module" : "junit-4-munit-0.7.28",

‎dd-java-agent/instrumentation/junit/junit-4/junit-4.10/src/test/groovy/JUnit4Test.groovy‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,9 @@ class JUnit4Test extends CiVisibilityInstrumentationTest {
199199
testcaseName | success | tests | attemptToFix | quarantined | disabled
200200
"test-attempt-to-fix-failed" | false | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] | [] | []
201201
"test-attempt-to-fix-succeeded" | true | [TestSucceed] | [new TestFQN("org.example.TestSucceed", "test_succeed")] | [] | []
202-
"test-attempt-to-fix-quarantined-failed" | true | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] | [new TestFQN("org.example.TestFailed", "test_failed")] | []
202+
"test-attempt-to-fix-quarantined-failed" | false | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] | [new TestFQN("org.example.TestFailed", "test_failed")] | []
203203
"test-attempt-to-fix-quarantined-succeeded" | true | [TestSucceed] | [new TestFQN("org.example.TestSucceed", "test_succeed")] | [new TestFQN("org.example.TestSucceed", "test_succeed")] | []
204-
"test-attempt-to-fix-disabled-failed" | true | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] | [] | [new TestFQN("org.example.TestFailed", "test_failed")]
204+
"test-attempt-to-fix-disabled-failed" | false | [TestFailed] | [new TestFQN("org.example.TestFailed", "test_failed")] | [] | [new TestFQN("org.example.TestFailed", "test_failed")]
205205
"test-attempt-to-fix-disabled-succeeded" | true | [TestSucceed] | [new TestFQN("org.example.TestSucceed", "test_succeed")] | [] | [new TestFQN("org.example.TestSucceed", "test_succeed")]
206206
}
207207

0 commit comments

Comments
 (0)