Merge branch 'vuex-action-helper-correctly-fail' into 'master'

Correctly fail Vuex action helper

Closes #44735

See merge request gitlab-org/gitlab-ce!18072

spec/javascripts/helpers/vuex_action_helper.js

-/* eslint-disable */
-
 /**
- * helper for testing action with expected mutations
+ * helper for testing action with expected mutations inspired in
  * https://vuex.vuejs.org/en/testing.html
+ *
+ * @example
+ * testAction(
+ *   actions.actionName, // action
+ *   { }, // mocked response
+ *   state, // state
+ *   [
+ *    { type: types.MUTATION}
+ *    { type: types.MUTATION_1, payload: {}}
+ *   ], // mutations
+ *   [
+ *    { type: 'actionName', payload: {}},
+ *    { type: 'actionName1', payload: {}}
+ *   ] //actions
+ *   done,
+ * );
  */
-export default (action, payload, state, expectedMutations, done) => {
-  let count = 0;
+export default (action, payload, state, expectedMutations, expectedActions, done) => {
+  let mutationsCount = 0;
+  let actionsCount = 0;
 
   // mock commit
-  const commit = (type, payload) => {
-    const mutation = expectedMutations[count];
-
-    try {
-      expect(mutation.type).to.equal(type);
-      if (payload) {
-        expect(mutation.payload).to.deep.equal(payload);
-      }
-    } catch (error) {
-      done(error);
+  const commit = (type, mutationPayload) => {
+    const mutation = expectedMutations[mutationsCount];
+
+    expect(mutation.type).toEqual(type);
+
+    if (mutation.payload) {
+      expect(mutation.payload).toEqual(mutationPayload);
     }
 
-    count++;
-    if (count >= expectedMutations.length) {
+    mutationsCount += 1;
+    if (mutationsCount >= expectedMutations.length) {
+      done();
+    }
+  };
+
+  // mock dispatch
+  const dispatch = (type, actionPayload) => {
+    const actionExpected = expectedActions[actionsCount];
+
+    expect(actionExpected.type).toEqual(type);
+
+    if (actionExpected.payload) {
+      expect(actionExpected.payload).toEqual(actionPayload);
+    }
+
+    actionsCount += 1;
+    if (actionsCount >= expectedActions.length) {
       done();
     }
   };
 
   // call the action with mocked store and arguments
-  action({ commit, state }, payload);
+  action({ commit, state, dispatch }, payload);
 
   // check if no mutations should have been dispatched
   if (expectedMutations.length === 0) {
-    expect(count).to.equal(0);
+    expect(mutationsCount).toEqual(0);
+    done();
+  }
+
+  // check if no mutations should have been dispatched
+  if (expectedActions.length === 0) {
+    expect(actionsCount).toEqual(0);
     done();
   }
 };

spec/javascripts/notes/stores/actions_spec.js

@@ -5,7 +5,13 @@ import * as actions from '~/notes/stores/actions';
 import store from '~/notes/stores';
 import testAction from '../../helpers/vuex_action_helper';
 import { resetStore } from '../helpers';
-import { discussionMock, notesDataMock, userDataMock, noteableDataMock, individualNote } from '../mock_data';
+import {
+  discussionMock,
+  notesDataMock,
+  userDataMock,
+  noteableDataMock,
+  individualNote,
+} from '../mock_data';
 
 describe('Actions Notes Store', () => {
   afterEach(() => {
@@ -13,66 +19,103 @@ describe('Actions Notes Store', () => {
   });
 
   describe('setNotesData', () => {
-    it('should set received notes data', (done) => {
-      testAction(actions.setNotesData, null, { notesData: {} }, [
-        { type: 'SET_NOTES_DATA', payload: notesDataMock },
-      ], done);
+    it('should set received notes data', done => {
+      testAction(
+        actions.setNotesData,
+        notesDataMock,
+        { notesData: {} },
+        [{ type: 'SET_NOTES_DATA', payload: notesDataMock }],
+        [],
+        done,
+      );
     });
   });
 
   describe('setNoteableData', () => {
-    it('should set received issue data', (done) => {
-      testAction(actions.setNoteableData, null, { noteableData: {} }, [
-        { type: 'SET_NOTEABLE_DATA', payload: noteableDataMock },
-      ], done);
+    it('should set received issue data', done => {
+      testAction(
+        actions.setNoteableData,
+        noteableDataMock,
+        { noteableData: {} },
+        [{ type: 'SET_NOTEABLE_DATA', payload: noteableDataMock }],
+        [],
+        done,
+      );
     });
   });
 
   describe('setUserData', () => {
-    it('should set received user data', (done) => {
-      testAction(actions.setUserData, null, { userData: {} }, [
-        { type: 'SET_USER_DATA', payload: userDataMock },
-      ], done);
+    it('should set received user data', done => {
+      testAction(
+        actions.setUserData,
+        userDataMock,
+        { userData: {} },
+        [{ type: 'SET_USER_DATA', payload: userDataMock }],
+        [],
+        done,
+      );
     });
   });
 
   describe('setLastFetchedAt', () => {
-    it('should set received timestamp', (done) => {
-      testAction(actions.setLastFetchedAt, null, { lastFetchedAt: {} }, [
-        { type: 'SET_LAST_FETCHED_AT', payload: 'timestamp' },
-      ], done);
+    it('should set received timestamp', done => {
+      testAction(
+        actions.setLastFetchedAt,
+        'timestamp',
+        { lastFetchedAt: {} },
+        [{ type: 'SET_LAST_FETCHED_AT', payload: 'timestamp' }],
+        [],
+        done,
+      );
     });
   });
 
   describe('setInitialNotes', () => {
-    it('should set initial notes', (done) => {
-      testAction(actions.setInitialNotes, null, { notes: [] }, [
-        { type: 'SET_INITIAL_NOTES', payload: [individualNote] },
-      ], done);
+    it('should set initial notes', done => {
+      testAction(
+        actions.setInitialNotes,
+        [individualNote],
+        { notes: [] },
+        [{ type: 'SET_INITIAL_NOTES', payload: [individualNote] }],
+        [],
+        done,
+      );
     });
   });
 
   describe('setTargetNoteHash', () => {
-    it('should set target note hash', (done) => {
-      testAction(actions.setTargetNoteHash, null, { notes: [] }, [
-        { type: 'SET_TARGET_NOTE_HASH', payload: 'hash' },
-      ], done);
+    it('should set target note hash', done => {
+      testAction(
+        actions.setTargetNoteHash,
+        'hash',
+        { notes: [] },
+        [{ type: 'SET_TARGET_NOTE_HASH', payload: 'hash' }],
+        [],
+        done,
+      );
     });
   });
 
   describe('toggleDiscussion', () => {
-    it('should toggle discussion', (done) => {
-      testAction(actions.toggleDiscussion, null, { notes: [discussionMock] }, [
-        { type: 'TOGGLE_DISCUSSION', payload: { discussionId: discussionMock.id } },
-      ], done);
+    it('should toggle discussion', done => {
+      testAction(
+        actions.toggleDiscussion,
+        { discussionId: discussionMock.id },
+        { notes: [discussionMock] },
+        [{ type: 'TOGGLE_DISCUSSION', payload: { discussionId: discussionMock.id } }],
+        [],
+        done,
+      );
     });
   });
 
   describe('async methods', () => {
     const interceptor = (request, next) => {
-      next(request.respondWith(JSON.stringify({}), {
-        status: 200,
-      }));
+      next(
+        request.respondWith(JSON.stringify({}), {
+          status: 200,
+        }),
+      );
     };
 
     beforeEach(() => {
@@ -84,8 +127,9 @@ describe('Actions Notes Store', () => {
     });
 
     describe('closeIssue', () => {
-      it('sets state as closed', (done) => {
-        store.dispatch('closeIssue', { notesData: { closeIssuePath: '' } })
+      it('sets state as closed', done => {
+        store
+          .dispatch('closeIssue', { notesData: { closeIssuePath: '' } })
           .then(() => {
             expect(store.state.noteableData.state).toEqual('closed');
             expect(store.state.isToggleStateButtonLoading).toEqual(false);
@@ -96,8 +140,9 @@ describe('Actions Notes Store', () => {
     });
 
     describe('reopenIssue', () => {
-      it('sets state as reopened', (done) => {
-        store.dispatch('reopenIssue', { notesData: { reopenIssuePath: '' } })
+      it('sets state as reopened', done => {
+        store
+          .dispatch('reopenIssue', { notesData: { reopenIssuePath: '' } })
           .then(() => {
             expect(store.state.noteableData.state).toEqual('reopened');
             expect(store.state.isToggleStateButtonLoading).toEqual(false);
@@ -110,7 +155,7 @@ describe('Actions Notes Store', () => {
 
   describe('emitStateChangedEvent', () => {
     it('emits an event on the document', () => {
-      document.addEventListener('issuable_vue_app:change', (event) => {
+      document.addEventListener('issuable_vue_app:change', event => {
         expect(event.detail.data).toEqual({ id: '1', state: 'closed' });
         expect(event.detail.isClosed).toEqual(false);
       });
@@ -120,40 +165,47 @@ describe('Actions Notes Store', () => {
   });
 
   describe('toggleStateButtonLoading', () => {
-    it('should set loading as true', (done) => {
-      testAction(actions.toggleStateButtonLoading, true, {}, [
-        { type: 'TOGGLE_STATE_BUTTON_LOADING', payload: true },
-      ], done);
+    it('should set loading as true', done => {
+      testAction(
+        actions.toggleStateButtonLoading,
+        true,
+        {},
+        [{ type: 'TOGGLE_STATE_BUTTON_LOADING', payload: true }],
+        [],
+        done,
+      );
     });
 
-    it('should set loading as false', (done) => {
-      testAction(actions.toggleStateButtonLoading, false, {}, [
-        { type: 'TOGGLE_STATE_BUTTON_LOADING', payload: false },
-      ], done);
+    it('should set loading as false', done => {
+      testAction(
+        actions.toggleStateButtonLoading,
+        false,
+        {},
+        [{ type: 'TOGGLE_STATE_BUTTON_LOADING', payload: false }],
+        [],
+        done,
+      );
     });
   });
 
   describe('toggleIssueLocalState', () => {
-    it('sets issue state as closed', (done) => {
-      testAction(actions.toggleIssueLocalState, 'closed', {}, [
-        { type: 'CLOSE_ISSUE', payload: 'closed' },
-      ], done);
+    it('sets issue state as closed', done => {
+      testAction(actions.toggleIssueLocalState, 'closed', {}, [{ type: 'CLOSE_ISSUE' }], [], done);
     });
 
-    it('sets issue state as reopened', (done) => {
-      testAction(actions.toggleIssueLocalState, 'reopened', {}, [
-        { type: 'REOPEN_ISSUE', payload: 'reopened' },
-      ], done);
+    it('sets issue state as reopened', done => {
+      testAction(actions.toggleIssueLocalState, 'reopened', {}, [{ type: 'REOPEN_ISSUE' }], [], done);
     });
   });
 
   describe('poll', () => {
-    beforeEach((done) => {
+    beforeEach(done => {
       jasmine.clock().install();
 
       spyOn(Vue.http, 'get').and.callThrough();
 
-      store.dispatch('setNotesData', notesDataMock)
+      store
+        .dispatch('setNotesData', notesDataMock)
         .then(done)
         .catch(done.fail);
     });
@@ -162,23 +214,29 @@ describe('Actions Notes Store', () => {
       jasmine.clock().uninstall();
     });
 
-    it('calls service with last fetched state', (done) => {
+    it('calls service with last fetched state', done => {
       const interceptor = (request, next) => {
-        next(request.respondWith(JSON.stringify({
-          notes: [],
-          last_fetched_at: '123456',
-        }), {
-          status: 200,
-          headers: {
-            'poll-interval': '1000',
-          },
-        }));
+        next(
+          request.respondWith(
+            JSON.stringify({
+              notes: [],
+              last_fetched_at: '123456',
+            }),
+            {
+              status: 200,
+              headers: {
+                'poll-interval': '1000',
+              },
+            },
+          ),
+        );
       };
 
       Vue.http.interceptors.push(interceptor);
       Vue.http.interceptors.push(headersInterceptor);
 
-      store.dispatch('poll')
+      store
+        .dispatch('poll')
         .then(() => new Promise(resolve => requestAnimationFrame(resolve)))
         .then(() => {
           expect(Vue.http.get).toHaveBeenCalledWith(jasmine.anything(), {
@@ -192,9 +250,12 @@ describe('Actions Notes Store', () => {
 
           jasmine.clock().tick(1500);
         })
-        .then(() => new Promise((resolve) => {
-          requestAnimationFrame(resolve);
-        }))
+        .then(
+          () =>
+            new Promise(resolve => {
+              requestAnimationFrame(resolve);
+            }),
+        )
         .then(() => {
           expect(Vue.http.get.calls.count()).toBe(2);
           expect(Vue.http.get.calls.mostRecent().args[1].headers).toEqual({

spec/javascripts/registry/stores/actions_spec.js

@@ -29,57 +29,96 @@ describe('Actions Registry Store', () => {
     describe('fetchRepos', () => {
       beforeEach(() => {
         interceptor = (request, next) => {
-          next(request.respondWith(JSON.stringify(reposServerResponse), {
-            status: 200,
-          }));
+          next(
+            request.respondWith(JSON.stringify(reposServerResponse), {
+              status: 200,
+            }),
+          );
         };
 
         Vue.http.interceptors.push(interceptor);
       });
 
-      it('should set receveived repos', (done) => {
-        testAction(actions.fetchRepos, null, mockedState, [
-          { type: types.TOGGLE_MAIN_LOADING },
-          { type: types.SET_REPOS_LIST, payload: reposServerResponse },
-        ], done);
+      it('should set receveived repos', done => {
+        testAction(
+          actions.fetchRepos,
+          null,
+          mockedState,
+          [
+            { type: types.TOGGLE_MAIN_LOADING },
+            { type: types.TOGGLE_MAIN_LOADING },
+            { type: types.SET_REPOS_LIST, payload: reposServerResponse },
+          ],
+          [],
+          done,
+        );
       });
     });
 
     describe('fetchList', () => {
       beforeEach(() => {
         interceptor = (request, next) => {
-          next(request.respondWith(JSON.stringify(registryServerResponse), {
-            status: 200,
-          }));
+          next(
+            request.respondWith(JSON.stringify(registryServerResponse), {
+              status: 200,
+            }),
+          );
         };
 
         Vue.http.interceptors.push(interceptor);
       });
 
-      it('should set received list', (done) => {
+      it('should set received list', done => {
         mockedState.repos = parsedReposServerResponse;
 
-        testAction(actions.fetchList, { repo: mockedState.repos[1] }, mockedState, [
-          { type: types.TOGGLE_REGISTRY_LIST_LOADING },
-          { type: types.SET_REGISTRY_LIST, payload: registryServerResponse },
-        ], done);
+        const repo = mockedState.repos[1];
+
+        testAction(
+          actions.fetchList,
+          { repo },
+          mockedState,
+          [
+            { type: types.TOGGLE_REGISTRY_LIST_LOADING, payload: repo },
+            { type: types.TOGGLE_REGISTRY_LIST_LOADING, payload: repo },
+            {
+              type: types.SET_REGISTRY_LIST,
+              payload: {
+                repo,
+                resp: registryServerResponse,
+                headers: jasmine.anything(),
+              },
+            },
+          ],
+          [],
+          done,
+        );
       });
     });
   });
 
   describe('setMainEndpoint', () => {
-    it('should commit set main endpoint', (done) => {
-      testAction(actions.setMainEndpoint, 'endpoint', mockedState, [
-        { type: types.SET_MAIN_ENDPOINT, payload: 'endpoint' },
-      ], done);
+    it('should commit set main endpoint', done => {
+      testAction(
+        actions.setMainEndpoint,
+        'endpoint',
+        mockedState,
+        [{ type: types.SET_MAIN_ENDPOINT, payload: 'endpoint' }],
+        [],
+        done,
+      );
     });
   });
 
   describe('toggleLoading', () => {
-    it('should commit toggle main loading', (done) => {
-      testAction(actions.toggleLoading, null, mockedState, [
-        { type: types.TOGGLE_MAIN_LOADING },
-      ], done);
+    it('should commit toggle main loading', done => {
+      testAction(
+        actions.toggleLoading,
+        null,
+        mockedState,
+        [{ type: types.TOGGLE_MAIN_LOADING }],
+        [],
+        done,
+      );
     });
   });
 });