From 436f1711f73c3b146b564e2d5a2c36adac628a82 Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Thu, 6 Feb 2020 14:39:46 -0600 Subject: [PATCH 1/6] Update Karma vuex_action_helper to export Jests --- .../javascripts/helpers/vuex_action_helper.js | 103 +----------------- 1 file changed, 1 insertion(+), 102 deletions(-) diff --git a/spec/javascripts/helpers/vuex_action_helper.js b/spec/javascripts/helpers/vuex_action_helper.js index c5de31a413809f..3911382a7a3705 100644 --- a/spec/javascripts/helpers/vuex_action_helper.js +++ b/spec/javascripts/helpers/vuex_action_helper.js @@ -1,102 +1 @@ -const noop = () => {}; - -/** - * 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 payload - * state, //state - * // expected mutations - * [ - * { type: types.MUTATION} - * { type: types.MUTATION_1, payload: jasmine.any(Number)} - * ], - * // expected 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 = noop, -) => { - const mutations = []; - const actions = []; - - // mock commit - const commit = (type, mutationPayload) => { - const mutation = { type }; - - if (typeof mutationPayload !== 'undefined') { - mutation.payload = mutationPayload; - } - - mutations.push(mutation); - }; - - // mock dispatch - const dispatch = (type, actionPayload) => { - const dispatchedAction = { type }; - - if (typeof actionPayload !== 'undefined') { - dispatchedAction.payload = actionPayload; - } - - actions.push(dispatchedAction); - }; - - const validateResults = () => { - expect({ - mutations, - actions, - }).toEqual({ - mutations: expectedMutations, - actions: expectedActions, - }); - done(); - }; - - const result = action( - { commit, state, dispatch, rootState: state, rootGetters: state, getters: state }, - payload, - ); - - return new Promise(setImmediate) - .then(() => result) - .catch(error => { - validateResults(); - throw error; - }) - .then(data => { - validateResults(); - return data; - }); -}; +export { default } from '../../frontend/helpers/vuex_action_helper'; -- GitLab From 5d38a55df420a198e3902f18f3eb1983d6c24e58 Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Thu, 6 Feb 2020 14:40:30 -0600 Subject: [PATCH 2/6] Update testAction to reject return values --- spec/frontend/helpers/vuex_action_helper.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/frontend/helpers/vuex_action_helper.js b/spec/frontend/helpers/vuex_action_helper.js index 6c3569a2247d52..79e39667da3869 100644 --- a/spec/frontend/helpers/vuex_action_helper.js +++ b/spec/frontend/helpers/vuex_action_helper.js @@ -71,6 +71,8 @@ export default ( } actions.push(dispatchedAction); + + return Promise.resolve(); }; const validateResults = () => { @@ -95,7 +97,8 @@ export default ( throw error; }) .then(data => { + expect(data).toBeFalsy(); + validateResults(); - return data; }); }; -- GitLab From 3cd1af0b21f37a261f663bdc250fd606574cbfaa Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Thu, 6 Feb 2020 16:36:34 -0600 Subject: [PATCH 3/6] fixup! Update Karma vuex_action_helper to export Jests --- .../helpers/vuex_action_helper_spec.js | 166 ------------------ 1 file changed, 166 deletions(-) delete mode 100644 spec/javascripts/helpers/vuex_action_helper_spec.js diff --git a/spec/javascripts/helpers/vuex_action_helper_spec.js b/spec/javascripts/helpers/vuex_action_helper_spec.js deleted file mode 100644 index 09f0bd395c3548..00000000000000 --- a/spec/javascripts/helpers/vuex_action_helper_spec.js +++ /dev/null @@ -1,166 +0,0 @@ -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(); - }); - }); - }); - - it('should work with async actions not returning promises', done => { - const data = { FOO: 'BAR' }; - - const promiseAction = ({ commit, dispatch }) => { - dispatch('ACTION'); - - axios - .get(TEST_HOST) - .then(() => { - commit('SUCCESS'); - return data; - }) - .catch(error => { - commit('ERROR'); - throw error; - }); - }; - - mock.onGet(TEST_HOST).replyOnce(200, 42); - - assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; - - testAction(promiseAction, null, {}, assertion.mutations, assertion.actions, done); - }); -}); -- GitLab From 2f159982e8d2f1c03daeb5ad82512fbb94ca851a Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Thu, 6 Feb 2020 16:54:39 -0600 Subject: [PATCH 4/6] fixup! Update testAction to reject return values --- spec/frontend/helpers/vuex_action_helper.js | 8 +-- .../helpers/vuex_action_helper_spec.js | 56 ++++++------------- 2 files changed, 22 insertions(+), 42 deletions(-) diff --git a/spec/frontend/helpers/vuex_action_helper.js b/spec/frontend/helpers/vuex_action_helper.js index 79e39667da3869..ee6e70f6ccbbbf 100644 --- a/spec/frontend/helpers/vuex_action_helper.js +++ b/spec/frontend/helpers/vuex_action_helper.js @@ -75,7 +75,9 @@ export default ( return Promise.resolve(); }; - const validateResults = () => { + const validateResults = returnValue => { + expect(returnValue).toBeUndefined(); + expect({ mutations, actions, @@ -97,8 +99,6 @@ export default ( throw error; }) .then(data => { - expect(data).toBeFalsy(); - - validateResults(); + validateResults(data); }); }; diff --git a/spec/frontend/helpers/vuex_action_helper_spec.js b/spec/frontend/helpers/vuex_action_helper_spec.js index 61d05762a04526..c7269acbf8f358 100644 --- a/spec/frontend/helpers/vuex_action_helper_spec.js +++ b/spec/frontend/helpers/vuex_action_helper_spec.js @@ -4,30 +4,15 @@ 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 = () => {}; beforeEach(() => { mock = new MockAdapter(axios); - /** - * In order to test the helper properly, we need to overwrite the Jest - * `expect` helper. We test that the testAction helper properly passes the - * dispatched actions/committed mutations to the Jest helper. - */ - originalExpect = expect; - assertion = null; - global.expect = actual => ({ - toEqual: () => { - originalExpect(actual).toEqual(assertion); - }, - }); }); afterEach(() => { mock.restore(); - global.expect = originalExpect; }); it('properly passes state and payload to action', () => { @@ -35,13 +20,11 @@ describe('VueX test helper (testAction)', () => { const examplePayload = { BAZ: 73, BIZ: 55 }; const action = ({ state }, payload) => { - originalExpect(state).toEqual(exampleState); - originalExpect(payload).toEqual(examplePayload); + expect(state).toEqual(exampleState); + expect(payload).toEqual(examplePayload); }; - assertion = { mutations: [], actions: [] }; - - testAction(action, examplePayload, exampleState); + return testAction(action, examplePayload, exampleState); }); describe('given a sync action', () => { @@ -50,7 +33,7 @@ describe('VueX test helper (testAction)', () => { commit('MUTATION'); }; - assertion = { mutations: [{ type: 'MUTATION' }], actions: [] }; + const assertion = { mutations: [{ type: 'MUTATION' }], actions: [] }; testAction(action, null, {}, assertion.mutations, assertion.actions, noop); }); @@ -60,19 +43,19 @@ describe('VueX test helper (testAction)', () => { dispatch('ACTION'); }; - assertion = { actions: [{ type: 'ACTION' }], mutations: [] }; + const assertion = { actions: [{ type: 'ACTION' }], mutations: [] }; testAction(action, null, {}, assertion.mutations, assertion.actions, noop); }); it('works with done callback once finished', done => { - assertion = { mutations: [], actions: [] }; + const assertion = { mutations: [], actions: [] }; testAction(noop, null, {}, assertion.mutations, assertion.actions, done); }); it('returns a promise', done => { - assertion = { mutations: [], actions: [] }; + const assertion = { mutations: [], actions: [] }; testAction(noop, null, {}, assertion.mutations, assertion.actions) .then(done) @@ -82,7 +65,6 @@ describe('VueX test helper (testAction)', () => { describe('given an async action (returning a promise)', () => { let lastError; - const data = { FOO: 'BAR' }; const asyncAction = ({ commit, dispatch }) => { dispatch('ACTION'); @@ -96,7 +78,6 @@ describe('VueX test helper (testAction)', () => { }) .then(() => { commit('SUCCESS'); - return data; }); }; @@ -107,33 +88,32 @@ describe('VueX test helper (testAction)', () => { it('works with done callback once finished', done => { mock.onGet(TEST_HOST).replyOnce(200, 42); - assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; + const assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; testAction(asyncAction, null, {}, assertion.mutations, assertion.actions, done); }); - it('returns original data of successful promise while checking actions/mutations', done => { + it('fails spec if action returns data', () => { mock.onGet(TEST_HOST).replyOnce(200, 42); - assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; + const badAction = () => Promise.resolve('something'); - testAction(asyncAction, null, {}, assertion.mutations, assertion.actions) - .then(res => { - originalExpect(res).toEqual(data); - done(); - }) - .catch(done.fail); + return expect(testAction(badAction, null, {})).rejects.toThrow( + expect.objectContaining({ + message: expect.stringContaining('toBeUndefined'), + }), + ); }); it('returns original error of rejected promise while checking actions/mutations', done => { mock.onGet(TEST_HOST).replyOnce(500, ''); - assertion = { mutations: [{ type: 'ERROR' }], actions: [{ type: 'ACTION' }] }; + const assertion = { mutations: [{ type: 'ERROR' }], actions: [{ type: 'ACTION' }] }; testAction(asyncAction, null, {}, assertion.mutations, assertion.actions) .then(done.fail) .catch(error => { - originalExpect(error).toBe(lastError); + expect(error).toBe(lastError); done(); }); }); @@ -159,7 +139,7 @@ describe('VueX test helper (testAction)', () => { mock.onGet(TEST_HOST).replyOnce(200, 42); - assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; + const assertion = { mutations: [{ type: 'SUCCESS' }], actions: [{ type: 'ACTION' }] }; testAction(asyncAction, null, {}, assertion.mutations, assertion.actions, done); }); -- GitLab From 6cd15ee5c92b4c7988c1d38a935b4f93b959ab62 Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Thu, 6 Feb 2020 17:08:40 -0600 Subject: [PATCH 5/6] Fix actions with return values --- .../javascripts/ide/stores/modules/clientside/actions.js | 2 +- .../javascripts/ide/stores/modules/file_templates/actions.js | 2 +- .../javascripts/monitoring/components/dashboards_dropdown.vue | 2 ++ app/assets/javascripts/monitoring/stores/actions.js | 3 +-- app/assets/javascripts/registry/list/stores/actions.js | 4 ++-- ee/app/assets/javascripts/dependencies/store/actions.js | 4 ++-- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/assets/javascripts/ide/stores/modules/clientside/actions.js b/app/assets/javascripts/ide/stores/modules/clientside/actions.js index eb3bcdff2ae275..7beab5d2c6e271 100644 --- a/app/assets/javascripts/ide/stores/modules/clientside/actions.js +++ b/app/assets/javascripts/ide/stores/modules/clientside/actions.js @@ -5,7 +5,7 @@ export const pingUsage = ({ rootGetters }) => { const url = `${projectUrl}/usage_ping/web_ide_clientside_preview`; - return axios.post(url); + return axios.post(url).then(() => {}); }; // prevent babel-plugin-rewire from generating an invalid default during karma tests diff --git a/app/assets/javascripts/ide/stores/modules/file_templates/actions.js b/app/assets/javascripts/ide/stores/modules/file_templates/actions.js index 59ead8a3dcfdbf..cde3b6aec00e70 100644 --- a/app/assets/javascripts/ide/stores/modules/file_templates/actions.js +++ b/app/assets/javascripts/ide/stores/modules/file_templates/actions.js @@ -43,7 +43,7 @@ export const fetchTemplateTypes = ({ dispatch, state, rootState }) => { }) .catch(() => dispatch('receiveTemplateTypesError')); - return fetchPages(); + return fetchPages().then(() => {}); }; export const setSelectedTemplateType = ({ commit, dispatch, rootGetters }, type) => { diff --git a/app/assets/javascripts/monitoring/components/dashboards_dropdown.vue b/app/assets/javascripts/monitoring/components/dashboards_dropdown.vue index 8f3e0a6ec75054..0cc93df3e5d0d8 100644 --- a/app/assets/javascripts/monitoring/components/dashboards_dropdown.vue +++ b/app/assets/javascripts/monitoring/components/dashboards_dropdown.vue @@ -96,6 +96,8 @@ export default { const dashboard = this.form.branch === this.defaultBranch ? createdDashboard : this.selectedDashboard; this.$emit(events.selectDashboard, dashboard); + + throw 'TODO! Please fix! We should not be receiving any data from an action.'; }) .catch(error => { this.loading = false; diff --git a/app/assets/javascripts/monitoring/stores/actions.js b/app/assets/javascripts/monitoring/stores/actions.js index 29000475bd4bd1..2baa16a8c9d2c3 100644 --- a/app/assets/javascripts/monitoring/stores/actions.js +++ b/app/assets/javascripts/monitoring/stores/actions.js @@ -239,8 +239,7 @@ export const duplicateSystemDashboard = ({ state }, payload) => { return axios .post(state.dashboardsEndpoint, params) - .then(response => response.data) - .then(data => data.dashboard) + .then(() => {}) .catch(error => { const { response } = error; if (response && response.data && response.data.error) { diff --git a/app/assets/javascripts/registry/list/stores/actions.js b/app/assets/javascripts/registry/list/stores/actions.js index 6afba6184863f9..d8ea0cb9e17d0c 100644 --- a/app/assets/javascripts/registry/list/stores/actions.js +++ b/app/assets/javascripts/registry/list/stores/actions.js @@ -34,9 +34,9 @@ export const fetchList = ({ commit }, { repo, page }) => { }); }; -export const deleteItem = (_, item) => axios.delete(item.destroyPath); +export const deleteItem = (_, item) => axios.delete(item.destroyPath).then(() => {}); export const multiDeleteItems = (_, { path, items }) => - axios.delete(path, { params: { ids: items } }); + axios.delete(path, { params: { ids: items } }).then(() => {}); export const setMainEndpoint = ({ commit }, data) => commit(types.SET_MAIN_ENDPOINT, data); export const setIsDeleteDisabled = ({ commit }, data) => commit(types.SET_IS_DELETE_DISABLED, data); diff --git a/ee/app/assets/javascripts/dependencies/store/actions.js b/ee/app/assets/javascripts/dependencies/store/actions.js index ded743bf0df317..17be86e7fbc2a0 100644 --- a/ee/app/assets/javascripts/dependencies/store/actions.js +++ b/ee/app/assets/javascripts/dependencies/store/actions.js @@ -7,12 +7,12 @@ export const setDependenciesEndpoint = ({ state, dispatch }, endpoint) => state.listTypes.map(({ namespace }) => dispatch(`${namespace}/setDependenciesEndpoint`, endpoint), ), - ); + ).then(() => {}); export const fetchDependencies = ({ state, dispatch }, payload) => Promise.all( state.listTypes.map(({ namespace }) => dispatch(`${namespace}/fetchDependencies`, payload)), - ); + ).then(() => {}); export const setCurrentList = ({ state, commit }, payload) => { if (state.listTypes.map(({ namespace }) => namespace).includes(payload)) { -- GitLab From 525b23a577b9ae3ad139c7af63ccca6d91426aff Mon Sep 17 00:00:00 2001 From: Paul Slaughter Date: Thu, 6 Feb 2020 17:33:54 -0600 Subject: [PATCH 6/6] fixup! Fix actions with return values --- app/assets/javascripts/diffs/components/app.vue | 2 ++ app/assets/javascripts/notes/stores/actions.js | 1 + 2 files changed, 3 insertions(+) diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 878b54f7d5378d..c1584f3757efae 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -250,6 +250,7 @@ export default { if (this.glFeatures.diffsBatchLoad) { this.fetchDiffFilesMeta() .then(({ real_size }) => { + throw 'TODO! Please fix! We should not be receiving any data from an action.'; this.diffFilesLength = parseInt(real_size, 10); if (toggleTree) this.hideTreeListIfJustOneFile(); @@ -274,6 +275,7 @@ export default { } else { this.fetchDiffFiles() .then(({ real_size }) => { + throw 'TODO! Please fix! We should not be receiving any data from an action.'; this.diffFilesLength = parseInt(real_size, 10); if (toggleTree) { this.hideTreeListIfJustOneFile(); diff --git a/app/assets/javascripts/notes/stores/actions.js b/app/assets/javascripts/notes/stores/actions.js index f3dc6187c3fee6..a62033ebc99404 100644 --- a/app/assets/javascripts/notes/stores/actions.js +++ b/app/assets/javascripts/notes/stores/actions.js @@ -253,6 +253,7 @@ export const saveNote = ({ commit, dispatch }, noteData) => { } const processQuickActions = res => { + throw 'TODO! We are using actions wrong if we are looking for a certain response from them... This is going to be a tough one to untangle though :/'; const { errors: { commands_only: message } = { commands_only: null } } = res; /* The following reply means that quick actions have been successfully applied: -- GitLab