Skip to content

Commit 72e2f50

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. (cherry picked from commit 8097c1a) # Conflicts: # x-pack/plugin/security/qa/profile/build.gradle
1 parent 3d56ecd commit 72e2f50

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;
@@ -51,6 +52,9 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
5152
@Inject
5253
protected abstract ProviderFactory getProviderFactory();
5354

55+
@Inject
56+
protected abstract BuildFeatures getBuildFeatures();
57+
5458
@Override
5559
public void apply(Project project) {
5660
project.getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class);
@@ -158,9 +162,11 @@ public void execute(Task t) {
158162
);
159163
test.systemProperties(sysprops);
160164

161-
// ignore changing test seed when build is passed -Dignore.tests.seed for cacheability experimentation
162-
if (System.getProperty("ignore.tests.seed") != null) {
163-
nonInputProperties.systemProperty("tests.seed", buildParams.get().getTestSeed());
165+
// ignore changing test seed when build is passed -Dignore.tests.seed for cacheability
166+
// also ignore when configuration cache is on since the test seed as task input would break
167+
// configuration cache reuse.
168+
if (System.getProperty("ignore.tests.seed") != null || getBuildFeatures().getConfigurationCache().getActive().get()) {
169+
nonInputProperties.systemProperty("tests.seed", buildParams.get().getTestSeedProvider());
164170
} else {
165171
test.systemProperty("tests.seed", buildParams.get().getTestSeed());
166172
}

‎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()
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)