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

[Security Solutions] Fixes 11 different flakey FTR/e2e tests and scenarios #115688

Merged
merged 4 commits into from
Oct 20, 2021

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Oct 20, 2021

Summary

Fixes flakes across tests that have either been skipped or have been a source of flake in the categories of:

  • Sorting fixes because Elasticsearch can return hits/arrays back in different orders
  • Flat array fixes because Elasticsearch can sometimes return [] or [[]] in-deterministically in some cases 🤷 , so we just flatten the array out completely and test for [] within those tests.
  • waitForSignalsToBePresent was missing in a test and sometimes we would get an empty array response which would fail CI.

Also I audited other tests for [[]] and waitForSignalsToBePresent and fixed them where they were present or if the waitForSignalsToBePresent count was incorrect. This should give us more stability when the CI is under pressure.

Sorting fixes:
#115554
#115321
#115319
#114581

Flat array fixes:
#89052
#115315
#115308
#115304
#115313
#113418

Missing additional check for "waitForSignalsToBePresent" or incorrect number of signals to wait for fixes:
#115310

Checklist

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@@ -344,6 +343,7 @@ export default ({ getService }: FtrProviderContext) => {
],
]);
await waitForRuleSuccessOrStatus(supertest, id);
await waitForSignalsToBePresent(supertest, 4, [id]);
Copy link
Contributor Author

@FrankHassanabad FrankHassanabad Oct 20, 2021

Choose a reason for hiding this comment

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

NOTE: This waitForSignalsToBePresent was missing and this is what caused the direct skipping from here:
#115310

@FrankHassanabad FrankHassanabad self-assigned this Oct 20, 2021
@FrankHassanabad FrankHassanabad added v8.0.0 v7.16.0 auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Security Solution Platform Security Solution Platform Team labels Oct 20, 2021
@FrankHassanabad FrankHassanabad marked this pull request as ready for review October 20, 2021 00:11
@FrankHassanabad FrankHassanabad requested a review from a team as a code owner October 20, 2021 00:11
@FrankHassanabad FrankHassanabad changed the title Fix flake tests [Security Solutions] Fixes 11 flakey tests Oct 20, 2021
@FrankHassanabad FrankHassanabad changed the title [Security Solutions] Fixes 11 flakey tests [Security Solutions] Fixes 11 flakey FTR/e2e tests Oct 20, 2021
@FrankHassanabad FrankHassanabad changed the title [Security Solutions] Fixes 11 flakey FTR/e2e tests [Security Solutions] Fixes 11 different flakey FTR/e2e tests and scenarios Oct 20, 2021
@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -499,7 +499,7 @@ export default ({ getService }: FtrProviderContext) => {
],
]);
await waitForRuleSuccessOrStatus(supertest, id);
await waitForSignalsToBePresent(supertest, 1, [id]);
await waitForSignalsToBePresent(supertest, 3, [id]);
Copy link
Contributor

Choose a reason for hiding this comment

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

This was just running before all 3 were present before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct, I went ahead and adjusted it. I don't think it failed before but to be consistent with the others I decided to change any of these that were incorrect counts which should reduce chances of flake.

Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

This is awesome. Code LGTM!

@FrankHassanabad FrankHassanabad enabled auto-merge (squash) October 20, 2021 02:51
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @FrankHassanabad

@FrankHassanabad FrankHassanabad merged commit a01165a into elastic:master Oct 20, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 20, 2021
…arios (elastic#115688)

## Summary

Fixes flakes across tests that have either been skipped or have been a source of flake in the categories of:
* Sorting fixes because Elasticsearch can return hits/arrays back in different orders
* Flat array fixes because Elasticsearch can sometimes return `[]` or `[[]]` in-deterministically in some cases 🤷 , so we just flatten the array out completely and test for `[]` within those tests.
* `waitForSignalsToBePresent` was missing in a test and sometimes we would get an empty array response which would fail CI.

Also I audited other tests for `[[]]` and `waitForSignalsToBePresent` and fixed them where they were present or if the `waitForSignalsToBePresent` count was incorrect. This should give us more stability when the CI is under pressure.

Sorting fixes:
elastic#115554
elastic#115321
elastic#115319
elastic#114581


Flat array fixes:
elastic#89052
elastic#115315
elastic#115308
elastic#115304
elastic#115313
elastic#113418

Missing additional check for "waitForSignalsToBePresent" or incorrect number of signals to wait for fixes:
elastic#115310


### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Oct 20, 2021
…arios (#115688) (#115699)

## Summary

Fixes flakes across tests that have either been skipped or have been a source of flake in the categories of:
* Sorting fixes because Elasticsearch can return hits/arrays back in different orders
* Flat array fixes because Elasticsearch can sometimes return `[]` or `[[]]` in-deterministically in some cases 🤷 , so we just flatten the array out completely and test for `[]` within those tests.
* `waitForSignalsToBePresent` was missing in a test and sometimes we would get an empty array response which would fail CI.

Also I audited other tests for `[[]]` and `waitForSignalsToBePresent` and fixed them where they were present or if the `waitForSignalsToBePresent` count was incorrect. This should give us more stability when the CI is under pressure.

Sorting fixes:
#115554
#115321
#115319
#114581


Flat array fixes:
#89052
#115315
#115308
#115304
#115313
#113418

Missing additional check for "waitForSignalsToBePresent" or incorrect number of signals to wait for fixes:
#115310


### Checklist

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
@FrankHassanabad FrankHassanabad deleted the fix-flake-tests branch October 20, 2021 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Security Solution Platform Security Solution Platform Team v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants