Skip to content
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ private TestEventsHandlerFactory(
services, repoServices, coverageServices, executionSettings);
} else {
sessionFactory =
headlessTestFrameworkEssionFactory(
headlessTestFrameworkSessionFactory(
services, repoServices, coverageServices, executionSettings);
}
}
Expand Down Expand Up @@ -259,7 +259,7 @@ private static TestFrameworkSession.Factory childTestFrameworkSessionFactory(
};
}

private static TestFrameworkSession.Factory headlessTestFrameworkEssionFactory(
private static TestFrameworkSession.Factory headlessTestFrameworkSessionFactory(
CiVisibilityServices services,
CiVisibilityRepoServices repoServices,
CiVisibilityCoverageServices.Child coverageServices,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ public class CiVisibilitySettings {

public static final CiVisibilitySettings DEFAULT =
new CiVisibilitySettings(
false, false, false, false, false, false, false, EarlyFlakeDetectionSettings.DEFAULT);
false,
false,
false,
false,
false,
false,
false,
EarlyFlakeDetectionSettings.DEFAULT,
TestManagementSettings.DEFAULT);

private final boolean itrEnabled;
private final boolean codeCoverage;
Expand All @@ -19,6 +27,7 @@ public class CiVisibilitySettings {
private final boolean impactedTestsDetectionEnabled;
private final boolean knownTestsEnabled;
private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
private final TestManagementSettings testManagementSettings;

CiVisibilitySettings(
boolean itrEnabled,
Expand All @@ -28,7 +37,8 @@ public class CiVisibilitySettings {
boolean flakyTestRetriesEnabled,
boolean impactedTestsDetectionEnabled,
boolean knownTestsEnabled,
EarlyFlakeDetectionSettings earlyFlakeDetectionSettings) {
EarlyFlakeDetectionSettings earlyFlakeDetectionSettings,
TestManagementSettings testManagementSettings) {
this.itrEnabled = itrEnabled;
this.codeCoverage = codeCoverage;
this.testsSkipping = testsSkipping;
Expand All @@ -37,6 +47,7 @@ public class CiVisibilitySettings {
this.impactedTestsDetectionEnabled = impactedTestsDetectionEnabled;
this.knownTestsEnabled = knownTestsEnabled;
this.earlyFlakeDetectionSettings = earlyFlakeDetectionSettings;
this.testManagementSettings = testManagementSettings;
}

public boolean isItrEnabled() {
Expand Down Expand Up @@ -71,6 +82,10 @@ public EarlyFlakeDetectionSettings getEarlyFlakeDetectionSettings() {
return earlyFlakeDetectionSettings;
}

public TestManagementSettings getTestManagementSettings() {
return testManagementSettings;
}

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

@Override
Expand All @@ -100,7 +116,8 @@ public int hashCode() {
flakyTestRetriesEnabled,
impactedTestsDetectionEnabled,
knownTestsEnabled,
earlyFlakeDetectionSettings);
earlyFlakeDetectionSettings,
testManagementSettings);
}

public interface Factory {
Expand All @@ -126,7 +143,9 @@ public CiVisibilitySettings fromJson(Map<String, Object> json) {
getBoolean(json, "impacted_tests_enabled", false),
getBoolean(json, "known_tests_enabled", false),
EarlyFlakeDetectionSettingsJsonAdapter.INSTANCE.fromJson(
(Map<String, Object>) json.get("early_flake_detection")));
(Map<String, Object>) json.get("early_flake_detection")),
TestManagementSettingsJsonAdapter.INSTANCE.fromJson(
(Map<String, Object>) json.get("test_management")));
}

private static boolean getBoolean(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import datadog.trace.api.civisibility.telemetry.tag.ItrSkipEnabled;
import datadog.trace.api.civisibility.telemetry.tag.KnownTestsEnabled;
import datadog.trace.api.civisibility.telemetry.tag.RequireGit;
import datadog.trace.api.civisibility.telemetry.tag.TestManagementEnabled;
import datadog.trace.civisibility.communication.TelemetryListener;
import datadog.trace.util.RandomUtils;
import java.io.File;
Expand Down Expand Up @@ -146,6 +147,7 @@ public CiVisibilitySettings getSettings(TracerEnvironment tracerEnvironment) thr
settings.isFlakyTestRetriesEnabled() ? FlakyTestRetriesEnabled.TRUE : null,
settings.isKnownTestsEnabled() ? KnownTestsEnabled.TRUE : null,
settings.isImpactedTestsDetectionEnabled() ? ImpactedTestsDetectionEnabled.TRUE : null,
settings.getTestManagementSettings().isEnabled() ? TestManagementEnabled.TRUE : null,
settings.isGitUploadRequired() ? RequireGit.TRUE : null);

return settings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class ExecutionSettings {
false,
false,
EarlyFlakeDetectionSettings.DEFAULT,
TestManagementSettings.DEFAULT,
null,
Collections.emptyMap(),
Collections.emptyMap(),
Expand All @@ -39,6 +40,7 @@ public class ExecutionSettings {
private final boolean flakyTestRetriesEnabled;
private final boolean impactedTestsDetectionEnabled;
@Nonnull private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
@Nonnull private final TestManagementSettings testManagementSettings;
@Nullable private final String itrCorrelationId;
@Nonnull private final Map<TestIdentifier, TestMetadata> skippableTests;
@Nonnull private final Map<String, BitSet> skippableTestsCoverage;
Expand All @@ -54,6 +56,7 @@ public ExecutionSettings(
boolean flakyTestRetriesEnabled,
boolean impactedTestsDetectionEnabled,
@Nonnull EarlyFlakeDetectionSettings earlyFlakeDetectionSettings,
@Nonnull TestManagementSettings testManagementSettings,
@Nullable String itrCorrelationId,
@Nonnull Map<TestIdentifier, TestMetadata> skippableTests,
@Nonnull Map<String, BitSet> skippableTestsCoverage,
Expand All @@ -67,6 +70,7 @@ public ExecutionSettings(
this.flakyTestRetriesEnabled = flakyTestRetriesEnabled;
this.impactedTestsDetectionEnabled = impactedTestsDetectionEnabled;
this.earlyFlakeDetectionSettings = earlyFlakeDetectionSettings;
this.testManagementSettings = testManagementSettings;
this.itrCorrelationId = itrCorrelationId;
this.skippableTests = skippableTests;
this.skippableTestsCoverage = skippableTestsCoverage;
Expand Down Expand Up @@ -105,6 +109,11 @@ public EarlyFlakeDetectionSettings getEarlyFlakeDetectionSettings() {
return earlyFlakeDetectionSettings;
}

@Nonnull
public TestManagementSettings getTestManagementSettings() {
return testManagementSettings;
}

@Nullable
public String getItrCorrelationId() {
return itrCorrelationId;
Expand Down Expand Up @@ -162,6 +171,7 @@ public boolean equals(Object o) {
&& codeCoverageEnabled == that.codeCoverageEnabled
&& testSkippingEnabled == that.testSkippingEnabled
&& Objects.equals(earlyFlakeDetectionSettings, that.earlyFlakeDetectionSettings)
&& Objects.equals(testManagementSettings, that.testManagementSettings)
&& Objects.equals(itrCorrelationId, that.itrCorrelationId)
&& Objects.equals(skippableTests, that.skippableTests)
&& Objects.equals(skippableTestsCoverage, that.skippableTestsCoverage)
Expand All @@ -178,6 +188,7 @@ public int hashCode() {
codeCoverageEnabled,
testSkippingEnabled,
earlyFlakeDetectionSettings,
testManagementSettings,
itrCorrelationId,
skippableTests,
skippableTestsCoverage,
Expand Down Expand Up @@ -211,6 +222,8 @@ public static ByteBuffer serialize(ExecutionSettings settings) {

EarlyFlakeDetectionSettingsSerializer.serialize(s, settings.earlyFlakeDetectionSettings);

TestManagementSettingsSerializer.serialize(s, settings.testManagementSettings);

s.write(settings.itrCorrelationId);
s.write(
settings.skippableTests,
Expand Down Expand Up @@ -238,6 +251,9 @@ public static ExecutionSettings deserialize(ByteBuffer buffer) {
EarlyFlakeDetectionSettings earlyFlakeDetectionSettings =
EarlyFlakeDetectionSettingsSerializer.deserialize(buffer);

TestManagementSettings testManagementSettings =
TestManagementSettingsSerializer.deserialize(buffer);

String itrCorrelationId = Serializer.readString(buffer);

Map<TestIdentifier, TestMetadata> skippableTests =
Expand All @@ -262,6 +278,7 @@ public static ExecutionSettings deserialize(ByteBuffer buffer) {
flakyTestRetriesEnabled,
impactedTestsDetectionEnabled,
earlyFlakeDetectionSettings,
testManagementSettings,
itrCorrelationId,
skippableTests,
skippableTestsCoverage,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package datadog.trace.civisibility.config;

import datadog.trace.api.Config;
import datadog.trace.api.ConfigDefaults;
import datadog.trace.api.civisibility.CIConstants;
import datadog.trace.api.civisibility.CiVisibilityWellKnownTags;
import datadog.trace.api.civisibility.config.TestIdentifier;
Expand All @@ -27,6 +28,7 @@
import java.util.concurrent.Future;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.function.Function;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -177,6 +179,17 @@ private Map<String, ExecutionSettings> doCreate(
settings,
CiVisibilitySettings::isKnownTestsEnabled,
Config::isCiVisibilityKnownTestsRequestEnabled);
boolean testManagementEnabled =
isFeatureEnabled(
settings,
s -> s.getTestManagementSettings().isEnabled(),
Comment thread
nikita-tkachenko-datadog marked this conversation as resolved.
Outdated
Config::isCiVisibilityTestManagementEnabled);
if (testManagementEnabled) {
overrideIntegerSetting(
Config::getCiVisibilityTestManagementAttemptToFixRetries,
value -> value != ConfigDefaults.DEFAULT_TEST_MANAGEMENT_ATTEMPT_TO_FIX_RETRIES,
value -> settings.getTestManagementSettings().setAttemptToFixRetries(value));
}

LOGGER.info(
"CI Visibility settings ({}, {}/{}/{}):\n"
Expand All @@ -186,7 +199,8 @@ private Map<String, ExecutionSettings> doCreate(
+ "Early flakiness detection - {},\n"
+ "Impacted tests detection - {},\n"
+ "Known tests marking - {},\n"
+ "Auto test retries - {}",
+ "Auto test retries - {},\n"
+ "Test Management - {}",
repositoryRoot,
tracerEnvironment.getConfigurations().getRuntimeName(),
tracerEnvironment.getConfigurations().getRuntimeVersion(),
Expand All @@ -197,7 +211,8 @@ private Map<String, ExecutionSettings> doCreate(
earlyFlakeDetectionEnabled,
impactedTestsEnabled,
knownTestsRequest,
flakyTestRetriesEnabled);
flakyTestRetriesEnabled,
testManagementEnabled);

Future<SkippableTests> skippableTestsFuture =
executor.submit(() -> getSkippableTests(tracerEnvironment, itrEnabled));
Expand Down Expand Up @@ -228,6 +243,9 @@ private Map<String, ExecutionSettings> doCreate(
earlyFlakeDetectionEnabled
? settings.getEarlyFlakeDetectionSettings()
: EarlyFlakeDetectionSettings.DEFAULT,
testManagementEnabled
? settings.getTestManagementSettings()
: TestManagementSettings.DEFAULT,
skippableTests.getCorrelationId(),
skippableTests
.getIdentifiersByModule()
Expand Down Expand Up @@ -270,6 +288,16 @@ private boolean isFeatureEnabled(
return remoteSetting.apply(ciVisibilitySettings) && killSwitch.apply(config);
}

private void overrideIntegerSetting(
Function<Config, Integer> valueGetter,
Function<Integer, Boolean> overrideCheck,
Consumer<Integer> overrideAction) {
Integer value = valueGetter.apply(config);
if (value != null && overrideCheck.apply(value)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that we want to apply the overriding local value only if it's different from the -1 default, but the function is so generic that it's takes a bit of effort to comprehend.
Perhaps we could make the Config field Integer instead of int, and apply the override if the value is not null, dropping the overrideCheck?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought having a very generic way of overriding values might be beneficial in the future, but it might not make much sense for now as it is only used in this specific scenario. Overriding (or, better yet, creating a new instance as the next comment indicates) would be much clearer if the Config field was an Integer with null as default, I agree

overrideAction.accept(value);
}
}

@Nonnull
private SkippableTests getSkippableTests(
TracerEnvironment tracerEnvironment, boolean itrEnabled) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package datadog.trace.civisibility.config;

import java.util.Objects;

public class TestManagementSettings {

public static final TestManagementSettings DEFAULT = new TestManagementSettings(false, -1);

private final boolean enabled;
private int attemptToFixRetries;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be better to make the class immutable (marking both fields as final) and create a new instance if there're overrides. Mutability makes it more difficult to reason about the possible outcomes, and has thread-safety implications


public TestManagementSettings(boolean enabled, int attemptToFixRetries) {
this.enabled = enabled;
this.attemptToFixRetries = attemptToFixRetries;
}

public boolean isEnabled() {
return enabled;
}

public int getAttemptToFixRetries() {
return attemptToFixRetries;
}

public void setAttemptToFixRetries(int value) {
attemptToFixRetries = value;
}

@Override
public boolean equals(Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}

TestManagementSettings that = (TestManagementSettings) o;
return enabled == that.enabled && attemptToFixRetries == that.attemptToFixRetries;
}

@Override
public int hashCode() {
return Objects.hash(enabled, attemptToFixRetries);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package datadog.trace.civisibility.config;

import com.squareup.moshi.FromJson;
import java.util.Map;

public class TestManagementSettingsJsonAdapter {
public static final TestManagementSettingsJsonAdapter INSTANCE =
new TestManagementSettingsJsonAdapter();

@FromJson
public TestManagementSettings fromJson(Map<String, Object> json) {
if (json == null) {
return TestManagementSettings.DEFAULT;
}

Boolean enabled = (Boolean) json.get("enabled");
Double attemptToFixRetries = (Double) json.get("attempt_to_fix_retries");

return new TestManagementSettings(
enabled != null ? enabled : false,
attemptToFixRetries != null ? attemptToFixRetries.intValue() : -1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package datadog.trace.civisibility.config;

import datadog.trace.civisibility.ipc.serialization.Serializer;
import java.nio.ByteBuffer;

public class TestManagementSettingsSerializer {
public static void serialize(Serializer serializer, TestManagementSettings settings) {
if (!settings.isEnabled()) {
serializer.write((byte) 0);
return;
}
serializer.write((byte) 1);
serializer.write(settings.getAttemptToFixRetries());
}

public static TestManagementSettings deserialize(ByteBuffer buf) {
boolean enabled = Serializer.readByte(buf) != 0;
if (!enabled) {
return TestManagementSettings.DEFAULT;
}

int attemptToFixRetries = Serializer.readInt(buf);
return new TestManagementSettings(enabled, attemptToFixRetries);
}
}
Loading