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 first 1/2 x-pack plugins from integTest to [yaml | java]RestTest or internalClusterTest #60630

Merged
merged 28 commits into from
Sep 2, 2020

Conversation

jakelandis
Copy link
Contributor

@jakelandis jakelandis commented Aug 3, 2020

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

@jakelandis jakelandis changed the title Convert most x-pack plugins from integTest to [yaml | java]RestTest or internalClusterTest Convert first 1/2 x-pack plugins from integTest to [yaml | java]RestTest or internalClusterTest Aug 5, 2020
@@ -9,3 +9,17 @@ dependencies {
// TOML parser for EqlActionIT tests
api 'io.ous:jtoml:2.0.0'
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved some test only dependencies out of the main source set and into the test source set and added expose it as a test artifact.

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 the reasoning behind that? If we share code with other projects doing so w/o sharing the entire tests jar is definitely preferable. While "main" is not the best descriport (we should be using the gradle testfixture plugin) it's still better then slamming everything in "test" and then sharing that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can move it back... it just seemed odd to have test only code in the main source set, even for a qa project.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's not really "main" code, but its the most convenient place to put it.

@@ -242,59 +240,6 @@ private void refresh() {
future.actionGet();
}

public static SamlServiceProviderDocument randomDocument() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to utility class (in the the test sourceset) so it can be shared easier.


private SamlServiceProviderTestUtils(){} //utility class

public static SamlServiceProviderDocument randomDocument() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no functional changes here ... just copy pasta to allow re-use across source sets.

@@ -29,6 +28,8 @@

public class MlBasicMultiNodeIT extends ESRestTestCase {

private static final String BASE_PATH = "/_ml/";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Quite a few of the ML REST tests had a dependency on the ML main code for ONLY this base path reference in the tests. Adding to each test like here is a bit redundant, but allows us to break that class path dependency and the REST path for ml is unlikely to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call. And if it does, that's a breaking change anyhow.


dependencies {
testImplementation project(":x-pack:plugin:core")
testImplementation project(path: xpackModule('ml'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here is an example where duplicating the BASE_PATH allows us to remove the dependency.

@jakelandis jakelandis marked this pull request as ready for review August 5, 2020 22:29
@jakelandis jakelandis added the :Delivery/Build Build or test infrastructure label Aug 5, 2020
@jakelandis
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

@jakelandis jakelandis merged commit 51d0bca into elastic:master Sep 2, 2020
@jakelandis jakelandis deleted the yamlRestTest_xpack_plugins branch September 2, 2020 14:22
jakelandis added a commit that referenced this pull request Sep 2, 2020
…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
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Sep 2, 2020
…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
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Sep 2, 2020
…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
jakelandis added a commit that referenced this pull request Sep 2, 2020
…]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
jakelandis added a commit that referenced this pull request Sep 2, 2020
…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
@javanna
Copy link
Member

javanna commented Sep 4, 2020

heya, not sure if you are already aware, but after this change (I think), I get a compile error on IntelliJ on both master and 7.x.

/home/javanna/elastic/elasticsearch/x-pack/plugin/ml/qa/ml-with-security/src/yamlRestTest/java/org/elasticsearch/smoketest/MlWithSecurityIT.java:19:66
java: cannot find symbol
  symbol:   class UsernamePasswordToken
  location: package org.elasticsearch.xpack.core.security.authc.support

Compiling from command line works ok, I tried to reimport the project in IntelliJ but that did not fix it.

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.10.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants