Create plugin for internalClusterTest task #55896

rjernst · 2020-04-28T21:46:27Z

This commit creates a new gradle plugin to provide a separate task name
and source set for running ESIntegTestCase tests. The only project
converted to use the new plugin in this PR is server, as an example. The
remaining cases in x-pack will be handled in followups.

This commit creates a new gradle plugin to provide a separate task name and source set for running ESIntegTestCase tests. The only project converted to use the new plugin in this PR is server, as an example. The remaining cases in x-pack will be handled in followups.

elasticmachine · 2020-04-28T21:46:29Z

Pinging @elastic/es-core-infra (:Core/Infra/Build)

jakelandis · 2020-04-28T23:33:00Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

+     * IDEs are also configured if setup, and the test task is added to check. The new test source
+     * set extends from the normal test source set to allow sharing of utilities.
+     */
+    public static void addTestSourceSet(Project project, String sourceSetName) {


Can you break up this up into sourceSet creation and task creation methods ?

I am converting the yamlTests to independent sourceSets and task, and would like to re-use the sourceSet parts here. However, I don't think the task part is re-usabe. Below is my requirement for task creation:

RestIntegTestTask yamlTestTask = project.getTasks() .create("yamlTest", RestIntegTestTask.class, task -> { task.dependsOn(project.getTasks().getByName("copyRestApiSpecsTask")); });

requires a custom Test.class, can't use task configuration avoidance (register vs. create) due to the constructor of RestIntegTestTask, needs a dependency.

I have a WIP for the YAML tests here: https://github.com/elastic/elasticsearch/pull/55833/files#diff-c8d3f49f55d848e6358cf9ebf14b7a79R48

I know that we use RestIntegTestTask, that exists for a mixture of reasons that I believe are all no longer relevant (eg task ordering which is now solved by testclusters, or common setup which is solved by your plugin instead of inside task creation). Regarding needing a dependency, that can be done by your plugin after the task is created.

We could have this method take a Function<? super SourceSet, ? extends TaskProvider<?>> factory argument such that the caller defines how the "test task" should be created, and the utility would handle wiring it to check, etc.

jakelandis · 2020-04-28T23:44:10Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

+        SourceSet mainSourceSet = sourceSets.getByName("main");
+        SourceSet testSourceSet = sourceSets.getByName("test");
+
+        extendConfiguration(project, testSourceSet, extraTestSourceSet, SourceSet::getCompileConfigurationName);


If I understand this (which I may not :) ) this applies the test configuration to the new source set ? (e.g. if testCompile depends on foo, then so does myNewTest sourceSet ?)

Looking at this for the requirements for YAML tests, I wonder if this should be optional since we want the YAML tests to have no dependencies beyond the test framework.

We could have this utility method accept a list of source sets from which the new source set should extend.

I can work on that, although I'd like to do it in a followup. At first I tried having the tests only extend main. However, we have several IT tests in server which use static utility methods from unit test classes. It will take some work to sort that all out.

jakelandis · 2020-04-28T23:49:42Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

+        TaskProvider<Test> testTask = project.getTasks().register(sourceSetName, Test.class);
+        testTask.configure(task -> {
+            task.setTestClassesDirs(extraTestSourceSet.getOutput().getClassesDirs());
+            task.setClasspath(extraTestSourceSet.getRuntimeClasspath());


should this task.setGroup(JavaBasePlugin.VERIFICATION_GROUP); ? (It helps with the Intellij Gradle view)

good call, done

jakelandis · 2020-04-28T23:56:13Z

server/build.gradle

@@ -134,11 +135,15 @@ dependencies {
    // tests use the locally compiled version of server
    exclude group: 'org.elasticsearch', module: 'server'
  }
+  internalClusterTestCompile(project(":test:framework")) {


should the help method setup the TestCompile dependency on :test:framework ? (admittedly, I don't know how to handle the exclude here...but i think all tests will need that dependency)

Possibly, but I'd like to consolidate that after we have converted all the xpack projects to using this plugin. For now, I want to stay simple as possible and only make it more complicated as we see universal commonalities.

mark-vieira

LGTM, do we need to update any documentation so folks don't run :server:integTest anymore?

mark-vieira · 2020-04-29T19:18:29Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

+            task.setTestClassesDirs(extraTestSourceSet.getOutput().getClassesDirs());
+            task.setClasspath(extraTestSourceSet.getRuntimeClasspath());
+        });
+        SourceSet mainSourceSet = sourceSets.getByName("main");


Let's use the SourceSet constants here.

mark-vieira · 2020-04-29T19:19:51Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

+        SourceSet mainSourceSet = sourceSets.getByName("main");
+        SourceSet testSourceSet = sourceSets.getByName("test");
+
+        extendConfiguration(project, testSourceSet, extraTestSourceSet, SourceSet::getCompileConfigurationName);


We could have this utility method accept a list of source sets from which the new source set should extend.

mark-vieira · 2020-04-29T19:23:21Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

+        Configuration extraTestRuntimeConfig = project.getConfigurations()
+            .getByName(extraTestSourceSet.getRuntimeClasspathConfigurationName());
+        extraTestSourceSet.setCompileClasspath(
+            project.getObjects().fileCollection().from(mainSourceSet.getOutput(), testSourceSet.getOutput(), extraTestCompileConfig)


This could also just be mainSourceSet.getOutput().plus(testSourceSet.getOutput()).plus(extraTestCompileConfig)

mark-vieira · 2020-04-29T19:29:13Z

buildSrc/src/main/java/org/elasticsearch/gradle/util/GradleUtils.java

+     * IDEs are also configured if setup, and the test task is added to check. The new test source
+     * set extends from the normal test source set to allow sharing of utilities.
+     */
+    public static void addTestSourceSet(Project project, String sourceSetName) {


We could have this method take a Function<? super SourceSet, ? extends TaskProvider<?>> factory argument such that the caller defines how the "test task" should be created, and the utility would handle wiring it to check, etc.

rjernst · 2020-04-29T20:10:59Z

@mark-vieira @jakelandis I've pushed afdeb9b which I believe addresses most of your asks. The utility method now only extends main, and the internal cluster test plugin adds test separately. Additionally, the utility method returns the task provider, so additional configuration can be done on the task.

mark-vieira · 2020-04-29T20:13:22Z

Additionally, the utility method returns the task provider, so additional configuration can be done on the task.

Unfortunately that still won't be sufficient for the yaml test use case as it's still a Test task instead of a RestTestRunnerTask. I think we still need to provide a mechanism for the caller to create this task.

mark-vieira · 2020-04-29T20:13:53Z

FWIW, I'm good with @jakelandis doing that refactoring as part of his work. That way we know the solution certainly solves his use case.

jakelandis

LGTM. If I have additional needs, i can introduce them. Thanks for changes!

We made a small mistake when breaking out the `ESIntegTestCase` subclasses that confused eclipse. This makes it happy again. Poor eclipse! Relates elastic#55896

We made a small mistake when breaking out the `ESIntegTestCase` subclasses that confused eclipse. This makes it happy again. Poor eclipse! Relates #55896

This commit creates a new Gradle plugin to provide a separate task name and source set for running YAML based REST tests. The only project converted to use the new plugin in this PR is distribution/archives/integ-test-zip. For which the testing has been moved to :rest-api-spec since it makes the most sense and it avoids a small but awkward change to the distribution plugin. The remaining cases in modules, plugins, and x-pack will be handled in followups. This plugin is distinctly different from the plugin introduced in #55896 since the YAML REST tests are intended to be black box tests over HTTP. As such they should not (by default) have access to the classpath for that which they are testing. The YAML based REST tests will be moved to separate source sets (yamlRestTest). The which source is the target for the test resources is dependent on if this new plugin is applied. If it is not applied, it will default to the test source set. Further, this introduces a breaking change for plugin developers that use the YAML testing framework. They will now need to either use the new source set and matching task, or configure the rest resources to use the old "test" source set that matches the old integTest task. (The former should be preferred). As part of this change (which is also breaking for plugin developers) the rest resources plugin has been removed from the build plugin and now requires either explicit application or application via the new YAML REST test plugin. Plugin developers should be able to fix the breaking changes to the YAML tests by adding apply plugin: 'elasticsearch.yaml-rest-test' and moving the YAML tests under a yamlRestTest folder (instead of test)

This commit creates a new Gradle plugin to provide a separate task name and source set for running YAML based REST tests. The only project converted to use the new plugin in this PR is distribution/archives/integ-test-zip. For which the testing has been moved to :rest-api-spec since it makes the most sense and it avoids a small but awkward change to the distribution plugin. The remaining cases in modules, plugins, and x-pack will be handled in followups. This plugin is distinctly different from the plugin introduced in elastic#55896 since the YAML REST tests are intended to be black box tests over HTTP. As such they should not (by default) have access to the classpath for that which they are testing. The YAML based REST tests will be moved to separate source sets (yamlRestTest). The which source is the target for the test resources is dependent on if this new plugin is applied. If it is not applied, it will default to the test source set. Further, this introduces a breaking change for plugin developers that use the YAML testing framework. They will now need to either use the new source set and matching task, or configure the rest resources to use the old "test" source set that matches the old integTest task. (The former should be preferred). As part of this change (which is also breaking for plugin developers) the rest resources plugin has been removed from the build plugin and now requires either explicit application or application via the new YAML REST test plugin. Plugin developers should be able to fix the breaking changes to the YAML tests by adding apply plugin: 'elasticsearch.yaml-rest-test' and moving the YAML tests under a yamlRestTest folder (instead of test)

This commit creates a new Gradle plugin to provide a separate task name and source set for running YAML based REST tests. The only project converted to use the new plugin in this PR is distribution/archives/integ-test-zip. For which the testing has been moved to :rest-api-spec since it makes the most sense and it avoids a small but awkward change to the distribution plugin. The remaining cases in modules, plugins, and x-pack will be handled in followups. This plugin is distinctly different from the plugin introduced in #55896 since the YAML REST tests are intended to be black box tests over HTTP. As such they should not (by default) have access to the classpath for that which they are testing. The YAML based REST tests will be moved to separate source sets (yamlRestTest). The which source is the target for the test resources is dependent on if this new plugin is applied. If it is not applied, it will default to the test source set. Further, this introduces a breaking change for plugin developers that use the YAML testing framework. They will now need to either use the new source set and matching task, or configure the rest resources to use the old "test" source set that matches the old integTest task. (The former should be preferred). As part of this change (which is also breaking for plugin developers) the rest resources plugin has been removed from the build plugin and now requires either explicit application or application via the new YAML REST test plugin. Plugin developers should be able to fix the breaking changes to the YAML tests by adding apply plugin: 'elasticsearch.yaml-rest-test' and moving the YAML tests under a yamlRestTest folder (instead of test)

…est or internalClusterTest (#60630) For 1/2 the plugins in x-pack, the integTest task is now a no-op and all of the tests are now executed via a test, yamlRestTest, javaRestTest, or internalClusterTest. This includes the following projects: async-search, autoscaling, ccr, enrich, eql, frozen-indicies, data-streams, graph, ilm, mapper-constant-keyword, mapper-flattened, ml A few of the more specialized qa projects within these plugins have not been changed with this PR due to additional complexity which should be addressed separately. A follow up PR will address the remaining x-pack plugins (this PR is big enough as-is). related: #61802 related: #56841 related: #59939 related: #55896

…Test or internalClusterTest (#61802) For 1/2 the plugins in x-pack, the integTest task is now a no-op and all of the tests are now executed via a test, yamlRestTest, javaRestTest, or internalClusterTest. This includes the following projects: security, spatial, stack, transform, vecotrs, voting-only-node, and watcher. A few of the more specialized qa projects within these plugins have not been changed with this PR due to additional complexity which should be addressed separately. related: #60630 related: #56841 related: #59939 related: #55896

…est or internalClusterTest (elastic#60630) For 1/2 the plugins in x-pack, the integTest task is now a no-op and all of the tests are now executed via a test, yamlRestTest, javaRestTest, or internalClusterTest. This includes the following projects: async-search, autoscaling, ccr, enrich, eql, frozen-indicies, data-streams, graph, ilm, mapper-constant-keyword, mapper-flattened, ml A few of the more specialized qa projects within these plugins have not been changed with this PR due to additional complexity which should be addressed separately. A follow up PR will address the remaining x-pack plugins (this PR is big enough as-is). related: elastic#61802 related: elastic#56841 related: elastic#59939 related: elastic#55896 # Conflicts: # x-pack/plugin/identity-provider/src/internalClusterTest/java/org/elasticsearch/xpack/idp/saml/sp/SamlServiceProviderIndexTests.java # x-pack/plugin/ilm/qa/with-security/build.gradle

…Test or internalClusterTest (elastic#61802) For 1/2 the plugins in x-pack, the integTest task is now a no-op and all of the tests are now executed via a test, yamlRestTest, javaRestTest, or internalClusterTest. This includes the following projects: security, spatial, stack, transform, vecotrs, voting-only-node, and watcher. A few of the more specialized qa projects within these plugins have not been changed with this PR due to additional complexity which should be addressed separately. related: elastic#60630 related: elastic#56841 related: elastic#59939 related: elastic#55896 # Conflicts: # x-pack/plugin/security/src/internalClusterTest/java/org/elasticsearch/integration/IndexPrivilegeIntegTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/integration/IndexPrivilegeIntegTests.java # x-pack/plugin/security/src/test/java/org/elasticsearch/integration/IndexPrivilegeTests.java # x-pack/plugin/transform/qa/multi-node-tests/build.gradle # x-pack/plugin/transform/qa/single-node-tests/build.gradle # x-pack/plugin/watcher/build.gradle

…]RestTest or internalClusterTest (#60630) (#61855) For 1/2 the plugins in x-pack, the integTest task is now a no-op and all of the tests are now executed via a test, yamlRestTest, javaRestTest, or internalClusterTest. This includes the following projects: async-search, autoscaling, ccr, enrich, eql, frozen-indicies, data-streams, graph, ilm, mapper-constant-keyword, mapper-flattened, ml A few of the more specialized qa projects within these plugins have not been changed with this PR due to additional complexity which should be addressed separately. A follow up PR will address the remaining x-pack plugins (this PR is big enough as-is). related: #61802 related: #56841 related: #59939 related: #55896

…a]RestTest or internalClusterTest (#61802) (#61856) For 1/2 the plugins in x-pack, the integTest task is now a no-op and all of the tests are now executed via a test, yamlRestTest, javaRestTest, or internalClusterTest. This includes the following projects: security, spatial, stack, transform, vecotrs, voting-only-node, and watcher. A few of the more specialized qa projects within these plugins have not been changed with this PR due to additional complexity which should be addressed separately. related: #60630 related: #56841 related: #59939 related: #55896

This commit removes `integTest` task from all es-plugins. Most relevant projects have been converted to use yamlRestTest, javaRestTest, or internalClusterTest in prior PRs. A few projects needed to be adjusted to allow complete removal of this task * x-pack/plugin - converted to use yamlRestTest and javaRestTest * plugins/repository-hdfs - kept the integTest task, but use `rest-test` plugin to define the task * qa/die-with-dignity - convert to javaRestTest * x-pack/qa/security-example-spi-extension - convert to javaRestTest * multiple projects - remove the integTest.enabled = false (yay!) related: #61802 related: #60630 related: #59444 related: #59089 related: #56841 related: #59939 related: #55896

This commit removes `integTest` task from all es-plugins. Most relevant projects have been converted to use yamlRestTest, javaRestTest, or internalClusterTest in prior PRs. A few projects needed to be adjusted to allow complete removal of this task * x-pack/plugin - converted to use yamlRestTest and javaRestTest * plugins/repository-hdfs - kept the integTest task, but use `rest-test` plugin to define the task * qa/die-with-dignity - convert to javaRestTest * x-pack/qa/security-example-spi-extension - convert to javaRestTest * multiple projects - remove the integTest.enabled = false (yay!) related: elastic#61802 related: elastic#60630 related: elastic#59444 related: elastic#59089 related: elastic#56841 related: elastic#59939 related: elastic#55896 # Conflicts: # qa/die-with-dignity/src/javaRestTest/java/org/elasticsearch/qa/die_with_dignity/DieWithDignityIT.java # x-pack/qa/security-example-spi-extension/build.gradle

This commit removes `integTest` task from all es-plugins. Most relevant projects have been converted to use yamlRestTest, javaRestTest, or internalClusterTest in prior PRs. A few projects needed to be adjusted to allow complete removal of this task * x-pack/plugin - converted to use yamlRestTest and javaRestTest * plugins/repository-hdfs - kept the integTest task, but use `rest-test` plugin to define the task * qa/die-with-dignity - convert to javaRestTest * x-pack/qa/security-example-spi-extension - convert to javaRestTest * multiple projects - remove the integTest.enabled = false (yay!) related: #61802 related: #60630 related: #59444 related: #59089 related: #56841 related: #59939 related: #55896

rjernst added >test Issues or PRs that are addressing/adding tests :Delivery/Build Build or test infrastructure v8.0.0 v7.8.0 v7.7.1 labels Apr 28, 2020

elasticmachine added the Team:Core/Infra Meta label for core/infra team label Apr 28, 2020

Merge branch 'master' into buildsplit3

6ef6e79

jakelandis reviewed Apr 28, 2020

View reviewed changes

rjernst added 3 commits April 29, 2020 11:39

checkstyle

407062a

Merge branch 'master' into buildsplit3

dd82383

add task to verification group

335a16e

mark-vieira approved these changes Apr 29, 2020

View reviewed changes

rjernst added 2 commits April 29, 2020 13:08

address feedback

afdeb9b

Merge branch 'master' into buildsplit3

6eb2b70

Merge branch 'master' into buildsplit3

2031cd1

jakelandis approved these changes Apr 29, 2020

View reviewed changes

rjernst added 6 commits April 29, 2020 16:23

Merge branch 'master' into buildsplit3

ae36d8e

spotless

0e2fce7

delay forbidden apis resolution of classpaths

7da0617

only use test task for testing conventions

1ec2e3e

Merge branch 'master' into buildsplit3

dff2d36

spotless

f47d8de

nik9000 mentioned this pull request May 7, 2020

Build: fix eclipse after icTests #56362

Merged

nik9000 added a commit that referenced this pull request May 7, 2020

Build: fix eclipse after icTests (#56362)

5bf8ab9

We made a small mistake when breaking out the `ESIntegTestCase` subclasses that confused eclipse. This makes it happy again. Poor eclipse! Relates #55896

nik9000 added a commit that referenced this pull request May 7, 2020

Build: fix eclipse after icTests (#56362)

ca9bec7

We made a small mistake when breaking out the `ESIntegTestCase` subclasses that confused eclipse. This makes it happy again. Poor eclipse! Relates #55896

nik9000 added a commit that referenced this pull request May 7, 2020

Build: fix eclipse after icTests (#56362)

3859396

We made a small mistake when breaking out the `ESIntegTestCase` subclasses that confused eclipse. This makes it happy again. Poor eclipse! Relates #55896

nik9000 added a commit that referenced this pull request May 7, 2020

Build: fix eclipse after icTests (#56362)

691ee6d

We made a small mistake when breaking out the `ESIntegTestCase` subclasses that confused eclipse. This makes it happy again. Poor eclipse! Relates #55896

jakelandis mentioned this pull request May 15, 2020

Create plugin for yamlTest task #56841

Merged

jakelandis mentioned this pull request Jul 6, 2020

7.x - Create plugin for yamlTest task (#56841) #59090

Merged

jakelandis mentioned this pull request Jul 6, 2020

7.8 - Create plugin for yamlTest task (#56841) #59092

Merged

jakelandis mentioned this pull request Aug 5, 2020

Convert first 1/2 x-pack plugins from integTest to [yaml | java]RestTest or internalClusterTest #60630

Merged

jakelandis mentioned this pull request Sep 1, 2020

Convert second 1/2 x-pack plugins from integTest to [yaml | java]RestTest or internalClusterTest #61802

Merged

jakelandis mentioned this pull request Sep 2, 2020

Remove integTest task from PluginBuildPlugin #61879

Merged

mark-vieira added Team:Delivery Meta label for Delivery team and removed Team:Core/Infra Meta label for core/infra team labels Nov 11, 2020

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create plugin for internalClusterTest task #55896

Create plugin for internalClusterTest task #55896

rjernst commented Apr 28, 2020

elasticmachine commented Apr 28, 2020

jakelandis Apr 28, 2020 •

edited

Loading

rjernst Apr 29, 2020

mark-vieira Apr 29, 2020

jakelandis Apr 28, 2020

mark-vieira Apr 29, 2020

rjernst Apr 29, 2020

jakelandis Apr 28, 2020

rjernst Apr 29, 2020

jakelandis Apr 28, 2020 •

edited

Loading

rjernst Apr 29, 2020

mark-vieira left a comment

mark-vieira Apr 29, 2020

mark-vieira Apr 29, 2020

mark-vieira Apr 29, 2020

mark-vieira Apr 29, 2020

rjernst commented Apr 29, 2020

mark-vieira commented Apr 29, 2020

mark-vieira commented Apr 29, 2020

jakelandis left a comment

Create plugin for internalClusterTest task #55896

Create plugin for internalClusterTest task #55896

Conversation

rjernst commented Apr 28, 2020

elasticmachine commented Apr 28, 2020

jakelandis Apr 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakelandis Apr 28, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mark-vieira left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjernst commented Apr 29, 2020

mark-vieira commented Apr 29, 2020

mark-vieira commented Apr 29, 2020

jakelandis left a comment

Choose a reason for hiding this comment

jakelandis Apr 28, 2020 •

edited

Loading

jakelandis Apr 28, 2020 •

edited

Loading