From ef0b75765b0820baa0afa4cd5a4e0117a49afce9 Mon Sep 17 00:00:00 2001 From: Mark Vieira Date: Tue, 28 May 2019 17:52:35 -0700 Subject: [PATCH] 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 --- .../elasticsearch/gradle/BuildPlugin.groovy | 21 ++------ ...emPropertyCommandLineArgumentProvider.java | 30 +++++++++++ .../testfixtures/TestFixturesPlugin.java | 53 +++++++++++-------- 3 files changed, 63 insertions(+), 41 deletions(-) create mode 100644 buildSrc/src/main/java/org/elasticsearch/gradle/SystemPropertyCommandLineArgumentProvider.java diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy index 9afbd436400a6..595bd1737309c 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/BuildPlugin.groovy @@ -843,7 +843,7 @@ class BuildPlugin implements Plugin { } test.jvmArgumentProviders.add(nonInputProperties) - test.extensions.getByType(ExtraPropertiesExtension).set('nonInputProperties', nonInputProperties) + test.extensions.add('nonInputProperties', nonInputProperties) test.executable = "${ext.get('runtimeJavaHome')}/bin/java" test.workingDir = project.file("${project.buildDir}/testrun/${test.name}") @@ -865,7 +865,8 @@ class BuildPlugin implements Plugin { } // we use './temp' since this is per JVM and tests are forbidden from writing to CWD - test.systemProperties 'java.io.tmpdir': './temp', + test.systemProperties 'gradle.dist.lib': new File(project.class.location.toURI()).parent, + 'java.io.tmpdir': './temp', 'java.awt.headless': 'true', 'tests.gradle': 'true', 'tests.artifact': project.name, @@ -881,7 +882,6 @@ class BuildPlugin implements Plugin { } // don't track these as inputs since they contain absolute paths and break cache relocatability - nonInputProperties.systemProperty('gradle.dist.lib', new File(project.class.location.toURI()).parent) nonInputProperties.systemProperty('gradle.worker.jar', "${project.gradle.getGradleUserHomeDir()}/caches/${project.gradle.gradleVersion}/workerMain/gradle-worker.jar") nonInputProperties.systemProperty('gradle.user.home', project.gradle.getGradleUserHomeDir()) @@ -1007,19 +1007,4 @@ class BuildPlugin implements Plugin { }) } } - - private static class SystemPropertyCommandLineArgumentProvider implements CommandLineArgumentProvider { - private final Map systemProperties = [:] - - void systemProperty(String key, Object value) { - systemProperties.put(key, value) - } - - @Override - Iterable asArguments() { - return systemProperties.collect { key, value -> - "-D${key}=${value.toString()}".toString() - } - } - } } diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/SystemPropertyCommandLineArgumentProvider.java b/buildSrc/src/main/java/org/elasticsearch/gradle/SystemPropertyCommandLineArgumentProvider.java new file mode 100644 index 0000000000000..7e808724035df --- /dev/null +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/SystemPropertyCommandLineArgumentProvider.java @@ -0,0 +1,30 @@ +package org.elasticsearch.gradle; + +import org.gradle.api.tasks.Input; +import org.gradle.process.CommandLineArgumentProvider; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.stream.Collectors; + +public class SystemPropertyCommandLineArgumentProvider implements CommandLineArgumentProvider { + private final Map systemProperties = new LinkedHashMap<>(); + + public void systemProperty(String key, Object value) { + systemProperties.put(key, value); + } + + @Override + public Iterable asArguments() { + return systemProperties.entrySet() + .stream() + .map(entry -> "-D" + entry.getKey() + "=" + entry.getValue()) + .collect(Collectors.toList()); + } + + // Track system property keys as an input so our build cache key will change if we add properties but values are still ignored + @Input + public Iterable getPropertyNames() { + return systemProperties.keySet(); + } +} diff --git a/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java b/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java index 81b431772c2a3..556e938875e26 100644 --- a/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java +++ b/buildSrc/src/main/java/org/elasticsearch/gradle/testfixtures/TestFixturesPlugin.java @@ -22,7 +22,9 @@ import com.avast.gradle.dockercompose.DockerComposePlugin; import com.avast.gradle.dockercompose.tasks.ComposeUp; import org.elasticsearch.gradle.OS; +import org.elasticsearch.gradle.SystemPropertyCommandLineArgumentProvider; import org.elasticsearch.gradle.precommit.TestingConventionsTasks; +import org.gradle.api.Action; import org.gradle.api.DefaultTask; import org.gradle.api.Plugin; import org.gradle.api.Project; @@ -142,7 +144,8 @@ public void apply(Project project) { configureServiceInfoForTask( task, fixtureProject, - task::systemProperty + (name, host) -> + task.getExtensions().getByType(SystemPropertyCommandLineArgumentProvider.class).systemProperty(name, host) ); task.dependsOn(fixtureProject.getTasks().getByName("postProcessFixture")); }) @@ -165,28 +168,32 @@ private void conditionTaskByType(TaskContainer tasks, TestFixtureExtension exten private void configureServiceInfoForTask(Task task, Project fixtureProject, BiConsumer consumer) { // Configure ports for the tests as system properties. // We only know these at execution time so we need to do it in doFirst - task.doFirst(theTask -> - fixtureProject.getExtensions().getByType(ComposeExtension.class).getServicesInfos() - .forEach((service, infos) -> { - infos.getTcpPorts() - .forEach((container, host) -> { - String name = "test.fixtures." + service + ".tcp." + container; - theTask.getLogger().info("port mapping property: {}={}", name, host); - consumer.accept( - name, - host - ); - }); - infos.getUdpPorts() - .forEach((container, host) -> { - String name = "test.fixtures." + service + ".udp." + container; - theTask.getLogger().info("port mapping property: {}={}", name, host); - consumer.accept( - name, - host - ); - }); - }) + task.doFirst(new Action() { + @Override + public void execute(Task theTask) { + fixtureProject.getExtensions().getByType(ComposeExtension.class).getServicesInfos() + .forEach((service, infos) -> { + infos.getTcpPorts() + .forEach((container, host) -> { + String name = "test.fixtures." + service + ".tcp." + container; + theTask.getLogger().info("port mapping property: {}={}", name, host); + consumer.accept( + name, + host + ); + }); + infos.getUdpPorts() + .forEach((container, host) -> { + String name = "test.fixtures." + service + ".udp." + container; + theTask.getLogger().info("port mapping property: {}={}", name, host); + consumer.accept( + name, + host + ); + }); + }); + } + } ); }