-
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
[Discover] Improve saveSearch functional test handling #73626
[Discover] Improve saveSearch functional test handling #73626
Conversation
Pinging @elastic/kibana-app (Team:KibanaApp) |
await retry.waitFor(`saved search can be persisted name ${searchName}`, async () => { | ||
await testSubjects.setValue('savedObjectTitle', searchName); | ||
await testSubjects.click('confirmSaveSavedObjectButton'); | ||
return (await header.isGlobalLoadingIndicatorVisible()) === true; |
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 thought we might need to wait for popup to disappear
return (await testSubjects.exists('confirmSaveSavedObjectButton', { timeout: 5000 })) === false
Is there a chance that popup will remain in place and header.isGlobalLoadingIndicatorVisible()
will return true
?
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.
good point 👍 , I will take a closer look here!
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 wondering if we can always catch the loading indicator after clicking save. Could it be that in some cases the save is quick enough that we never see the loading indicator?
Also, if we do find the loading indicator, I think we should then wait for it to be hidden before returning from saveSearch method. I think waitUntilLoadingHasFinished was doing that.
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've changed the code to check for the save button not to be disabled, this should catch the occasional
flakiness when for whatever reason the title wasn't entered, and the save button stayed disabled therefore
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.
flaky test runner says, it's ok this way: https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/712/
…-29-improve-discover-page-functional
💚 Build SucceededBuild metrics
History
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.
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 - thanks for working on this! I didn't pull code and test. Only code review and Jenkins results.
* Check for submit button to be disabled, before submitting the form to prevent occasional flakiness
* Check for submit button to be disabled, before submitting the form to prevent occasional flakiness
* master: (54 commits) [ML] Migrate to React BrowserRouter and Kibana provided History. (elastic#71941) [Discover] Improve saveSearch functional test handling (elastic#73626) [Metrics UI] Fix all threshold alert conditions disappearing due to alert prefill (elastic#73708) [Metrics UI] Fix alert previews of ungrouped alerts (elastic#73735) [SIEM] Fixes "include building block button" to operate (elastic#73900) [Metrics UI] Fix alert management to open without refresh (elastic#73739) [Security Solution][Lists] - Tests cleanup and remove unnecessary import (elastic#73865) [Ingest Management] main branch uses epr-snapshot. Others production (elastic#73555) [Canvas][tech-debt] Fix SVG not shrinking vertically properly (elastic#73867) [Maps] upgrade turf (elastic#73816) [Security Solution][Telemetry] Concurrent telemetry requests (elastic#73558) [Security Solution][Exceptions] - Update how nested entries are displayed in exceptions viewer (elastic#73745) [Security Solution][Exceptions] Adds autocomplete workaround for .text fields (elastic#73761) [Metrics UI] Fix previewing of No Data results (elastic#73753) Closes elastic#72914 by hiding anomaly detection settings links when the ml plugin is disabled. (elastic#73638) [Ingest Manager] Fix config selection in enrollment flyout from config list page (elastic#73833) [DOCS] Fixes typo in Alerting actions (elastic#73756) [APM] fixes linking errors to ML and Discover (elastic#73758) Handle promise rejections when building artifacts (elastic#73831) [Security Solution][Detections] Change from sha1 to sha256 (elastic#73741) ...
* master: (38 commits) [Discover] Context unskip date nanos functional tests (elastic#73781) [ML] Migrate to React BrowserRouter and Kibana provided History. (elastic#71941) [Discover] Improve saveSearch functional test handling (elastic#73626) [Metrics UI] Fix all threshold alert conditions disappearing due to alert prefill (elastic#73708) [Metrics UI] Fix alert previews of ungrouped alerts (elastic#73735) [SIEM] Fixes "include building block button" to operate (elastic#73900) [Metrics UI] Fix alert management to open without refresh (elastic#73739) [Security Solution][Lists] - Tests cleanup and remove unnecessary import (elastic#73865) [Ingest Management] main branch uses epr-snapshot. Others production (elastic#73555) [Canvas][tech-debt] Fix SVG not shrinking vertically properly (elastic#73867) [Maps] upgrade turf (elastic#73816) [Security Solution][Telemetry] Concurrent telemetry requests (elastic#73558) [Security Solution][Exceptions] - Update how nested entries are displayed in exceptions viewer (elastic#73745) [Security Solution][Exceptions] Adds autocomplete workaround for .text fields (elastic#73761) [Metrics UI] Fix previewing of No Data results (elastic#73753) Closes elastic#72914 by hiding anomaly detection settings links when the ml plugin is disabled. (elastic#73638) [Ingest Manager] Fix config selection in enrollment flyout from config list page (elastic#73833) [DOCS] Fixes typo in Alerting actions (elastic#73756) [APM] fixes linking errors to ML and Discover (elastic#73758) Handle promise rejections when building artifacts (elastic#73831) ...
Summary
Improve functional test
saveSearch
method to prevent occasional flakiness, when a search name wasn't inserted and the persisting hasn't started therefore.Fixes #68706
https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/62931/console
Here's the flaky test runner to be sure, no errors reported:
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/702/testReport/Chrome%20X-Pack%20UI%20Functional%20Tests/x-pack_test_functional_apps_discover_reporting·ts/
Checklist