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 flaky test around search cancellation #193008

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

mikecote
Copy link
Contributor

@mikecote mikecote commented Sep 16, 2024

Resolves #192914

In this PR, I'm fixing the flakiness in tests where sometimes rules fail with a different message on timeout. This is expected as it's a race condition between the Elasticsearch request timing out and the alerting rule getting cancelled. So we can expect one of two messages.

Note: Test is not skipped as of PR creation

@mikecote mikecote added Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v9.0.0 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) v8.16.0 labels Sep 16, 2024
@mikecote mikecote self-assigned this Sep 16, 2024
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

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

cc @mikecote

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#6944

[✅] x-pack/test/alerting_api_integration/spaces_only/tests/alerting/group4/config.ts: 100/100 tests passed.

see run history

@mikecote mikecote marked this pull request as ready for review September 16, 2024 16:29
@mikecote mikecote requested a review from a team as a code owner September 16, 2024 16:29
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

Copy link
Contributor

@ymao1 ymao1 left a comment

Choose a reason for hiding this comment

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

LGTM

@mikecote mikecote merged commit 0c63a7b into elastic:main Sep 16, 2024
36 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 16, 2024
Resolves elastic#192914

In this PR, I'm fixing the flakiness in tests where sometimes rules fail
with a different message on timeout. This is expected as it's a race
condition between the Elasticsearch request timing out and the alerting
rule getting cancelled. So we can expect one of two messages.

Note: Test is not skipped as of PR creation
(cherry picked from commit 0c63a7b)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 16, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Fix flaky test around search cancellation
(#193008)](#193008)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Mike
Côté","email":"mikecote@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-16T17:45:08Z","message":"Fix
flaky test around search cancellation (#193008)\n\nResolves
https://github.com/elastic/kibana/issues/192914\r\n\r\nIn this PR, I'm
fixing the flakiness in tests where sometimes rules fail\r\nwith a
different message on timeout. This is expected as it's a
race\r\ncondition between the Elasticsearch request timing out and the
alerting\r\nrule getting cancelled. So we can expect one of two
messages.\r\n\r\nNote: Test is not skipped as of PR
creation","sha":"0c63a7b73394e9bfe348de8b2d0755c557098eae","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Alerting","release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.16.0"],"title":"Fix
flaky test around search
cancellation","number":193008,"url":"https://github.com/elastic/kibana/pull/193008","mergeCommit":{"message":"Fix
flaky test around search cancellation (#193008)\n\nResolves
https://github.com/elastic/kibana/issues/192914\r\n\r\nIn this PR, I'm
fixing the flakiness in tests where sometimes rules fail\r\nwith a
different message on timeout. This is expected as it's a
race\r\ncondition between the Elasticsearch request timing out and the
alerting\r\nrule getting cancelled. So we can expect one of two
messages.\r\n\r\nNote: Test is not skipped as of PR
creation","sha":"0c63a7b73394e9bfe348de8b2d0755c557098eae"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193008","number":193008,"mergeCommit":{"message":"Fix
flaky test around search cancellation (#193008)\n\nResolves
https://github.com/elastic/kibana/issues/192914\r\n\r\nIn this PR, I'm
fixing the flakiness in tests where sometimes rules fail\r\nwith a
different message on timeout. This is expected as it's a
race\r\ncondition between the Elasticsearch request timing out and the
alerting\r\nrule getting cancelled. So we can expect one of two
messages.\r\n\r\nNote: Test is not skipped as of PR
creation","sha":"0c63a7b73394e9bfe348de8b2d0755c557098eae"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
mikecote added a commit that referenced this pull request Oct 3, 2024
Resolves #192914.

Follow up from #193008 as the
GitHub issue got re-opened
(#192914 (comment)).

In the first PR, I fixed the assertion on the error message, in this PR,
I'm fixing the assertion on the status reason which varies from
`timeout` and `execute` depending on where the error initiated from
(abort controller vs ES client).

Note: Test is not skipped as of PR creation

Flaky test runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7065
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 3, 2024
Resolves elastic#192914.

Follow up from elastic#193008 as the
GitHub issue got re-opened
(elastic#192914 (comment)).

In the first PR, I fixed the assertion on the error message, in this PR,
I'm fixing the assertion on the status reason which varies from
`timeout` and `execute` depending on where the error initiated from
(abort controller vs ES client).

Note: Test is not skipped as of PR creation

Flaky test runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7065

(cherry picked from commit 3308625)
kibanamachine added a commit that referenced this pull request Oct 3, 2024
# Backport

This will backport the following commits from `main` to `8.x`:
- [Fix flaky test around search cancellation pt2
(#194687)](#194687)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Mike
Côté","email":"mikecote@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-10-03T12:23:57Z","message":"Fix
flaky test around search cancellation pt2 (#194687)\n\nResolves
https://github.com/elastic/kibana/issues/192914.\r\n\r\nFollow up from
#193008 as the\r\nGitHub issue got
re-opened\r\n(https://github.com/elastic/kibana/issues/192914#issuecomment-2383972707).\r\n\r\nIn
the first PR, I fixed the assertion on the error message, in this
PR,\r\nI'm fixing the assertion on the status reason which varies
from\r\n`timeout` and `execute` depending on where the error initiated
from\r\n(abort controller vs ES client).\r\n\r\nNote: Test is not
skipped as of PR creation\r\n\r\nFlaky test
runner:\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7065","sha":"330862524d89a9ab9fcf8931169759a99e5e44a6","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Feature:Alerting","release_note:skip","Team:ResponseOps","v9.0.0","backport:prev-minor","v8.16.0"],"title":"Fix
flaky test around search cancellation
pt2","number":194687,"url":"https://github.com/elastic/kibana/pull/194687","mergeCommit":{"message":"Fix
flaky test around search cancellation pt2 (#194687)\n\nResolves
https://github.com/elastic/kibana/issues/192914.\r\n\r\nFollow up from
#193008 as the\r\nGitHub issue got
re-opened\r\n(https://github.com/elastic/kibana/issues/192914#issuecomment-2383972707).\r\n\r\nIn
the first PR, I fixed the assertion on the error message, in this
PR,\r\nI'm fixing the assertion on the status reason which varies
from\r\n`timeout` and `execute` depending on where the error initiated
from\r\n(abort controller vs ES client).\r\n\r\nNote: Test is not
skipped as of PR creation\r\n\r\nFlaky test
runner:\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7065","sha":"330862524d89a9ab9fcf8931169759a99e5e44a6"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194687","number":194687,"mergeCommit":{"message":"Fix
flaky test around search cancellation pt2 (#194687)\n\nResolves
https://github.com/elastic/kibana/issues/192914.\r\n\r\nFollow up from
#193008 as the\r\nGitHub issue got
re-opened\r\n(https://github.com/elastic/kibana/issues/192914#issuecomment-2383972707).\r\n\r\nIn
the first PR, I fixed the assertion on the error message, in this
PR,\r\nI'm fixing the assertion on the status reason which varies
from\r\n`timeout` and `execute` depending on where the error initiated
from\r\n(abort controller vs ES client).\r\n\r\nNote: Test is not
skipped as of PR creation\r\n\r\nFlaky test
runner:\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7065","sha":"330862524d89a9ab9fcf8931169759a99e5e44a6"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Mike Côté <mikecote@users.noreply.github.com>
tiansivive pushed a commit to tiansivive/kibana that referenced this pull request Oct 7, 2024
Resolves elastic#192914.

Follow up from elastic#193008 as the
GitHub issue got re-opened
(elastic#192914 (comment)).

In the first PR, I fixed the assertion on the error message, in this PR,
I'm fixing the assertion on the status reason which varies from
`timeout` and `execute` depending on where the error initiated from
(abort controller vs ES client).

Note: Test is not skipped as of PR creation

Flaky test runner:
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7065
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v8.16.0 v9.0.0
Projects
None yet
5 participants