Skip to content

Commit 813e57d

Browse files
mark-vieiramark-vieira
authored
Add explicit build flag for experimenting with test execution cacheability (#42649)
* Add build flag for ignoring random test seed as task input * Fix checkstyle violations
1 parent a1e7858 commit 813e57d

File tree

3 files changed

+72
-42
lines changed

3 files changed

+72
-42
lines changed

‎buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ class BuildPlugin implements Plugin<Project> {
817817
}
818818

819819
test.jvmArgumentProviders.add(nonInputProperties)
820-
test.extensions.getByType(ExtraPropertiesExtension).set('nonInputProperties', nonInputProperties)
820+
test.extensions.add('nonInputProperties', nonInputProperties)
821821

822822
test.executable = "${ext.get('runtimeJavaHome')}/bin/java"
823823
test.workingDir = project.file("${project.buildDir}/testrun/${test.name}")
@@ -842,17 +842,25 @@ class BuildPlugin implements Plugin<Project> {
842842

843843
// we use './temp' since this is per JVM and tests are forbidden from writing to CWD
844844
test.systemProperties 'gradle.dist.lib': new File(project.class.location.toURI()).parent,
845-
'gradle.worker.jar': "${project.gradle.getGradleUserHomeDir()}/caches/${project.gradle.gradleVersion}/workerMain/gradle-worker.jar",
846-
'gradle.user.home': project.gradle.getGradleUserHomeDir(),
847845
'java.io.tmpdir': './temp',
848846
'java.awt.headless': 'true',
849847
'tests.gradle': 'true',
850848
'tests.artifact': project.name,
851849
'tests.task': test.path,
852850
'tests.security.manager': 'true',
853-
'tests.seed': project.property('testSeed'),
854851
'jna.nosys': 'true'
855852

853+
// ignore changing test seed when build is passed -Dignore.tests.seed for cacheability experimentation
854+
if (System.getProperty('ignore.tests.seed') != null) {
855+
nonInputProperties.systemProperty('tests.seed', project.property('testSeed'))
856+
} else {
857+
test.systemProperty('tests.seed', project.property('testSeed'))
858+
}
859+
860+
// don't track these as inputs since they contain absolute paths and break cache relocatability
861+
nonInputProperties.systemProperty('gradle.worker.jar', "${project.gradle.getGradleUserHomeDir()}/caches/${project.gradle.gradleVersion}/workerMain/gradle-worker.jar")
862+
nonInputProperties.systemProperty('gradle.user.home', project.gradle.getGradleUserHomeDir())
863+
856864
nonInputProperties.systemProperty('compiler.java', "${-> (ext.get('compilerJavaVersion') as JavaVersion).getMajorVersion()}")
857865

858866
// TODO: remove setting logging level via system property
@@ -965,19 +973,4 @@ class BuildPlugin implements Plugin<Project> {
965973
})
966974
}
967975
}
968-
969-
private static class SystemPropertyCommandLineArgumentProvider implements CommandLineArgumentProvider {
970-
private final Map<String, Object> systemProperties = [:]
971-
972-
void systemProperty(String key, Object value) {
973-
systemProperties.put(key, value)
974-
}
975-
976-
@Override
977-
Iterable<String> asArguments() {
978-
return systemProperties.collect { key, value ->
979-
"-D${key}=${value.toString()}".toString()
980-
}
981-
}
982-
}
983976
}

‎buildSrc/src/main/java/org/elasticsearch/gradle/SystemPropertyCommandLineArgumentProvider.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package org.elasticsearch.gradle;
2+
3+
import org.gradle.api.tasks.Input;
4+
import org.gradle.process.CommandLineArgumentProvider;
5+
6+
import java.util.LinkedHashMap;
7+
import java.util.Map;
8+
import java.util.stream.Collectors;
9+
10+
public class SystemPropertyCommandLineArgumentProvider implements CommandLineArgumentProvider {
11+
private final Map<String, Object> systemProperties = new LinkedHashMap<>();
12+
13+
public void systemProperty(String key, Object value) {
14+
systemProperties.put(key, value);
15+
}
16+
17+
@Override
18+
public Iterable<String> asArguments() {
19+
return systemProperties.entrySet()
20+
.stream()
21+
.map(entry -> "-D" + entry.getKey() + "=" + entry.getValue())
22+
.collect(Collectors.toList());
23+
}
24+
25+
// Track system property keys as an input so our build cache key will change if we add properties but values are still ignored
26+
@Input
27+
public Iterable<String> getPropertyNames() {
28+
return systemProperties.keySet();
29+
}
30+
}

‎buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@
2222
import com.avast.gradle.dockercompose.DockerComposePlugin;
2323
import com.avast.gradle.dockercompose.tasks.ComposeUp;
2424
import org.elasticsearch.gradle.OS;
25+
import org.elasticsearch.gradle.SystemPropertyCommandLineArgumentProvider;
2526
import org.elasticsearch.gradle.precommit.JarHellTask;
2627
import org.elasticsearch.gradle.precommit.TestingConventionsTasks;
2728
import org.elasticsearch.gradle.precommit.ThirdPartyAuditTask;
29+
import org.gradle.api.Action;
2830
import org.gradle.api.DefaultTask;
2931
import org.gradle.api.Plugin;
3032
import org.gradle.api.Project;
@@ -122,7 +124,8 @@ public void apply(Project project) {
122124
configureServiceInfoForTask(
123125
task,
124126
fixtureProject,
125-
task::systemProperty
127+
(name, host) ->
128+
task.getExtensions().getByType(SystemPropertyCommandLineArgumentProvider.class).systemProperty(name, host)
126129
);
127130
task.dependsOn(fixtureProject.getTasks().getByName("postProcessFixture"));
128131
})
@@ -143,28 +146,32 @@ private void conditionTaskByType(TaskContainer tasks, TestFixtureExtension exten
143146
private void configureServiceInfoForTask(Task task, Project fixtureProject, BiConsumer<String, Integer> consumer) {
144147
// Configure ports for the tests as system properties.
145148
// We only know these at execution time so we need to do it in doFirst
146-
task.doFirst(theTask ->
147-
fixtureProject.getExtensions().getByType(ComposeExtension.class).getServicesInfos()
148-
.forEach((service, infos) -> {
149-
infos.getTcpPorts()
150-
.forEach((container, host) -> {
151-
String name = "test.fixtures." + service + ".tcp." + container;
152-
theTask.getLogger().info("port mapping property: {}={}", name, host);
153-
consumer.accept(
154-
name,
155-
host
156-
);
157-
});
158-
infos.getUdpPorts()
159-
.forEach((container, host) -> {
160-
String name = "test.fixtures." + service + ".udp." + container;
161-
theTask.getLogger().info("port mapping property: {}={}", name, host);
162-
consumer.accept(
163-
name,
164-
host
165-
);
166-
});
167-
})
149+
task.doFirst(new Action<Task>() {
150+
@Override
151+
public void execute(Task theTask) {
152+
fixtureProject.getExtensions().getByType(ComposeExtension.class).getServicesInfos()
153+
.forEach((service, infos) -> {
154+
infos.getTcpPorts()
155+
.forEach((container, host) -> {
156+
String name = "test.fixtures." + service + ".tcp." + container;
157+
theTask.getLogger().info("port mapping property: {}={}", name, host);
158+
consumer.accept(
159+
name,
160+
host
161+
);
162+
});
163+
infos.getUdpPorts()
164+
.forEach((container, host) -> {
165+
String name = "test.fixtures." + service + ".udp." + container;
166+
theTask.getLogger().info("port mapping property: {}={}", name, host);
167+
consumer.accept(
168+
name,
169+
host
170+
);
171+
});
172+
});
173+
}
174+
}
168175
);
169176
}
170177

0 commit comments

Comments
 (0)