-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Add error handling/retry logic for search source alert tests #196443
Add error handling/retry logic for search source alert tests #196443
Conversation
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#7157[✅] x-pack/test/functional_with_es_ssl/apps/discover_ml_uptime/config.ts: 25/25 tests passed. |
@wayneseymour Do you mind taking a quick look at this and see if it's along the lines of what you were thinking? |
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.
@lukasolson I would strongly recommend running the serverless tests through MKI, from your local
One major reason is to ensure the default timeout durations encoded in x-pack/test_serverless/functional/test_suites/common/discover_ml_uptime/discover/search_source_alert.ts
, actually work for real serverless as well.
PR CI only runs fake-serverless.
Running against real serverless (mki) will give you useful feedback.
- network access is typically slower, etc
Ran From my Local
Ran the stateful test, no issues.
Ran the serverless test against a real serverless oblt project (mki), I am seeing errors:
└-: Discover alerting
└-> "before all" hook: beforeTestSuite.trigger in "Discover alerting"
└-> "before all" hook in "Discover alerting"
└-: Search source Alert
└-> "before all" hook: beforeTestSuite.trigger for "should create an alert when there is no data view"
└-> "before all" hook for "should create an alert when there is no data view"
│ERROR timeout of 60000ms exceeded
│ Waiting 15000 ms before the next attempt
└-> should create an alert when there is no data view
└-> "before each" hook: global before each for "should create an alert when there is no data view"
└- ✓ pass (23.1s)
└-> should show time field validation error
└-> "before each" hook: global before each for "should show time field validation error"
└- ✓ pass (14.8s)
└-> should navigate to alert results via view in app link
└-> "before each" hook: global before each for "should navigate to alert results via view in app link"
└- ✓ pass (40.0s)
└-> should navigate to alert results via link provided in notification
└-> "before each" hook: global before each for "should navigate to alert results via link provided in notification"
└- ✓ pass (37.8s)
└-> should display prev rule state after params update on clicking prev generated link
└-> "before each" hook: global before each for "should display prev rule state after params update on clicking prev generated link"
└- ✖ fail: Discover alerting Search source Alert should display prev rule state after params update on clicking prev generated link
│ Error: retry.try reached timeout 120000 ms
│ TimeoutError: Waiting for element to be located By(css selector, [data-test-subj="saveEditedRuleButton"])
│ Wait timed out after 10054ms
│ at /Users/trezworkbox/dev/main/node_modules/selenium-webdriver/lib/webdriver.js:923:22
│ at processTicksAndRejections (node:internal/process/task_queues:95:5)
│ at onFailure (retry_for_success.ts:18:9)
│ at retryForSuccess (retry_for_success.ts:85:7)
│ at Proxy.try (retry.ts:52:12)
│ at Proxy.clickByCssSelector (find.ts:420:5)
│ at Proxy.click (test_subjects.ts:169:5)
│ at Context.<anonymous> (search_source_alert.ts:509:7)
│ at Object.apply (wrap_function.js:74:16)
│
│
└-> should display actual state after rule params update on clicking viewInApp link
└-> "before each" hook: global before each for "should display actual state after rule params update on clicking viewInApp link"
└- ✖ fail: Discover alerting Search source Alert should display actual state after rule params update on clicking viewInApp link
│ Error: expected '' to equal 'message:msg-1'
│ at Assertion.assert (expect.js:100:11)
│ at Assertion.apply (expect.js:227:8)
│ at Function.equal (expect.js:531:15)
│ at checkUpdatedRuleParamsState (search_source_alert.ts:343:31)
│ at processTicksAndRejections (node:internal/process/task_queues:95:5)
│ at Context.<anonymous> (search_source_alert.ts:522:7)
│ at Object.apply (wrap_function.js:74:16)
│
│
└-> should display prev data view state after update on clicking prev generated link
└-> "before each" hook: global before each for "should display prev data view state after update on clicking prev generated link"
....
I recommend running it against search and security projects as well.
…kasolson/kibana into search_source_alert_ft_error_handling
…_ft_error_handling
Pinging @elastic/kibana-data-discovery (Team:DataDiscovery) |
Flaky Test Runner Stats🎉 All tests passed! - kibana-flaky-test-suite-runner#7454[✅] x-pack/test_serverless/functional/test_suites/search/common_configs/config.group6.ts: 25/25 tests passed. |
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.
Code changes LGTM, but it looks like a different test is failing now in this PR. Possibly related to #198987.
…lert_ft_error_handling
…kasolson/kibana into search_source_alert_ft_error_handling
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, thanks for fixing them and adding the issue!
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.
I'm guessing the conditionals for buttons and inputs are for O11y projects? We should probably consider splitting these out since they could diverge more, but let's get them unskipped for now.
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.
That's right, and yes I totally agree. There was already conditionals like these in these tests but I agree, it's probably time now to split these.
💚 Build Succeeded
Metrics [docs]
History
cc @lukasolson |
@lukasolson Did we only want this merged for 9.0 or should it also be backported to an 8.x branch? |
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…#196443) ## Summary Resolves elastic#193842. Adds error handling & retry logic for search source alerts that are causing failures on MKI. ### Checklist - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed (cherry picked from commit 5acba96)
…#196443) ## Summary Resolves elastic#193842. Adds error handling & retry logic for search source alerts that are causing failures on MKI. ### Checklist - [x] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed
…196443) (#203878) # Backport This will backport the following commits from `main` to `8.x`: - [Add error handling/retry logic for search source alert tests (#196443)](#196443) <!--- Backport version: 8.9.8 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Lukas Olson","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-12-10T20:27:36Z","message":"Add error handling/retry logic for search source alert tests (#196443)\n\n## Summary\r\n\r\nResolves https://github.com/elastic/kibana/issues/193842.\r\n\r\nAdds error handling & retry logic for search source alerts that are\r\ncausing failures on MKI.\r\n\r\n### Checklist\r\n\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed","sha":"5acba9678aef16b460bb7b578dcdf4ce10a012a4","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","backport:skip","v9.0.0","Team:DataDiscovery"],"number":196443,"url":"https://github.com/elastic/kibana/pull/196443","mergeCommit":{"message":"Add error handling/retry logic for search source alert tests (#196443)\n\n## Summary\r\n\r\nResolves https://github.com/elastic/kibana/issues/193842.\r\n\r\nAdds error handling & retry logic for search source alerts that are\r\ncausing failures on MKI.\r\n\r\n### Checklist\r\n\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed","sha":"5acba9678aef16b460bb7b578dcdf4ce10a012a4"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/196443","number":196443,"mergeCommit":{"message":"Add error handling/retry logic for search source alert tests (#196443)\n\n## Summary\r\n\r\nResolves https://github.com/elastic/kibana/issues/193842.\r\n\r\nAdds error handling & retry logic for search source alerts that are\r\ncausing failures on MKI.\r\n\r\n### Checklist\r\n\r\n- [x] [Flaky Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was\r\nused on any tests changed","sha":"5acba9678aef16b460bb7b578dcdf4ce10a012a4"}}]}] BACKPORT-->
Summary
Resolves #193842.
Adds error handling & retry logic for search source alerts that are causing failures on MKI.
Checklist