Fix flakiness on custom time range saved searches #165454

gsoldevila · 2023-09-01T11:08:15Z

A bunch of tests on dashboards are customising some of the panels settings and providing custom time ranges:

Currently, the logic is not waiting for the quick toggle animation to complete, before proceeding to select a time range.
This can cause a flaky behavior if the logic tries to customize the range before the button is actually available, as seen on this failed test. (part of this failed CI build)

The goal of this PR is to add a small waiting period, to make sure the toggle animation has completed, and that the time range controls are visible and clickable.

I used the opportunity to cleanup some "await delay millis" calls, reusing existing logic instead.

elasticmachine · 2023-09-01T11:09:05Z

Pinging @elastic/kibana-qa (Team:QA)

…-ref HEAD~1..HEAD --fix'

ThomThomson · 2023-09-01T16:20:17Z

If I remember correctly, animations are disabled for CI runs, so there really shouldn't be any flakiness caused by an animation. Additionally, adding timed sleeps is usually flakier than adding a retry or awaiting the presence of some element, so IMO it might be better to restructure some of these calls to do that rather than just sleeping.

gsoldevila · 2023-09-04T10:58:06Z

I checked with QA folks and it seems that they are indeed disabled.

Interestingly, I have a bunch of flaky test runner failures that show a screenshot of the welcome page interstitial fading in (half way through): https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2851

Additionally, adding timed sleeps is usually flakier than adding a retry or awaiting the presence of some element, so IMO it might be better to restructure some of these calls to do that rather than just sleeping

Fair point, I'll update the PR accordingly.

…rches

gsoldevila · 2023-09-05T10:41:08Z

@ThomThomson you were right, I confirmed animations are disabled (both on CI an locally).
A detailed look at the debug logs showed that the custom time range was actually not opening at all (either missed click or clicked twice 🤷🏼‍♂️ ).

Either way, I added an optional boolean to these toggles, so that when we intend to enable them, we will only click if they are disabled (and viceversa).

stratoula

Visualizations team changes LGTM

…-fix'

nickpeihl

dashboard functional test changes lgtm!

thanks for fixing the flakiness!

nreese · 2023-09-05T14:29:38Z

test/functional/services/dashboard/panel_settings.ts

@@ -55,10 +56,12 @@ export function DashboardCustomizePanelProvider({ getService }: FtrProviderConte
      return await this.findFlyoutTestSubject('superDatePickerToggleQuickMenuButton');
    }

-    public async clickToggleQuickMenuButton() {
+    public async clickToggleQuickMenuButton(open?: boolean) {


Instead of toggle methods with an open parameter, how about create separate enable/disable methods? So something like enableCustomTimeRange and disableCustomTimeRange.

Sure, we can do that too!

nreese · 2023-09-06T13:24:47Z

test/functional/services/dashboard/panel_settings.ts

@@ -115,10 +122,18 @@ export function DashboardCustomizePanelProvider({ getService, getPageObject }: F
      });
    }

-    public async clickToggleShowCustomTimeRange(enable?: boolean) {
-      log.debug('clickToggleShowCustomTimeRange');
+    public async enableCustomTimeRange() {


This looks great. One more thing, since it sounds like missing clicks is one cause of flakyness, how about adding a retry

const toggle = await testSubjects.find(this.TOGGLE_TIME_RANGE_TEST_SUBJ); await retry.try(async () => { if (!(await toggle.isSelected())) { await toggle.click(); if (!(await toggle.isSelected())) { throw new Error('Switch click missed'); } } });

…rches

gsoldevila · 2023-09-14T13:08:27Z

Triggered 150x runs with the current changes: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3128

davismcphee

@gsoldevila Thanks for the effort on this! Unfortunately it looks like the Discover test is the only one still failing in your flaky test run... @jughosta recently looked into this failure too and opened a draft PR #166239 that seems to fix the flakiness (she also did a 150x run). If you prefer, feel free to leave the Discover test skipped in this PR so you can get it merged, and we can follow up with the fix from #166239 instead.

gsoldevila · 2023-09-14T21:16:23Z

@davismcphee Frankly, I don't think that PR tackles the actual source of flakiness, which IMO is a lost click on the "Enable custom time range toggle".
I've just checked the new CI failure stack trace in my runs and came to the exact same conclusion.

As for my PR, I believe I had a working solution with the retry.try() but changed it to retry.waitFor() instead, thinking that it would re-attempt the click if I passed an onFailureBlock handler (it seems it does not).

But since it's your ownership, feel free to go ahead and merge it.
I'll keep mine around just in case the flakiness strikes back (I'll rebase / solve conflicts).

davismcphee · 2023-09-15T02:26:54Z

@gsoldevila Thanks for the explanation! In that case we'll hold off on merging #166239. I didn't want to block the PR if you were hoping to merge, but we can definitely continue with this one instead if you think you've got a reliable fix for it. Looks like the test has been an issue for quite a while so it would be great to see it gone for good.

…rches

gsoldevila · 2023-09-21T14:04:16Z

I updated the branch to use a retry mechanism.
Triggered 150x runs of the flaky test runner: https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3207
@jughosta I also updated the opening of the date picker quick menu (2nd action in the spot) using the logic in your PR.

kibana-ci · 2023-09-21T15:56:13Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 28132c7

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #10 / serverless common UI Index Management Index Templates "before all" hook for "renders the index templates tab"
[job] [logs] FTR Configs #1 / serverless examples UI Partial Results Example "before all" hook for "should trace mouse events"

Metrics [docs]

✅ unchanged

History

💔 Build #160824 failed 5bf114e
💛 Build #159102 was flaky 5b033d2
💚 Build #159063 succeeded 76ab208
💔 Build #156841 failed cc323cc
💔 Build #156702 failed 46a7566
💔 Build #156346 failed a1c6678

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

davismcphee

Latest changes LGTM and the flakiness seems to be gone! 🎉 @jughosta can you give the final approval on this one since you're most familiar with these tests?

A bunch of tests on dashboards are customising some of the panels settings and providing custom time ranges: <img width="409" alt="image" src="https://github.com/elastic/kibana/assets/25349407/c869c1a3-f7db-4ccd-ad00-c5403f2b4201"> Currently, the logic is not waiting for the quick toggle animation to complete, before proceeding to select a time range. This can cause a flaky behavior if the logic tries to customize the range before the button is actually available, as seen on [this failed test](https://s3.amazonaws.com/buildkiteartifacts.com/e0f3970e-3a75-4621-919f-e6c773e2bb12/0fda5127-f57f-42fb-8e5a-146b3d535916/018a4c44-5497-48b6-8521-a8a79a2f63b4/018a4c46-0e7a-4b69-9a3d-9c54c27165b0/target/test_failures/018a4c46-0e7a-4b69-9a3d-9c54c27165b0_4fcbc47e71644919129e320eea8bb3bc.html?response-content-type=text%2Fhtml&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAQPCP3C7LZWZ5UB5F%2F20230901%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20230901T094837Z&X-Amz-Expires=600&X-Amz-Security-Token=IQoJb3JpZ2luX2VjEB0aCXVzLWVhc3QtMSJGMEQCIGCyKcVLGPUawZubNzZdt5oZNb5v0saiIuPqXwI7rmwlAiAsOj%2Fiep94v%2BYZJtLY3Gw0m%2FmK5mJw2IcIBdNKFXgK%2BCr6Awjm%2F%2F%2F%2F%2F%2F%2F%2F%2F%2F8BEAAaDDAzMjM3OTcwNTMwMyIMXOd1Hm6ks%2FNE37V0Ks4DgMUso7syv87hnPcC%2BB1soxvFFnj4JnNZc6ZgkLUe93z99iPFBUsqH%2BRbUTfSbjVOEJYBKGYuvp32xvSWsYNVPXKmcej18LC0yNi%2BBzoG2X%2Bj80g%2BbGMm6YfTncjPhOE0CHHqOWXts9nQ8WpDy8XOl0zfMtuiPjzOXHo9lvw2mgYDZIJIMV72FYB9JGg8FPbLQtD3rysLGNE0VDKgl5LCnYwhY1pwRCRHnVW41QfV0pwK%2FbjNf9HjdK31LQvMY%2FGPuB3M6O2CUZLsvLGfWBeGYHtkqb0hrL9ijO1Uo28ZSS1FytPftEdF0e1kAC9C5zD56HtYm55aktOWtaaC0XPWLdWWGUq%2FKQzhxSCiXK6ovATU3zI3yPNoZs92YBYmIPMOpEI40dCCpksjPwAMCiQd%2F9gMNKP5Qp5CbYd2Khy%2FeXaT8J7HOZCueN63O0j%2FtX1tbwfznhbr74lAcRQjueRYmwboZaGSDZUQ33lSSmyZk1V9WF9eJyt88oHvIx0q9bIjvOlW05DiNKfEFWYwfBywdGuvRU6eGMs1QcDNu33Lb%2BhymudM2JZmQKIjZOcb2l3Fzctp614owH4JcRlmF4%2BIa4xHeBdRlTMysS8bTIsgMK7axacGOqYBzIpC1wgZWJ1kZ0agLWCNaMIdUl%2B4xrr7w%2Fz0843WWMhRrvbJhDTHqk5UclF%2FSROAMe0FH2XEXiQ65ILyUPlrUMels5tfQ3Pp%2FJWPi9NsQJUQ1n9uLN%2BFPDOoMo8Uxg4%2FkG2O7yTkrIdArfA6pWN9I21gFMW%2BFZy9BMYltt5T65ZKOyYAIFGpLhgfBySIBCUMgwR1kusfDhf1%2FRTvtDKD2sJKN5a0IA%3D%3D&X-Amz-SignedHeaders=host&X-Amz-Signature=35fabe908aa7514e4a92de0ed12973af85ccfb439984fc3bdd7ef3bb8fe3419b). (part of this [failed CI build](https://buildkite.com/elastic/kibana-pull-request/builds/155285#018a4c46-0e7a-4b69-9a3d-9c54c27165b0)) The goal of this PR is to add a small waiting period, to make sure the toggle animation has completed, and that the time range controls are visible and clickable. I used the opportunity to cleanup some "await delay millis" calls, reusing existing logic instead. --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit b98b6d0)

kibanamachine · 2023-09-25T07:39:34Z

💚 All backports created successfully

Status	Branch	Result
✅	8.10

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