Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert modules to use yamlRestTest #59089

Merged
merged 11 commits into from
Jul 13, 2020

Conversation

jakelandis
Copy link
Contributor

@jakelandis jakelandis commented Jul 6, 2020

This commit moves the modules REST tests to the
newly introduced yamlRestTest source set. A few
tests have also been re-named to include the correct
IT suffix. Without changing the names, the testing
conventions task would fail since now that the YAML
tests are no longer present pacify the convention.
These tests have moved to the internalClusterTest
source set.

related: #56841

@elasticmachine
Copy link
Collaborator

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

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jul 6, 2020
Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of comments.

@@ -17,6 +17,7 @@
* under the License.
*/
apply plugin: 'elasticsearch.rest-resources'
apply plugin: 'elasticsearch.yaml-rest-test'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't yaml-rest-test apply the rest-resources plugin?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, wasn't sure if we wanted to keep it explicit since most projects are configured in some way or if we should just let it be implicit via the yaml-rest-test.

I don't have a strong opinion either way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to be explicit. We are saying this is a "yaml test project", we should assume that plugin will apply any dependencies.

@@ -28,3 +29,5 @@ restResources {
includeCore '_common', 'indices', 'cluster', 'index', 'search', 'nodes'
}
}

integTest.enabled = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's creating this task? Can we structure things somehow such that projects only with yaml tests simply don't have this task at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the PluginBuildPlugin :( I can't really remove it or conditionally apply it until we get javaYamlTest (and all internalClusterTest are correctly moved too).

I should be able to get to that after all of the yamlRestTests are converted.

artifacts {
restTests(new File(projectDir, "src/test/resources/rest-api-spec/test"))
restTests(new File(projectDir, "src/yamlRestTest/resources/rest-api-spec/test"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's use file() instead of new File().

Copy link
Contributor

@mark-vieira mark-vieira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes LGTM 👍

@jakelandis
Copy link
Contributor Author

@elasticmachine update branch

@jakelandis
Copy link
Contributor Author

@elasticmachine update branch

@Mpdreamz Mpdreamz added v7.8.2 and removed v7.8.1 labels Jul 13, 2020
@jakelandis jakelandis merged commit ddd882b into elastic:master Jul 13, 2020
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Jul 13, 2020
This commit moves the modules REST tests to the
newly introduced yamlRestTest source set. A few
tests have also been re-named to include the correct
IT suffix. Without changing the names, the testing
conventions task would fail since now that the YAML
tests are no longer present pacify the convention.
These tests have moved to the internalClusterTest
source set.

related: elastic#56841
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Jul 13, 2020
This commit moves the modules REST tests to the
newly introduced yamlRestTest source set. A few
tests have also been re-named to include the correct
IT suffix. Without changing the names, the testing
conventions task would fail since now that the YAML
tests are no longer present pacify the convention.
These tests have moved to the internalClusterTest
source set.

related: elastic#56841
jakelandis added a commit that referenced this pull request Jul 13, 2020
This commit moves the modules REST tests to the
newly introduced yamlRestTest source set. A few
tests have also been re-named to include the correct
IT suffix. Without changing the names, the testing
conventions task would fail since now that the YAML
tests are no longer present pacify the convention.
These tests have moved to the internalClusterTest
source set.

related: #56841
jakelandis added a commit that referenced this pull request Jul 13, 2020
This commit moves the modules REST tests to the
newly introduced yamlRestTest source set. A few
tests have also been re-named to include the correct
IT suffix. Without changing the names, the testing
conventions task would fail since now that the YAML
tests are no longer present pacify the convention.
These tests have moved to the internalClusterTest
source set.

related: #56841
jakelandis added a commit that referenced this pull request Sep 8, 2020
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 a commit to jakelandis/elasticsearch that referenced this pull request Sep 8, 2020
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
jakelandis added a commit that referenced this pull request Sep 9, 2020
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
@mark-vieira 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Delivery/Build Build or test infrastructure >refactoring Team:Delivery Meta label for Delivery team v7.8.2 v7.9.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants