Merge branch 'vuex-test-helper-improvements' into 'master'

EE-Port of gitlab-ce!20513

Closes gitlab-ce#44912

See merge request gitlab-org/gitlab-ee!6482

app/assets/javascripts/ide/stores/modules/merge_requests/actions.js

@@ -31,7 +31,7 @@ export const fetchMergeRequests = ({ dispatch, state: { state } }, { type, searc
   dispatch('requestMergeRequests', type);
   dispatch('resetMergeRequests', type);
 
-  Api.mergeRequests({ scope, state, search })
+  return Api.mergeRequests({ scope, state, search })
     .then(({ data }) => dispatch('receiveMergeRequestsSuccess', { type, data }))
     .catch(() => dispatch('receiveMergeRequestsError', { type, search }));
 };

app/assets/javascripts/ide/stores/modules/pipelines/actions.js

@@ -102,7 +102,7 @@ export const receiveJobsSuccess = ({ commit }, { id, data }) =>
 export const fetchJobs = ({ dispatch }, stage) => {
   dispatch('requestJobs', stage.id);
 
-  axios
+  return axios
     .get(stage.dropdownPath)
     .then(({ data }) => dispatch('receiveJobsSuccess', { id: stage.id, data }))
     .catch(() => dispatch('receiveJobsError', stage));

spec/javascripts/frequent_items/store/actions_spec.js

@@ -205,7 +205,7 @@ describe('Frequent Items Dropdown Store Actions', () => {
         actions.setSearchQuery,
         { query: 'test' },
         mockedState,
-        [{ type: types.SET_SEARCH_QUERY }],
+        [{ type: types.SET_SEARCH_QUERY, payload: { query: 'test' } }],
         [{ type: 'fetchSearchedItems', payload: { query: 'test' } }],
         done,
       );
@@ -216,7 +216,7 @@ describe('Frequent Items Dropdown Store Actions', () => {
         actions.setSearchQuery,
         null,
         mockedState,
-        [{ type: types.SET_SEARCH_QUERY }],
+        [{ type: types.SET_SEARCH_QUERY, payload: null }],
         [{ type: 'fetchFrequentItems' }],
         done,
       );

spec/javascripts/helpers/vuex_action_helper.js

+const noop = () => {};
+
 /**
- * helper for testing action with expected mutations inspired in
+ * Helper for testing action with expected mutations inspired in
  * https://vuex.vuejs.org/en/testing.html
  *
+ * @param {Function} action to be tested
+ * @param {Object} payload will be provided to the action
+ * @param {Object} state will be provided to the action
+ * @param {Array} [expectedMutations=[]] mutations expected to be committed
+ * @param {Array} [expectedActions=[]] actions expected to be dispatched
+ * @param {Function} [done=noop] to be executed after the tests
+ * @return {Promise}
+ *
  * @example
  * testAction(
  *   actions.actionName, // action
- *   { }, // mocked response
- *   state, // state
+ *   { }, // mocked payload
+ *   state, //state
+ *   // expected mutations
  *   [
  *    { type: types.MUTATION}
- *    { type: types.MUTATION_1, payload: {}}
- *   ], // mutations
+ *    { type: types.MUTATION_1, payload: jasmine.any(Number)}
+ *   ],
+ *   // expected actions
  *   [
- *    { type: 'actionName', payload: {}},
- *    { type: 'actionName1', payload: {}}
- *   ] //actions
+ *    { type: 'actionName', payload: {param: 'foobar'}},
+ *    { type: 'actionName1'}
+ *   ]
  *   done,
  * );
+ *
+ * @example
+ * testAction(
+ *   actions.actionName, // action
+ *   { }, // mocked payload
+ *   state, //state
+ *   [ { type: types.MUTATION} ], // expected mutations
+ *   [], // expected actions
+ * ).then(done)
+ * .catch(done.fail);
  */
-export default (action, payload, state, expectedMutations, expectedActions, done) => {
-  let mutationsCount = 0;
-  let actionsCount = 0;
+export default (
+  action,
+  payload,
+  state,
+  expectedMutations = [],
+  expectedActions = [],
+  done = noop,
+) => {
+  const mutations = [];
+  const actions = [];
 
   // mock commit
   const commit = (type, mutationPayload) => {
-    const mutation = expectedMutations[mutationsCount];
-
-    expect(mutation.type).toEqual(type);
+    const mutation = { type };
 
-    if (mutation.payload) {
-      expect(mutation.payload).toEqual(mutationPayload);
+    if (typeof mutationPayload !== 'undefined') {
+      mutation.payload = mutationPayload;
     }
 
-    mutationsCount += 1;
-    if (mutationsCount >= expectedMutations.length) {
-      done();
-    }
+    mutations.push(mutation);
   };
 
   // mock dispatch
   const dispatch = (type, actionPayload) => {
-    const actionExpected = expectedActions[actionsCount];
-
-    expect(actionExpected.type).toEqual(type);
+    const dispatchedAction = { type };
 
-    if (actionExpected.payload) {
-      expect(actionExpected.payload).toEqual(actionPayload);
+    if (typeof actionPayload !== 'undefined') {
+      dispatchedAction.payload = actionPayload;
     }
 
-    actionsCount += 1;
-    if (actionsCount >= expectedActions.length) {
-      done();
-    }
+    actions.push(dispatchedAction);
   };
 
-  // call the action with mocked store and arguments
-  action({ commit, state, dispatch, rootState: state }, payload);
-
-  // check if no mutations should have been dispatched
-  if (expectedMutations.length === 0) {
-    expect(mutationsCount).toEqual(0);
+  const validateResults = () => {
+    expect({
+      mutations,
+      actions,
+    }).toEqual({
+      mutations: expectedMutations,
+      actions: expectedActions,
+    });
     done();
-  }
+  };
 
-  // check if no mutations should have been dispatched
-  if (expectedActions.length === 0) {
-    expect(actionsCount).toEqual(0);
-    done();
-  }
+  return new Promise((resolve, reject) => {
+    try {
+      const result = action({ commit, state, dispatch, rootState: state }, payload);
+      resolve(result);
+    } catch (e) {
+      reject(e);
+    }
+  })
+    .catch(error => {
+      validateResults();
+      throw error;
+    })
+    .then(data => {
+      validateResults();
+      return data;
+    });
 };

spec/javascripts/helpers/vuex_action_helper_spec.js

+import MockAdapter from 'axios-mock-adapter';
+import { TEST_HOST } from 'spec/test_constants';
+import axios from '~/lib/utils/axios_utils';
+import testAction from './vuex_action_helper';
+
+describe('VueX test helper (testAction)', () => {
+  let originalExpect;
+  let assertion;
+  let mock;
+  const noop = () => {};
+
+  beforeAll(() => {
+    mock = new MockAdapter(axios);
+    /*
+    In order to test the helper properly, we need to overwrite the jasmine `expect` helper.
+    We test that the testAction helper properly passes the dispatched actions/committed mutations
+    to the jasmine helper.
+     */
+    originalExpect = expect;
+    assertion = null;
+    global.expect = actual => ({
+      toEqual: () => {
+        originalExpect(actual).toEqual(assertion);
+      },
+    });
+  });
+
+  afterAll(() => {
+    mock.restore();
+    global.expect = originalExpect;
+  });
+
+  it('should properly pass on state and payload', () => {
+    const exampleState = { FOO: 12, BAR: 3 };
+    const examplePayload = { BAZ: 73, BIZ: 55 };
+
+    const action = ({ state }, payload) => {
+      originalExpect(state).toEqual(exampleState);
+      originalExpect(payload).toEqual(examplePayload);
+    };
+
+    assertion = { mutations: [], actions: [] };
+
+    testAction(action, examplePayload, exampleState);
+  });
+
+  describe('should work with synchronous actions', () => {
+    it('committing mutation', () => {
+      const action = ({ commit }) => {
+        commit('MUTATION');
+      };
+
+      assertion = { mutations: [{ type: 'MUTATION' }], actions: [] };
+
+      testAction(action, null, {}, assertion.mutations, assertion.actions, noop);
+    });
+
+    it('dispatching action', () => {
+      const action = ({ dispatch }) => {
+        dispatch('ACTION');
+      };
+
+      assertion = { actions: [{ type: 'ACTION' }], mutations: [] };
+
+      testAction(action, null, {}, assertion.mutations, assertion.actions, noop);
+    });
+
+    it('work with jasmine done once finished', done => {
+      assertion = { mutations: [], actions: [] };
+
+      testAction(noop, null, {}, assertion.mutations, assertion.actions, done);
+    });
+
+    it('provide promise interface', done => {
+      assertion = { mutations: [], actions: [] };
+
+      testAction(noop, null, {}, assertion.mutations, assertion.actions)
+        .then(done)
+        .catch(done.fail);
+    });
+  });
+
+  describe('should work with promise based actions (fetch action)', () => {
+    let lastError;
+    const data = { FOO: 'BAR' };
+
+    const promiseAction = ({ commit, dispatch }) => {
+      dispatch('ACTION');
+
+      return axios
+        .get(TEST_HOST)
+        .catch(error => {
+          commit('ERROR');
+          lastError = error;
+          throw error;
+        })
+        .then(() => {
+          commit('SUCCESS');
+          return data;
+        });
+    };
+
+    beforeEach(() => {
+      lastError = null;
+    });
+
+    it('work with jasmine done once finished', done => {
+      mock.onGet(TEST_HOST).replyOnce(200, 42);
+
+      assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] };
+
+      testAction(promiseAction, null, {}, assertion.mutations, assertion.actions, done);
+    });
+
+    it('return original data of successful promise while checking actions/mutations', done => {
+      mock.onGet(TEST_HOST).replyOnce(200, 42);
+
+      assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] };
+
+      testAction(promiseAction, null, {}, assertion.mutations, assertion.actions)
+        .then(res => {
+          originalExpect(res).toEqual(data);
+          done();
+        })
+        .catch(done.fail);
+    });
+
+    it('return original error of rejected promise while checking actions/mutations', done => {
+      mock.onGet(TEST_HOST).replyOnce(500, '');
+
+      assertion = { mutations: [{ type: 'ERROR' }], actions: [{ type: 'ACTION' }] };
+
+      testAction(promiseAction, null, {}, assertion.mutations, assertion.actions)
+        .then(done.fail)
+        .catch(error => {
+          originalExpect(error).toBe(lastError);
+          done();
+        });
+    });
+  });
+});

spec/javascripts/ide/stores/actions/file_spec.js

@@ -601,10 +601,7 @@ describe('IDE store file actions', () => {
         actions.unstageChange,
         'path',
         store.state,
-        [
-          { type: types.UNSTAGE_CHANGE, payload: 'path' },
-          { type: types.SET_LAST_COMMIT_MSG, payload: '' },
-        ],
+        [{ type: types.UNSTAGE_CHANGE, payload: 'path' }],
         [],
         done,
       );

spec/javascripts/ide/stores/actions/project_spec.js

@@ -73,6 +73,7 @@ describe('IDE store project actions', () => {
           branchId: store.state.currentBranchId,
         },
         store.state,
+        // mutations
         [
           {
             type: 'SET_BRANCH_COMMIT',
@@ -82,17 +83,9 @@ describe('IDE store project actions', () => {
               commit: { id: '123' },
             },
           },
-        ], // mutations
-        [
-          {
-            type: 'getLastCommitPipeline',
-            payload: {
-              projectId: 'abc/def',
-              projectIdNumber: store.state.projects['abc/def'].id,
-              branchId: 'master',
-            },
-          },
-        ], // action
+        ],
+        // action
+        [],
         done,
       );
     });

spec/javascripts/ide/stores/actions/tree_spec.js

@@ -192,11 +192,8 @@ describe('Multi-file store tree actions', () => {
         showTreeEntry,
         'grandparent/parent/child.txt',
         store.state,
-        [
-          { type: types.SET_TREE_OPEN, payload: 'grandparent/parent' },
-          { type: types.SET_TREE_OPEN, payload: 'grandparent' },
-        ],
-        [{ type: 'showTreeEntry' }],
+        [{ type: types.SET_TREE_OPEN, payload: 'grandparent/parent' }],
+        [{ type: 'showTreeEntry', payload: 'grandparent/parent' }],
         done,
       );
     });

spec/javascripts/ide/stores/modules/merge_requests/actions_spec.js

@@ -122,21 +122,6 @@ describe('IDE merge requests actions', () => {
         });
       });
 
-      it('dispatches request', done => {
-        testAction(
-          fetchMergeRequests,
-          { type: 'created' },
-          mockedState,
-          [],
-          [
-            { type: 'requestMergeRequests' },
-            { type: 'resetMergeRequests' },
-            { type: 'receiveMergeRequestsSuccess' },
-          ],
-          done,
-        );
-      });
-
       it('dispatches success with received data', done => {
         testAction(
           fetchMergeRequests,
@@ -144,8 +129,8 @@ describe('IDE merge requests actions', () => {
           mockedState,
           [],
           [
-            { type: 'requestMergeRequests' },
-            { type: 'resetMergeRequests' },
+            { type: 'requestMergeRequests', payload: 'created' },
+            { type: 'resetMergeRequests', payload: 'created' },
             {
               type: 'receiveMergeRequestsSuccess',
               payload: { type: 'created', data: mergeRequests },
@@ -168,9 +153,9 @@ describe('IDE merge requests actions', () => {
           mockedState,
           [],
           [
-            { type: 'requestMergeRequests' },
-            { type: 'resetMergeRequests' },
-            { type: 'receiveMergeRequestsError' },
+            { type: 'requestMergeRequests', payload: 'created' },
+            { type: 'resetMergeRequests', payload: 'created' },
+            { type: 'receiveMergeRequestsError', payload: { type: 'created', search: '' } },
           ],
           done,
         );

spec/javascripts/ide/stores/modules/pipelines/actions_spec.js

@@ -315,7 +315,7 @@ describe('IDE pipelines actions', () => {
         'job',
         mockedState,
         [{ type: types.SET_DETAIL_JOB, payload: 'job' }],
-        [{ type: 'setRightPane' }],
+        [{ type: 'setRightPane', payload: 'jobs-detail' }],
         done,
       );
     });
@@ -325,7 +325,7 @@ describe('IDE pipelines actions', () => {
         setDetailJob,
         null,
         mockedState,
-        [{ type: types.SET_DETAIL_JOB }],
+        [{ type: types.SET_DETAIL_JOB, payload: null }],
         [{ type: 'setRightPane', payload: rightSidebarViews.pipelines }],
         done,
       );
@@ -336,7 +336,7 @@ describe('IDE pipelines actions', () => {
         setDetailJob,
         'job',
         mockedState,
-        [{ type: types.SET_DETAIL_JOB }],
+        [{ type: types.SET_DETAIL_JOB, payload: 'job' }],
         [{ type: 'setRightPane', payload: rightSidebarViews.jobsDetail }],
         done,
       );

spec/javascripts/vue_shared/security_reports/store/actions_spec.js

@@ -275,13 +275,16 @@ describe('security reports actions', () => {
 
   describe('receiveSastError', () => {
     it('should commit sast error mutation', done => {
+      const error = new Error('test');
+
       testAction(
         receiveSastError,
-        null,
+        error,
         mockedState,
         [
           {
             type: types.RECEIVE_SAST_REPORTS_ERROR,
+            payload: error,
           },
         ],
         [],
@@ -475,13 +478,16 @@ describe('security reports actions', () => {
 
   describe('receiveSastContainerError', () => {
     it('should commit sast error mutation', done => {
+      const error = new Error('test');
+
       testAction(
         receiveSastContainerError,
-        null,
+        error,
         mockedState,
         [
           {
             type: types.RECEIVE_SAST_CONTAINER_ERROR,
+            payload: error,
           },
         ],
         [],
@@ -681,13 +687,16 @@ describe('security reports actions', () => {
 
   describe('receiveDastError', () => {
     it('should commit sast error mutation', done => {
+      const error = new Error('test');
+
       testAction(
         receiveDastError,
-        null,
+        error,
         mockedState,
         [
           {
             type: types.RECEIVE_DAST_ERROR,
+            payload: error,
           },
         ],
         [],
@@ -883,13 +892,16 @@ describe('security reports actions', () => {
 
   describe('receiveDependencyScanningError', () => {
     it('should commit sast error mutation', done => {
+      const error = new Error('test');
+
       testAction(
         receiveDependencyScanningError,
-        null,
+        error,
         mockedState,
         [
           {
             type: types.RECEIVE_DEPENDENCY_SCANNING_ERROR,
+            payload: error,
           },
         ],
         [],
@@ -1260,6 +1272,7 @@ describe('security reports actions', () => {
           },
           {
             type: 'receiveDismissIssueError',
+            payload: 'There was an error dismissing the vulnerability. Please try again.',
           },
         ],
         done,
@@ -1426,6 +1439,7 @@ describe('security reports actions', () => {
           },
           {
             type: 'receiveDismissIssueError',
+            payload: 'There was an error reverting the dismissal. Please try again.',
           },
         ],
         done,
@@ -1526,6 +1540,7 @@ describe('security reports actions', () => {
           },
           {
             type: 'receiveCreateIssueError',
+            payload: 'There was an error creating the issue. Please try again.',
           },
         ],
         done,
@@ -1535,13 +1550,16 @@ describe('security reports actions', () => {
 
   describe('updateSastIssue', () => {
     it('commits update sast issue', done => {
+      const payload = { foo: 'bar' };
+
       testAction(
         updateSastIssue,
-        null,
+        payload,
         mockedState,
         [
           {
             type: types.UPDATE_SAST_ISSUE,
+            payload,
           },
         ],
         [],
@@ -1552,13 +1570,16 @@ describe('security reports actions', () => {
 
   describe('updateDependencyScanningIssue', () => {
     it('commits update dependency scanning issue', done => {
+      const payload = { foo: 'bar' };
+
       testAction(
         updateDependencyScanningIssue,
-        null,
+        payload,
         mockedState,
         [
           {
             type: types.UPDATE_DEPENDENCY_SCANNING_ISSUE,
+            payload,
           },
         ],
         [],
@@ -1569,13 +1590,16 @@ describe('security reports actions', () => {
 
   describe('updateContainerScanningIssue', () => {
     it('commits update container scanning issue', done => {
+      const payload = { foo: 'bar' };
+
       testAction(
         updateContainerScanningIssue,
-        null,
+        payload,
         mockedState,
         [
           {
             type: types.UPDATE_CONTAINER_SCANNING_ISSUE,
+            payload,
           },
         ],
         [],
@@ -1586,13 +1610,16 @@ describe('security reports actions', () => {
 
   describe('updateDastIssue', () => {
     it('commits update dast issue', done => {
+      const payload = { foo: 'bar' };
+
       testAction(
         updateDastIssue,
-        null,
+        payload,
         mockedState,
         [
           {
             type: types.UPDATE_DAST_ISSUE,
+            payload,
           },
         ],
         [],