Skip to content

Commit c4928eb

Browse files
nikita-tkachenko-datadognikita-tkachenko-datadog
committed
Implement EFD for modified tests
1 parent a8b31bf commit c4928eb

File tree

15 files changed

+196
-68
lines changed

15 files changed

+196
-68
lines changed

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

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
import datadog.trace.civisibility.source.SourcePathResolver;
3333
import datadog.trace.civisibility.source.index.RepoIndexProvider;
3434
import datadog.trace.civisibility.source.index.RepoIndexSourcePathResolver;
35+
import datadog.trace.util.Strings;
36+
import java.io.File;
3537
import java.nio.file.Path;
3638
import java.nio.file.Paths;
3739
import java.util.Map;
@@ -66,8 +68,8 @@ public class CiVisibilityRepoServices {
6668
LOGGER.info("PR detected: {}", pullRequestInfo);
6769
}
6870

69-
repoRoot = ciInfo.getNormalizedCiWorkspace();
70-
moduleName = getModuleName(services.config, path, ciInfo);
71+
repoRoot = appendSlashIfNeeded(getRepoRoot(ciInfo, services.gitClientFactory));
72+
moduleName = getModuleName(services.config, repoRoot, path);
7173
ciTags = new CITagsProvider().getCiTags(ciInfo, pullRequestInfo);
7274

7375
gitDataUploader =
@@ -98,13 +100,41 @@ public class CiVisibilityRepoServices {
98100
}
99101
}
100102

101-
static String getModuleName(Config config, Path path, CIInfo ciInfo) {
103+
private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFactory) {
104+
String ciWorkspace = ciInfo.getNormalizedCiWorkspace();
105+
if (Strings.isNotBlank(ciWorkspace)) {
106+
return ciWorkspace;
107+
108+
} else {
109+
try {
110+
return gitClientFactory.create(".").getRepoRoot();
111+
112+
} catch (InterruptedException e) {
113+
Thread.currentThread().interrupt();
114+
LOGGER.error("Interrupted while getting repo root", e);
115+
return null;
116+
117+
} catch (Exception e) {
118+
LOGGER.error("Error while getting repo root", e);
119+
return null;
120+
}
121+
}
122+
}
123+
124+
private static String appendSlashIfNeeded(String repoRoot) {
125+
if (repoRoot != null && !repoRoot.endsWith(File.separator)) {
126+
return repoRoot + File.separator;
127+
} else {
128+
return repoRoot;
129+
}
130+
}
131+
132+
static String getModuleName(Config config, String repoRoot, Path path) {
102133
// if parent process is instrumented, it will provide build system's module name
103134
String parentModuleName = config.getCiVisibilityModuleName();
104135
if (parentModuleName != null) {
105136
return parentModuleName;
106137
}
107-
String repoRoot = ciInfo.getNormalizedCiWorkspace();
108138
if (repoRoot != null && path.startsWith(repoRoot)) {
109139
String relativePath = Paths.get(repoRoot).relativize(path).toString();
110140
if (!relativePath.isEmpty()) {

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,11 @@ private static TestFrameworkSession.Factory childTestFrameworkSessionFactory(
238238
new TestDecoratorImpl(component, sessionName, testCommand, repoServices.ciTags);
239239

240240
ExecutionStrategy executionStrategy =
241-
new ExecutionStrategy(services.config, executionSettings);
241+
new ExecutionStrategy(
242+
services.config,
243+
executionSettings,
244+
repoServices.sourcePathResolver,
245+
services.linesResolver);
242246

243247
return new ProxyTestSession(
244248
services.processHierarchy.parentProcessModuleContext,
@@ -268,7 +272,11 @@ private static TestFrameworkSession.Factory headlessTestFrameworkEssionFactory(
268272
new TestDecoratorImpl(component, sessionName, projectName, repoServices.ciTags);
269273

270274
ExecutionStrategy executionStrategy =
271-
new ExecutionStrategy(services.config, executionSettings);
275+
new ExecutionStrategy(
276+
services.config,
277+
executionSettings,
278+
repoServices.sourcePathResolver,
279+
services.linesResolver);
272280
return new HeadlessTestSession(
273281
projectName,
274282
startTime,

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,7 @@ private TracerEnvironment buildTracerEnvironment(
181181
codeCoverageEnabled,
182182
testSkippingEnabled,
183183
flakyTestRetriesEnabled,
184-
// knownTests being null covers the following cases:
185-
// - early flake detection is disabled in remote settings
186-
// - early flake detection is disabled via local config killswitch
187-
// - the list of known tests could not be obtained
188-
knownTestsByModule != null
184+
earlyFlakeDetectionEnabled
189185
? ciVisibilitySettings.getEarlyFlakeDetectionSettings()
190186
: EarlyFlakeDetectionSettings.DEFAULT,
191187
itrCorrelationId,
@@ -278,8 +274,7 @@ private Map<String, Collection<TestIdentifier>> getFlakyTestsByModule(
278274
return configurationApi.getFlakyTestsByModule(tracerEnvironment);
279275

280276
} catch (Exception e) {
281-
LOGGER.error(
282-
"Could not obtain list of flaky tests, flaky test retries will not be available", e);
277+
LOGGER.error("Could not obtain list of flaky tests", e);
283278
return Collections.emptyMap();
284279
}
285280
}
@@ -290,9 +285,7 @@ private Map<String, Collection<TestIdentifier>> getKnownTestsByModule(
290285
return configurationApi.getKnownTestsByModule(tracerEnvironment);
291286

292287
} catch (Exception e) {
293-
LOGGER.error(
294-
"Could not obtain list of known tests, early flakiness detection will not be available",
295-
e);
288+
LOGGER.error("Could not obtain list of known tests", e);
296289
return null;
297290
}
298291
}
@@ -302,6 +295,9 @@ private Diff getPullRequestDiff() {
302295
if (repositoryRoot == null) {
303296
return Diff.EMPTY;
304297
}
298+
// FIXME nikita: return empty diff if impacted tests detection is disabled (killswitch)
299+
// FIXME nikita: add telemetry
300+
// FIXME nikita: add file-based granularity fallback if Git executable is not available?
305301
try {
306302
GitClient gitClient = gitClientFactory.create(repositoryRoot);
307303
return gitClient.getGitDiff(

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package datadog.trace.civisibility.domain;
22

33
import datadog.trace.api.civisibility.config.TestIdentifier;
4+
import datadog.trace.api.civisibility.config.TestSourceData;
45
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
56
import datadog.trace.api.civisibility.telemetry.tag.TestFrameworkInstrumentation;
67
import javax.annotation.Nonnull;
@@ -27,6 +28,8 @@ TestSuiteImpl testSuiteStart(
2728

2829
boolean isFlaky(TestIdentifier test);
2930

31+
boolean isModified(TestSourceData testSourceData);
32+
3033
/**
3134
* Checks if a given test should be skipped with Intelligent Test Runner or not
3235
*
@@ -45,7 +48,7 @@ TestSuiteImpl testSuiteStart(
4548
boolean skip(TestIdentifier test);
4649

4750
@Nonnull
48-
TestRetryPolicy retryPolicy(TestIdentifier test);
51+
TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource);
4952

5053
void end(Long startTime);
5154
}

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import datadog.trace.api.Config;
44
import datadog.trace.api.DDTraceId;
55
import datadog.trace.api.civisibility.config.TestIdentifier;
6+
import datadog.trace.api.civisibility.config.TestSourceData;
67
import datadog.trace.api.civisibility.coverage.CoverageStore;
78
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
89
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
@@ -93,6 +94,11 @@ public boolean isFlaky(TestIdentifier test) {
9394
return executionStrategy.isFlaky(test);
9495
}
9596

97+
@Override
98+
public boolean isModified(TestSourceData testSourceData) {
99+
return executionStrategy.isModified(testSourceData);
100+
}
101+
96102
@Override
97103
public boolean shouldBeSkipped(TestIdentifier test) {
98104
return executionStrategy.shouldBeSkipped(test);
@@ -105,8 +111,8 @@ public boolean skip(TestIdentifier test) {
105111

106112
@Override
107113
@Nonnull
108-
public TestRetryPolicy retryPolicy(TestIdentifier test) {
109-
return executionStrategy.retryPolicy(test);
114+
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) {
115+
return executionStrategy.retryPolicy(test, testSource);
110116
}
111117

112118
@Override
@@ -136,7 +142,7 @@ private void sendModuleExecutionResult() {
136142
executionSettings.getEarlyFlakeDetectionSettings();
137143
boolean earlyFlakeDetectionEnabled = earlyFlakeDetectionSettings.isEnabled();
138144
boolean earlyFlakeDetectionFaulty =
139-
earlyFlakeDetectionEnabled && executionStrategy.isEarlyFlakeDetectionLimitReached();
145+
earlyFlakeDetectionEnabled && executionStrategy.isEFDLimitReached();
140146
long testsSkippedTotal = executionStrategy.getTestsSkipped();
141147

142148
signalClient.send(

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import datadog.trace.api.DDTags;
55
import datadog.trace.api.civisibility.CIConstants;
66
import datadog.trace.api.civisibility.config.TestIdentifier;
7+
import datadog.trace.api.civisibility.config.TestSourceData;
78
import datadog.trace.api.civisibility.coverage.CoverageStore;
89
import datadog.trace.api.civisibility.retry.TestRetryPolicy;
910
import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector;
@@ -78,6 +79,11 @@ public boolean isFlaky(TestIdentifier test) {
7879
return executionStrategy.isFlaky(test);
7980
}
8081

82+
@Override
83+
public boolean isModified(TestSourceData testSourceData) {
84+
return executionStrategy.isModified(testSourceData);
85+
}
86+
8187
@Override
8288
public boolean shouldBeSkipped(TestIdentifier test) {
8389
return executionStrategy.shouldBeSkipped(test);
@@ -90,8 +96,8 @@ public boolean skip(TestIdentifier test) {
9096

9197
@Override
9298
@Nonnull
93-
public TestRetryPolicy retryPolicy(TestIdentifier test) {
94-
return executionStrategy.retryPolicy(test);
99+
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) {
100+
return executionStrategy.retryPolicy(test, testSource);
95101
}
96102

97103
@Override
@@ -116,7 +122,7 @@ public void end(@Nullable Long endTime) {
116122
executionSettings.getEarlyFlakeDetectionSettings();
117123
if (earlyFlakeDetectionSettings.isEnabled()) {
118124
setTag(Tags.TEST_EARLY_FLAKE_ENABLED, true);
119-
if (executionStrategy.isEarlyFlakeDetectionLimitReached()) {
125+
if (executionStrategy.isEFDLimitReached()) {
120126
setTag(Tags.TEST_EARLY_FLAKE_ABORT_REASON, CIConstants.EFD_ABORT_REASON_FAULTY);
121127
}
122128
}

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ public void onTestStart(
160160
test.setTag(Tags.TEST_IS_NEW, true);
161161
}
162162

163+
if (testModule.isModified(testSourceData)) {
164+
test.setTag(Tags.TEST_IS_MODIFIED, true);
165+
}
166+
163167
if (testFramework != null) {
164168
test.setTag(Tags.TEST_FRAMEWORK, testFramework);
165169
if (testFrameworkVersion != null) {
@@ -267,8 +271,8 @@ public boolean shouldBeSkipped(TestIdentifier test) {
267271

268272
@Override
269273
@Nonnull
270-
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData source) {
271-
return testModule.retryPolicy(test);
274+
public TestRetryPolicy retryPolicy(TestIdentifier test, TestSourceData testSource) {
275+
return testModule.retryPolicy(test, testSource);
272276
}
273277

274278
@Override

‎dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,24 @@ public void unshallow(@Nullable String remoteCommitReference)
184184
.trim());
185185
}
186186

187+
/**
188+
* Returns the absolute path to Git repository
189+
*
190+
* @return absolute path
191+
* @throws IOException If an error was encountered while writing command input or reading output
192+
* @throws TimeoutException If timeout was reached while waiting for Git command to finish
193+
* @throws InterruptedException If current thread was interrupted while waiting for Git command to
194+
* finish
195+
*/
196+
public @NonNull String getRepoRoot() throws IOException, TimeoutException, InterruptedException {
197+
return executeCommand(
198+
Command.OTHER,
199+
() ->
200+
commandExecutor
201+
.executeCommand(IOUtils::readFully, "git", "rev-parse", "--show-toplevel")
202+
.trim());
203+
}
204+
187205
/**
188206
* Returns URL of the remote with the given name
189207
*

0 commit comments

Comments
 (0)