Skip to content

Commit f29f7f0

Browse files
breskebybreskeby
committed
[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.
1 parent b1907fb commit f29f7f0

File tree

11 files changed

+141
-39
lines changed

11 files changed

+141
-39
lines changed

‎build-tools-internal/src/integTest/groovy/org/elasticsearch/gradle/internal/ElasticsearchTestBasePluginFuncTest.groovy‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,4 +108,65 @@ class ElasticsearchTestBasePluginFuncTest extends AbstractGradleFuncTest {
108108
then:
109109
result.task(':test').outcome == TaskOutcome.UP_TO_DATE
110110
}
111+
112+
def "uses new test seed for every invocation"() {
113+
given:
114+
file("src/test/java/acme/SomeTests.java").text = """
115+
116+
public class SomeTests {
117+
@org.junit.Test
118+
public void printTestSeed() {
119+
System.out.println("TESTSEED=[" + System.getProperty("tests.seed") + "]");
120+
}
121+
}
122+
123+
"""
124+
buildFile.text = """
125+
plugins {
126+
id 'java'
127+
id 'elasticsearch.test-base'
128+
}
129+
130+
tasks.named('test').configure {
131+
testLogging {
132+
showStandardStreams = true
133+
}
134+
}
135+
136+
tasks.register('test2', Test) {
137+
classpath = sourceSets.test.runtimeClasspath
138+
testClassesDirs = sourceSets.test.output.classesDirs
139+
testLogging {
140+
showStandardStreams = true
141+
}
142+
}
143+
144+
repositories {
145+
mavenCentral()
146+
}
147+
148+
dependencies {
149+
testImplementation 'junit:junit:4.12'
150+
}
151+
152+
"""
153+
154+
when:
155+
def result1 = gradleRunner("cleanTest", "cleanTest2", "test", "test2").build()
156+
def result2 = gradleRunner("cleanTest", "cleanTest2", "test", "test2").build()
157+
158+
then:
159+
def seeds1 = result1.output.findAll(/(?m)TESTSEED=\[([^\]]+)\]/) { it[1] }
160+
def seeds2 = result2.output.findAll(/(?m)TESTSEED=\[([^\]]+)\]/) { it[1] }
161+
162+
seeds1.unique().size() == 1
163+
seeds2.unique().size() == 1
164+
165+
verifyAll {
166+
seeds1[0] != null
167+
seeds2[0] != null
168+
seeds1[0] != seeds2[0]
169+
}
170+
result2.output.contains("Configuration cache entry reused.")
171+
}
111172
}

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/ElasticsearchTestBasePlugin.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.gradle.api.Project;
2525
import org.gradle.api.Task;
2626
import org.gradle.api.artifacts.Configuration;
27+
import org.gradle.api.configuration.BuildFeatures;
2728
import org.gradle.api.file.FileCollection;
2829
import org.gradle.api.plugins.JavaPlugin;
2930
import org.gradle.api.provider.ProviderFactory;
@@ -56,6 +57,9 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
5657
@Inject
5758
protected abstract ProviderFactory getProviderFactory();
5859

60+
@Inject
61+
protected abstract BuildFeatures getBuildFeatures();
62+
5963
@Override
6064
public void apply(Project project) {
6165
project.getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class);
@@ -164,9 +168,11 @@ public void execute(Task t) {
164168
);
165169
test.systemProperties(sysprops);
166170

167-
// ignore changing test seed when build is passed -Dignore.tests.seed for cacheability experimentation
168-
if (System.getProperty("ignore.tests.seed") != null) {
169-
nonInputProperties.systemProperty("tests.seed", buildParams.get().getTestSeed());
171+
// ignore changing test seed when build is passed -Dignore.tests.seed for cacheability
172+
// also ignore when configuration cache is on since the test seed as task input would break
173+
// configuration cache reuse.
174+
if (System.getProperty("ignore.tests.seed") != null || getBuildFeatures().getConfigurationCache().getActive().get()) {
175+
nonInputProperties.systemProperty("tests.seed", buildParams.get().getTestSeedProvider());
170176
} else {
171177
test.systemProperty("tests.seed", buildParams.get().getTestSeed());
172178
}

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/BuildParameterExtension.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ public interface BuildParameterExtension {
5656

5757
String getTestSeed();
5858

59+
Provider<String> getTestSeedProvider();
60+
5961
Boolean getCi();
6062

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

6769
Provider<BwcVersions> getBwcVersionsProvider();
6870

69-
Random getRandom();
71+
Provider<Random> getRandom();
7072

7173
Boolean getGraalVmRuntime();
7274
}

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/DefaultBuildParameterExtension.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public abstract class DefaultBuildParameterExtension implements BuildParameterEx
3939
private final Provider<String> gitRevision;
4040

4141
private transient AtomicReference<ZonedDateTime> buildDate = new AtomicReference<>();
42-
private final String testSeed;
42+
private final Provider<String> testSeed;
4343
private final Boolean isCi;
4444
private final Integer defaultParallel;
4545
private final Boolean snapshotBuild;
@@ -58,7 +58,7 @@ public DefaultBuildParameterExtension(
5858
JavaVersion gradleJavaVersion,
5959
Provider<String> gitRevision,
6060
Provider<String> gitOrigin,
61-
String testSeed,
61+
Provider<String> testSeed,
6262
boolean isCi,
6363
int defaultParallel,
6464
final boolean isSnapshotBuild,
@@ -181,6 +181,11 @@ public ZonedDateTime getBuildDate() {
181181

182182
@Override
183183
public String getTestSeed() {
184+
return testSeed.get();
185+
}
186+
187+
@Override
188+
public Provider<String> getTestSeedProvider() {
184189
return testSeed;
185190
}
186191

@@ -205,8 +210,8 @@ public BwcVersions getBwcVersions() {
205210
}
206211

207212
@Override
208-
public Random getRandom() {
209-
return new Random(Long.parseUnsignedLong(testSeed.split(":")[0], 16));
213+
public Provider<Random> getRandom() {
214+
return getTestSeedProvider().map(seed -> new Random(Long.parseUnsignedLong(seed.split(":")[0], 16)));
210215
}
211216

212217
@Override

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.gradle.api.NamedDomainObjectContainer;
3030
import org.gradle.api.Plugin;
3131
import org.gradle.api.Project;
32+
import org.gradle.api.configuration.BuildFeatures;
3233
import org.gradle.api.logging.Logger;
3334
import org.gradle.api.logging.Logging;
3435
import org.gradle.api.model.ObjectFactory;
@@ -60,7 +61,6 @@
6061
import java.util.List;
6162
import java.util.Locale;
6263
import java.util.Properties;
63-
import java.util.Random;
6464
import java.util.concurrent.atomic.AtomicReference;
6565
import java.util.regex.Matcher;
6666
import java.util.regex.Pattern;
@@ -87,7 +87,7 @@ public class GlobalBuildInfoPlugin implements Plugin<Project> {
8787
private final JavaInstallationRegistry javaInstallationRegistry;
8888
private final JvmMetadataDetector metadataDetector;
8989
private final ProviderFactory providers;
90-
private final BranchesFileParser branchesFileParser;
90+
private final BuildFeatures buildFeatures;
9191
private JavaToolchainService toolChainService;
9292
private Project project;
9393

@@ -96,13 +96,14 @@ public GlobalBuildInfoPlugin(
9696
ObjectFactory objectFactory,
9797
JavaInstallationRegistry javaInstallationRegistry,
9898
JvmMetadataDetector metadataDetector,
99-
ProviderFactory providers
99+
ProviderFactory providers,
100+
BuildFeatures buildFeatures
100101
) {
101102
this.objectFactory = objectFactory;
102103
this.javaInstallationRegistry = javaInstallationRegistry;
103104
this.metadataDetector = new ErrorTraceMetadataDetector(metadataDetector);
104105
this.providers = providers;
105-
this.branchesFileParser = new BranchesFileParser(new ObjectMapper());
106+
this.buildFeatures = buildFeatures;
106107
}
107108

108109
@Override
@@ -113,6 +114,7 @@ public void apply(Project project) {
113114
this.project = project;
114115
project.getPlugins().apply(JvmToolchainsPlugin.class);
115116
project.getPlugins().apply(JdkDownloadPlugin.class);
117+
project.getPlugins().apply(VersionPropertiesPlugin.class);
116118
Provider<GitInfo> gitInfo = project.getPlugins().apply(GitInfoPlugin.class).getGitInfo();
117119

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

124-
project.getPlugins().apply(VersionPropertiesPlugin.class);
125126
Properties versionProperties = (Properties) project.getExtensions().getByName(VERSIONS_EXT);
126127
JavaVersion minimumCompilerVersion = JavaVersion.toVersion(versionProperties.get("minimumCompilerJava"));
127128
JavaVersion minimumRuntimeVersion = JavaVersion.toVersion(versionProperties.get("minimumRuntimeJava"));
128-
129129
Version elasticsearchVersionProperty = Version.fromString(versionProperties.getProperty("elasticsearch"));
130130

131131
RuntimeJava runtimeJavaHome = findRuntimeJavaHome();
@@ -233,6 +233,7 @@ private List<DevelopmentBranch> getDevelopmentBranches() {
233233
}
234234
}
235235

236+
var branchesFileParser = new BranchesFileParser(new ObjectMapper());
236237
return branchesFileParser.parse(branchesBytes);
237238
}
238239

@@ -266,7 +267,11 @@ private void logGlobalBuildInfo(BuildParameterExtension buildParams) {
266267
if (javaToolchainHome != null) {
267268
LOGGER.quiet(" JAVA_TOOLCHAIN_HOME : " + javaToolchainHome);
268269
}
269-
LOGGER.quiet(" Random Testing Seed : " + buildParams.getTestSeed());
270+
271+
if (buildFeatures.getConfigurationCache().getActive().get() == false) {
272+
// if configuration cache is enabled, resolving the test seed early breaks configuration cache reuse
273+
LOGGER.quiet(" Random Testing Seed : " + buildParams.getTestSeed());
274+
}
270275
LOGGER.quiet(" In FIPS 140 mode : " + buildParams.getInFipsJvm());
271276
LOGGER.quiet("=======================================");
272277
}
@@ -321,16 +326,8 @@ private Stream<InstallationLocation> getAvailableJavaInstallationLocationSteam()
321326
);
322327
}
323328

324-
private static String getTestSeed() {
325-
String testSeedProperty = System.getProperty("tests.seed");
326-
final String testSeed;
327-
if (testSeedProperty == null) {
328-
long seed = new Random(System.currentTimeMillis()).nextLong();
329-
testSeed = Long.toUnsignedString(seed, 16).toUpperCase(Locale.ROOT);
330-
} else {
331-
testSeed = testSeedProperty;
332-
}
333-
return testSeed;
329+
private Provider<String> getTestSeed() {
330+
return project.getProviders().of(TestSeedValueSource.class, spec -> {});
334331
}
335332

336333
private static void throwInvalidJavaHomeException(String description, File javaHome, int expectedVersion, int actualVersion) {

‎build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/TestSeedValueSource.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the "Elastic License
4+
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
5+
* Public License v 1"; you may not use this file except in compliance with, at
6+
* your election, the "Elastic License 2.0", the "GNU Affero General Public
7+
* License v3.0 only", or the "Server Side Public License, v 1".
8+
*/
9+
10+
package org.elasticsearch.gradle.internal.info;
11+
12+
import org.gradle.api.provider.ValueSource;
13+
import org.gradle.api.provider.ValueSourceParameters;
14+
15+
import java.util.Locale;
16+
import java.util.Random;
17+
18+
public abstract class TestSeedValueSource implements ValueSource<String, ValueSourceParameters.None> {
19+
20+
@Override
21+
public ValueSourceParameters.None getParameters() {
22+
return null;
23+
}
24+
25+
@Override
26+
public String obtain() {
27+
String testSeedProperty = System.getProperty("tests.seed");
28+
final String testSeed;
29+
if (testSeedProperty == null) {
30+
long seed = new Random(System.currentTimeMillis()).nextLong();
31+
testSeed = Long.toUnsignedString(seed, 16).toUpperCase(Locale.ROOT);
32+
} else {
33+
testSeed = testSeedProperty;
34+
}
35+
return testSeed;
36+
}
37+
}

‎x-pack/plugin/security/qa/profile/build.gradle‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,7 @@ dependencies {
1212
javaRestTestImplementation project(':x-pack:plugin:security')
1313
}
1414

15-
boolean literalUsername = buildParams.random.nextBoolean()
16-
1715
tasks.named("javaRestTest").configure {
1816
usesDefaultDistribution("to be triaged")
19-
systemProperty 'test.literalUsername', literalUsername
17+
systemProperty 'test.literalUsername', buildParams.random.map{r -> r.nextBoolean()}.get()
2018
}

‎x-pack/plugin/sql/qa/jdbc/security/build.gradle‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
11
import org.elasticsearch.gradle.internal.test.RestIntegTestTask
2-
import org.elasticsearch.gradle.testclusters.TestClusterValueSource
3-
import org.elasticsearch.gradle.testclusters.TestClustersPlugin
4-
import org.elasticsearch.gradle.testclusters.TestClustersRegistry
5-
import org.elasticsearch.gradle.util.GradleUtils
62

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

‎x-pack/qa/multi-cluster-search-security/legacy-with-basic-license/build.gradle‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ restResources {
1919
}
2020

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

2424
def fulfillingCluster = testClusters.register('fulfilling-cluster') {
2525
setting 'xpack.security.enabled', 'true'
@@ -57,7 +57,7 @@ def queryingCluster = testClusters.register('querying-cluster') {
5757
user username: "test_user", password: "x-pack-test-password"
5858
setting 'xpack.ml.enabled', 'false'
5959
setting 'cluster.remote.my_remote_cluster.skip_unavailable', 'false'
60-
if (proxyMode) {
60+
if (proxyModeProvider.get()) {
6161
setting 'cluster.remote.my_remote_cluster.mode', 'proxy'
6262
setting 'cluster.remote.my_remote_cluster.proxy_address', {
6363
"\"${fulfillingCluster.get().getAllTransportPortURI().get(0)}\""
@@ -79,7 +79,7 @@ tasks.register('querying-cluster', RestIntegTestTask) {
7979
useCluster fulfillingCluster
8080
useCluster queryingCluster
8181
systemProperty 'tests.rest.suite', 'querying_cluster'
82-
if (proxyMode) {
82+
if (proxyModeProvider.get()) {
8383
systemProperty 'tests.rest.blacklist', [
8484
'querying_cluster/10_basic/Add persistent remote cluster based on the preset cluster',
8585
'querying_cluster/20_info/Add persistent remote cluster based on the preset cluster and check remote info',

‎x-pack/qa/multi-cluster-search-security/legacy-with-full-license/build.gradle‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ restResources {
1919
}
2020

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

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

46-
if (proxyMode) {
46+
if (proxyModeProvider.get()) {
4747
setting 'cluster.remote.my_remote_cluster.mode', 'proxy'
4848
setting 'cluster.remote.my_remote_cluster.proxy_address', {
4949
"\"${fulfillingCluster.get().getAllTransportPortURI().get(0)}\""

0 commit comments

Comments
 (0)