Polish and cleanup test related plugins (#74673) (#74896)

- 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
elastic · Jul 5, 2021 · ac2ceb4 · ac2ceb4
1 parent e0b9e98
commit ac2ceb4
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 71 deletions.
diff --git a/...ols-internal/src/main/java/org/elasticsearch/gradle/internal/test/RestTestBasePlugin.java b/...ols-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.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 void apply(Project 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 void apply(Project 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();
+    }
 }
diff --git a/...ternal/src/main/java/org/elasticsearch/gradle/internal/test/StandaloneRestTestPlugin.java b/...ternal/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 void apply(final Project 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 void apply(final Project 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));

diff --git a/...nternal/src/main/java/org/elasticsearch/gradle/internal/test/rest/JavaRestTestPlugin.java b/...nternal/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));
     }
 }
diff --git a/...ools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/RestTestUtil.java b/...ools-internal/src/main/java/org/elasticsearch/gradle/internal/test/rest/RestTestUtil.java
@@ -8,22 +8,19 @@
 
 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.testclusters.ElasticsearchCluster;
 import org.elasticsearch.gradle.util.GradleUtils;
-import org.gradle.api.NamedDomainObjectContainer;
 import org.gradle.api.Project;
+import org.gradle.api.NamedDomainObjectContainer;
 import org.gradle.api.plugins.JavaBasePlugin;
 import org.gradle.api.plugins.JavaPlugin;
 import org.gradle.api.provider.Provider;
 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.
@@ -45,9 +42,9 @@ static ElasticsearchCluster createTestCluster(Project project, SourceSet sourceS
     /**
      * 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 +64,12 @@ public static Provider<RestIntegTestTask> registerTask(Project project, SourceSe
                 }
             });
         });
-
-        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"));
     }
 

diff --git a/...nternal/src/main/java/org/elasticsearch/gradle/internal/test/rest/YamlRestTestPlugin.java b/...nternal/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 void apply(Project project) {
                     .map(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));
     }
 }
diff --git a/x-pack/plugin/core/build.gradle b/x-pack/plugin/core/build.gradle
@@ -146,7 +146,7 @@ restResources {
   }
 }
 
-testClusters.yamlRestTest {
+testClusters.matching{ it.name == 'yamlRestTest' }.configureEach {
   testDistribution = 'default'
   setting 'xpack.security.enabled', 'true'
   setting 'xpack.license.self_generated.type', 'trial'

diff --git a/x-pack/plugin/security/qa/security-trial/build.gradle b/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
 

diff --git a/x-pack/plugin/security/qa/service-account/build.gradle b/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
 

diff --git a/x-pack/plugin/security/qa/smoke-test-all-realms/build.gradle b/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"
 }
-
diff --git a/x-pack/plugin/security/qa/tls-basic/build.gradle b/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
 

diff --git a/x-pack/plugin/transform/qa/multi-node-tests/build.gradle b/x-pack/plugin/transform/qa/multi-node-tests/build.gradle
@@ -23,7 +23,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'