From 83d4b6775dfc59dffa3f07a4f77767216fbc08ab Mon Sep 17 00:00:00 2001 From: Marina Mosti <mmosti@gitlab.com> Date: Tue, 25 Feb 2025 13:35:15 +0000 Subject: [PATCH] test: fix vue 3 tests router spec --- doc/development/testing_guide/testing_vue3.md | 10 +++++ .../assets/javascripts/ci/secrets/router.js | 2 + ee/spec/frontend/ci/secrets/router_spec.js | 42 ++++++++++--------- scripts/frontend/quarantined_vue3_specs.txt | 1 - 4 files changed, 34 insertions(+), 21 deletions(-) diff --git a/doc/development/testing_guide/testing_vue3.md b/doc/development/testing_guide/testing_vue3.md index dd7de5706f96e..d93d749382b8f 100644 --- a/doc/development/testing_guide/testing_vue3.md +++ b/doc/development/testing_guide/testing_vue3.md @@ -308,6 +308,16 @@ wrapper.findComponent(GlKeysetNavigation).vm.$emit('push'); await waitForPromises() ``` +#### Navigation guards + +Navigation guards must call their third argument `next` exactly once in any given pass through the navigation guard. This is necessary in both `vue-router@3` and `vue-router@4`, but is more important in `vue-router@4` as all navigations are asynchronous and must be awaited. + +Failing to call `next` can produce [hard to debug errors](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/181946#note_2355522500), e.g.: + +```shell +Timeout - Async callback was not invoked within the 5000 ms timeout specified by jest.setTimeout +``` + #### Debugging More often than not you will find yourself running into cryptic errors like the one below. diff --git a/ee/app/assets/javascripts/ci/secrets/router.js b/ee/app/assets/javascripts/ci/secrets/router.js index fd61e8a13d1d6..31219f86e9002 100644 --- a/ee/app/assets/javascripts/ci/secrets/router.js +++ b/ee/app/assets/javascripts/ci/secrets/router.js @@ -23,6 +23,7 @@ export const initNavigationGuards = ({ router, base, props, location }) => { router.beforeEach((to, _, next) => { if (to.name === INDEX_ROUTE_NAME) { visitUrl(props.projectSecretsSettingsPath); + next(); } else next(); }); } else { @@ -31,6 +32,7 @@ export const initNavigationGuards = ({ router, base, props, location }) => { router.beforeEach((to, _, next) => { if (to.name !== INDEX_ROUTE_NAME) { visitUrl(base + to.fullPath); + next(); } else next(); }); } diff --git a/ee/spec/frontend/ci/secrets/router_spec.js b/ee/spec/frontend/ci/secrets/router_spec.js index 70ae856252ee4..34f10cfe36998 100644 --- a/ee/spec/frontend/ci/secrets/router_spec.js +++ b/ee/spec/frontend/ci/secrets/router_spec.js @@ -9,6 +9,7 @@ import SecretsTable from 'ee/ci/secrets/components/secrets_table/secrets_table.v import createRouter, { initNavigationGuards } from 'ee/ci/secrets/router'; import { ENTITY_GROUP } from 'ee/ci/secrets/constants'; import SecretsApp from 'ee//ci/secrets/components/secrets_app.vue'; +import { getMatchedComponents } from '~/lib/utils/vue3compat/vue_router'; Vue.use(VueRouter); jest.mock('~/lib/utils/url_utility', () => ({ @@ -45,10 +46,10 @@ describe('Secrets router', () => { }); }; - const createSecretsApp = ({ route, props, location = defaultLocation } = {}) => { + const createSecretsApp = async ({ route, props, location = defaultLocation } = {}) => { router = createRouter(base, props, location); if (route) { - router.push(route); + await router.push(route); } wrapper = mountExtended(SecretsApp, { @@ -79,20 +80,21 @@ describe('Secrets router', () => { ${'/secretName/edit'} | ${'SecretFormWrapper'} | ${[SecretFormWrapper]} `('uses $componentNames for path "$path"', ({ path, components }) => { router = createRouter(base, groupProps, defaultLocation); + const componentsForRoute = getMatchedComponents(router, path); - expect(router.getMatchedComponents(path)).toStrictEqual(components); + expect(componentsForRoute).toStrictEqual(components); }); it.each` path | redirect - ${'/secretName'} | ${'/secretName/details'} - ${'/secretName/unknownroute'} | ${'/'} + ${'/secretName'} | ${'details'} + ${'/secretName/unknownroute'} | ${'index'} `('redirects from $path to $redirect', async ({ path, redirect }) => { router = createRouter(base, groupProps, defaultLocation); await router.push(path); - expect(router.currentRoute.path).toBe(redirect); + expect(router.currentRoute.name).toBe(redirect); }); describe.each` @@ -100,8 +102,8 @@ describe('Secrets router', () => { ${'group'} | ${groupProps} | ${groupProps.groupPath} ${'project'} | ${projectProps} | ${projectProps.projectPath} `('$entity secrets form', ({ entity, props, fullPath }) => { - it('provides the correct props when visiting the index', () => { - createSecretsApp({ route: '/', props, location: defaultLocation }); + it('provides the correct props when visiting the index', async () => { + await createSecretsApp({ route: '/', props, location: defaultLocation }); expect(wrapper.findComponent(SecretsTable).props()).toMatchObject({ isGroup: entity === ENTITY_GROUP, @@ -109,8 +111,8 @@ describe('Secrets router', () => { }); }); - it('provides the correct props when visiting the create form', () => { - createSecretsApp({ route: '/new', props, location: defaultLocation }); + it('provides the correct props when visiting the create form', async () => { + await createSecretsApp({ route: '/new', props, location: defaultLocation }); expect(wrapper.findComponent(SecretFormWrapper).props()).toMatchObject({ entity, @@ -118,8 +120,8 @@ describe('Secrets router', () => { }); }); - it('provides the correct props when visiting the edit form', () => { - createSecretsApp({ route: editRoute, props, location: defaultLocation }); + it('provides the correct props when visiting the edit form', async () => { + await createSecretsApp({ route: editRoute, props, location: defaultLocation }); expect(wrapper.findComponent(SecretFormWrapper).props()).toMatchObject({ entity, @@ -140,16 +142,16 @@ describe('Secrets router', () => { createRouterWithNavigationGuards(secretsBase, projectProps, settingsLocation); }); - it('navigating within the index route does not redirect', () => { - router.push('/?page=2'); + it('navigating within the index route does not redirect', async () => { + await router.push('/?page=2'); expect(visitUrl).not.toHaveBeenCalled(); }); it.each([editRoute, '/new', '/secretName/details', '/secretName/edit'])( 'navigating to the non-index route %s redirects to the appropriate route in /-/secrets', - (route) => { - router.push(route); + async (route) => { + await router.push(route); expect(visitUrl).toHaveBeenCalledWith(expect.stringContaining(secretsBase)); }, @@ -163,15 +165,15 @@ describe('Secrets router', () => { it.each([editRoute, '/new', '/secretName/details', '/secretName/edit'])( 'navigating to the non-index route %s does not redirect', - (route) => { - router.push(route); + async (route) => { + await router.push(route); expect(visitUrl).not.toHaveBeenCalled(); }, ); - it('navigating to the index route redirects to /-/settings/ci_cd', () => { - router.push('/'); + it('navigating to the index route redirects to /-/settings/ci_cd', async () => { + await router.push('/'); expect(visitUrl).toHaveBeenCalledWith('/path/to/project/-/settings/ci_cd'); }); diff --git a/scripts/frontend/quarantined_vue3_specs.txt b/scripts/frontend/quarantined_vue3_specs.txt index 47915ae1898ee..c5f79e64f5fe5 100644 --- a/scripts/frontend/quarantined_vue3_specs.txt +++ b/scripts/frontend/quarantined_vue3_specs.txt @@ -31,7 +31,6 @@ ee/spec/frontend/boards/components/epics_swimlanes_spec.js ee/spec/frontend/ci/pipeline_details/header/pipeline_header_spec.js ee/spec/frontend/ci/runner/components/runner_usage_spec.js ee/spec/frontend/ci/secrets/components/secrets_breadcrumbs_spec.js -ee/spec/frontend/ci/secrets/router_spec.js ee/spec/frontend/compliance_dashboard/components/frameworks_report/edit_framework/components/policies_section_spec.js ee/spec/frontend/compliance_dashboard/components/frameworks_report/report_spec.js ee/spec/frontend/dependencies/components/app_spec.js -- GitLab