Maintainer feedback: Refactor tests

Leverage recent changes made to the specs to get closer to
replicating actual user interaction, instead of reaching into
implementation details, such as mutating a view model.

ee/app/assets/javascripts/license_compliance/components/app.vue

@@ -137,18 +137,18 @@ export default {
     <!-- TODO: Remove feature flag -->
     <template v-if="hasLicensePolicyList">
       <gl-tabs v-model="tabIndex" content-class="pt-0">
-        <gl-tab>
+        <gl-tab data-testid="licensesTab">
           <template #title>
-            {{ s__('Licenses|Detected in Project') }}
+            <span data-testid="licensesTabTitle">{{ s__('Licenses|Detected in Project') }}</span>
             <gl-badge pill>{{ licenseCount }}</gl-badge>
           </template>
 
           <detected-licenses-table />
         </gl-tab>
 
-        <gl-tab>
+        <gl-tab data-testid="policiesTab">
           <template #title>
-            {{ s__('Licenses|Policies') }}
+            <span data-testid="policiesTabTitle">{{ s__('Licenses|Policies') }}</span>
             <gl-badge pill>{{ policyCount }}</gl-badge>
           </template>
 

ee/spec/frontend/license_compliance/components/app_spec.js

@@ -81,13 +81,11 @@ const createComponent = ({ state, props, options }) => {
     },
     ...options,
     store: fakeStore,
-    stubs: {
-      GlTabs,
-      GlTab,
-    },
   });
 };
 
+const findByTestId = testId => wrapper.find(`[data-testid="${testId}"]`);
+
 describe('Project Licenses', () => {
   afterEach(() => {
     wrapper.destroy();
@@ -188,66 +186,62 @@ describe('Project Licenses', () => {
       expect(wrapper.findAll(GlTab)).toHaveLength(2);
     });
 
+    it('it renders the "Detected in project" table', () => {
+      expect(wrapper.find(DetectedLicensesTable).exists()).toBe(true);
+    });
+
+    it('it renders the "Policies" table', () => {
+      expect(wrapper.find(LicenseManagement).exists()).toBe(true);
+    });
+
+    it('renders the pipeline info', () => {
+      expect(wrapper.find(PipelineInfo).exists()).toBe(true);
+    });
+
     describe.each`
-      givenLocationHash | expectedTabIndex
-      ${'licenses'}     | ${0}
-      ${'policies'}     | ${1}
-      ${'foo'}          | ${0}
-      ${'bar'}          | ${0}
+      givenLocationHash | expectedActiveTab
+      ${'#licenses'}    | ${'licenses'}
+      ${'#policies'}    | ${'policies'}
     `(
-      'when the url contains $givenLocationHash hash',
-      ({ givenLocationHash, expectedTabIndex }) => {
+      'when window.location contains the hash "$givenLocationHash"',
+      ({ givenLocationHash, expectedActiveTab }) => {
+        const originalLocation = window.location;
+
         beforeEach(() => {
-          setWindowLocation({
-            href: `${TEST_HOST}#${givenLocationHash}`,
-          });
+          setWindowLocation(`http://foo.com/index${givenLocationHash}`);
 
           createComponent({
             state: {
               initialized: true,
+              isLoading: false,
+              licenses: [
+                {
+                  name: 'MIT',
+                  classification: LICENSE_APPROVAL_CLASSIFICATION.DENIED,
+                  components: [],
+                },
+              ],
+              pageInfo: 1,
             },
             options: {
               provide: {
                 glFeatures: { licensePolicyList: true },
               },
+              mount: true,
             },
           });
         });
 
-        it(`sets the tabIndex to be "${expectedTabIndex}"`, () => {
-          expect(wrapper.find(GlTabs).attributes('value')).toBe(`${expectedTabIndex}`);
+        afterEach(() => {
+          window.location = originalLocation;
         });
-      },
-    );
 
-    it.each`
-      givenTabIndex | expectedLocationHash
-      ${0}          | ${'licenses'}
-      ${1}          | ${'policies'}
-    `(
-      'sets the location hash to "tabName" when the corresponding tab is activated',
-      ({ givenTabIndex, expectedLocationHash }) => {
-        wrapper.setData({ tabIndex: givenTabIndex });
-        wrapper.vm.$forceUpdate();
-
-        return wrapper.vm.$nextTick().then(() => {
-          expect(window.location.hash).toBe(expectedLocationHash);
+        it(`sets the active tab to be "${expectedActiveTab}"`, () => {
+          expect(findByTestId(`${expectedActiveTab}Tab`).classes()).toContain('active');
         });
       },
     );
 
-    it('it renders the "Detected in project" table', () => {
-      expect(wrapper.find(DetectedLicensesTable).exists()).toBe(true);
-    });
-
-    it('it renders the "Policies" table', () => {
-      expect(wrapper.find(LicenseManagement).exists()).toBe(true);
-    });
-
-    it('renders the pipeline info', () => {
-      expect(wrapper.find(PipelineInfo).exists()).toBe(true);
-    });
-
     describe('when the tabs are rendered', () => {
       const pageInfo = {
         total: 1,
@@ -281,6 +275,21 @@ describe('Project Licenses', () => {
         });
       });
 
+      it.each`
+        givenActiveTab | expectedLocationHash
+        ${'policies'}  | ${'#policies'}
+        ${'licenses'}  | ${'#licenses'}
+      `(
+        'sets the location hash to "$expectedLocationHash" when the "$givenTab" tab is activate',
+        ({ givenActiveTab, expectedLocationHash }) => {
+          findByTestId(`${givenActiveTab}TabTitle`).trigger('click');
+
+          return wrapper.vm.$nextTick().then(() => {
+            expect(window.location.hash).toBe(expectedLocationHash);
+          });
+        },
+      );
+
       it('it renders the correct count in "Detected in project" tab', () => {
         expect(
           wrapper

spec/frontend/helpers/set_window_location_helper.js

 /**
- * setWindowLocation allows for setting properties of `window.location`
+ * setWindowLocation allows for setting `window.location`
  * (doing so directly is causing an error in jsdom)
  *
  * Example usage:
  * assert(window.location.hash === undefined);
- * setWindowLocation({
- *    href: 'http://example.com#foo'
- * })
+ * setWindowLocation('http://example.com#foo')
  * assert(window.location.hash === '#foo');
  *
  * More information:
  * https://github.com/facebook/jest/issues/890
  *
- * @param value
+ * @param url
  */
-export default function setWindowLocation(value) {
+export default function setWindowLocation(url) {
+  const parsedUrl = new URL(url);
+
+  const newLocationValue = [
+    'hash',
+    'host',
+    'hostname',
+    'href',
+    'origin',
+    'pathname',
+    'port',
+    'protocol',
+    'search',
+  ].reduce(
+    (location, prop) => ({
+      ...location,
+      [prop]: parsedUrl[prop],
+    }),
+    {},
+  );
+
   Object.defineProperty(window, 'location', {
+    value: newLocationValue,
     writable: true,
-    value,
   });
 }

spec/frontend/helpers/set_window_location_helper_spec.js

-import setWindowLocationHelper from './set_window_location_helper';
+import setWindowLocation from './set_window_location_helper';
 
-describe('setWindowLocationHelper', () => {
+describe('setWindowLocation', () => {
   const originalLocation = window.location;
 
   afterEach(() => {
@@ -8,22 +8,33 @@ describe('setWindowLocationHelper', () => {
   });
 
   it.each`
-    property      | value
-    ${'hash'}     | ${'foo'}
-    ${'host'}     | ${'gitlab.com'}
-    ${'hostname'} | ${'gitlab.com'}
-    ${'href'}     | ${'https://gitlab.com/foo'}
-    ${'origin'}   | ${'https://gitlab.com'}
-    ${'origin'}   | ${'/foo'}
-    ${'port'}     | ${'80'}
-    ${'protocol'} | ${'https:'}
-  `('sets "window.location.$property" to be $value', ({ property, value }) => {
-    expect(window.location).toBe(originalLocation);
+    url                                    | property      | value
+    ${'https://gitlab.com#foo'}            | ${'hash'}     | ${'#foo'}
+    ${'http://gitlab.com'}                 | ${'host'}     | ${'gitlab.com'}
+    ${'http://gitlab.org'}                 | ${'hostname'} | ${'gitlab.org'}
+    ${'http://gitlab.org/foo#bar'}         | ${'href'}     | ${'http://gitlab.org/foo#bar'}
+    ${'http://gitlab.com'}                 | ${'origin'}   | ${'http://gitlab.com'}
+    ${'http://gitlab.com/foo/bar/baz'}     | ${'pathname'} | ${'/foo/bar/baz'}
+    ${'https://gitlab.com'}                | ${'protocol'} | ${'https:'}
+    ${'http://gitlab.com#foo'}             | ${'protocol'} | ${'http:'}
+    ${'http://gitlab.com:8080'}            | ${'port'}     | ${'8080'}
+    ${'http://gitlab.com?foo=bar&bar=foo'} | ${'search'}   | ${'?foo=bar&bar=foo'}
+  `(
+    'sets "window.location.$property" to be "$value" when called with: "$url"',
+    ({ url, property, value }) => {
+      expect(window.location).toBe(originalLocation);
 
-    setWindowLocationHelper({
-      [property]: value,
-    });
+      setWindowLocation(url);
 
-    expect(window.location[property]).toBe(value);
-  });
+      expect(window.location[property]).toBe(value);
+    },
+  );
+
+  it.each([null, 1, undefined, false, '', 'gitlab.com'])(
+    'throws an error when called with an invalid url: "%s"',
+    invalidUrl => {
+      expect(() => setWindowLocation(invalidUrl)).toThrow(new TypeError('Invalid URL'));
+      expect(window.location).toBe(originalLocation);
+    },
+  );
 });