Skip to content

Do not use auto_expand_replica in mget yaml rest tests#97427

Merged
pxsalehi merged 2 commits intoelastic:mainfrom
pxsalehi:ps230706-fixTest-mgetYamlRestMixedCluster
Jul 7, 2023
Merged

Do not use auto_expand_replica in mget yaml rest tests#97427
pxsalehi merged 2 commits intoelastic:mainfrom
pxsalehi:ps230706-fixTest-mgetYamlRestMixedCluster

Conversation

@pxsalehi
Copy link
Member

@pxsalehi pxsalehi commented Jul 6, 2023

As described in the issue, the change in #96763
has made the MixedClusterClientYamlTestSuiteIT for mget fail very
often. For now, let's take the same approach that we have for get.

Closes #97236

@pxsalehi pxsalehi added >test Issues or PRs that are addressing/adding tests :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. labels Jul 6, 2023
@pxsalehi pxsalehi marked this pull request as ready for review July 6, 2023 15:04
@elasticsearchmachine elasticsearchmachine added the Team:Distributed Meta label for distributed team. label Jul 6, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)


- do:
cluster.health:
wait_for_status: green
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to completely remove this section? Should we at least wait for yellow?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems to be safe. For some of the tests changed here and in my PR linked above, the original test didn't even have the wait, see e.g. https://github.com/elastic/elasticsearch/blob/8.8/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/mget/40_routing.yml#L47-L64.

The get tests use this setup and they seem to be ok. The writes would have to wait I think anyway. See the comment of this commit 5010402.

@pxsalehi pxsalehi requested a review from idegtiarenko July 6, 2023 15:29
@pxsalehi
Copy link
Member Author

pxsalehi commented Jul 7, 2023

@elasticmachine run elasticsearch-ci/part-1

There were no test failures, but somehow the Jenkins job failed.

@pxsalehi
Copy link
Member Author

pxsalehi commented Jul 7, 2023

@elasticmachine update branch

@pxsalehi pxsalehi added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Jul 7, 2023
@pxsalehi pxsalehi merged commit 34eb74f into elastic:main Jul 7, 2023
@pxsalehi pxsalehi deleted the ps230706-fixTest-mgetYamlRestMixedCluster branch July 7, 2023 12:45
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.9

pxsalehi added a commit to pxsalehi/elasticsearch that referenced this pull request Jul 7, 2023
As described in the issue, the change in elastic#96763
has made the MixedClusterClientYamlTestSuiteIT for mget fail very
often. For now, let's take the same approach that we have for get.

Closes elastic#97236
elasticsearchmachine pushed a commit that referenced this pull request Jul 7, 2023
As described in the issue, the change in #96763
has made the MixedClusterClientYamlTestSuiteIT for mget fail very
often. For now, let's take the same approach that we have for get.

Closes #97236
@rjernst rjernst added v8.9.0 and removed v8.9.1 labels Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. Team:Distributed Meta label for distributed team. >test Issues or PRs that are addressing/adding tests v8.9.0 v8.10.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CI] MixedClusterClientYamlTestSuiteIT test {p0=mget/40_routing/routing} failing

5 participants