Skip to content

Allowing _cat/allocation to return more than 0 shards in test#84539

Merged
masseyke merged 6 commits intoelastic:masterfrom
masseyke:fix/cat-allocation-test-failure
Mar 24, 2022
Merged

Allowing _cat/allocation to return more than 0 shards in test#84539
masseyke merged 6 commits intoelastic:masterfrom
masseyke:fix/cat-allocation-test-failure

Conversation

@masseyke
Copy link
Copy Markdown
Member

@masseyke masseyke commented Mar 1, 2022

We delete all indices in a cluster before running ClientYamlTestSuiteIT. However some system indices can regenerate.
The cat.allocation, cat.indices, and cat.shards tests were failing because they assume that no indices not created by the tests exist. They can now all handle system indices being created during the test.
Closes #83719
Closes #84765
Closes #84972
Closes #82151
Closes #82660

@masseyke masseyke added >test Issues or PRs that are addressing/adding tests :Core/Infra/CAT APIs Text APIs behind /_cat team-discuss auto-backport-and-merge v8.2.0 v8.1.1 v8.0.2 labels Mar 1, 2022
@elasticmachine elasticmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Mar 1, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@masseyke
Copy link
Copy Markdown
Member Author

masseyke commented Mar 1, 2022

I'm adding team-discuss to this one because I'm not completely sure this is the right approach to fixing these kinds of tests (there will probably be more).

$body: |
/^
( 0 \s+
( \d \s+ #usually 0, unless some system index has been recreated before this runs
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's strange to me is that we do this above in the "all nodes" test with a \d+, but then in the "column headers" test we also assert that it's 0, so I think we'd need to change that one also if we went with this fix.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point. I added that. What's going to be trickier (and maybe make us rethink these tests) are failures like #84765. That's doing a call to _cat/indices, and failing because it's getting back an unexpected second row for the index .tasks.

@dnhatn dnhatn added v8.1.2 and removed v8.1.1 labels Mar 17, 2022
@masseyke
Copy link
Copy Markdown
Member Author

@elasticmachine update branch

@masseyke
Copy link
Copy Markdown
Member Author

I think I've updated the tests for _cat/allocation, _cat/indices, and _cat/shards so that they still meet the original spirit of the tests but now won't fail if system indices appear (unless those system indices start with .i, but I don't think that happens and we can easily add a fix if it does start happening).

@masseyke masseyke requested a review from dakrone March 24, 2022 18:20
Copy link
Copy Markdown
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for fixing these!

@masseyke masseyke merged commit c4b1f18 into elastic:master Mar 24, 2022
@masseyke masseyke deleted the fix/cat-allocation-test-failure branch March 24, 2022 19:08
masseyke added a commit to masseyke/elasticsearch that referenced this pull request Mar 24, 2022
…c#84539)

We delete all indices in a cluster before running ClientYamlTestSuiteIT. However some system indices can regenerate.
The cat.allocation, cat.indices, and cat.shards tests were failing because they assume that no indices not created by the tests exist. They can now all handle system indices being created during the test.
Closes elastic#83719
Closes elastic#84765
Closes elastic#84972
Closes elastic#82151
Closes elastic#82660
masseyke added a commit to masseyke/elasticsearch that referenced this pull request Mar 24, 2022
…c#84539)

We delete all indices in a cluster before running ClientYamlTestSuiteIT. However some system indices can regenerate.
The cat.allocation, cat.indices, and cat.shards tests were failing because they assume that no indices not created by the tests exist. They can now all handle system indices being created during the test.
Closes elastic#83719
Closes elastic#84765
Closes elastic#84972
Closes elastic#82151
Closes elastic#82660
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

💚 Backport successful

Status Branch Result
8.0
8.1

@masseyke masseyke removed the v8.0.2 label Mar 24, 2022
elasticsearchmachine pushed a commit that referenced this pull request Mar 24, 2022
#85342)

We delete all indices in a cluster before running ClientYamlTestSuiteIT. However some system indices can regenerate.
The cat.allocation, cat.indices, and cat.shards tests were failing because they assume that no indices not created by the tests exist. They can now all handle system indices being created during the test.
Closes #83719
Closes #84765
Closes #84972
Closes #82151
Closes #82660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment