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

Fix AllocateRoutedStepTests reusing keys for random values #51016

Merged
merged 2 commits into from
Jan 15, 2020

Conversation

dakrone
Copy link
Member

@dakrone dakrone commented Jan 15, 2020

In these tests there was a very small chance that keys could collide,
which causes test failures.

Resolves #49307

In these tests there was a very small chance that keys could collide,
which causes test failures.

Resolves elastic#49307
@dakrone dakrone added >non-issue >test Issues or PRs that are addressing/adding tests :Data Management/ILM+SLM Index and Snapshot lifecycle management v8.0.0 v7.6.0 labels Jan 15, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/ILM+SLM)

@@ -114,7 +114,7 @@ public void testInvalidNumberOfReplicas() {
Map<String, String> map = new HashMap<>();
int numIncludes = randomIntBetween(minEntries, maxEntries);
for (int i = 0; i < numIncludes; i++) {
map.put(randomAlphaOfLengthBetween(1, 20), randomAlphaOfLengthBetween(1, 20));
map.put(randomAlphaOfLengthBetween(2, 20), randomAlphaOfLengthBetween(2, 20));
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 reason behind this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

So the test that actually caused the failure generated n as the key in the map, by increasing it to at least 2, we decrease the chance of collisions. The other changes make sure we don't have collisions, but this at least decreases the amount of retries since it's less likely to collide.

Copy link
Contributor

@andreidan andreidan left a comment

Choose a reason for hiding this comment

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

thanks for showing tests some ❤ @dakrone

@polyfractal polyfractal added v7.7.0 and removed v7.6.0 labels Jan 15, 2020
@dakrone
Copy link
Member Author

dakrone commented Jan 15, 2020

@elasticmachine update branch

@elasticmachine
Copy link
Collaborator

user doesn't have permission to update head repository

@dakrone dakrone merged commit 2b79d32 into elastic:master Jan 15, 2020
@dakrone dakrone deleted the fix-allocate-action-tests-49307 branch January 15, 2020 17:39
dakrone added a commit to dakrone/elasticsearch that referenced this pull request Jan 15, 2020
…1016)

In these tests there was a very small chance that keys could collide,
which causes test failures.

Resolves elastic#49307
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
…1016)

In these tests there was a very small chance that keys could collide,
which causes test failures.

Resolves elastic#49307
droberts195 pushed a commit to droberts195/elasticsearch that referenced this pull request Mar 23, 2020
In these tests there was a very small chance that keys could collide,
which causes test failures.

Backport of elastic#51016
droberts195 added a commit that referenced this pull request Mar 23, 2020
In these tests there was a very small chance that keys could collide,
which causes test failures.

Backport of #51016

Co-authored-by: Lee Hinman <dakrone@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/ILM+SLM Index and Snapshot lifecycle management >non-issue >test Issues or PRs that are addressing/adding tests v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] AllocationRoutedStepTests testExecuteAllocateNotCompleteOnlyOneCopyAllocated failed on 7.x
5 participants