unskips application leave confirm & application deep links tests #168741

TinaHeiligers · 2023-10-12T20:42:41Z

I modified the deep links tests because the side nav was overlaying the in-app nav.
While, theoretically, the side nav should work for the tests, it tends to be flaky.

I added logs for the url so that if these tests do fail, we'll have a bit more data to go on for debugging.
These tests pass on local test runs.

latest flaky test runs (50): https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3604

Unit or functional tests were updated or added to match the most common scenarios

TinaHeiligers · 2023-10-12T21:47:43Z

@elasticmachine merge upstream

TinaHeiligers

Unskips tests and adds debug log for the current url.

TinaHeiligers · 2023-10-12T23:10:54Z

test/plugin_functional/test_suites/core_plugins/application_leave_confirm.ts

-  // Failing: See https://github.com/elastic/kibana/issues/75963
-  // Failing: See https://github.com/elastic/kibana/issues/166838
-  describe.skip('application using leave confirmation', () => {
+  describe('application using leave confirmation', () => {


This test suite needed a complete overhaul to account for increased delays that various background tasks perform (ebt data collection, fleet setup etc).

TinaHeiligers · 2023-10-13T16:05:06Z

@elasticmachine merge upstream

TinaHeiligers · 2023-10-14T20:08:12Z

@elasticmachine merge upstream

TinaHeiligers · 2023-10-15T20:27:48Z

test/plugin_functional/test_suites/core_plugins/application_deep_links.ts

@@ -39,8 +39,7 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide
    });
  };

-  // Failing: See https://github.com/elastic/kibana/issues/166893
-  describe.skip('application deep links navigation', function describeDeepLinksTests() {
+  describe('application deep links navigation', function describeDeepLinksTests() {


@elastic/appex-sharedux I could use a hand in figuring out why tests using appMenu.clickLink fail in CI and pass locally. The most recent changes to relevant components seems to be with CollapsibleNav but that was 4 months ago.

The appLeave test failed because the modal wasn't visible.

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

TinaHeiligers · 2023-10-16T14:14:31Z

@elasticmachine merge upstream

TinaHeiligers · 2023-10-16T21:46:33Z

@elasticmachine merge upstream

elasticmachine · 2023-10-16T23:16:42Z

Pinging @elastic/kibana-core (Team:Core)

TinaHeiligers · 2023-10-16T23:20:59Z

@elasticmachine merge upstream

TinaHeiligers · 2023-10-17T16:39:14Z

@elasticmachine merge upstream

…eiligers/kibana into skipped-tests-app-leave-deep-links

TinaHeiligers · 2023-10-17T19:07:32Z

None of the test failures are related to this PR.

TinaHeiligers · 2023-10-17T19:07:46Z

@elasticmachine merge upstream

…eiligers/kibana into skipped-tests-app-leave-deep-links

…count. The delays stem from multiple background tasks that run simultaneously and cannot be turned off

TinaHeiligers

I ended up completely overhauling the app leave tests. With all the background events that now happen when kibana runs, it's no surprise these tests are flaky!

TinaHeiligers · 2023-10-17T21:08:25Z

test/plugin_functional/test_suites/core_plugins/application_leave_confirm.ts

    describe('when navigating to another app', () => {
-      it('prevents navigation if user click cancel on the confirmation dialog', async () => {
+      it('allows navigation if user click confirm on the confirmation dialog', async () => {


I swapped the order to test if it's an actual test failure or if it's the first test in the suite that fails. Turns out it's the first test that fails in CI because the modal doesn't show.

TinaHeiligers · 2023-10-18T22:08:15Z

test/functional/page_objects/common_page.ts

@@ -411,9 +411,18 @@ export class CommonPageObject extends FtrService {
   * Clicks cancel button on modal
   * @param overlayWillStay pass in true if your test will show multiple modals in succession
   */
-  async clickCancelOnModal(overlayWillStay = true) {
+  async clickCancelOnModal(overlayWillStay = true, ignorePageLeaveWarning = false) {


Extracted to allow re-calling similar test flow for both navigation options (cancel | confirm)

TinaHeiligers · 2023-10-18T22:08:55Z

test/functional/page_objects/common_page.ts

-    await this.testSubjects.click('confirmModalCancelButton');
+    await this.testSubjects.exists('confirmModalTitleText');
+
+    await this.retry.try(async () => {


we have to wait for the modal to fade in, which takes time, likely even longer in CI

TinaHeiligers · 2023-10-18T22:10:41Z

test/functional/services/apps_menu.ts

@@ -59,6 +59,7 @@ export class AppsMenuService extends FtrService {
    if (!(await this.testSubjects.exists('collapsibleNav'))) {
      await this.testSubjects.click('toggleNavButton');
    }
+    await this.testSubjects.exists('collapsibleNav');


This helper wasn't confirming a 'happy-path', we should assert that the nav flyout does appear before progressing to use any of the nav methods (clicking an app title for example)

TinaHeiligers · 2023-10-18T22:11:40Z

test/plugin_functional/plugins/core_plugin_appleave/public/plugin.tsx

@@ -15,6 +15,8 @@ export class CoreAppLeavePlugin
    core.application.register({
      id: 'appleave1',
      title: 'AppLeave 1',
+      appRoute: '/app/appleave1',
+      category: DEFAULT_APP_CATEGORIES.kibana,


I cheated here a little, adding the test plugins to the Analytics section to find them without having to scroll through all the other plugins in the nav flyout.

TinaHeiligers · 2023-10-18T22:14:01Z

test/plugin_functional/plugins/core_plugin_deep_links/public/plugin.tsx

@@ -29,7 +29,7 @@ export class CorePluginDeepLinksPlugin
        },
        {
          id: 'pageA',
-          title: 'DL Page A',
+          title: 'DL page A',


The test plugins and test suites themselves weren't using consistent case, making it hard to debug if the tests fail. Inconsistent case can also easily be missed in future refactors so keep it simple: Use consistent case.

TinaHeiligers · 2023-10-18T22:15:52Z

test/plugin_functional/test_suites/core_plugins/application_leave_confirm.ts

+    });
+  };
+
+  const ensureModalOpen = async (


test helper to make it easier to recall

TinaHeiligers · 2023-10-18T22:19:37Z

test/plugin_functional/test_suites/core_plugins/application_leave_confirm.ts

-        await testSubjects.existOrFail('appLeaveConfirmModal');
-        await PageObjects.common.clickConfirmOnModal();
-        expect(await browser.getCurrentUrl()).to.eql(getKibanaUrl('/app/appleave2'));
+        await appsMenu.clickLink('AppLeave 2');


Navigating to another app kicks off ebt metrics collection along with a host of other automated background tasks. We need to give those time to execute.

TinaHeiligers · 2023-10-18T22:20:22Z

test/plugin_functional/test_suites/core_plugins/application_leave_confirm.ts

+        });
+        const currentUrl = await browser.getCurrentUrl();
+        expect(currentUrl).to.contain('appleave2');
+        await PageObjects.common.navigateToApp('home');


Navigating away completely is the only way to ensure the browser doesn't get "stuck" in a rendering frenzy.

yikes 😄 , do you know what the source is of the rendering frenzy?

It's the side nav, the react hooks and the multiple ways navigation can occur. Mostly, though, it's the browser + delays in fading in and out.

TinaHeiligers · 2023-10-19T01:54:26Z

@elasticmachine merge upstream

kibana-ci · 2023-10-19T02:47:05Z

💚 Build Succeeded

Buildkite Build
Commit: 0f5277f

Metrics [docs]

✅ unchanged

History

💔 Build #169276 failed 8d1c8a2
💔 Build #169274 failed 81579c4
💔 Build #169253 failed 1e08998
💛 Build #168750 was flaky ca94111
💔 Build #168700 failed 16bb946

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

jloleysens

Did not run locally, but overall LGTM!

jloleysens · 2023-10-19T09:34:01Z

test/plugin_functional/test_suites/core_plugins/application_leave_confirm.ts

+        });
+        const currentUrl = await browser.getCurrentUrl();
+        expect(currentUrl).to.contain('appleave2');
+        await PageObjects.common.navigateToApp('home');


yikes 😄 , do you know what the source is of the rendering frenzy?

dokmic

Great job fixing this 👍
This PR also resolves #75963, as it has the same origin as #166838.

kibanamachine · 2023-10-19T13:10:47Z

💚 All backports created successfully

Status	Branch	Result
✅	8.11

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

Questions ?

Please refer to the Backport tool documentation

…stic#168741) fix elastic#166838 fix elastic#166893 fix elastic#75963 I modified the deep links tests because the side nav was overlaying the in-app nav. While, theoretically, the side nav should work for the tests, it tends to be flaky. I added logs for the url so that if these tests do fail, we'll have a bit more data to go on for debugging. These tests pass on local test runs. latest flaky test runs (50): https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3604 - [X] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Kibana Machine <[email protected]> (cherry picked from commit e811b62)

…ts (#168741) (#169395) # Backport This will backport the following commits from `main` to `8.11`: - [unskips application leave confirm & application deep links tests (#168741)](#168741)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Christiane (Tina) Heiligers <[email protected]>

…0228) ## Summary Attempt at fixing #166893 PR [#168741](#168741) forgot to update one of the tests to **exclusively** use `navigateToAppLinks`. Thus, the impacted test had a duplicated navigation logic. This PR: * removes that unintended call (this should hopefully fix flakiness). * simplifies and unifies test logic, improving readability. Flaky test runner pipeline - 100x 🟢 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3840

…stic#170228) ## Summary Attempt at fixing elastic#166893 PR [elastic#168741](elastic#168741) forgot to update one of the tests to **exclusively** use `navigateToAppLinks`. Thus, the impacted test had a duplicated navigation logic. This PR: * removes that unintended call (this should hopefully fix flakiness). * simplifies and unifies test logic, improving readability. Flaky test runner pipeline - 100x 🟢 https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/3840

unskips tests failing in CI

0feeb53

TinaHeiligers added backport Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v8.11.0 labels Oct 12, 2023

Merge branch 'main' into skipped-tests-app-leave-deep-links

a49010c

TinaHeiligers commented Oct 12, 2023

View reviewed changes

Merge branch 'main' into skipped-tests-app-leave-deep-links

5fc9a09

Merge branch 'main' into skipped-tests-app-leave-deep-links

c61ef3d

TinaHeiligers commented Oct 15, 2023

View reviewed changes

TinaHeiligers and others added 3 commits October 15, 2023 15:17

Use consistent nav link title, name

4713ed5

adds debug log for current URL

15096a6

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

4c080ea

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

kibanamachine and others added 2 commits October 16, 2023 10:14

Merge branch 'main' into skipped-tests-app-leave-deep-links

59995c8

Improves tests

cc6f57f

Merge branch 'main' into skipped-tests-app-leave-deep-links

6f2f392

TinaHeiligers marked this pull request as ready for review October 16, 2023 23:16

TinaHeiligers requested a review from a team as a code owner October 16, 2023 23:16

Merge branch 'main' into skipped-tests-app-leave-deep-links

41e0dcb

kibanamachine and others added 3 commits October 16, 2023 19:21

Merge branch 'main' into skipped-tests-app-leave-deep-links

06e050d

Ensure app navigates to app home url

eb25bbc

Swap test order to force browser response

d0ae7d2

Merge branch 'skipped-tests-app-leave-deep-links' of github.com:TinaH…

77ceb45

…eiligers/kibana into skipped-tests-app-leave-deep-links

kibanamachine and others added 4 commits October 17, 2023 15:07

Merge branch 'main' into skipped-tests-app-leave-deep-links

ca94111

check exists for the correct modal test subject

18abd6b

Merge branch 'skipped-tests-app-leave-deep-links' of github.com:TinaH…

fac7c79

…eiligers/kibana into skipped-tests-app-leave-deep-links

Refactors tests, helpers and plugin to take delayed rendering into ac…

1e08998

…count. The delays stem from multiple background tasks that run simultaneously and cannot be turned off

TinaHeiligers commented Oct 18, 2023

View reviewed changes

TinaHeiligers added 4 commits October 18, 2023 15:56

Increase waitFor timeout more

473c1fd

Change data test subject

d725990

increase timeout even more

81579c4

Move nav click event into retry

8d1c8a2

Merge branch 'main' into skipped-tests-app-leave-deep-links

0f5277f

jloleysens approved these changes Oct 19, 2023

View reviewed changes

dokmic approved these changes Oct 19, 2023

View reviewed changes

TinaHeiligers merged commit e811b62 into elastic:main Oct 19, 2023

kibanamachine added the v8.12.0 label Oct 19, 2023

kibanamachine mentioned this pull request Oct 19, 2023

[8.11] unskips application leave confirm & application deep links tests (#168741) #169395

Merged

gsoldevila mentioned this pull request Oct 31, 2023

[application_deep_links] Unify / improve navigation between apps #170228

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unskips application leave confirm & application deep links tests #168741

unskips application leave confirm & application deep links tests #168741

TinaHeiligers commented Oct 12, 2023 •

edited by kibanamachine

Loading

TinaHeiligers commented Oct 12, 2023

TinaHeiligers left a comment •

edited

Loading

TinaHeiligers Oct 12, 2023 •

edited

Loading

TinaHeiligers commented Oct 13, 2023

TinaHeiligers commented Oct 14, 2023

TinaHeiligers Oct 15, 2023 •

edited

Loading

TinaHeiligers commented Oct 16, 2023

TinaHeiligers commented Oct 16, 2023

elasticmachine commented Oct 16, 2023

TinaHeiligers commented Oct 16, 2023

TinaHeiligers commented Oct 17, 2023

TinaHeiligers commented Oct 17, 2023

TinaHeiligers commented Oct 17, 2023

TinaHeiligers left a comment

TinaHeiligers Oct 17, 2023

TinaHeiligers Oct 18, 2023

TinaHeiligers Oct 18, 2023

TinaHeiligers Oct 18, 2023

TinaHeiligers Oct 18, 2023

TinaHeiligers Oct 18, 2023

TinaHeiligers Oct 18, 2023

TinaHeiligers Oct 18, 2023

TinaHeiligers Oct 18, 2023

jloleysens Oct 19, 2023

TinaHeiligers Oct 19, 2023

TinaHeiligers commented Oct 19, 2023

kibana-ci commented Oct 19, 2023

jloleysens left a comment

jloleysens Oct 19, 2023

dokmic left a comment

kibanamachine commented Oct 19, 2023

unskips application leave confirm & application deep links tests #168741

unskips application leave confirm & application deep links tests #168741

Conversation

TinaHeiligers commented Oct 12, 2023 • edited by kibanamachine Loading

TinaHeiligers commented Oct 12, 2023

TinaHeiligers left a comment • edited Loading

Choose a reason for hiding this comment

TinaHeiligers Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

TinaHeiligers commented Oct 13, 2023

TinaHeiligers commented Oct 14, 2023

TinaHeiligers Oct 15, 2023 • edited Loading

Choose a reason for hiding this comment

TinaHeiligers commented Oct 16, 2023

TinaHeiligers commented Oct 16, 2023

elasticmachine commented Oct 16, 2023

TinaHeiligers commented Oct 16, 2023

TinaHeiligers commented Oct 17, 2023

TinaHeiligers commented Oct 17, 2023

TinaHeiligers commented Oct 17, 2023

TinaHeiligers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TinaHeiligers commented Oct 19, 2023

kibana-ci commented Oct 19, 2023

💚 Build Succeeded

Metrics [docs]

History

jloleysens left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dokmic left a comment

Choose a reason for hiding this comment

kibanamachine commented Oct 19, 2023

💚 All backports created successfully

Questions ?

TinaHeiligers commented Oct 12, 2023 •

edited by kibanamachine

Loading

TinaHeiligers left a comment •

edited

Loading

TinaHeiligers Oct 12, 2023 •

edited

Loading

TinaHeiligers Oct 15, 2023 •

edited

Loading