[Alerting] Fixes Failing test: X-Pack Alerting API Integration Tests.x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting - alerting api integration security and spaces enabled Alerts do stuff when AAD is broken #84707
Conversation
ymao1
added
Feature:Alerting
release_note:skip
|
Pinging @elastic/kibana-alerting-services (Team:Alerting Services) |
x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/update.ts
Outdated
Show resolved
Hide resolved
| @@ -301,6 +301,9 @@ export default function createDeleteTests({ getService }: FtrProviderContext) { | |||
| .send(getTestAlertData()) | |||
| .expect(200); | |||
|
|
|||
| // Delay before performing update to avoid 409 errors | |||
| await new Promise((resolve) => setTimeout(resolve, 1000)); | |||
There was a problem hiding this comment.
I wonder if we should wrap the call below in a retry.try instead of adding timeouts in the tests? That way it can keep trying for a certain amount of time but in most cases finish the test at normal speed. Unless this was a consistent issue?
There was a problem hiding this comment.
I wonder if we should wrap the call below in a
retry.tryinstead of adding timeouts in the tests? That way it can keep trying for a certain amount of time but in most cases finish the test at normal speed. Unless this was a consistent issue?
Good call. The failures don't happen very often. I've updated the PR to use retry.try. Here are the flaky test runner results after that update: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1039/
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/delete.ts
Show resolved
Hide resolved
x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/execution_status.ts
Outdated
Show resolved
Hide resolved
💚 Build SucceededMetrics [docs]Distributable file count
History
To update your PR or re-run it, just comment with: |
…x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting - alerting api integration security and spaces enabled Alerts do stuff when AAD is broken (elastic#84707) * Adding delay between creating and updating alert to avoid 409 conflicts * Unskipping update test * Using retry.try instead of delay * PR fixes Co-authored-by: Kibana Machine <[email protected]>
…x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting - alerting api integration security and spaces enabled Alerts do stuff when AAD is broken (#84707) (#85161) * Adding delay between creating and updating alert to avoid 409 conflicts * Unskipping update test * Using retry.try instead of delay * PR fixes Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
Resolves #84007
Resolves #84216
Resolves #84324
Summary
For all the tests that test for broken AAD, added a delay between creating the alert and updating the alert in order to avoid occasional 409 conflict errors.
Flaky test runner results: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1035/
The other issue is that the 409 errors come back as 500 "Internal Server Errors". I traced this back to how it's handled within the http router, which only forwards 401 errors from the ES client and throws an internal error for any other status code.
https://github.com/elastic/kibana/blob/master/src/core/server/http/router/router.ts#L252-289
Checklist