Skip to content

Commit 1dff23c

Browse files
Fix Test Optimization init when repo root cannot be determined (#8533)
1 parent 5d109aa commit 1dff23c

File tree

10 files changed

+71
-39
lines changed

10 files changed

+71
-39
lines changed

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

+7-6
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.nio.file.Paths;
4040
import java.util.Map;
4141
import java.util.concurrent.CompletableFuture;
42+
import javax.annotation.Nullable;
4243
import org.slf4j.Logger;
4344
import org.slf4j.LoggerFactory;
4445

@@ -47,7 +48,7 @@ public class CiVisibilityRepoServices {
4748

4849
private static final Logger LOGGER = LoggerFactory.getLogger(CiVisibilityRepoServices.class);
4950

50-
final String repoRoot;
51+
@Nullable final String repoRoot;
5152
final String moduleName;
5253
final Provider ciProvider;
5354
final Map<String, String> ciTags;
@@ -135,7 +136,7 @@ private static String appendSlashIfNeeded(String repoRoot) {
135136
}
136137
}
137138

138-
static String getModuleName(Config config, String repoRoot, Path path) {
139+
static String getModuleName(Config config, @Nullable String repoRoot, Path path) {
139140
// if parent process is instrumented, it will provide build system's module name
140141
String parentModuleName = config.getCiVisibilityModuleName();
141142
if (parentModuleName != null) {
@@ -175,7 +176,7 @@ private static ExecutionSettingsFactory buildExecutionSettingsFactory(
175176
GitRepoUnshallow gitRepoUnshallow,
176177
GitDataUploader gitDataUploader,
177178
PullRequestInfo pullRequestInfo,
178-
String repoRoot) {
179+
@Nullable String repoRoot) {
179180
ConfigurationApi configurationApi;
180181
if (backendApi == null) {
181182
LOGGER.warn(
@@ -208,7 +209,7 @@ private static GitDataUploader buildGitDataUploader(
208209
GitClient gitClient,
209210
GitRepoUnshallow gitRepoUnshallow,
210211
BackendApi backendApi,
211-
String repoRoot) {
212+
@Nullable String repoRoot) {
212213
if (!config.isCiVisibilityGitUploadEnabled()) {
213214
return () -> CompletableFuture.completedFuture(null);
214215
}
@@ -239,7 +240,7 @@ private static GitDataUploader buildGitDataUploader(
239240
}
240241

241242
private static SourcePathResolver buildSourcePathResolver(
242-
String repoRoot, RepoIndexProvider indexProvider) {
243+
@Nullable String repoRoot, RepoIndexProvider indexProvider) {
243244
SourcePathResolver compilerAidedResolver =
244245
repoRoot != null
245246
? new CompilerAidedSourcePathResolver(repoRoot)
@@ -248,7 +249,7 @@ private static SourcePathResolver buildSourcePathResolver(
248249
return new BestEffortSourcePathResolver(compilerAidedResolver, indexResolver);
249250
}
250251

251-
private static Codeowners buildCodeowners(String repoRoot) {
252+
private static Codeowners buildCodeowners(@Nullable String repoRoot) {
252253
if (repoRoot != null) {
253254
return new CodeownersProvider().build(repoRoot);
254255
} else {

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

+5-7
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public class ExecutionSettingsFactoryImpl implements ExecutionSettingsFactory {
5454
private final GitRepoUnshallow gitRepoUnshallow;
5555
private final GitDataUploader gitDataUploader;
5656
private final PullRequestInfo pullRequestInfo;
57-
private final String repositoryRoot;
57+
@Nullable private final String repositoryRoot;
5858

5959
public ExecutionSettingsFactoryImpl(
6060
Config config,
@@ -63,7 +63,7 @@ public ExecutionSettingsFactoryImpl(
6363
GitRepoUnshallow gitRepoUnshallow,
6464
GitDataUploader gitDataUploader,
6565
PullRequestInfo pullRequestInfo,
66-
String repositoryRoot) {
66+
@Nullable String repositoryRoot) {
6767
this.config = config;
6868
this.configurationApi = configurationApi;
6969
this.gitClient = gitClient;
@@ -75,21 +75,19 @@ public ExecutionSettingsFactoryImpl(
7575

7676
/** @return Executions settings by module name */
7777
public Map<String, ExecutionSettings> create(@Nonnull JvmInfo jvmInfo) {
78-
TracerEnvironment tracerEnvironment = buildTracerEnvironment(repositoryRoot, jvmInfo, null);
78+
TracerEnvironment tracerEnvironment = buildTracerEnvironment(jvmInfo, null);
7979
return create(tracerEnvironment);
8080
}
8181

8282
@Override
8383
public ExecutionSettings create(@Nonnull JvmInfo jvmInfo, @Nullable String moduleName) {
84-
TracerEnvironment tracerEnvironment =
85-
buildTracerEnvironment(repositoryRoot, jvmInfo, moduleName);
84+
TracerEnvironment tracerEnvironment = buildTracerEnvironment(jvmInfo, moduleName);
8685
Map<String, ExecutionSettings> settingsByModule = create(tracerEnvironment);
8786
ExecutionSettings settings = settingsByModule.get(moduleName);
8887
return settings != null ? settings : settingsByModule.get(DEFAULT_SETTINGS);
8988
}
9089

91-
private TracerEnvironment buildTracerEnvironment(
92-
String repositoryRoot, JvmInfo jvmInfo, @Nullable String moduleName) {
90+
private TracerEnvironment buildTracerEnvironment(JvmInfo jvmInfo, @Nullable String moduleName) {
9391
GitInfo gitInfo = GitInfoProvider.INSTANCE.getGitInfo(repositoryRoot);
9492

9593
TracerEnvironment.Builder builder = TracerEnvironment.builder();

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/percentage/JacocoCoverageCalculator.java

+36-11
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.io.FileOutputStream;
2020
import java.io.IOException;
2121
import java.io.InputStream;
22+
import java.io.Reader;
2223
import java.nio.file.Files;
2324
import java.nio.file.Paths;
2425
import java.util.BitSet;
@@ -42,6 +43,7 @@
4243
import org.jacoco.core.data.SessionInfoStore;
4344
import org.jacoco.report.FileMultiReportOutput;
4445
import org.jacoco.report.IReportVisitor;
46+
import org.jacoco.report.ISourceFileLocator;
4547
import org.jacoco.report.InputStreamSourceFileLocator;
4648
import org.jacoco.report.html.HTMLFormatter;
4749
import org.jacoco.report.xml.XMLFormatter;
@@ -55,13 +57,13 @@ public static final class Factory
5557
implements CoverageCalculator.Factory<JacocoCoverageCalculator> {
5658
private final Config config;
5759
private final RepoIndexProvider repoIndexProvider;
58-
private final String repoRoot;
60+
@Nullable private final String repoRoot;
5961
private final ModuleSignalRouter moduleSignalRouter;
6062

6163
public Factory(
6264
Config config,
6365
RepoIndexProvider repoIndexProvider,
64-
String repoRoot,
66+
@Nullable String repoRoot,
6567
ModuleSignalRouter moduleSignalRouter) {
6668
this.config = config;
6769
this.repoIndexProvider = repoIndexProvider;
@@ -100,7 +102,7 @@ public JacocoCoverageCalculator moduleCoverage(
100102

101103
private final RepoIndexProvider repoIndexProvider;
102104

103-
private final String repoRoot;
105+
@Nullable private final String repoRoot;
104106

105107
private final long eventId;
106108

@@ -116,7 +118,10 @@ public JacocoCoverageCalculator moduleCoverage(
116118
private final Collection<File> outputClassesDirs = new HashSet<>();
117119

118120
private JacocoCoverageCalculator(
119-
Config config, RepoIndexProvider repoIndexProvider, String repoRoot, long sessionId) {
121+
Config config,
122+
RepoIndexProvider repoIndexProvider,
123+
@Nullable String repoRoot,
124+
long sessionId) {
120125
this.parent = null;
121126
this.config = config;
122127
this.repoIndexProvider = repoIndexProvider;
@@ -128,7 +133,7 @@ private JacocoCoverageCalculator(
128133
Config config,
129134
RepoIndexProvider repoIndexProvider,
130135
ExecutionSettings executionSettings,
131-
String repoRoot,
136+
@Nullable String repoRoot,
132137
long moduleId,
133138
@Nullable BuildModuleLayout moduleLayout,
134139
ModuleSignalRouter moduleSignalRouter,
@@ -297,29 +302,33 @@ private void dumpCoverageReport(IBundleCoverage coverageBundle, File reportFolde
297302
final IReportVisitor htmlVisitor =
298303
htmlFormatter.createVisitor(new FileMultiReportOutput(reportFolder));
299304
htmlVisitor.visitInfo(Collections.emptyList(), Collections.emptyList());
300-
htmlVisitor.visitBundle(
301-
coverageBundle, new RepoIndexFileLocator(repoIndexProvider.getIndex(), repoRoot));
305+
htmlVisitor.visitBundle(coverageBundle, createSourceFileLocator());
302306
htmlVisitor.visitEnd();
303307

304308
File xmlReport = new File(reportFolder, "jacoco.xml");
305309
try (FileOutputStream xmlReportStream = new FileOutputStream(xmlReport)) {
306310
XMLFormatter xmlFormatter = new XMLFormatter();
307311
IReportVisitor xmlVisitor = xmlFormatter.createVisitor(xmlReportStream);
308312
xmlVisitor.visitInfo(Collections.emptyList(), Collections.emptyList());
309-
xmlVisitor.visitBundle(
310-
coverageBundle, new RepoIndexFileLocator(repoIndexProvider.getIndex(), repoRoot));
313+
xmlVisitor.visitBundle(coverageBundle, createSourceFileLocator());
311314
xmlVisitor.visitEnd();
312315
}
313316
} catch (Exception e) {
314317
LOGGER.error("Error while creating report in {}", reportFolder, e);
315318
}
316319
}
317320

321+
private ISourceFileLocator createSourceFileLocator() {
322+
return repoRoot != null
323+
? new RepoIndexFileLocator(repoIndexProvider.getIndex(), repoRoot)
324+
: NoOpFileLocator.INSTANCE;
325+
}
326+
318327
private static final class RepoIndexFileLocator extends InputStreamSourceFileLocator {
319328
private final RepoIndex repoIndex;
320-
private final String repoRoot;
329+
@Nonnull private final String repoRoot;
321330

322-
private RepoIndexFileLocator(RepoIndex repoIndex, String repoRoot) {
331+
private RepoIndexFileLocator(RepoIndex repoIndex, @Nonnull String repoRoot) {
323332
super("utf-8", 4);
324333
this.repoIndex = repoIndex;
325334
this.repoRoot = repoRoot;
@@ -343,6 +352,22 @@ protected InputStream getSourceStream(String path) throws IOException {
343352
}
344353
}
345354

355+
private static final class NoOpFileLocator implements ISourceFileLocator {
356+
private static final NoOpFileLocator INSTANCE = new NoOpFileLocator();
357+
358+
private NoOpFileLocator() {}
359+
360+
@Override
361+
public Reader getSourceFile(String s, String s1) {
362+
return null;
363+
}
364+
365+
@Override
366+
public int getTabWidth() {
367+
return 0;
368+
}
369+
}
370+
346371
private long getCoveragePercentage(IBundleCoverage coverageBundle) {
347372
if (backendCoverageData.isEmpty()) {
348373
return getLocalCoveragePercentage(coverageBundle);

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import java.util.Collection;
1616
import javax.annotation.Nonnull;
1717
import javax.annotation.Nullable;
18-
import org.jetbrains.annotations.NotNull;
1918

2019
public class NoOpTestEventsHandler<SuiteKey, TestKey>
2120
implements TestEventsHandler<SuiteKey, TestKey> {
@@ -99,7 +98,7 @@ public SkipReason skipReason(TestIdentifier test) {
9998
return null;
10099
}
101100

102-
@NotNull
101+
@Nonnull
103102
@Override
104103
public TestExecutionPolicy executionPolicy(
105104
TestIdentifier test, TestSourceData source, Collection<String> testTags) {

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,6 @@ LineDiff getGitDiff(String baseCommit, String targetCommit)
7777
throws IOException, TimeoutException, InterruptedException;
7878

7979
interface Factory {
80-
GitClient create(String repoRoot);
80+
GitClient create(@Nullable String repoRoot);
8181
}
8282
}

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

+5-5
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
import java.util.Collection;
66
import java.util.Collections;
77
import java.util.List;
8-
import org.jetbrains.annotations.NotNull;
9-
import org.jetbrains.annotations.Nullable;
8+
import javax.annotation.Nonnull;
9+
import javax.annotation.Nullable;
1010

1111
public class NoOpGitClient implements GitClient {
1212

@@ -54,7 +54,7 @@ public String getCurrentBranch() {
5454
return null;
5555
}
5656

57-
@NotNull
57+
@Nonnull
5858
@Override
5959
public List<String> getTags(String commit) {
6060
return Collections.emptyList();
@@ -108,13 +108,13 @@ public String getCommitterDate(String commit) {
108108
return null;
109109
}
110110

111-
@NotNull
111+
@Nonnull
112112
@Override
113113
public List<String> getLatestCommits() {
114114
return Collections.emptyList();
115115
}
116116

117-
@NotNull
117+
@Nonnull
118118
@Override
119119
public List<String> getObjects(
120120
Collection<String> commitsToSkip, Collection<String> commitsToInclude) {

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

+9-4
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public class ShellGitClient implements GitClient {
5151
*/
5252
ShellGitClient(
5353
CiVisibilityMetricCollector metricCollector,
54-
String repoRoot,
54+
@Nonnull String repoRoot,
5555
String latestCommitsSince,
5656
int latestCommitsLimit,
5757
long timeoutMillis) {
@@ -651,10 +651,15 @@ public Factory(Config config, CiVisibilityMetricCollector metricCollector) {
651651
}
652652

653653
@Override
654-
public GitClient create(String repoRoot) {
654+
public GitClient create(@Nullable String repoRoot) {
655655
long commandTimeoutMillis = config.getCiVisibilityGitCommandTimeoutMillis();
656-
return new ShellGitClient(
657-
metricCollector, repoRoot, "1 month ago", 1000, commandTimeoutMillis);
656+
if (repoRoot != null) {
657+
return new ShellGitClient(
658+
metricCollector, repoRoot, "1 month ago", 1000, commandTimeoutMillis);
659+
} else {
660+
LOGGER.debug("Could not determine repository root, using no-op git client");
661+
return NoOpGitClient.INSTANCE;
662+
}
658663
}
659664
}
660665
}

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/CachingRepoIndexBuilderFactory.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import datadog.trace.api.cache.DDCache;
55
import datadog.trace.api.cache.DDCaches;
66
import java.nio.file.FileSystem;
7+
import javax.annotation.Nullable;
78

89
public class CachingRepoIndexBuilderFactory implements RepoIndexProvider.Factory {
910

@@ -25,7 +26,7 @@ public CachingRepoIndexBuilderFactory(
2526
}
2627

2728
@Override
28-
public RepoIndexProvider create(String repoRoot) {
29+
public RepoIndexProvider create(@Nullable String repoRoot) {
2930
if (repoRoot == null) {
3031
return () -> RepoIndex.EMPTY;
3132
}

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.HashSet;
2020
import java.util.Map;
2121
import java.util.concurrent.atomic.AtomicInteger;
22+
import javax.annotation.Nonnull;
2223
import org.slf4j.Logger;
2324
import org.slf4j.LoggerFactory;
2425

@@ -37,7 +38,7 @@ public class RepoIndexBuilder implements RepoIndexProvider {
3738

3839
public RepoIndexBuilder(
3940
Config config,
40-
String repoRoot,
41+
@Nonnull String repoRoot,
4142
PackageResolver packageResolver,
4243
ResourceResolver resourceResolver,
4344
FileSystem fileSystem) {
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
package datadog.trace.civisibility.source.index;
22

3+
import javax.annotation.Nullable;
4+
35
public interface RepoIndexProvider {
46
RepoIndex getIndex();
57

68
interface Factory {
7-
RepoIndexProvider create(String repoRoot);
9+
RepoIndexProvider create(@Nullable String repoRoot);
810
}
911
}

0 commit comments

Comments
 (0)