From f29f7f097f134efcac1c6844bd9da14987a1d7f2 Mon Sep 17 00:00:00 2001 From: Rene Groeschke Date: Thu, 4 Sep 2025 16:14:41 +0200 Subject: [PATCH] [Gradle] Fix test seed handling when running with cc (#133811) This ensures we provide a new test seed per build invocation when running with configuration cache enabled while still ensuring the configuration cache can be reused. basically test seed beeing ignored as cc input. --- ...ElasticsearchTestBasePluginFuncTest.groovy | 61 +++++++++++++++++++ .../internal/ElasticsearchTestBasePlugin.java | 12 +++- .../info/BuildParameterExtension.java | 4 +- .../info/DefaultBuildParameterExtension.java | 13 ++-- .../internal/info/GlobalBuildInfoPlugin.java | 31 +++++----- .../internal/info/TestSeedValueSource.java | 37 +++++++++++ .../plugin/security/qa/profile/build.gradle | 4 +- .../plugin/sql/qa/jdbc/security/build.gradle | 4 -- .../legacy-with-basic-license/build.gradle | 6 +- .../legacy-with-full-license/build.gradle | 4 +- .../legacy-with-restricted-trust/build.gradle | 4 +- 11 files changed, 141 insertions(+), 39 deletions(-) create mode 100644 build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/TestSeedValueSource.java diff --git a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/ElasticsearchTestBasePluginFuncTest.groovy b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/ElasticsearchTestBasePluginFuncTest.groovy index e6ddad01933fb..69defbee8816e 100644 --- a/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/ElasticsearchTestBasePluginFuncTest.groovy +++ b/build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/ElasticsearchTestBasePluginFuncTest.groovy @@ -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.") + } } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java index 387ef5523b23b..0bd766752a377 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java @@ -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; @@ -56,6 +57,9 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin { @Inject protected abstract ProviderFactory getProviderFactory(); + @Inject + protected abstract BuildFeatures getBuildFeatures(); + @Override public void apply(Project project) { project.getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class); @@ -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()); } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/BuildParameterExtension.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/BuildParameterExtension.java index c3c2a9cdfe672..cac13eac83fe6 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/BuildParameterExtension.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/BuildParameterExtension.java @@ -56,6 +56,8 @@ public interface BuildParameterExtension { String getTestSeed(); + Provider getTestSeedProvider(); + Boolean getCi(); Integer getDefaultParallel(); @@ -66,7 +68,7 @@ public interface BuildParameterExtension { Provider getBwcVersionsProvider(); - Random getRandom(); + Provider getRandom(); Boolean getGraalVmRuntime(); } diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/DefaultBuildParameterExtension.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/DefaultBuildParameterExtension.java index ccda893cc9500..afa3ed26cfb9f 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/DefaultBuildParameterExtension.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/DefaultBuildParameterExtension.java @@ -39,7 +39,7 @@ public abstract class DefaultBuildParameterExtension implements BuildParameterEx private final Provider gitRevision; private transient AtomicReference buildDate = new AtomicReference<>(); - private final String testSeed; + private final Provider testSeed; private final Boolean isCi; private final Integer defaultParallel; private final Boolean snapshotBuild; @@ -58,7 +58,7 @@ public DefaultBuildParameterExtension( JavaVersion gradleJavaVersion, Provider gitRevision, Provider gitOrigin, - String testSeed, + Provider testSeed, boolean isCi, int defaultParallel, final boolean isSnapshotBuild, @@ -181,6 +181,11 @@ public ZonedDateTime getBuildDate() { @Override public String getTestSeed() { + return testSeed.get(); + } + + @Override + public Provider getTestSeedProvider() { return testSeed; } @@ -205,8 +210,8 @@ public BwcVersions getBwcVersions() { } @Override - public Random getRandom() { - return new Random(Long.parseUnsignedLong(testSeed.split(":")[0], 16)); + public Provider getRandom() { + return getTestSeedProvider().map(seed -> new Random(Long.parseUnsignedLong(seed.split(":")[0], 16))); } @Override diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java index fba659a39982d..d733da2a4b690 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java @@ -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; @@ -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; @@ -87,7 +87,7 @@ public class GlobalBuildInfoPlugin implements Plugin { 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; @@ -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()); + this.buildFeatures = buildFeatures; } @Override @@ -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); Provider gitInfo = project.getPlugins().apply(GitInfoPlugin.class).getGitInfo(); toolChainService = project.getExtensions().getByType(JavaToolchainService.class); @@ -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(); @@ -233,6 +233,7 @@ private List getDevelopmentBranches() { } } + var branchesFileParser = new BranchesFileParser(new ObjectMapper()); return branchesFileParser.parse(branchesBytes); } @@ -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()); + } LOGGER.quiet(" In FIPS 140 mode : " + buildParams.getInFipsJvm()); LOGGER.quiet("======================================="); } @@ -321,16 +326,8 @@ private Stream 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 getTestSeed() { + return project.getProviders().of(TestSeedValueSource.class, spec -> {}); } private static void throwInvalidJavaHomeException(String description, File javaHome, int expectedVersion, int actualVersion) { diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/TestSeedValueSource.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/TestSeedValueSource.java new file mode 100644 index 0000000000000..df0731bf999ca --- /dev/null +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/TestSeedValueSource.java @@ -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 { + + @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; + } +} diff --git a/x-pack/plugin/security/qa/profile/build.gradle b/x-pack/plugin/security/qa/profile/build.gradle index 1c3a0503d2499..a1f9ec0d9db2f 100644 --- a/x-pack/plugin/security/qa/profile/build.gradle +++ b/x-pack/plugin/security/qa/profile/build.gradle @@ -12,9 +12,7 @@ dependencies { javaRestTestImplementation project(':x-pack:plugin:security') } -boolean literalUsername = buildParams.random.nextBoolean() - tasks.named("javaRestTest").configure { usesDefaultDistribution("to be triaged") - systemProperty 'test.literalUsername', literalUsername + systemProperty 'test.literalUsername', buildParams.random.map{r -> r.nextBoolean()}.get() } diff --git a/x-pack/plugin/sql/qa/jdbc/security/build.gradle b/x-pack/plugin/sql/qa/jdbc/security/build.gradle index bed7ff60107b2..09a7f8b5d8d36 100644 --- a/x-pack/plugin/sql/qa/jdbc/security/build.gradle +++ b/x-pack/plugin/sql/qa/jdbc/security/build.gradle @@ -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' diff --git a/x-pack/qa/multi-cluster-search-security/legacy-with-basic-license/build.gradle b/x-pack/qa/multi-cluster-search-security/legacy-with-basic-license/build.gradle index 83c231da7529c..79dcc098667c8 100644 --- a/x-pack/qa/multi-cluster-search-security/legacy-with-basic-license/build.gradle +++ b/x-pack/qa/multi-cluster-search-security/legacy-with-basic-license/build.gradle @@ -19,7 +19,7 @@ restResources { } // 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' @@ -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)}\"" @@ -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', diff --git a/x-pack/qa/multi-cluster-search-security/legacy-with-full-license/build.gradle b/x-pack/qa/multi-cluster-search-security/legacy-with-full-license/build.gradle index 6e95d718b19de..bc9515bb9d04f 100644 --- a/x-pack/qa/multi-cluster-search-security/legacy-with-full-license/build.gradle +++ b/x-pack/qa/multi-cluster-search-security/legacy-with-full-license/build.gradle @@ -19,7 +19,7 @@ restResources { } // 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' @@ -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)}\"" diff --git a/x-pack/qa/multi-cluster-search-security/legacy-with-restricted-trust/build.gradle b/x-pack/qa/multi-cluster-search-security/legacy-with-restricted-trust/build.gradle index 5c6235e092458..f95b597b189aa 100644 --- a/x-pack/qa/multi-cluster-search-security/legacy-with-restricted-trust/build.gradle +++ b/x-pack/qa/multi-cluster-search-security/legacy-with-restricted-trust/build.gradle @@ -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' @@ -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)}\""