Merge branch '336917-reset-location-before-each-test' into 'master'

Reset window location before every test [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!67353

doc/development/testing_guide/frontend_testing.md

@@ -425,6 +425,10 @@ it('does something', () => {
 
 ### Mocking the current location in Jest
 
+NOTE:
+The value of `window.location.href` is reset before every test to avoid earlier
+tests affecting later ones.
+
 If your tests require `window.location.href` to take a particular value, use
 the `setWindowLocation` helper:
 
@@ -442,7 +446,6 @@ it('passes', () => {
 });
 ```
 
-NOTE:
 To modify only the hash, use either the `setWindowLocation` helper, or assign
 directly to `window.location.hash`, e.g.:
 

ee/spec/frontend/on_demand_scans/components/on_demand_scans_form_spec.js

@@ -610,8 +610,8 @@ describe('OnDemandScansForm', () => {
       createShallowComponent();
       await wrapper.vm.$nextTick();
 
-      expect(findScannerProfilesSelector().attributes('value')).toBe(scannerProfile.id);
-      expect(findSiteProfilesSelector().attributes('value')).toBe(siteProfile.id);
+      expect(findScannerProfilesSelector().attributes('value')).toBe(dastScan.scannerProfileId);
+      expect(findSiteProfilesSelector().attributes('value')).toBe(dastScan.siteProfileId);
     });
   });
 

ee/spec/frontend/requirements/components/requirements_root_spec.js

@@ -24,6 +24,7 @@ import { mockTracking, unmockTracking } from 'helpers/tracking_helper';
 import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import waitForPromises from 'helpers/wait_for_promises';
 import createFlash from '~/flash';
+import { queryToObject } from '~/lib/utils/url_utility';
 import FilteredSearchBarRoot from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue';
 
 import {
@@ -904,9 +905,12 @@ describe('RequirementsRoot', () => {
 
         expect(wrapper.vm.prevPageCursor).toBe('');
         expect(wrapper.vm.nextPageCursor).toBe(mockPageInfo.endCursor);
-        expect(global.window.location.href).toBe(
-          `${TEST_HOST}/?page=2&state=opened&sort=created_desc&next=${mockPageInfo.endCursor}`,
-        );
+        expect(queryToObject(window.location.search)).toEqual({
+          page: '2',
+          state: 'opened',
+          sort: 'created_desc',
+          next: mockPageInfo.endCursor,
+        });
         expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_navigation', {
           label: 'next',
         });
@@ -926,9 +930,12 @@ describe('RequirementsRoot', () => {
 
         expect(wrapper.vm.prevPageCursor).toBe(mockPageInfo.startCursor);
         expect(wrapper.vm.nextPageCursor).toBe('');
-        expect(global.window.location.href).toBe(
-          `${TEST_HOST}/?page=1&state=opened&sort=created_desc&prev=${mockPageInfo.startCursor}`,
-        );
+        expect(queryToObject(window.location.search)).toEqual({
+          page: '1',
+          state: 'opened',
+          sort: 'created_desc',
+          prev: mockPageInfo.startCursor,
+        });
         expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_navigation', {
           label: 'prev',
         });

spec/frontend/__helpers__/mock_window_location_helper.js

@@ -10,7 +10,7 @@
  */
 const useMockLocation = (fn) => {
   const origWindowLocation = window.location;
-  let currentWindowLocation;
+  let currentWindowLocation = origWindowLocation;
 
   Object.defineProperty(window, 'location', {
     get: () => currentWindowLocation,

spec/frontend/__helpers__/set_window_location_helper_spec.js

 import setWindowLocation from './set_window_location_helper';
+import { TEST_HOST } from './test_constants';
 
 describe('helpers/set_window_location_helper', () => {
   const originalLocation = window.location.href;
@@ -103,4 +104,30 @@ describe('helpers/set_window_location_helper', () => {
       );
     });
   });
+
+  // This set of tests relies on Jest executing tests in source order, which is
+  // at the time of writing the only order they will execute, by design.
+  // See https://github.com/facebook/jest/issues/4386 for more details.
+  describe('window.location resetting by global beforeEach', () => {
+    const overridden = 'https://gdk.test:1234/';
+    const initial = `${TEST_HOST}/`;
+
+    it('works before an override', () => {
+      expect(window.location.href).toBe(initial);
+    });
+
+    describe('overriding', () => {
+      beforeEach(() => {
+        setWindowLocation(overridden);
+      });
+
+      it('works', () => {
+        expect(window.location.href).toBe(overridden);
+      });
+    });
+
+    it('works after an override', () => {
+      expect(window.location.href).toBe(initial);
+    });
+  });
 });

spec/frontend/diffs/components/app_spec.js

@@ -259,6 +259,8 @@ describe('diffs/components/app', () => {
   });
 
   it('marks current diff file based on currently highlighted row', async () => {
+    window.location.hash = 'ABC_123';
+
     createComponent({
       shouldShow: true,
     });
@@ -429,9 +431,8 @@ describe('diffs/components/app', () => {
       jest.spyOn(wrapper.vm, 'refetchDiffData').mockImplementation(() => {});
       jest.spyOn(wrapper.vm, 'adjustView').mockImplementation(() => {});
     };
-    const location = window.location.href;
 
-    beforeAll(() => {
+    beforeEach(() => {
       setWindowLocation(COMMIT_URL);
       document.title = 'My Title';
     });
@@ -440,10 +441,6 @@ describe('diffs/components/app', () => {
       jest.spyOn(urlUtils, 'updateHistory');
     });
 
-    afterAll(() => {
-      setWindowLocation(location);
-    });
-
     it('when the commit changes and the app is not loading it should update the history, refetch the diff data, and update the view', async () => {
       createComponent({}, ({ state }) => {
         state.diffs.commit = { ...state.diffs.commit, id: 'OLD' };

spec/frontend/monitoring/utils_spec.js

@@ -448,7 +448,7 @@ describe('monitoring/utils', () => {
       input                                                               | urlParams
       ${[]}                                                               | ${''}
       ${[{ name: 'env', value: 'prod' }]}                                 | ${'?var-env=prod'}
-      ${[{ name: 'env1', value: 'prod' }, { name: 'env2', value: null }]} | ${'?var-env=prod&var-env1=prod'}
+      ${[{ name: 'env1', value: 'prod' }, { name: 'env2', value: null }]} | ${'?var-env1=prod'}
     `(
       'setCustomVariablesFromUrl updates history with query "$urlParams" with input $input',
       ({ input, urlParams }) => {

spec/frontend/test_setup.js

@@ -3,6 +3,8 @@ import * as jqueryMatchers from 'custom-jquery-matchers';
 import Vue from 'vue';
 import 'jquery';
 import { setGlobalDateToFakeDate } from 'helpers/fake_date';
+import setWindowLocation from 'helpers/set_window_location_helper';
+import { TEST_HOST } from 'helpers/test_constants';
 import Translate from '~/vue_shared/translate';
 import { getJSONFixture, loadHTMLFixture, setHTMLFixture } from './__helpers__/fixtures';
 import { initializeTestTimeout } from './__helpers__/timeout';
@@ -88,8 +90,13 @@ Object.assign(global, {
   },
 });
 
-// make sure that each test actually tests something
-// see https://jestjs.io/docs/en/expect#expecthasassertions
 beforeEach(() => {
+  // make sure that each test actually tests something
+  // see https://jestjs.io/docs/en/expect#expecthasassertions
   expect.hasAssertions();
+
+  // Reset the mocked window.location. This ensures tests don't interfere with
+  // each other, and removes the need to tidy up if it was changed for a given
+  // test.
+  setWindowLocation(TEST_HOST);
 });

spec/frontend/vue_shared/components/url_sync_spec.js

 import { shallowMount } from '@vue/test-utils';
-import setWindowLocation from 'helpers/set_window_location_helper';
 import { historyPushState } from '~/lib/utils/common_utils';
 import { mergeUrlParams } from '~/lib/utils/url_utility';
 import UrlSyncComponent from '~/vue_shared/components/url_sync.vue';
@@ -15,9 +14,6 @@ jest.mock('~/lib/utils/common_utils', () => ({
 describe('url sync component', () => {
   let wrapper;
   const mockQuery = { group_id: '5014437163714', project_ids: ['5014437608314'] };
-  const TEST_HOST = 'http://testhost/';
-
-  setWindowLocation(TEST_HOST);
 
   const findButton = () => wrapper.find('button');
 
@@ -35,7 +31,9 @@ describe('url sync component', () => {
 
   const expectUrlSync = (query, times, mergeUrlParamsReturnValue) => {
     expect(mergeUrlParams).toHaveBeenCalledTimes(times);
-    expect(mergeUrlParams).toHaveBeenCalledWith(query, TEST_HOST, { spreadArrays: true });
+    expect(mergeUrlParams).toHaveBeenCalledWith(query, window.location.href, {
+      spreadArrays: true,
+    });
 
     expect(historyPushState).toHaveBeenCalledTimes(times);
     expect(historyPushState).toHaveBeenCalledWith(mergeUrlParamsReturnValue);