-
Notifications
You must be signed in to change notification settings - Fork 8.2k
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 race condition in flaky alerting test #60438
Fix race condition in flaky alerting test #60438
Conversation
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
I hate that we need this though, I'd really like to expose a proper TM api for this kind of thing.
Agreed, I'm also hoping the advanced API key invalidation (#53868) will help reduce the need for code like this. Since the general invalidation of API keys (#53732), we've had a lot of race conditions in our tests. |
Found some flakiness (fixed in 413b6d2), doing another flaky test runner run here: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/264/. |
…cleanup-race-condition
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👌
* Fix race condition in flaky test * Fix flakiness in test * Fix more flakiness
In this PR, I'm fixing a race condition where the alert gets disabled before the cleanup of the action task params can finish.
I'm renaming in the task manager utils
waitForIdle
towaitForEmpty
and adding a newwaitForAllTasksIdle
function that will be called before disabling an alert. This allows the actions that run after an alert execution to also finish running.Reproducing steps
You can reproduce the flakiness by adding
await new Promise(r => setTimeout(r, 3000));
here:
kibana/x-pack/plugins/actions/server/lib/task_runner_factory.ts
Line 103 in 8def60e
Fixes #58991
Fixes #58643