Skip to content
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.

Sign up for GitHub

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

unskips application leave confirm & application deep links tests #168741

Merged
merged 26 commits into from
Oct 19, 2023
Merged

unskips application leave confirm & application deep links tests #168741

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
0feeb53
unskips tests failing in CI
TinaHeiligers Oct 12, 2023
a49010c
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 12, 2023
5fc9a09
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 13, 2023
c61ef3d
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 14, 2023
4713ed5
Use consistent nav link title, name
TinaHeiligers Oct 15, 2023
15096a6
adds debug log for current URL
TinaHeiligers Oct 15, 2023
4c080ea
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Oct 16, 2023
59995c8
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 16, 2023
cc6f57f
Improves tests
TinaHeiligers Oct 16, 2023
6f2f392
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 16, 2023
41e0dcb
Merge branch 'main' into skipped-tests-app-leave-deep-links
TinaHeiligers Oct 16, 2023
06e050d
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 16, 2023
eb25bbc
Ensure app navigates to app home url
TinaHeiligers Oct 17, 2023
d0ae7d2
Swap test order to force browser response
TinaHeiligers Oct 17, 2023
16bb946
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 17, 2023
cd2f7a5
updates test plugin and page objects methods to accomodate latest beh…
TinaHeiligers Oct 17, 2023
77ceb45
Merge branch 'skipped-tests-app-leave-deep-links' of github.com:TinaH…
TinaHeiligers Oct 17, 2023
ca94111
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 17, 2023
18abd6b
check exists for the correct modal test subject
TinaHeiligers Oct 17, 2023
fac7c79
Merge branch 'skipped-tests-app-leave-deep-links' of github.com:TinaH…
TinaHeiligers Oct 17, 2023
1e08998
Refactors tests, helpers and plugin to take delayed rendering into ac…
TinaHeiligers Oct 18, 2023
473c1fd
Increase waitFor timeout more
TinaHeiligers Oct 18, 2023
d725990
Change data test subject
TinaHeiligers Oct 18, 2023
81579c4
increase timeout even more
TinaHeiligers Oct 18, 2023
8d1c8a2
Move nav click event into retry
TinaHeiligers Oct 18, 2023
0f5277f
Merge branch 'main' into skipped-tests-app-leave-deep-links
kibanamachine Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/plugin_functional/plugins/core_plugin_deep_links/public/application.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const PageA = () => (
<EuiPageHeader>
<EuiPageHeaderSection>
<EuiTitle size="l">
<h1>DL Page A</h1>
<h1>DL page A</h1>
</EuiTitle>
</EuiPageHeaderSection>
</EuiPageHeader>
Expand All @@ -70,7 +70,7 @@ const PageB = () => (
<EuiPageHeader>
<EuiPageHeaderSection>
<EuiTitle size="l">
<h1>DL Page B</h1>
<h1>DL page B</h1>
</EuiTitle>
</EuiPageHeaderSection>
</EuiPageHeader>
Expand Down
6 changes: 3 additions & 3 deletions test/plugin_functional/plugins/core_plugin_deep_links/public/plugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class CorePluginDeepLinksPlugin
},
{
id: 'pageA',
title: 'DL Page A',
title: 'DL page A',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

path: '/page-a',
navLinkStatus: AppNavLinkStatus.visible,
},
Expand All @@ -39,15 +39,15 @@ export class CorePluginDeepLinksPlugin
deepLinks: [
{
id: 'pageB',
title: 'DL Page B',
title: 'DL page B',
path: '/page-b',
navLinkStatus: AppNavLinkStatus.visible,
},
],
},
{
id: 'pageC',
title: 'DL Page C',
title: 'DL page C',
path: '/page-c',
// navLinkStatus hidden by default
},
Expand Down
30 changes: 20 additions & 10 deletions test/plugin_functional/test_suites/core_plugins/application_deep_links.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide
const testSubjects = getService('testSubjects');
const retry = getService('retry');
const esArchiver = getService('esArchiver');
const log = getService('log');

const loadingScreenNotShown = async () =>
expect(await testSubjects.exists('kbnLoadingMessage')).to.be(false);
Expand All @@ -35,12 +36,20 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide
const waitForUrlToBe = (pathname?: string, search?: string) => {
const expectedUrl = getKibanaUrl(pathname, search);
return retry.waitFor(`Url to be ${expectedUrl}`, async () => {
return (await browser.getCurrentUrl()) === expectedUrl;
const currentUrl = await browser.getCurrentUrl();
log?.debug(`waiting for currentUrl ${currentUrl} to be expectedUrl ${expectedUrl}`);
return currentUrl === expectedUrl;
});
};

// Failing: See https://github.com/elastic/kibana/issues/166893
describe.skip('application deep links navigation', function describeDeepLinksTests() {
const navigateToAppLinks = async (subject: string) => {
if (!(await testSubjects.exists(subject))) {
log.debug(`side nav in app not in DOM`);
}
await testSubjects.click(subject);
};

describe('application deep links navigation', function describeDeepLinksTests() {
Copy link
Contributor Author

@TinaHeiligers TinaHeiligers Oct 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

before(async () => {
await esArchiver.emptyKibanaIndex();
await PageObjects.common.navigateToApp('dl');
Expand All @@ -51,28 +60,29 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide
});

it('should navigate to page A when navlink is clicked', async () => {
await appsMenu.clickLink('DL Page A');
await navigateToAppLinks('dlNavPageA');
await waitForUrlToBe('/app/dl/page-a');
await loadingScreenNotShown();
await testSubjects.existOrFail('dlAppPageA');
await testSubjects.existOrFail('dlNavPageA');
});

it('should be able to use the back button to navigate back to previous deep link', async () => {
await browser.goBack();
await waitForUrlToBe('/app/dl/home');
await loadingScreenNotShown();
await testSubjects.existOrFail('dlAppHome');
await testSubjects.existOrFail('dlNavHome');
});

it('should navigate to nested page B when navlink is clicked', async () => {
await appsMenu.clickLink('DL Page B');
await navigateToAppLinks('dlNavDeepPageB');
await waitForUrlToBe('/app/dl/page-b');
await loadingScreenNotShown();
await testSubjects.existOrFail('dlAppPageB');
await testSubjects.existOrFail('dlNavDeepPageB');
});

it('should navigate to Home when navlink is clicked inside the defined category group', async () => {
await appsMenu.clickLink('DL Home', { category: 'securitySolution' });
await navigateToAppLinks('dlAppHome');
await waitForUrlToBe('/app/dl/home');
await loadingScreenNotShown();
await testSubjects.existOrFail('dlAppHome');
Expand All @@ -82,14 +92,14 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide
await testSubjects.click('dlNavDeepPageB');
await waitForUrlToBe('/app/dl/page-b');
await loadingScreenNotShown();
await testSubjects.existOrFail('dlAppPageB');
await testSubjects.existOrFail('dlNavDeepPageB');
});

it('should navigate to nested page A using navigateToApp deepLinkId', async () => {
await testSubjects.click('dlNavDeepPageAById');
await waitForUrlToBe('/app/dl/page-a');
await loadingScreenNotShown();
await testSubjects.existOrFail('dlAppPageA');
await testSubjects.existOrFail('dlNavPageA');
});

it('should not display hidden deep links', async () => {
Expand Down
41 changes: 30 additions & 11 deletions test/plugin_functional/test_suites/core_plugins/application_leave_confirm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,45 @@ export default function ({ getService, getPageObjects }: PluginFunctionalProvide
const browser = getService('browser');
const appsMenu = getService('appsMenu');
const testSubjects = getService('testSubjects');
const log = getService('log');
const retry = getService('retry');

// Failing: See https://github.com/elastic/kibana/issues/75963
// Failing: See https://github.com/elastic/kibana/issues/166838
describe.skip('application using leave confirmation', () => {
const waitForUrlToBe = (pathname?: string, search?: string) => {
const expectedUrl = getKibanaUrl(pathname, search);
return retry.waitFor(`Url to be ${expectedUrl}`, async () => {
const currentUrl = await browser.getCurrentUrl();
log.debug(`waiting for currentUrl ${currentUrl} to be expectedUrl ${expectedUrl}`);
return currentUrl === expectedUrl;
});
};

describe('application using leave confirmation', () => {
Copy link
Contributor Author

@TinaHeiligers TinaHeiligers Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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 () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

await PageObjects.common.navigateToApp('appleave1');
await appsMenu.clickLink('AppLeave 2');
await waitForUrlToBe('/app/appleave1');

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

const currentUrl = await browser.getCurrentUrl();
const kibanaUrl = getKibanaUrl('/app/appleave2');
log.debug(`currentUrl ${currentUrl} kibanaUrl ${kibanaUrl}`);
expect(currentUrl).to.eql(kibanaUrl);
});
it('allows navigation if user click confirm on the confirmation dialog', async () => {
it('prevents navigation if user click cancel on the confirmation dialog', async () => {
await PageObjects.common.navigateToApp('appleave1');
await appsMenu.clickLink('AppLeave 2');
await waitForUrlToBe('/app/appleave1');

await appsMenu.clickLink('AppLeave 2');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

await testSubjects.existOrFail('appLeaveConfirmModal');
await PageObjects.common.clickConfirmOnModal();
expect(await browser.getCurrentUrl()).to.eql(getKibanaUrl('/app/appleave2'));
await PageObjects.common.clickCancelOnModal(false);

const currentUrl = await browser.getCurrentUrl();
const kibanaUrl = getKibanaUrl('/app/appleave1');
log.debug(`currentUrl ${currentUrl} kibanaUrl ${kibanaUrl}`);
expect(currentUrl).to.eql(kibanaUrl);
});
});
});
Expand Down