Introduce javaRestTest source set/task and convert modules #59939

jakelandis · 2020-07-20T23:34:43Z

Introduce a javaRestTest source set and task to compliment the yamlRestTest.
javaRestTest differs such that the code is sourced from Java and may have
different dependencies and setup requirements for the test clusters. This also
allows the tests to run in parallel in different cluster instances to prevent any
cross test contamination between the two types of tests.

Included in this PR is all :modules no longer use the integTest task. The tests
are now driven by test, yamlRestTest, javaRestTest, and internalClusterTest.
Since only :modules (and :rest-api-spec) have been converted to yamlRestTest
we can now disable the integTest task if either yamlRestTest or javaRestTest have
been applied. Once all projects are converted, we can delete the integTest task.

related: #56841
related: #59444

jakelandis · 2020-07-21T14:02:15Z

buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/AbstractRestTestPlugin.java

+/**
+ * Helper parent to configure the necessary tasks and dependencies.
+ */
+public abstract class AbstractRestTestPlugin implements Plugin<Project> {


this was pulled from YamlRestTestPlugin so it can be reused with JavaRestTestPlugin. There are no substantive changes introduced here ... just copy/pasted.

All these methods can be static and therefore live in a utility class rather than an abstract class that must be extended.

elasticmachine · 2020-07-21T17:00:37Z

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

rjernst

I have just a couple small comments. I'll leave a more thorough review of the gradle changes to @mark-vieira.

rjernst · 2020-07-21T18:39:37Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

            if (isModule == false || isXPackModule) {
                addNoticeGeneration(project, extension1)
            }
+            if(project.pluginManager.hasPlugin("elasticsearch.yaml-rest-test")


nit: space after if
However, should the integTest task be disabled through onlyIf? There is no guarantee the plugins checked for here will be applied before esplugin.

fixed nit.

There is no guarantee the plugins checked for here will be applied before esplugin.

This is part of an project.afterEvaluate block, so I think it is guaranteed to be applied.

rjernst · 2020-07-21T18:46:25Z

modules/transport-netty4/build.gradle

 }

-test {
+internalClusterTest {


Dont' we still need this system property for test?

indeed. d5e4572 thanks!

mark-vieira

Looks great, Jake. Just a couple of comments and questions.

mark-vieira · 2020-07-21T20:42:22Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

            if (isModule == false || isXPackModule) {
                addNoticeGeneration(project, extension1)
            }
+            if (project.pluginManager.hasPlugin("elasticsearch.yaml-rest-test")


Let's not use afterEvaluate + hasPlugin. This is subject to configuration ordering. Let's instead move this outside of afterEvaluate and just use withPlugin to disable that integTest task whenever either of these plugins is applied.

mark-vieira · 2020-07-21T20:44:25Z

buildSrc/src/main/java/org/elasticsearch/gradle/test/rest/AbstractRestTestPlugin.java

+/**
+ * Helper parent to configure the necessary tasks and dependencies.
+ */
+public abstract class AbstractRestTestPlugin implements Plugin<Project> {


All these methods can be static and therefore live in a utility class rather than an abstract class that must be extended.

mark-vieira · 2020-07-21T20:46:01Z

buildSrc/src/main/groovy/org/elasticsearch/gradle/plugin/PluginBuildPlugin.groovy

+            if (project.pluginManager.hasPlugin("elasticsearch.yaml-rest-test")
+                || project.pluginManager.hasPlugin("elasticsearch.java-rest-test")) {
+                //disable integTest task if project has been converted to use yaml or java rest test plugin
+                project.tasks.integTest.enabled = false


So this doesn't actually remove this task, we simply disable it in a central location to remove noise from build scripts. What's keeping us from actually getting rid of this altogether?

So this doesn't actually remove this task, we simply disable it in a central location to remove noise from build scripts.

correct.

What's keeping us from actually getting rid of this altogether?

This PR only covers modules. I am in the process of converting all of the projects to this pattern. Once that is done, I can (and will) remove integTest from here.

mark-vieira

👍

…9939) Introduce a javaRestTest source set and task to compliment the yamlRestTest. javaRestTest differs such that the code is sourced from Java and may have different dependencies and setup requirements for the test clusters. This also allows the tests to run in parallel in different cluster instances to prevent any cross test contamination between the two types of tests. Included in this PR is all :modules no longer use the integTest task. The tests are now driven by test, yamlRestTest, javaRestTest, and internalClusterTest. Since only :modules (and :rest-api-spec) have been converted to yamlRestTest we can now disable the integTest task if either yamlRestTest or javaRestTest have been applied. Once all projects are converted, we can delete the integTest task. related: elastic#56841 related: elastic#59444

) (#60026) Introduce a javaRestTest source set and task to compliment the yamlRestTest. javaRestTest differs such that the code is sourced from Java and may have different dependencies and setup requirements for the test clusters. This also allows the tests to run in parallel in different cluster instances to prevent any cross test contamination between the two types of tests. Included in this PR is all :modules no longer use the integTest task. The tests are now driven by test, yamlRestTest, javaRestTest, and internalClusterTest. Since only :modules (and :rest-api-spec) have been converted to yamlRestTest we can now disable the integTest task if either yamlRestTest or javaRestTest have been applied. Once all projects are converted, we can delete the integTest task. related: #56841 related: #59444

…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

jakelandis added 3 commits July 20, 2020 17:30

new javaRestTest plugin and 1/2 modules converted

a65914c

should all work

45bf3d8

spotless

319d6fa

jakelandis commented Jul 21, 2020

View reviewed changes

jakelandis changed the title ~~wip - Java rest test~~ Introduce javaRestTest source set/task and convert modules Jul 21, 2020

jakelandis requested review from mark-vieira and rjernst July 21, 2020 14:28

jakelandis marked this pull request as ready for review July 21, 2020 14:28

jakelandis mentioned this pull request Jul 21, 2020

Convert most OSS plugins from integTest to [yaml | java]RestTest or internalClusterTest #59444

Merged

jakelandis added :Delivery/Build Build or test infrastructure >refactoring v7.10.0 v7.9.1 v8.0.0 labels Jul 21, 2020

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

rjernst reviewed Jul 21, 2020

View reviewed changes

minor review changes

d5e4572

mark-vieira reviewed Jul 21, 2020

View reviewed changes

withPlugin and static methods

01521f9

mark-vieira approved these changes Jul 21, 2020

View reviewed changes

jakelandis removed the v7.9.1 label Jul 21, 2020

jakelandis merged commit 7dd57c9 into elastic:master Jul 21, 2020

jakelandis deleted the javaRestTest branch July 21, 2020 22:17

jakelandis added the backport pending label Jul 21, 2020

jakelandis mentioned this pull request Jul 21, 2020

[7.x] Introduce javaRestTest source set/task and convert modules (#59939) #60026

Merged

jakelandis removed the backport pending label Jul 28, 2020

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

Introduce javaRestTest source set/task and convert modules #59939

Introduce javaRestTest source set/task and convert modules #59939

Uh oh!

Conversation

jakelandis commented Jul 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Jul 21, 2020

Uh oh!

rjernst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-vieira left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mark-vieira left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jakelandis commented Jul 20, 2020 •

edited

Loading