Skip to content

cleanup ILM qa structure #35110

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 31, 2018
Merged

cleanup ILM qa structure #35110

merged 3 commits into from
Oct 31, 2018

Conversation

@talevy
Copy link
Contributor

@talevy talevy commented Oct 30, 2018 •
edited
Loading

This commit does a few things

  • moves ILM-specifc rest yaml tests into plugin/ilm/qa, and creates special
    :plugin:ilm:qa:rest module to test them
  • removes the with-security tests of the yaml tests since they are covered in
    the rest tests now
  • moves ChangePolicyforIndexIT into the qa/multi-node project since that test is
    not currently running in main ilm since integTest is disabled

@talevy
cleanup ILM qa structure
8e82bae 
This commit does a few things

- moves ILM-specifc rest yaml tests into plugin/ilm/qa, and creates special
  :plugin:ilm:qa:rest module to test them
- removes the with-security tests of the yaml tests since they are covered in
  the rest tests now
- moves ChangePolicyforIndexIT into the qa/multi-node project since that test is
  not currently running in main ilm since integTest is disabled
@talevy talevy added >test Issues or PRs that are addressing/adding tests blocker :Data Management/ILM+SLM Index and Snapshot lifecycle management labels Oct 30, 2018
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 30, 2018

Pinging @elastic/es-core-infra

@talevy
Copy link
Contributor Author

talevy commented Oct 30, 2018 •
edited
Loading

As this build structure is borrowed from plugin/ccr, you will notice that there is no
:x-pack:plugin:ilm:qa:check, one must run each qa module separately, or run all with
:x-pack:plugin:ilm:check.

If this is not desired, we can modify (maybe in a followup, or here)

EDIT EDIT EDIT:
qa:check now works as committed

@talevy
Copy link
Contributor Author

talevy commented Oct 31, 2018 •
edited
Loading

nevermind. CI hates that qa's check is broken with no licenseHeaders to verify

:x-pack:plugin:ilm:qa:licenseHeaders FAILED

will dig into this, I may be missing something. running theory is that there are no src files to check, unlike in CCR where there is

alpar-t
alpar-t reviewed Oct 31, 2018
View reviewed changes
Copy link
Contributor

@alpar-t alpar-t left a comment

Choose a reason for hiding this comment

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

Left a minor comment, other than that LGTM

x-pack/plugin/ilm/qa/build.gradle

subprojects {
project.tasks.withType(RestIntegTestTask) {
final File xPackResources = new File(xpackProject('plugin').projectDir, 'src/test/resources')
Copy link
Contributor

@alpar-t alpar-t Oct 31, 2018

Choose a reason for hiding this comment

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

Gradle encodes this information as a SourceSet,
so you can reference this path as xpack('plugin').sourceSets.test.resources

Copy link
Contributor Author

@talevy talevy Oct 31, 2018

Choose a reason for hiding this comment

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

looks like xpack is not accessible from RestIntegTestTask

> Could not create task ':x-pack:plugin:ilm:qa:multi-node:integTest'.
  > Could not find method xpack() for arguments [plugin] on task ':x-pack:plugin:ilm:qa:multi-node:integTest' of type org.elasticsearch.gradle.test.RestIntegTestTask.

colings86
colings86 approved these changes Oct 31, 2018
View reviewed changes
Copy link
Contributor

@colings86 colings86 left a comment

Choose a reason for hiding this comment

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

LGTM on the structure side. Thanks @atorok for taking a look at the gradle side.

@talevy
Copy link
Contributor Author

talevy commented Oct 31, 2018

thank you @atorok and @colings86 for taking another look at this

@talevy talevy merged commit 5f4b23f into elastic:index-lifecycle Oct 31, 2018
@talevy talevy deleted the ilm-cleanup-qa branch October 31, 2018 18:50
talevy added a commit that referenced this pull request Oct 31, 2018
@talevy
cleanup ILM qa structure (#35110)
1e66b8b 
This commit does a few things

- moves ILM-specifc rest yaml tests into plugin/ilm/qa, and creates special
  :plugin:ilm:qa:rest module to test them
- removes the with-security tests of the yaml tests since they are covered in
  the rest tests now
- moves ChangePolicyforIndexIT into the qa/multi-node project since that test is
  not currently running in main ilm since integTest is disabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alpar-t alpar-t alpar-t left review comments

@colings86 colings86 colings86 approved these changes

@dakrone dakrone Awaiting requested review from dakrone

@gwbrown gwbrown Awaiting requested review from gwbrown

Assignees

No one assigned

Labels

blocker :Data Management/ILM+SLM Index and Snapshot lifecycle management >test Issues or PRs that are addressing/adding tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@talevy @elasticmachine @colings86 @alpar-t