Skip to content

[Gradle] Fix test seed handling when running with cc #133811

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 4, 2025
Merged

[Gradle] Fix test seed handling when running with cc #133811

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
381cd97
Fix testseed handling when running with cc
breskeby Aug 29, 2025
87d1aa8
[Gradle] Test seed should change per invocation and not break CC
breskeby Aug 29, 2025
d9f6c78
Fix more early resolutions of test seed
breskeby Aug 29, 2025
769fbb3
Cleanup
breskeby Aug 29, 2025
4a9f74a
Simplify test
breskeby Sep 4, 2025
eff11c9
Merge branch 'main' into fix-testseed-cc
breskeby Sep 4, 2025
c9ff966
Merge branch 'main' into fix-testseed-cc
breskeby Sep 4, 2025
a25ede5
Merge branch 'main' into fix-testseed-cc
breskeby Sep 4, 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
61 changes: 61 additions & 0 deletions ...gTest/groovy/org/elasticsearch/gradle/internal/ElasticsearchTestBasePluginFuncTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,65 @@ class ElasticsearchTestBasePluginFuncTest extends AbstractGradleFuncTest {
then:
result.task(':test').outcome == TaskOutcome.UP_TO_DATE
}

def "uses new test seed for every invocation"() {
given:
file("src/test/java/acme/SomeTests.java").text = """

public class SomeTests {
@org.junit.Test
public void printTestSeed() {
System.out.println("TESTSEED=[" + System.getProperty("tests.seed") + "]");
}
}

"""
buildFile.text = """
plugins {
id 'java'
id 'elasticsearch.test-base'
}

tasks.named('test').configure {
testLogging {
showStandardStreams = true
}
}

tasks.register('test2', Test) {
classpath = sourceSets.test.runtimeClasspath
testClassesDirs = sourceSets.test.output.classesDirs
testLogging {
showStandardStreams = true
}
}

repositories {
mavenCentral()
}

dependencies {
testImplementation 'junit:junit:4.12'
}

"""

when:
def result1 = gradleRunner("cleanTest", "cleanTest2", "test", "test2").build()
def result2 = gradleRunner("cleanTest", "cleanTest2", "test", "test2").build()

then:
def seeds1 = result1.output.findAll(/(?m)TESTSEED=\[([^\]]+)\]/) { it[1] }
def seeds2 = result2.output.findAll(/(?m)TESTSEED=\[([^\]]+)\]/) { it[1] }

seeds1.unique().size() == 1
seeds2.unique().size() == 1

verifyAll {
seeds1[0] != null
seeds2[0] != null
seeds1[0] != seeds2[0]
}
result2.output.contains("Configuration cache entry reused.")
}
}
12 changes: 9 additions & 3 deletions ...internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.gradle.api.Project;
import org.gradle.api.Task;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.configuration.BuildFeatures;
import org.gradle.api.file.FileCollection;
import org.gradle.api.plugins.JavaPlugin;
import org.gradle.api.provider.ProviderFactory;
Expand Down Expand Up @@ -56,6 +57,9 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
@Inject
protected abstract ProviderFactory getProviderFactory();

@Inject
protected abstract BuildFeatures getBuildFeatures();

@Override
public void apply(Project project) {
project.getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class);
Expand Down Expand Up @@ -164,9 +168,11 @@ public void execute(Task t) {
);
test.systemProperties(sysprops);

// ignore changing test seed when build is passed -Dignore.tests.seed for cacheability experimentation
if (System.getProperty("ignore.tests.seed") != null) {
nonInputProperties.systemProperty("tests.seed", buildParams.get().getTestSeed());
// ignore changing test seed when build is passed -Dignore.tests.seed for cacheability
// also ignore when configuration cache is on since the test seed as task input would break
// configuration cache reuse.
if (System.getProperty("ignore.tests.seed") != null || getBuildFeatures().getConfigurationCache().getActive().get()) {
nonInputProperties.systemProperty("tests.seed", buildParams.get().getTestSeedProvider());
} else {
test.systemProperty("tests.seed", buildParams.get().getTestSeed());
}
Expand Down
4 changes: 3 additions & 1 deletion ...nternal/src/main/java/org/elasticsearch/gradle/internal/info/BuildParameterExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public interface BuildParameterExtension {

String getTestSeed();

Provider<String> getTestSeedProvider();

Boolean getCi();

Integer getDefaultParallel();
Expand All @@ -66,7 +68,7 @@ public interface BuildParameterExtension {

Provider<BwcVersions> getBwcVersionsProvider();

Random getRandom();
Provider<Random> getRandom();

Boolean getGraalVmRuntime();
}
13 changes: 9 additions & 4 deletions .../src/main/java/org/elasticsearch/gradle/internal/info/DefaultBuildParameterExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public abstract class DefaultBuildParameterExtension implements BuildParameterEx
private final Provider<String> gitRevision;

private transient AtomicReference<ZonedDateTime> buildDate = new AtomicReference<>();
private final String testSeed;
private final Provider<String> testSeed;
private final Boolean isCi;
private final Integer defaultParallel;
private final Boolean snapshotBuild;
Expand All @@ -58,7 +58,7 @@ public DefaultBuildParameterExtension(
JavaVersion gradleJavaVersion,
Provider<String> gitRevision,
Provider<String> gitOrigin,
String testSeed,
Provider<String> testSeed,
boolean isCi,
int defaultParallel,
final boolean isSnapshotBuild,
Expand Down Expand Up @@ -181,6 +181,11 @@ public ZonedDateTime getBuildDate() {

@Override
public String getTestSeed() {
return testSeed.get();
}

@Override
public Provider<String> getTestSeedProvider() {
return testSeed;
}

Expand All @@ -205,8 +210,8 @@ public BwcVersions getBwcVersions() {
}

@Override
public Random getRandom() {
return new Random(Long.parseUnsignedLong(testSeed.split(":")[0], 16));
public Provider<Random> getRandom() {
return getTestSeedProvider().map(seed -> new Random(Long.parseUnsignedLong(seed.split(":")[0], 16)));
}

@Override
Expand Down
31 changes: 14 additions & 17 deletions ...-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.gradle.api.NamedDomainObjectContainer;
import org.gradle.api.Plugin;
import org.gradle.api.Project;
import org.gradle.api.configuration.BuildFeatures;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.model.ObjectFactory;
Expand Down Expand Up @@ -60,7 +61,6 @@
import java.util.List;
import java.util.Locale;
import java.util.Properties;
import java.util.Random;
import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -87,7 +87,7 @@ public class GlobalBuildInfoPlugin implements Plugin<Project> {
private final JavaInstallationRegistry javaInstallationRegistry;
private final JvmMetadataDetector metadataDetector;
private final ProviderFactory providers;
private final BranchesFileParser branchesFileParser;
private final BuildFeatures buildFeatures;
private JavaToolchainService toolChainService;
private Project project;

Expand All @@ -96,13 +96,14 @@ public GlobalBuildInfoPlugin(
ObjectFactory objectFactory,
JavaInstallationRegistry javaInstallationRegistry,
JvmMetadataDetector metadataDetector,
ProviderFactory providers
ProviderFactory providers,
BuildFeatures buildFeatures
) {
this.objectFactory = objectFactory;
this.javaInstallationRegistry = javaInstallationRegistry;
this.metadataDetector = new ErrorTraceMetadataDetector(metadataDetector);
this.providers = providers;
this.branchesFileParser = new BranchesFileParser(new ObjectMapper());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this seems only used at one point. therefore I removed the field property.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. I got the impression that it belongs in constructor, to avoid creating it on every method call, but the apply method will be called only once anyway.

this.buildFeatures = buildFeatures;
}

@Override
Expand All @@ -113,6 +114,7 @@ public void apply(Project project) {
this.project = project;
project.getPlugins().apply(JvmToolchainsPlugin.class);
project.getPlugins().apply(JdkDownloadPlugin.class);
project.getPlugins().apply(VersionPropertiesPlugin.class);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tend to follow the pattern of applying nested plugins as early as possible (there are edge cases where you might only do it on certain conditions) in the apply method. that makes reading way simpler and I think its common practice

Provider<GitInfo> gitInfo = project.getPlugins().apply(GitInfoPlugin.class).getGitInfo();

toolChainService = project.getExtensions().getByType(JavaToolchainService.class);
Expand All @@ -121,11 +123,9 @@ public void apply(Project project) {
throw new GradleException("Gradle " + minimumGradleVersion.getVersion() + "+ is required");
}

project.getPlugins().apply(VersionPropertiesPlugin.class);
Properties versionProperties = (Properties) project.getExtensions().getByName(VERSIONS_EXT);
JavaVersion minimumCompilerVersion = JavaVersion.toVersion(versionProperties.get("minimumCompilerJava"));
JavaVersion minimumRuntimeVersion = JavaVersion.toVersion(versionProperties.get("minimumRuntimeJava"));

Version elasticsearchVersionProperty = Version.fromString(versionProperties.getProperty("elasticsearch"));

RuntimeJava runtimeJavaHome = findRuntimeJavaHome();
Expand Down Expand Up @@ -233,6 +233,7 @@ private List<DevelopmentBranch> getDevelopmentBranches() {
}
}

var branchesFileParser = new BranchesFileParser(new ObjectMapper());
return branchesFileParser.parse(branchesBytes);
}

Expand Down Expand Up @@ -266,7 +267,11 @@ private void logGlobalBuildInfo(BuildParameterExtension buildParams) {
if (javaToolchainHome != null) {
LOGGER.quiet(" JAVA_TOOLCHAIN_HOME : " + javaToolchainHome);
}
LOGGER.quiet(" Random Testing Seed : " + buildParams.getTestSeed());

if (buildFeatures.getConfigurationCache().getActive().get() == false) {
// if configuration cache is enabled, resolving the test seed early breaks configuration cache reuse
LOGGER.quiet(" Random Testing Seed : " + buildParams.getTestSeed());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the test seed printing resolves the ValueSource early and breaks configuration cache. therefore only print if not running in cc

}
LOGGER.quiet(" In FIPS 140 mode : " + buildParams.getInFipsJvm());
LOGGER.quiet("=======================================");
}
Expand Down Expand Up @@ -321,16 +326,8 @@ private Stream<InstallationLocation> getAvailableJavaInstallationLocationSteam()
);
}

private static String getTestSeed() {
String testSeedProperty = System.getProperty("tests.seed");
final String testSeed;
if (testSeedProperty == null) {
long seed = new Random(System.currentTimeMillis()).nextLong();
testSeed = Long.toUnsignedString(seed, 16).toUpperCase(Locale.ROOT);
} else {
testSeed = testSeedProperty;
}
return testSeed;
private Provider<String> getTestSeed() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to be able to pass a changing value to Test tasks without breaking configuration cache reuse, we have to rely on a ValueSource here that we pass down all the way to "noninputproperties" CommandLineProvider we configure for tests tasks. ValueSource allows being opaque to the configuration cache. Other providers or plain string would always be interpreted as inputs to the configuration cache, resulting in the test seed not changing between multiple build invocation without configuration cache change

return project.getProviders().of(TestSeedValueSource.class, spec -> {});
}

private static void throwInvalidJavaHomeException(String description, File javaHome, int expectedVersion, int actualVersion) {
Expand Down
37 changes: 37 additions & 0 deletions ...ls-internal/src/main/java/org/elasticsearch/gradle/internal/info/TestSeedValueSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

package org.elasticsearch.gradle.internal.info;

import org.gradle.api.provider.ValueSource;
import org.gradle.api.provider.ValueSourceParameters;

import java.util.Locale;
import java.util.Random;

public abstract class TestSeedValueSource implements ValueSource<String, ValueSourceParameters.None> {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to model the TestSeed resolution as ValueSource as this allows passing the parameter transparently for the configuration cache down to the consumers (test cases, testcluster).

Taking this from the according javadoc:

Represents an external source of information used by a Gradle build. Examples of external sources include client environment variables, system properties, configuration files, shell commands, network services, among others.
Representing external sources as [ValueSource](https://docs.gradle.org/current/javadoc/org/gradle/api/provider/ValueSource.html)s allows Gradle to transparently manage [the configuration cache](https://docs.gradle.org/current/userguide/configuration_cache.html) as values obtained from those sources change. For example, a build might run a different set of tasks depending on whether the CI environment variable is set or not.


@Override
public ValueSourceParameters.None getParameters() {
return null;
}

@Override
public String obtain() {
String testSeedProperty = System.getProperty("tests.seed");
final String testSeed;
if (testSeedProperty == null) {
long seed = new Random(System.currentTimeMillis()).nextLong();
testSeed = Long.toUnsignedString(seed, 16).toUpperCase(Locale.ROOT);
} else {
testSeed = testSeedProperty;
}
return testSeed;
}
}
4 changes: 1 addition & 3 deletions x-pack/plugin/security/qa/profile/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ dependencies {
javaRestTestImplementation project(':x-pack:plugin:security')
}

boolean literalUsername = buildParams.random.nextBoolean()
Copy link
Contributor Author

@breskeby breskeby Sep 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

buildParams.random resolves the TestSeed and breaks configuration-cache. by moving this into the task configuration block we defer that. for this particular test cc will always be out of date. Longterm we might wand a more deterministic way of testing this behaviour

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The altenative would be to split the random from the testseed,, but this would break reproducability.


tasks.named("javaRestTest").configure {
usesDefaultDistribution("to be triaged")
systemProperty 'test.literalUsername', literalUsername
systemProperty 'test.literalUsername', buildParams.random.map{r -> r.nextBoolean()}.get()
}
4 changes: 0 additions & 4 deletions x-pack/plugin/sql/qa/jdbc/security/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import org.elasticsearch.gradle.internal.test.RestIntegTestTask
import org.elasticsearch.gradle.testclusters.TestClusterValueSource
import org.elasticsearch.gradle.testclusters.TestClustersPlugin
import org.elasticsearch.gradle.testclusters.TestClustersRegistry
import org.elasticsearch.gradle.util.GradleUtils

apply plugin: 'elasticsearch.internal-test-artifact'

Expand Down
6 changes: 3 additions & 3 deletions x-pack/qa/multi-cluster-search-security/legacy-with-basic-license/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ restResources {
}

// randomise between sniff and proxy modes
boolean proxyMode = buildParams.random.nextBoolean()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

since we only use this in deprecated TestCluster settings I did not invest to much time to sugar code that api here for now. Ultimately this moves into tests at one point 🤞

var proxyModeProvider = buildParams.random.map{r -> r.nextBoolean()}

def fulfillingCluster = testClusters.register('fulfilling-cluster') {
setting 'xpack.security.enabled', 'true'
Expand Down Expand Up @@ -57,7 +57,7 @@ def queryingCluster = testClusters.register('querying-cluster') {
user username: "test_user", password: "x-pack-test-password"
setting 'xpack.ml.enabled', 'false'
setting 'cluster.remote.my_remote_cluster.skip_unavailable', 'false'
if (proxyMode) {
if (proxyModeProvider.get()) {
setting 'cluster.remote.my_remote_cluster.mode', 'proxy'
setting 'cluster.remote.my_remote_cluster.proxy_address', {
"\"${fulfillingCluster.get().getAllTransportPortURI().get(0)}\""
Expand All @@ -79,7 +79,7 @@ tasks.register('querying-cluster', RestIntegTestTask) {
useCluster fulfillingCluster
useCluster queryingCluster
systemProperty 'tests.rest.suite', 'querying_cluster'
if (proxyMode) {
if (proxyModeProvider.get()) {
systemProperty 'tests.rest.blacklist', [
'querying_cluster/10_basic/Add persistent remote cluster based on the preset cluster',
'querying_cluster/20_info/Add persistent remote cluster based on the preset cluster and check remote info',
Expand Down
4 changes: 2 additions & 2 deletions x-pack/qa/multi-cluster-search-security/legacy-with-full-license/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ restResources {
}

// randomise between sniff and proxy modes
boolean proxyMode = buildParams.random.nextBoolean()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this resolves test seed early basically breaking configuration cache reusability every time

var proxyModeProvider = buildParams.random.map{r -> r.nextBoolean()}

def fulfillingCluster = testClusters.register('fulfilling-cluster') {
setting 'xpack.security.enabled', 'true'
Expand All @@ -43,7 +43,7 @@ def queryingCluster = testClusters.register('querying-cluster') {
setting 'cluster.remote.connections_per_cluster', "1"
user username: "test_user", password: "x-pack-test-password"

if (proxyMode) {
if (proxyModeProvider.get()) {
setting 'cluster.remote.my_remote_cluster.mode', 'proxy'
setting 'cluster.remote.my_remote_cluster.proxy_address', {
"\"${fulfillingCluster.get().getAllTransportPortURI().get(0)}\""
Expand Down
4 changes: 2 additions & 2 deletions x-pack/qa/multi-cluster-search-security/legacy-with-restricted-trust/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ tasks.register("copyCerts", Sync) {
}

// randomise between sniff and proxy modes
boolean proxyMode = buildParams.random.nextBoolean()
var proxyModeProvider = buildParams.random.map{r -> r.nextBoolean()}

def fulfillingCluster = testClusters.register('fulfilling-cluster') {
setting 'xpack.security.enabled', 'true'
Expand Down Expand Up @@ -66,7 +66,7 @@ def queryingCluster = testClusters.register('querying-cluster') {
setting 'cluster.remote.connections_per_cluster', "1"
user username: "test_user", password: "x-pack-test-password"

if (proxyMode) {
if (proxyModeProvider.get()) {
setting 'cluster.remote.my_remote_cluster.mode', 'proxy'
setting 'cluster.remote.my_remote_cluster.proxy_address', {
"\"${fulfillingCluster.get().getAllTransportPortURI().get(0)}\""
Expand Down