Browse Source

Polish and cleanup test related plugins (#74673)

- avoid eagerly created test cluster
- remove duplicate superflous configuration
- resolve system properties via provider factory
- move common test configuration / setup into rest test base plugin
Rene Groeschke 4 years ago
parent
commit
1db75f75ca

+ 3 - 3
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/rest/compat/YamlRestCompatTestPlugin.java

@@ -37,7 +37,7 @@ import java.io.File;
 import java.nio.file.Path;
 import java.util.Map;
 
-import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.setupDependencies;
+import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.setupTestDependenciesDefaults;
 
 /**
  * Apply this plugin to run the YAML based REST tests from a prior major version against this version's cluster.
@@ -162,7 +162,7 @@ public class YamlRestCompatTestPlugin implements Plugin<Project> {
             .flatMap(CopyRestTestsTask::getOutputResourceDir);
 
         // setup the yamlRestTest task
-        Provider<RestIntegTestTask> yamlRestCompatTestTask = RestTestUtil.registerTask(project, yamlCompatTestSourceSet);
+        Provider<RestIntegTestTask> yamlRestCompatTestTask = RestTestUtil.registerTestTask(project, yamlCompatTestSourceSet);
         project.getTasks().withType(RestIntegTestTask.class).named(SOURCE_SET_NAME).configure(testTask -> {
             // Use test runner and classpath from "normal" yaml source set
             testTask.setTestClassesDirs(
@@ -180,7 +180,7 @@ public class YamlRestCompatTestPlugin implements Plugin<Project> {
             testTask.onlyIf(t -> isEnabled(project));
         });
 
-        setupDependencies(project, yamlCompatTestSourceSet);
+        setupTestDependenciesDefaults(project, yamlCompatTestSourceSet);
 
         // setup IDE
         GradleUtils.setupIdeForTestSourceSet(project, yamlCompatTestSourceSet);

+ 24 - 4
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/RestTestBasePlugin.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.gradle.internal.test;
 
+import org.elasticsearch.gradle.internal.ElasticsearchJavaBasePlugin;
 import org.elasticsearch.gradle.internal.ElasticsearchTestBasePlugin;
 import org.elasticsearch.gradle.internal.FixtureStop;
 import org.elasticsearch.gradle.internal.InternalTestClustersPlugin;
@@ -17,16 +18,28 @@ import org.elasticsearch.gradle.testclusters.TestClustersPlugin;
 import org.gradle.api.NamedDomainObjectContainer;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
+import org.gradle.api.plugins.JavaBasePlugin;
+import org.gradle.api.provider.ProviderFactory;
+import org.jetbrains.annotations.Nullable;
+
+import javax.inject.Inject;
 
 public class RestTestBasePlugin implements Plugin<Project> {
     private static final String TESTS_REST_CLUSTER = "tests.rest.cluster";
     private static final String TESTS_CLUSTER = "tests.cluster";
     private static final String TESTS_CLUSTER_NAME = "tests.clustername";
+    private ProviderFactory providerFactory;
+
+    @Inject
+    public RestTestBasePlugin(ProviderFactory providerFactory) {
+        this.providerFactory = providerFactory;
+    }
 
     @Override
     public void apply(Project project) {
-        project.getPluginManager().apply(InternalTestClustersPlugin.class);
+        project.getPluginManager().apply(ElasticsearchJavaBasePlugin.class);
         project.getPluginManager().apply(ElasticsearchTestBasePlugin.class);
+        project.getPluginManager().apply(InternalTestClustersPlugin.class);
         project.getTasks().withType(RestIntegTestTask.class).configureEach(restIntegTestTask -> {
             @SuppressWarnings("unchecked")
             NamedDomainObjectContainer<ElasticsearchCluster> testClusters = (NamedDomainObjectContainer<ElasticsearchCluster>) project
@@ -36,8 +49,8 @@ public class RestTestBasePlugin implements Plugin<Project> {
             restIntegTestTask.useCluster(cluster);
             restIntegTestTask.include("**/*IT.class");
             restIntegTestTask.systemProperty("tests.rest.load_packaged", Boolean.FALSE.toString());
-            if (System.getProperty(TESTS_REST_CLUSTER) == null) {
-                if (System.getProperty(TESTS_CLUSTER) != null || System.getProperty(TESTS_CLUSTER_NAME) != null) {
+            if (systemProperty(TESTS_REST_CLUSTER) == null) {
+                if (systemProperty(TESTS_CLUSTER) != null || systemProperty(TESTS_CLUSTER_NAME) != null) {
                     throw new IllegalArgumentException(
                         String.format("%s, %s, and %s must all be null or non-null", TESTS_REST_CLUSTER, TESTS_CLUSTER, TESTS_CLUSTER_NAME)
                     );
@@ -48,15 +61,22 @@ public class RestTestBasePlugin implements Plugin<Project> {
                 runnerNonInputProperties.systemProperty(TESTS_CLUSTER, () -> String.join(",", cluster.getAllTransportPortURI()));
                 runnerNonInputProperties.systemProperty(TESTS_CLUSTER_NAME, cluster::getName);
             } else {
-                if (System.getProperty(TESTS_CLUSTER) == null || System.getProperty(TESTS_CLUSTER_NAME) == null) {
+                if (systemProperty(TESTS_CLUSTER) == null || systemProperty(TESTS_CLUSTER_NAME) == null) {
                     throw new IllegalArgumentException(
                         String.format("%s, %s, and %s must all be null or non-null", TESTS_REST_CLUSTER, TESTS_CLUSTER, TESTS_CLUSTER_NAME)
                     );
                 }
             }
         });
+
+        project.getTasks().named(JavaBasePlugin.CHECK_TASK_NAME).configure(check -> check.dependsOn(project.getTasks().withType(RestIntegTestTask.class)));
         project.getTasks()
             .withType(StandaloneRestIntegTestTask.class)
             .configureEach(t -> t.finalizedBy(project.getTasks().withType(FixtureStop.class)));
     }
+
+    @Nullable
+    private String systemProperty(String propName) {
+        return providerFactory.systemProperty(propName).forUseAtConfigurationTime().getOrNull();
+    }
 }

+ 2 - 11
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/StandaloneRestTestPlugin.java

@@ -8,20 +8,14 @@
 
 package org.elasticsearch.gradle.internal.test;
 
-import org.elasticsearch.gradle.internal.ElasticsearchJavaBasePlugin;
-import org.elasticsearch.gradle.internal.ElasticsearchJavaPlugin;
 import org.elasticsearch.gradle.internal.ExportElasticsearchBuildResourcesTask;
-import org.elasticsearch.gradle.internal.RepositoriesSetupPlugin;
-import org.elasticsearch.gradle.internal.info.BuildParams;
 import org.elasticsearch.gradle.internal.info.GlobalBuildInfoPlugin;
 import org.elasticsearch.gradle.internal.precommit.InternalPrecommitTasks;
-import org.elasticsearch.gradle.testclusters.TestClustersPlugin;
+import org.elasticsearch.gradle.internal.test.rest.RestTestUtil;
 import org.gradle.api.InvalidUserDataException;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
-import org.gradle.api.plugins.JavaBasePlugin;
 import org.gradle.api.plugins.JavaPlugin;
-import org.gradle.api.plugins.JavaPluginExtension;
 import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.SourceSetContainer;
 import org.gradle.api.tasks.testing.Test;
@@ -47,9 +41,6 @@ public class StandaloneRestTestPlugin implements Plugin<Project> {
         }
 
         project.getRootProject().getPluginManager().apply(GlobalBuildInfoPlugin.class);
-        project.getPluginManager().apply(ElasticsearchJavaBasePlugin.class);
-        project.getPluginManager().apply(TestClustersPlugin.class);
-        project.getPluginManager().apply(RepositoriesSetupPlugin.class);
         project.getPluginManager().apply(RestTestBasePlugin.class);
 
         project.getTasks().register("buildResources", ExportElasticsearchBuildResourcesTask.class);
@@ -65,7 +56,7 @@ public class StandaloneRestTestPlugin implements Plugin<Project> {
 
         // create a compileOnly configuration as others might expect it
         project.getConfigurations().create("compileOnly");
-        project.getDependencies().add("testImplementation", project.project(":test:framework"));
+        RestTestUtil.setupTestDependenciesDefaults(project, testSourceSet);
 
         EclipseModel eclipse = project.getExtensions().getByType(EclipseModel.class);
         eclipse.getClasspath().setSourceSets(Arrays.asList(testSourceSet));

+ 4 - 18
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/JavaRestTestPlugin.java

@@ -8,21 +8,15 @@
 
 package org.elasticsearch.gradle.internal.test.rest;
 
-import org.elasticsearch.gradle.internal.ElasticsearchJavaBasePlugin;
-import org.elasticsearch.gradle.internal.InternalTestClustersPlugin;
-import org.elasticsearch.gradle.internal.test.RestIntegTestTask;
 import org.elasticsearch.gradle.internal.test.RestTestBasePlugin;
 import org.elasticsearch.gradle.util.GradleUtils;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
-import org.gradle.api.plugins.JavaBasePlugin;
-import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.SourceSetContainer;
 
-import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.createTestCluster;
-import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.registerTask;
-import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.setupDependencies;
+import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.registerTestTask;
+import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.setupTestDependenciesDefaults;
 
 /**
  * Apply this plugin to run the Java based REST tests.
@@ -33,27 +27,19 @@ public class JavaRestTestPlugin implements Plugin<Project> {
 
     @Override
     public void apply(Project project) {
-        project.getPluginManager().apply(ElasticsearchJavaBasePlugin.class);
         project.getPluginManager().apply(RestTestBasePlugin.class);
-        project.getPluginManager().apply(InternalTestClustersPlugin.class);
 
         // create source set
         SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class);
         SourceSet javaTestSourceSet = sourceSets.create(SOURCE_SET_NAME);
 
-        // create the test cluster container
-        createTestCluster(project, javaTestSourceSet);
-
         // setup the javaRestTest task
-        Provider<RestIntegTestTask> javaRestTestTask = registerTask(project, javaTestSourceSet);
+        registerTestTask(project, javaTestSourceSet);
 
         // setup dependencies
-        setupDependencies(project, javaTestSourceSet);
+        setupTestDependenciesDefaults(project, javaTestSourceSet);
 
         // setup IDE
         GradleUtils.setupIdeForTestSourceSet(project, javaTestSourceSet);
-
-        // wire this task into check
-        project.getTasks().named(JavaBasePlugin.CHECK_TASK_NAME).configure(check -> check.dependsOn(javaRestTestTask));
     }
 }

+ 3 - 20
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/RestTestUtil.java

@@ -8,13 +8,8 @@
 
 package org.elasticsearch.gradle.internal.test.rest;
 
-import org.elasticsearch.gradle.VersionProperties;
-import org.elasticsearch.gradle.internal.info.BuildParams;
 import org.elasticsearch.gradle.internal.test.RestIntegTestTask;
-import org.elasticsearch.gradle.testclusters.ElasticsearchCluster;
-import org.elasticsearch.gradle.testclusters.TestClustersPlugin;
 import org.elasticsearch.gradle.util.GradleUtils;
-import org.gradle.api.NamedDomainObjectContainer;
 import org.gradle.api.Project;
 import org.gradle.api.plugins.JavaBasePlugin;
 import org.gradle.api.plugins.JavaPlugin;
@@ -23,7 +18,6 @@ import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.TaskProvider;
 import org.gradle.api.tasks.bundling.AbstractArchiveTask;
 import org.gradle.api.tasks.bundling.Zip;
-import org.gradle.api.tasks.testing.Test;
 
 /**
  * Utility class to configure the necessary tasks and dependencies.
@@ -33,21 +27,12 @@ public class RestTestUtil {
     private RestTestUtil() {
     }
 
-    public static ElasticsearchCluster createTestCluster(Project project, SourceSet sourceSet) {
-        // eagerly create the testCluster container so it is easily available for configuration
-        @SuppressWarnings("unchecked")
-        NamedDomainObjectContainer<ElasticsearchCluster> testClusters = (NamedDomainObjectContainer<ElasticsearchCluster>) project
-                .getExtensions()
-                .getByName(TestClustersPlugin.EXTENSION_NAME);
-        return testClusters.create(sourceSet.getName());
-    }
-
     /**
      * Creates a task with the source set name of type {@link RestIntegTestTask}
      */
-    public static Provider<RestIntegTestTask> registerTask(Project project, SourceSet sourceSet) {
+    public static Provider<RestIntegTestTask> registerTestTask(Project project, SourceSet sourceSet) {
         // lazily create the test task
-        Provider<RestIntegTestTask> testProvider = project.getTasks().register(sourceSet.getName(), RestIntegTestTask.class, testTask -> {
+        return project.getTasks().register(sourceSet.getName(), RestIntegTestTask.class, testTask -> {
             testTask.setGroup(JavaBasePlugin.VERIFICATION_GROUP);
             testTask.setDescription("Runs the REST tests against an external cluster");
             project.getPlugins().withType(JavaPlugin.class, t ->
@@ -67,14 +52,12 @@ public class RestTestUtil {
                 }
             });
         });
-
-        return testProvider;
     }
 
     /**
      * Setup the dependencies needed for the REST tests.
      */
-    public static void setupDependencies(Project project, SourceSet sourceSet) {
+    public static void setupTestDependenciesDefaults(Project project, SourceSet sourceSet) {
         project.getDependencies().add(sourceSet.getImplementationConfigurationName(), project.project(":test:framework"));
     }
 

+ 5 - 21
build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/YamlRestTestPlugin.java

@@ -8,22 +8,16 @@
 
 package org.elasticsearch.gradle.internal.test.rest;
 
-import org.elasticsearch.gradle.internal.ElasticsearchJavaBasePlugin;
 import org.elasticsearch.gradle.internal.ElasticsearchJavaPlugin;
-import org.elasticsearch.gradle.internal.test.RestIntegTestTask;
 import org.elasticsearch.gradle.internal.test.RestTestBasePlugin;
-import org.elasticsearch.gradle.testclusters.TestClustersPlugin;
 import org.elasticsearch.gradle.util.GradleUtils;
 import org.gradle.api.Plugin;
 import org.gradle.api.Project;
-import org.gradle.api.plugins.JavaBasePlugin;
-import org.gradle.api.provider.Provider;
 import org.gradle.api.tasks.SourceSet;
 import org.gradle.api.tasks.SourceSetContainer;
 
-import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.createTestCluster;
-import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.registerTask;
-import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.setupDependencies;
+import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.registerTestTask;
+import static org.elasticsearch.gradle.internal.test.rest.RestTestUtil.setupTestDependenciesDefaults;
 
 /**
  * Apply this plugin to run the YAML based REST tests.
@@ -34,25 +28,19 @@ public class YamlRestTestPlugin implements Plugin<Project> {
 
     @Override
     public void apply(Project project) {
-
-        project.getPluginManager().apply(ElasticsearchJavaBasePlugin.class);
-        project.getPluginManager().apply(TestClustersPlugin.class);
         project.getPluginManager().apply(RestTestBasePlugin.class);
         project.getPluginManager().apply(RestResourcesPlugin.class);
 
         ElasticsearchJavaPlugin.configureConfigurations(project);
+
         // create source set
         SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class);
         SourceSet yamlTestSourceSet = sourceSets.create(SOURCE_SET_NAME);
 
-        // create the test cluster container
-        createTestCluster(project, yamlTestSourceSet);
-
-        // setup the yamlRestTest task
-        Provider<RestIntegTestTask> yamlRestTestTask = registerTask(project, yamlTestSourceSet);
+        registerTestTask(project, yamlTestSourceSet);
 
         // setup the dependencies
-        setupDependencies(project, yamlTestSourceSet);
+        setupTestDependenciesDefaults(project, yamlTestSourceSet);
 
         // setup the copy for the rest resources
         project.getTasks().withType(CopyRestApiTask.class).configureEach(copyRestApiTask -> {
@@ -83,10 +71,6 @@ public class YamlRestTestPlugin implements Plugin<Project> {
                     .flatMap(CopyRestTestsTask::getOutputResourceDir)
             );
 
-        // setup IDE
         GradleUtils.setupIdeForTestSourceSet(project, yamlTestSourceSet);
-
-        // wire this task into check
-        project.getTasks().named(JavaBasePlugin.CHECK_TASK_NAME).configure(check -> check.dependsOn(yamlRestTestTask));
     }
 }

+ 1 - 1
x-pack/plugin/security/qa/security-trial/build.gradle

@@ -5,7 +5,7 @@ dependencies {
   javaRestTestImplementation(testArtifact(project(xpackModule('security'))))
 }
 
-testClusters.javaRestTest {
+testClusters.matching { it.name == 'javaRestTest' }.configureEach {
   testDistribution = 'DEFAULT'
   numberOfNodes = 2
 

+ 1 - 1
x-pack/plugin/security/qa/service-account/build.gradle

@@ -6,7 +6,7 @@ dependencies {
   javaRestTestImplementation project(':x-pack:plugin:security')
 }
 
-testClusters.javaRestTest {
+testClusters.matching { it.name == 'javaRestTest' }.configureEach {
   testDistribution = 'DEFAULT'
   numberOfNodes = 2
 

+ 1 - 2
x-pack/plugin/security/qa/smoke-test-all-realms/build.gradle

@@ -13,7 +13,7 @@ dependencies {
   javaRestTestImplementation(testArtifact(project(xpackModule('core'))))
 }
 
-testClusters.javaRestTest {
+testClusters.matching { it.name == 'javaRestTest' }.configureEach {
   testDistribution = 'DEFAULT'
   numberOfNodes = 2
 
@@ -81,4 +81,3 @@ testClusters.javaRestTest {
   user username: "admin_user", password: "admin-password"
   user username: "security_test_user", password: "security-test-password", role: "security_test_role"
 }
-

+ 1 - 1
x-pack/plugin/security/qa/tls-basic/build.gradle

@@ -12,7 +12,7 @@ if (BuildParams.inFipsJvm){
   tasks.named("javaRestTest").configure{enabled = false }
 }
 
-testClusters.javaRestTest {
+testClusters.matching { it.name == 'javaRestTest' }.configureEach {
   testDistribution = 'DEFAULT'
   numberOfNodes = 2
 

+ 1 - 1
x-pack/plugin/transform/qa/multi-node-tests/build.gradle

@@ -22,7 +22,7 @@ sourceSets.javaRestTest.resources.srcDir(keystoreDir)
 tasks.named("processJavaRestTestResources").configure { dependsOn("copyKeyCerts") }
 tasks.named("javaRestTest").configure { dependsOn "copyKeyCerts" }
 
-testClusters.javaRestTest {
+testClusters.matching { it.name == 'javaRestTest' }.configureEach {
   testDistribution = 'DEFAULT'
   setting 'xpack.security.enabled', 'true'
   setting 'xpack.license.self_generated.type', 'trial'