Skip to content

Commit dc06b5e

Browse files
nikita-tkachenko-datadognikita-tkachenko-datadog
committed
Generalize tests skipping logic
1 parent a85ec5f commit dc06b5e

File tree

32 files changed

+296
-165
lines changed

32 files changed

+296
-165
lines changed

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import datadog.trace.api.civisibility.config.TestIdentifier;
44
import datadog.trace.api.civisibility.config.TestSourceData;
55
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
6+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
67
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
78
import javax.annotation.Nonnull;
89
import javax.annotation.Nullable;
@@ -31,12 +32,13 @@ TestSuiteImpl testSuiteStart(
3132
boolean isModified(TestSourceData testSourceData);
3233

3334
/**
34-
* Checks if a given test can be skipped with Intelligent Test Runner or not.
35+
* Returns the reason for skipping a test, IF it can be skipped.
3536
*
3637
* @param test Test to be checked
37-
* @return {@code true} if the test can be skipped, {@code false} otherwise
38+
* @return skip reason, or {@code null} if the test cannot be skipped
3839
*/
39-
boolean isSkippable(TestIdentifier test);
40+
@Nullable
41+
SkipReason skipReason(TestIdentifier test);
4042

4143
@Nonnull
4244
TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource);

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
import datadog.trace.api.DDTraceId;
99
import datadog.trace.api.civisibility.CIConstants;
1010
import datadog.trace.api.civisibility.DDTest;
11-
import datadog.trace.api.civisibility.InstrumentationBridge;
1211
import datadog.trace.api.civisibility.InstrumentationTestBridge;
1312
import datadog.trace.api.civisibility.config.TestIdentifier;
1413
import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge;
@@ -23,6 +22,7 @@
2322
import datadog.trace.api.civisibility.telemetry.tag.IsRetry;
2423
import datadog.trace.api.civisibility.telemetry.tag.IsRum;
2524
import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
25+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
2626
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
2727
import datadog.trace.api.gateway.RequestContextSlot;
2828
import datadog.trace.bootstrap.instrumentation.api.AgentScope;
@@ -208,9 +208,10 @@ public void setSkipReason(String skipReason) {
208208
if (skipReason != null) {
209209
span.setTag(Tags.TEST_SKIP_REASON, skipReason);
210210

211-
if (skipReason.equals(InstrumentationBridge.ITR_SKIP_REASON)) {
211+
if (skipReason.equals(SkipReason.ITR.getDescription())) {
212212
span.setTag(Tags.TEST_SKIPPED_BY_ITR, true);
213213
metricCollector.add(CiVisibilityCountMetric.ITR_SKIPPED, 1, EventType.TEST);
214+
executionResults.incrementTestsSkippedByItr();
214215
}
215216
}
216217
}
@@ -262,10 +263,6 @@ public void end(@Nullable Long endTime) {
262263
span.finish();
263264
}
264265

265-
if (InstrumentationBridge.ITR_SKIP_REASON.equals(span.getTag(Tags.TEST_SKIP_REASON))) {
266-
executionResults.incrementTestsSkippedByItr();
267-
}
268-
269266
metricCollector.add(
270267
CiVisibilityCountMetric.EVENT_FINISHED,
271268
1,

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import datadog.trace.api.civisibility.coverage.CoverageStore;
88
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
99
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
10+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
1011
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1112
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1213
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -102,9 +103,10 @@ public boolean isModified(TestSourceData testSourceData) {
102103
return executionStrategy.isModified(testSourceData);
103104
}
104105

106+
@Nullable
105107
@Override
106-
public boolean isSkippable(TestIdentifier test) {
107-
return executionStrategy.isSkippable(test);
108+
public SkipReason skipReason(TestIdentifier test) {
109+
return executionStrategy.skipReason(test);
108110
}
109111

110112
@Override

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import datadog.trace.api.civisibility.coverage.CoverageStore;
99
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
1010
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
11+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
1112
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1213
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
1314
import datadog.trace.bootstrap.instrumentation.api.AgentSpanContext;
@@ -87,9 +88,10 @@ public boolean isModified(TestSourceData testSourceData) {
8788
return executionStrategy.isModified(testSourceData);
8889
}
8990

91+
@Nullable
9092
@Override
91-
public boolean isSkippable(TestIdentifier test) {
92-
return executionStrategy.isSkippable(test);
93+
public SkipReason skipReason(TestIdentifier test) {
94+
return executionStrategy.skipReason(test);
9395
}
9496

9597
@Override

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import datadog.trace.api.civisibility.config.TestSourceData;
77
import datadog.trace.api.civisibility.events.TestEventsHandler;
88
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
9+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
910
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1011
import datadog.trace.bootstrap.ContextStore;
1112
import datadog.trace.civisibility.retry.NeverRetry;
@@ -91,8 +92,8 @@ public void onTestIgnore(
9192
}
9293

9394
@Override
94-
public boolean isSkippable(TestIdentifier test) {
95-
return false;
95+
public SkipReason skipReason(TestIdentifier test) {
96+
return null;
9697
}
9798

9899
@NotNull

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric;
1313
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
1414
import datadog.trace.api.civisibility.telemetry.tag.EventType;
15+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
1516
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
1617
import datadog.trace.bootstrap.ContextStore;
1718
import datadog.trace.bootstrap.instrumentation.api.Tags;
@@ -187,7 +188,7 @@ public void onTestStart(
187188
test.setTag(Tags.TEST_ITR_UNSKIPPABLE, true);
188189
metricCollector.add(CiVisibilityCountMetric.ITR_UNSKIPPABLE, 1, EventType.TEST);
189190

190-
if (testModule.isSkippable(thisTest)) {
191+
if (testModule.skipReason(thisTest) == SkipReason.ITR) {
191192
test.setTag(Tags.TEST_ITR_FORCED_RUN, true);
192193
metricCollector.add(CiVisibilityCountMetric.ITR_FORCED_RUN, 1, EventType.TEST);
193194
}
@@ -276,9 +277,10 @@ public boolean isFlaky(TestIdentifier test) {
276277
return testModule.isFlaky(test);
277278
}
278279

280+
@Nullable
279281
@Override
280-
public boolean isSkippable(TestIdentifier test) {
281-
return testModule.isSkippable(test);
282+
public SkipReason skipReason(TestIdentifier test) {
283+
return testModule.skipReason(test);
282284
}
283285

284286
@Override

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

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import datadog.trace.api.civisibility.config.TestMetadata;
66
import datadog.trace.api.civisibility.config.TestSourceData;
77
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
8+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
89
import datadog.trace.civisibility.config.EarlyFlakeDetectionSettings;
910
import datadog.trace.civisibility.config.ExecutionSettings;
1011
import datadog.trace.civisibility.retry.NeverRetry;
@@ -17,6 +18,7 @@
1718
import java.util.Map;
1819
import java.util.concurrent.atomic.AtomicInteger;
1920
import javax.annotation.Nonnull;
21+
import javax.annotation.Nullable;
2022
import org.slf4j.Logger;
2123
import org.slf4j.LoggerFactory;
2224

@@ -58,18 +60,23 @@ public boolean isFlaky(TestIdentifier test) {
5860
return flakyTests != null && flakyTests.contains(test.withoutParameters());
5961
}
6062

61-
public boolean isSkippable(TestIdentifier test) {
63+
@Nullable
64+
public SkipReason skipReason(TestIdentifier test) {
6265
if (test == null) {
63-
return false;
66+
return null;
6467
}
6568
if (!executionSettings.isTestSkippingEnabled()) {
66-
return false;
69+
return null;
6770
}
6871
Map<TestIdentifier, TestMetadata> skippableTests = executionSettings.getSkippableTests();
6972
TestMetadata testMetadata = skippableTests.get(test);
70-
return testMetadata != null
71-
&& !(config.isCiVisibilityCoverageLinesEnabled()
72-
&& testMetadata.isMissingLineCodeCoverage());
73+
if (testMetadata == null) {
74+
return null;
75+
}
76+
if (config.isCiVisibilityCoverageLinesEnabled() && testMetadata.isMissingLineCodeCoverage()) {
77+
return null;
78+
}
79+
return SkipReason.ITR;
7380
}
7481

7582
@Nonnull

‎dd-java-agent/agent-ci-visibility/src/testFixtures/groovy/datadog/trace/civisibility/CiVisibilityTestUtils.groovy‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import com.jayway.jsonpath.JsonPath
77
import com.jayway.jsonpath.Option
88
import com.jayway.jsonpath.ReadContext
99
import com.jayway.jsonpath.WriteContext
10+
import freemarker.core.InvalidReferenceException
1011
import freemarker.template.Template
1112
import freemarker.template.TemplateExceptionHandler
1213
import org.skyscreamer.jsonassert.JSONAssert
@@ -130,6 +131,16 @@ abstract class CiVisibilityTestUtils {
130131
StringWriter coveragesOut = new StringWriter()
131132
coveragesTemplate.process(replacements, coveragesOut)
132133
return coveragesOut.toString()
134+
} catch (InvalidReferenceException e) {
135+
def missingExpression = e.getBlamedExpressionString()
136+
if (missingExpression != null) {
137+
replacements.put(missingExpression, "<VALUE_MISSING>") // to simplify debugging failures
138+
return getFreemarkerTemplate(templatePath, replacements, replacementsSource)
139+
140+
} else {
141+
throw new RuntimeException("Could not get Freemarker template " + templatePath + "; replacements map: " + replacements + "; replacements source: " + replacementsSource, e)
142+
}
143+
133144
} catch (Exception e) {
134145
throw new RuntimeException("Could not get Freemarker template " + templatePath + "; replacements map: " + replacements + "; replacements source: " + replacementsSource, e)
135146
}

‎dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberInstrumentation.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public String[] helperClassNames() {
3636
return new String[] {
3737
packageName + ".CucumberUtils",
3838
packageName + ".TestEventsHandlerHolder",
39-
packageName + ".SkippedByItr",
39+
packageName + ".SkippedByDatadog",
4040
packageName + ".JUnit4Utils",
4141
packageName + ".TracingListener",
4242
packageName + ".CucumberTracingListener",

dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberItrInstrumentation.java renamed to dd-java-agent/instrumentation/junit-4.10/cucumber-junit-4/src/main/java/datadog/trace/instrumentation/junit4/JUnit4CucumberSkipInstrumentation.java

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import datadog.trace.api.Config;
1212
import datadog.trace.api.civisibility.InstrumentationBridge;
1313
import datadog.trace.api.civisibility.config.TestIdentifier;
14+
import datadog.trace.api.civisibility.telemetry.tag.SkipReason;
1415
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
1516
import io.cucumber.core.gherkin.Pickle;
1617
import java.util.List;
@@ -24,10 +25,10 @@
2425
import org.junit.runner.notification.RunNotifier;
2526

2627
@AutoService(InstrumenterModule.class)
27-
public class JUnit4CucumberItrInstrumentation extends InstrumenterModule.CiVisibility
28+
public class JUnit4CucumberSkipInstrumentation extends InstrumenterModule.CiVisibility
2829
implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice {
2930

30-
public JUnit4CucumberItrInstrumentation() {
31+
public JUnit4CucumberSkipInstrumentation() {
3132
super("ci-visibility", "junit-4", "junit-4-cucumber");
3233
}
3334

@@ -51,7 +52,7 @@ public String[] helperClassNames() {
5152
return new String[] {
5253
packageName + ".CucumberUtils",
5354
packageName + ".TestEventsHandlerHolder",
54-
packageName + ".SkippedByItr",
55+
packageName + ".SkippedByDatadog",
5556
packageName + ".JUnit4Utils",
5657
packageName + ".TracingListener",
5758
packageName + ".CucumberTracingListener",
@@ -67,34 +68,36 @@ public Reference[] additionalMuzzleReferences() {
6768
public void methodAdvice(MethodTransformer transformer) {
6869
transformer.applyAdvice(
6970
named("run").and(takesArgument(0, named("org.junit.runner.notification.RunNotifier"))),
70-
JUnit4CucumberItrInstrumentation.class.getName() + "$CucumberItrAdvice");
71+
JUnit4CucumberSkipInstrumentation.class.getName() + "$CucumberSkipAdvice");
7172
}
7273

73-
public static class CucumberItrAdvice {
74+
public static class CucumberSkipAdvice {
75+
@SuppressWarnings("bytebuddy-exception-suppression")
7476
@SuppressFBWarnings("NP_BOOLEAN_RETURN_NULL")
7577
@Advice.OnMethodEnter(skipOn = Boolean.class)
7678
public static Boolean run(
7779
@Advice.FieldValue("pickle") Pickle pickle,
7880
@Advice.FieldValue("description") Description description,
7981
@Advice.Argument(0) RunNotifier notifier) {
8082

81-
List<String> tags = pickle.getTags();
82-
for (String tag : tags) {
83-
if (tag.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) {
84-
return null;
85-
}
86-
}
87-
8883
TestIdentifier test = CucumberUtils.toTestIdentifier(description);
89-
if (TestEventsHandlerHolder.TEST_EVENTS_HANDLER.isSkippable(test)) {
90-
notifier.fireTestAssumptionFailed(
91-
new Failure(
92-
description,
93-
new AssumptionViolatedException(InstrumentationBridge.ITR_SKIP_REASON)));
94-
return Boolean.FALSE;
95-
} else {
84+
SkipReason skipReason = TestEventsHandlerHolder.TEST_EVENTS_HANDLER.skipReason(test);
85+
if (skipReason == null) {
9686
return null;
9787
}
88+
89+
if (skipReason == SkipReason.ITR) {
90+
List<String> tags = pickle.getTags();
91+
for (String tag : tags) {
92+
if (tag.endsWith(InstrumentationBridge.ITR_UNSKIPPABLE_TAG)) {
93+
return null;
94+
}
95+
}
96+
}
97+
98+
notifier.fireTestAssumptionFailed(
99+
new Failure(description, new AssumptionViolatedException(skipReason.getDescription())));
100+
return Boolean.FALSE;
98101
}
99102
}
100103
}

0 commit comments

Comments
 (0)