Skip to content

Update impacted tests logic #8923

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
6980d02
remove backend request for diff
daniel-mohedano May 16, 2025
a7f738d
update PRInfo building to use partial information from CI vendors
daniel-mohedano May 20, 2025
a1238c0
synchronize repo unshallow with future, avoids performing several times
daniel-mohedano May 20, 2025
cbe2a95
add `default_branch` to settings response
daniel-mohedano May 22, 2025
b861e7f
update settings api test
daniel-mohedano May 22, 2025
cab317e
change repo unshallow synchronization from future-based to synchronized
daniel-mohedano May 22, 2025
e401549
include pr info checks to CITagsProviderTest
daniel-mohedano May 23, 2025
63e7236
add tests for prinfo merging
daniel-mohedano May 23, 2025
a109cfd
Revert "update PRInfo building to use partial information from CI ven…
daniel-mohedano Jun 2, 2025
4d18887
Revert "include pr info checks to CITagsProviderTest"
daniel-mohedano Jun 2, 2025
84e1c0f
Revert "add tests for prinfo merging"
daniel-mohedano Jun 2, 2025
ec152b6
base commit sha algorithm implementation
daniel-mohedano Jun 2, 2025
1e843af
example git repo for base commit sha testing
daniel-mohedano Jun 2, 2025
fc82fcc
simplify algo implementation for easier read
daniel-mohedano Jun 2, 2025
67339e9
implement git diff based on best candidate for base commit sha
daniel-mohedano Jun 2, 2025
bfd3ad4
add github actions cloning test
daniel-mohedano Jun 3, 2025
5ddb58c
add several testing cases and cleaned up debug messages
daniel-mohedano Jun 4, 2025
c558a86
remove repo_origin folder
daniel-mohedano Jun 4, 2025
eca82e1
Merge branch 'master' into daniel.mohedano/impacted-tests-update
daniel-mohedano Jun 4, 2025
0fac3ae
fix forbidden apis
daniel-mohedano Jun 4, 2025
0e6bc2d
remove smoke test (no backend implementation anymore)
daniel-mohedano Jun 4, 2025
2e3362f
remove FileDiff
daniel-mohedano Jun 5, 2025
4198fe3
correctly reset all settings tags for mock backend
daniel-mohedano Jun 5, 2025
a99cff8
create isBlank method to avoid double negation with isNotBlank
daniel-mohedano Jun 5, 2025
f060e73
clean up base branch sha logic
daniel-mohedano Jun 6, 2025
ce0da33
fix code violation
daniel-mohedano Jun 6, 2025
2970e68
junit 4 test import wildcard
daniel-mohedano Jun 6, 2025
99980be
use relative paths in remotes and add remote origin to resources
daniel-mohedano Jun 6, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.nio.file.Path;
import java.util.Map;
import java.util.Objects;
import javax.annotation.Nullable;

public class CiVisibilitySettings {

Expand All @@ -17,7 +18,8 @@ public class CiVisibilitySettings {
false,
false,
EarlyFlakeDetectionSettings.DEFAULT,
TestManagementSettings.DEFAULT);
TestManagementSettings.DEFAULT,
null);

private final boolean itrEnabled;
private final boolean codeCoverage;
Expand All @@ -28,6 +30,7 @@ public class CiVisibilitySettings {
private final boolean knownTestsEnabled;
private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
private final TestManagementSettings testManagementSettings;
@Nullable private final String defaultBranch;

CiVisibilitySettings(
boolean itrEnabled,
Expand All @@ -38,7 +41,8 @@ public class CiVisibilitySettings {
boolean impactedTestsDetectionEnabled,
boolean knownTestsEnabled,
EarlyFlakeDetectionSettings earlyFlakeDetectionSettings,
TestManagementSettings testManagementSettings) {
TestManagementSettings testManagementSettings,
@Nullable String defaultBranch) {
this.itrEnabled = itrEnabled;
this.codeCoverage = codeCoverage;
this.testsSkipping = testsSkipping;
Expand All @@ -48,6 +52,7 @@ public class CiVisibilitySettings {
this.knownTestsEnabled = knownTestsEnabled;
this.earlyFlakeDetectionSettings = earlyFlakeDetectionSettings;
this.testManagementSettings = testManagementSettings;
this.defaultBranch = defaultBranch;
}

public boolean isItrEnabled() {
Expand Down Expand Up @@ -86,6 +91,11 @@ public TestManagementSettings getTestManagementSettings() {
return testManagementSettings;
}

@Nullable
public String getDefaultBranch() {
return defaultBranch;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -103,7 +113,8 @@ public boolean equals(Object o) {
&& impactedTestsDetectionEnabled == that.impactedTestsDetectionEnabled
&& knownTestsEnabled == that.knownTestsEnabled
&& Objects.equals(earlyFlakeDetectionSettings, that.earlyFlakeDetectionSettings)
&& Objects.equals(testManagementSettings, that.testManagementSettings);
&& Objects.equals(testManagementSettings, that.testManagementSettings)
&& Objects.equals(defaultBranch, that.defaultBranch);
}

@Override
Expand All @@ -117,7 +128,8 @@ public int hashCode() {
impactedTestsDetectionEnabled,
knownTestsEnabled,
earlyFlakeDetectionSettings,
testManagementSettings);
testManagementSettings,
defaultBranch);
}

public interface Factory {
Expand Down Expand Up @@ -145,13 +157,20 @@ public CiVisibilitySettings fromJson(Map<String, Object> json) {
EarlyFlakeDetectionSettings.JsonAdapter.INSTANCE.fromJson(
(Map<String, Object>) json.get("early_flake_detection")),
TestManagementSettings.JsonAdapter.INSTANCE.fromJson(
(Map<String, Object>) json.get("test_management")));
(Map<String, Object>) json.get("test_management")),
getString(json, "default_branch", null));
}

private static boolean getBoolean(
Map<String, Object> json, String fieldName, boolean defaultValue) {
Object value = json.get(fieldName);
return value instanceof Boolean ? (Boolean) value : defaultValue;
}

private static String getString(
Map<String, Object> json, String fieldName, String defaultValue) {
Object value = json.get(fieldName);
return value instanceof String ? (String) value : defaultValue;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ public Map<TestSetting, Map<String, Collection<TestFQN>>> getTestManagementTests
TracerEnvironment tracerEnvironment) {
return Collections.emptyMap();
}

@Override
public ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) {
return ChangedFiles.EMPTY;
}
};

CiVisibilitySettings getSettings(TracerEnvironment tracerEnvironment) throws IOException;
Expand All @@ -58,6 +53,4 @@ Map<String, Collection<TestFQN>> getKnownTestsByModule(TracerEnvironment tracerE

Map<TestSetting, Map<String, Collection<TestFQN>>> getTestManagementTestsByModule(
TracerEnvironment tracerEnvironment) throws IOException;

ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ public class ConfigurationApiImpl implements ConfigurationApi {

private static final String SETTINGS_URI = "libraries/tests/services/setting";
private static final String SKIPPABLE_TESTS_URI = "ci/tests/skippable";
private static final String CHANGED_FILES_URI = "ci/tests/diffs";
private static final String FLAKY_TESTS_URI = "ci/libraries/tests/flaky";
private static final String KNOWN_TESTS_URI = "ci/libraries/tests";
private static final String TEST_MANAGEMENT_TESTS_URI = "test/libraries/test-management/tests";
Expand All @@ -68,7 +67,6 @@ public class ConfigurationApiImpl implements ConfigurationApi {
private final JsonAdapter<EnvelopeDto<KnownTestsDto>> testFullNamesResponseAdapter;
private final JsonAdapter<EnvelopeDto<TestManagementDto>> testManagementRequestAdapter;
private final JsonAdapter<EnvelopeDto<TestManagementTestsDto>> testManagementTestsResponseAdapter;
private final JsonAdapter<EnvelopeDto<ChangedFiles>> changedFilesResponseAdapter;

public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector metricCollector) {
this(backendApi, metricCollector, () -> RandomUtils.randomUUID().toString());
Expand Down Expand Up @@ -119,11 +117,6 @@ public ConfigurationApiImpl(BackendApi backendApi, CiVisibilityMetricCollector m
Types.newParameterizedTypeWithOwner(
ConfigurationApiImpl.class, EnvelopeDto.class, TestManagementTestsDto.class);
testManagementTestsResponseAdapter = moshi.adapter(testManagementTestsResponseType);

ParameterizedType changedFilesResponseAdapterType =
Types.newParameterizedTypeWithOwner(
ConfigurationApiImpl.class, EnvelopeDto.class, ChangedFiles.class);
changedFilesResponseAdapter = moshi.adapter(changedFilesResponseAdapterType);
}

@Override
Expand Down Expand Up @@ -416,37 +409,6 @@ private Map<TestSetting, Map<String, Collection<TestFQN>>> parseTestManagementTe
return testsByTypeByModule;
}

@Override
public ChangedFiles getChangedFiles(TracerEnvironment tracerEnvironment) throws IOException {
OkHttpUtils.CustomListener telemetryListener =
new TelemetryListener.Builder(metricCollector)
.requestCount(CiVisibilityCountMetric.IMPACTED_TESTS_DETECTION_REQUEST)
.requestErrors(CiVisibilityCountMetric.IMPACTED_TESTS_DETECTION_REQUEST_ERRORS)
.requestDuration(CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_REQUEST_MS)
.responseBytes(CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_BYTES)
.build();

String uuid = uuidGenerator.get();
EnvelopeDto<TracerEnvironment> request =
new EnvelopeDto<>(new DataDto<>(uuid, "ci_app_tests_diffs_request", tracerEnvironment));
String json = requestAdapter.toJson(request);
RequestBody requestBody = RequestBody.create(JSON, json);
ChangedFiles changedFiles =
backendApi.post(
CHANGED_FILES_URI,
requestBody,
is ->
changedFilesResponseAdapter.fromJson(Okio.buffer(Okio.source(is))).data.attributes,
telemetryListener,
false);

int filesCount = changedFiles.getFiles().size();
LOGGER.debug("Received {} changed files", filesCount);
metricCollector.add(
CiVisibilityDistributionMetric.IMPACTED_TESTS_DETECTION_RESPONSE_FILES, filesCount);
return changedFiles;
}

private static final class EnvelopeDto<T> {
private final DataDto<T> data;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import datadog.trace.api.git.GitInfoProvider;
import datadog.trace.civisibility.ci.PullRequestInfo;
import datadog.trace.civisibility.diff.Diff;
import datadog.trace.civisibility.diff.FileDiff;
import datadog.trace.civisibility.diff.LineDiff;
import datadog.trace.civisibility.git.tree.GitClient;
import datadog.trace.civisibility.git.tree.GitDataUploader;
Expand Down Expand Up @@ -215,7 +214,8 @@ private Map<String, ExecutionSettings> doCreate(
getTestManagementTestsByModule(
tracerEnvironment, testManagementSettings.isEnabled()));
Future<Diff> pullRequestDiffFuture =
executor.submit(() -> getPullRequestDiff(tracerEnvironment, impactedTestsEnabled));
executor.submit(
() -> getPullRequestDiff(impactedTestsEnabled, settings.getDefaultBranch()));

SkippableTests skippableTests = skippableTestsFuture.get();
Map<String, Collection<TestFQN>> flakyTestsByModule = flakyTestsFuture.get();
Expand Down Expand Up @@ -404,8 +404,7 @@ private Map<TestSetting, Map<String, Collection<TestFQN>>> getTestManagementTest
}

@Nonnull
private Diff getPullRequestDiff(
TracerEnvironment tracerEnvironment, boolean impactedTestsDetectionEnabled) {
private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled, String defaultBranch) {
if (!impactedTestsDetectionEnabled) {
return LineDiff.EMPTY;
}
Expand All @@ -414,49 +413,25 @@ private Diff getPullRequestDiff(
if (repositoryRoot != null) {
// ensure repo is not shallow before attempting to get git diff
gitRepoUnshallow.unshallow();
Diff diff =
gitClient.getGitDiff(
pullRequestInfo.getPullRequestBaseBranchSha(),
pullRequestInfo.getGitCommitHeadSha());

String baseCommitSha = pullRequestInfo.getPullRequestBaseBranchSha();
if (baseCommitSha == null) {
baseCommitSha =
gitClient.getBaseCommitSha(pullRequestInfo.getPullRequestBaseBranch(), defaultBranch);
}

Diff diff = gitClient.getGitDiff(baseCommitSha, pullRequestInfo.getGitCommitHeadSha());
if (diff != null) {
return diff;
}
}

} catch (InterruptedException e) {
LOGGER.error("Interrupted while getting git diff for PR: {}", pullRequestInfo, e);
Thread.currentThread().interrupt();

} catch (Exception e) {
LOGGER.error("Could not get git diff for PR: {}", pullRequestInfo, e);
}

if (config.isCiVisibilityImpactedTestsBackendRequestEnabled()) {
try {
ChangedFiles changedFiles = configurationApi.getChangedFiles(tracerEnvironment);

// attempting to use base SHA returned by the backend to calculate git diff
if (repositoryRoot != null) {
// ensure repo is not shallow before attempting to get git diff
gitRepoUnshallow.unshallow();
Diff diff = gitClient.getGitDiff(changedFiles.getBaseSha(), tracerEnvironment.getSha());
if (diff != null) {
return diff;
}
}

// falling back to file-level granularity
return new FileDiff(changedFiles.getFiles());

} catch (InterruptedException e) {
LOGGER.error("Interrupted while getting git diff for: {}", tracerEnvironment, e);
Thread.currentThread().interrupt();

} catch (Exception e) {
LOGGER.error("Could not get git diff for: {}", tracerEnvironment, e);
}
}

return LineDiff.EMPTY;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ List<String> getObjects(Collection<String> commitsToSkip, Collection<String> com
Path createPackFiles(List<String> objectHashes)
throws IOException, TimeoutException, InterruptedException;

@Nullable
String getBaseCommitSha(@Nullable String baseBranch, @Nullable String defaultBranch)
throws IOException, TimeoutException, InterruptedException;

@Nullable
LineDiff getGitDiff(String baseCommit, String targetCommit)
throws IOException, TimeoutException, InterruptedException;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public GitRepoUnshallow(Config config, GitClient gitClient) {
this.gitClient = gitClient;
}

public boolean unshallow() throws IOException, InterruptedException, TimeoutException {
public synchronized boolean unshallow()
throws IOException, InterruptedException, TimeoutException {
if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,12 @@ public Path createPackFiles(List<String> objectHashes) {
return null;
}

@Nullable
@Override
public String getBaseCommitSha(@Nullable String baseBranch, @Nullable String defaultBranch) {
return null;
}

@Nullable
@Override
public LineDiff getGitDiff(String baseCommit, String targetCommit) {
Expand Down
Loading