diff options
Diffstat (limited to 'spec/frontend/diffs')
31 files changed, 991 insertions, 529 deletions
diff --git a/spec/frontend/diffs/components/app_spec.js b/spec/frontend/diffs/components/app_spec.js index 416564b72c3..7fbeb33dd93 100644 --- a/spec/frontend/diffs/components/app_spec.js +++ b/spec/frontend/diffs/components/app_spec.js @@ -1,5 +1,6 @@ +import Vue, { nextTick } from 'vue'; import Vuex from 'vuex'; -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { shallowMount } from '@vue/test-utils'; import { GlLoadingIcon, GlPagination } from '@gitlab/ui'; import MockAdapter from 'axios-mock-adapter'; import { TEST_HOST } from 'spec/test_constants'; @@ -26,6 +27,8 @@ const TEST_ENDPOINT = `${TEST_HOST}/diff/endpoint`; const COMMIT_URL = `${TEST_HOST}/COMMIT/OLD`; const UPDATED_COMMIT_URL = `${TEST_HOST}/COMMIT/NEW`; +Vue.use(Vuex); + function getCollapsedFilesWarning(wrapper) { return wrapper.find(CollapsedFilesWarning); } @@ -37,7 +40,6 @@ describe('diffs/components/app', () => { let mock; function createComponent(props = {}, extendStore = () => {}, provisions = {}) { - const localVue = createLocalVue(); const provide = { ...provisions, glFeatures: { @@ -45,16 +47,13 @@ describe('diffs/components/app', () => { }, }; - localVue.use(Vuex); - store = createDiffsStore(); store.state.diffs.isLoading = false; store.state.diffs.isTreeLoaded = true; extendStore(store); - wrapper = shallowMount(localVue.extend(App), { - localVue, + wrapper = shallowMount(App, { propsData: { endpoint: TEST_ENDPOINT, endpointMetadata: `${TEST_HOST}/diff/endpointMetadata`, @@ -70,11 +69,6 @@ describe('diffs/components/app', () => { }, provide, store, - methods: { - isLatestVersion() { - return true; - }, - }, }); } @@ -102,13 +96,13 @@ describe('diffs/components/app', () => { }); describe('fetch diff methods', () => { - beforeEach(done => { + beforeEach(() => { const fetchResolver = () => { store.state.diffs.retrievingBatches = false; store.state.notes.discussions = 'test'; return Promise.resolve({ real_size: 100 }); }; - jest.spyOn(window, 'requestIdleCallback').mockImplementation(fn => fn()); + jest.spyOn(window, 'requestIdleCallback').mockImplementation((fn) => fn()); createComponent(); jest.spyOn(wrapper.vm, 'fetchDiffFilesMeta').mockImplementation(fetchResolver); jest.spyOn(wrapper.vm, 'fetchDiffFilesBatch').mockImplementation(fetchResolver); @@ -119,68 +113,55 @@ describe('diffs/components/app', () => { jest.spyOn(wrapper.vm, 'unwatchRetrievingBatches').mockImplementation(() => {}); store.state.diffs.retrievingBatches = true; store.state.diffs.diffFiles = []; - wrapper.vm.$nextTick(done); + return nextTick(); }); - it('calls batch methods if diffsBatchLoad is enabled, and not latest version', done => { + it('calls batch methods if diffsBatchLoad is enabled, and not latest version', async () => { expect(wrapper.vm.diffFilesLength).toEqual(0); - wrapper.vm.isLatestVersion = () => false; wrapper.vm.fetchData(false); - setImmediate(() => { - expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled(); - expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled(); - expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled(); - expect(wrapper.vm.fetchCoverageFiles).toHaveBeenCalled(); - expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled(); - expect(wrapper.vm.diffFilesLength).toEqual(100); - expect(wrapper.vm.unwatchRetrievingBatches).toHaveBeenCalled(); - done(); - }); + await nextTick(); + + expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled(); + expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled(); + expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled(); + expect(wrapper.vm.fetchCoverageFiles).toHaveBeenCalled(); + expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled(); + expect(wrapper.vm.diffFilesLength).toBe(100); + expect(wrapper.vm.unwatchRetrievingBatches).toHaveBeenCalled(); }); - it('calls batch methods if diffsBatchLoad is enabled, and latest version', done => { + it('calls batch methods if diffsBatchLoad is enabled, and latest version', async () => { expect(wrapper.vm.diffFilesLength).toEqual(0); wrapper.vm.fetchData(false); - setImmediate(() => { - expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled(); - expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled(); - expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled(); - expect(wrapper.vm.fetchCoverageFiles).toHaveBeenCalled(); - expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled(); - expect(wrapper.vm.diffFilesLength).toEqual(100); - expect(wrapper.vm.unwatchRetrievingBatches).toHaveBeenCalled(); - done(); - }); - }); - }); - - it('adds container-limiting classes when showFileTree is false with inline diffs', () => { - createComponent({}, ({ state }) => { - state.diffs.showTreeList = false; - state.diffs.isParallelView = false; - }); - - expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(true); - }); + await nextTick(); - it('does not add container-limiting classes when showFileTree is false with inline diffs', () => { - createComponent({}, ({ state }) => { - state.diffs.showTreeList = true; - state.diffs.isParallelView = false; + expect(wrapper.vm.startRenderDiffsQueue).toHaveBeenCalled(); + expect(wrapper.vm.fetchDiffFilesMeta).toHaveBeenCalled(); + expect(wrapper.vm.fetchDiffFilesBatch).toHaveBeenCalled(); + expect(wrapper.vm.fetchCoverageFiles).toHaveBeenCalled(); + expect(wrapper.vm.unwatchDiscussions).toHaveBeenCalled(); + expect(wrapper.vm.diffFilesLength).toBe(100); + expect(wrapper.vm.unwatchRetrievingBatches).toHaveBeenCalled(); }); - - expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(false); }); - it('does not add container-limiting classes when isFluidLayout', () => { - createComponent({ isFluidLayout: true }, ({ state }) => { - state.diffs.isParallelView = false; - }); - - expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(false); - }); + it.each` + props | state | expected + ${{ isFluidLayout: true }} | ${{ isParallelView: false }} | ${false} + ${{}} | ${{ isParallelView: false }} | ${true} + ${{}} | ${{ showTreeList: true, diffFiles: [{}], isParallelView: false }} | ${false} + ${{}} | ${{ showTreeList: false, diffFiles: [{}], isParallelView: false }} | ${true} + ${{}} | ${{ showTreeList: false, diffFiles: [], isParallelView: false }} | ${true} + `( + 'uses container-limiting classes ($expected) with state ($state) and props ($props)', + ({ props, state, expected }) => { + createComponent(props, ({ state: origState }) => Object.assign(origState.diffs, state)); + + expect(wrapper.find('.container-limited.limit-container-width').exists()).toBe(expected); + }, + ); it('displays loading icon on loading', () => { createComponent({}, ({ state }) => { @@ -216,28 +197,25 @@ describe('diffs/components/app', () => { window.location.hash = 'ABC_123'; }); - it('sets highlighted row if hash exists in location object', done => { + it('sets highlighted row if hash exists in location object', async () => { createComponent({ shouldShow: true, }); // Component uses $nextTick so we wait until that has finished - setImmediate(() => { - expect(store.state.diffs.highlightedRow).toBe('ABC_123'); + await nextTick(); - done(); - }); + expect(store.state.diffs.highlightedRow).toBe('ABC_123'); }); - it('marks current diff file based on currently highlighted row', () => { + it('marks current diff file based on currently highlighted row', async () => { createComponent({ shouldShow: true, }); // Component uses $nextTick so we wait until that has finished - return wrapper.vm.$nextTick().then(() => { - expect(store.state.diffs.currentDiffFileId).toBe('ABC'); - }); + await nextTick(); + expect(store.state.diffs.currentDiffFileId).toBe('ABC'); }); }); @@ -261,23 +239,23 @@ describe('diffs/components/app', () => { }); it('sets width of tree list', () => { - createComponent(); + createComponent({}, ({ state }) => { + state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }]; + }); expect(wrapper.find('.js-diff-tree-list').element.style.width).toEqual('320px'); }); }); - it('marks current diff file based on currently highlighted row', done => { + it('marks current diff file based on currently highlighted row', async () => { createComponent({ shouldShow: true, }); - // Component uses $nextTick so we wait until that has finished - setImmediate(() => { - expect(store.state.diffs.currentDiffFileId).toBe('ABC'); + // Component uses nextTick so we wait until that has finished + await nextTick(); - done(); - }); + expect(store.state.diffs.currentDiffFileId).toBe('ABC'); }); describe('empty state', () => { @@ -297,79 +275,43 @@ describe('diffs/components/app', () => { expect(wrapper.find(NoChanges).exists()).toBe(false); expect(wrapper.findAll(DiffFile).length).toBe(1); }); - - it('does not render empty state when versions match', () => { - createComponent({}, ({ state }) => { - state.diffs.startVersion = mergeRequestDiff; - state.diffs.mergeRequestDiff = mergeRequestDiff; - }); - - expect(wrapper.find(NoChanges).exists()).toBe(false); - }); }); describe('keyboard shortcut navigation', () => { let spies = []; - let jumpSpy; let moveSpy; + let jumpSpy; - function setup(componentProps, featureFlags) { - createComponent( - componentProps, - ({ state }) => { - state.diffs.commit = { id: 'SHA123' }; - }, - { glFeatures: { mrCommitNeighborNav: true, ...featureFlags } }, - ); + function setup(componentProps) { + createComponent(componentProps, ({ state }) => { + state.diffs.commit = { id: 'SHA123' }; + }); moveSpy = jest.spyOn(wrapper.vm, 'moveToNeighboringCommit').mockImplementation(() => {}); - jumpSpy = jest.fn(); + jumpSpy = jest.spyOn(wrapper.vm, 'jumpToFile').mockImplementation(() => {}); spies = [jumpSpy, moveSpy]; - wrapper.setMethods({ - jumpToFile: jumpSpy, - }); } describe('visible app', () => { it.each` - key | name | spy | args | featureFlags - ${'['} | ${'jumpToFile'} | ${0} | ${[-1]} | ${{}} - ${'k'} | ${'jumpToFile'} | ${0} | ${[-1]} | ${{}} - ${']'} | ${'jumpToFile'} | ${0} | ${[+1]} | ${{}} - ${'j'} | ${'jumpToFile'} | ${0} | ${[+1]} | ${{}} - ${'x'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'previous' }]} | ${{ mrCommitNeighborNav: true }} - ${'c'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'next' }]} | ${{ mrCommitNeighborNav: true }} + key | name | spy | args + ${'['} | ${'jumpToFile'} | ${0} | ${[-1]} + ${'k'} | ${'jumpToFile'} | ${0} | ${[-1]} + ${']'} | ${'jumpToFile'} | ${0} | ${[+1]} + ${'j'} | ${'jumpToFile'} | ${0} | ${[+1]} + ${'x'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'previous' }]} + ${'c'} | ${'moveToNeighboringCommit'} | ${1} | ${[{ direction: 'next' }]} `( 'calls `$name()` with correct parameters whenever the "$key" key is pressed', - ({ key, spy, args, featureFlags }) => { - setup({ shouldShow: true }, featureFlags); - - return wrapper.vm.$nextTick().then(() => { - expect(spies[spy]).not.toHaveBeenCalled(); - - Mousetrap.trigger(key); - - expect(spies[spy]).toHaveBeenCalledWith(...args); - }); - }, - ); - - it.each` - key | name | spy | featureFlags - ${'x'} | ${'moveToNeighboringCommit'} | ${1} | ${{ mrCommitNeighborNav: false }} - ${'c'} | ${'moveToNeighboringCommit'} | ${1} | ${{ mrCommitNeighborNav: false }} - `( - 'does not call `$name()` even when the correct key is pressed if the feature flag is disabled', - ({ key, spy, featureFlags }) => { - setup({ shouldShow: true }, featureFlags); + async ({ key, spy, args }) => { + setup({ shouldShow: true }); - return wrapper.vm.$nextTick().then(() => { - expect(spies[spy]).not.toHaveBeenCalled(); + await nextTick(); + expect(spies[spy]).not.toHaveBeenCalled(); - Mousetrap.trigger(key); + Mousetrap.trigger(key); - expect(spies[spy]).not.toHaveBeenCalled(); - }); + expect(spies[spy]).toHaveBeenCalledWith(...args); }, ); @@ -379,25 +321,23 @@ describe('diffs/components/app', () => { ${'r'} | ${'moveToNeighboringCommit'} | ${1} | ${['x', 'c']} `( `does not call \`$name()\` when a key that is not one of \`$allowed\` is pressed`, - ({ key, spy }) => { - setup({ shouldShow: true }, { mrCommitNeighborNav: true }); + async ({ key, spy }) => { + setup({ shouldShow: true }); - return wrapper.vm.$nextTick().then(() => { - Mousetrap.trigger(key); + await nextTick(); + Mousetrap.trigger(key); - expect(spies[spy]).not.toHaveBeenCalled(); - }); + expect(spies[spy]).not.toHaveBeenCalled(); }, ); }); describe('hidden app', () => { - beforeEach(() => { - setup({ shouldShow: false }, { mrCommitNeighborNav: true }); + beforeEach(async () => { + setup({ shouldShow: false }); - return wrapper.vm.$nextTick().then(() => { - Mousetrap.reset(); - }); + await nextTick(); + Mousetrap.reset(); }); it.each` @@ -420,8 +360,6 @@ describe('diffs/components/app', () => { let spy; beforeEach(() => { - spy = jest.fn(); - createComponent({}, () => { store.state.diffs.diffFiles = [ { file_hash: '111', file_path: '111.js' }, @@ -429,66 +367,49 @@ describe('diffs/components/app', () => { { file_hash: '333', file_path: '333.js' }, ]; }); - - wrapper.setMethods({ - scrollToFile: spy, - }); + spy = jest.spyOn(store, 'dispatch'); }); afterEach(() => { wrapper.destroy(); }); - it('jumps to next and previous files in the list', done => { - wrapper.vm - .$nextTick() - .then(() => { - wrapper.vm.jumpToFile(+1); + it('jumps to next and previous files in the list', async () => { + await nextTick(); + + wrapper.vm.jumpToFile(+1); - expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['222.js']); - store.state.diffs.currentDiffFileId = '222'; - wrapper.vm.jumpToFile(+1); + expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['diffs/scrollToFile', '222.js']); + store.state.diffs.currentDiffFileId = '222'; + wrapper.vm.jumpToFile(+1); - expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['333.js']); - store.state.diffs.currentDiffFileId = '333'; - wrapper.vm.jumpToFile(-1); + expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['diffs/scrollToFile', '333.js']); + store.state.diffs.currentDiffFileId = '333'; + wrapper.vm.jumpToFile(-1); - expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['222.js']); - }) - .then(done) - .catch(done.fail); + expect(spy.mock.calls[spy.mock.calls.length - 1]).toEqual(['diffs/scrollToFile', '222.js']); }); - it('does not jump to previous file from the first one', done => { - wrapper.vm - .$nextTick() - .then(() => { - store.state.diffs.currentDiffFileId = '333'; + it('does not jump to previous file from the first one', async () => { + await nextTick(); + store.state.diffs.currentDiffFileId = '333'; - expect(wrapper.vm.currentDiffIndex).toEqual(2); + expect(wrapper.vm.currentDiffIndex).toBe(2); - wrapper.vm.jumpToFile(+1); + wrapper.vm.jumpToFile(+1); - expect(wrapper.vm.currentDiffIndex).toEqual(2); - expect(spy).not.toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); + expect(wrapper.vm.currentDiffIndex).toBe(2); + expect(spy).not.toHaveBeenCalled(); }); - it('does not jump to next file from the last one', done => { - wrapper.vm - .$nextTick() - .then(() => { - expect(wrapper.vm.currentDiffIndex).toEqual(0); + it('does not jump to next file from the last one', async () => { + await nextTick(); + expect(wrapper.vm.currentDiffIndex).toBe(0); - wrapper.vm.jumpToFile(-1); + wrapper.vm.jumpToFile(-1); - expect(wrapper.vm.currentDiffIndex).toEqual(0); - expect(spy).not.toHaveBeenCalled(); - }) - .then(done) - .catch(done.fail); + expect(wrapper.vm.currentDiffIndex).toBe(0); + expect(spy).not.toHaveBeenCalled(); }); }); @@ -514,7 +435,7 @@ describe('diffs/components/app', () => { window.location = 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', () => { + 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' }; }); @@ -522,14 +443,13 @@ describe('diffs/components/app', () => { store.state.diffs.commit = { id: 'NEW' }; - return wrapper.vm.$nextTick().then(() => { - expect(urlUtils.updateHistory).toHaveBeenCalledWith({ - title: document.title, - url: UPDATED_COMMIT_URL, - }); - expect(wrapper.vm.refetchDiffData).toHaveBeenCalled(); - expect(wrapper.vm.adjustView).toHaveBeenCalled(); + await nextTick(); + expect(urlUtils.updateHistory).toHaveBeenCalledWith({ + title: document.title, + url: UPDATED_COMMIT_URL, }); + expect(wrapper.vm.refetchDiffData).toHaveBeenCalled(); + expect(wrapper.vm.adjustView).toHaveBeenCalled(); }); it.each` @@ -538,7 +458,7 @@ describe('diffs/components/app', () => { ${false} | ${'NEW'} | ${'NEW'} `( 'given `{ "isLoading": $isLoading, "oldSha": "$oldSha", "newSha": "$newSha" }`, nothing should happen', - ({ isLoading, oldSha, newSha }) => { + async ({ isLoading, oldSha, newSha }) => { createComponent({}, ({ state }) => { state.diffs.isLoading = isLoading; state.diffs.commit = { ...state.diffs.commit, id: oldSha }; @@ -547,11 +467,10 @@ describe('diffs/components/app', () => { store.state.diffs.commit = { id: newSha }; - return wrapper.vm.$nextTick().then(() => { - expect(urlUtils.updateHistory).not.toHaveBeenCalled(); - expect(wrapper.vm.refetchDiffData).not.toHaveBeenCalled(); - expect(wrapper.vm.adjustView).not.toHaveBeenCalled(); - }); + await nextTick(); + expect(urlUtils.updateHistory).not.toHaveBeenCalled(); + expect(wrapper.vm.refetchDiffData).not.toHaveBeenCalled(); + expect(wrapper.vm.adjustView).not.toHaveBeenCalled(); }, ); }); @@ -559,6 +478,7 @@ describe('diffs/components/app', () => { describe('diffs', () => { it('should render compare versions component', () => { createComponent({}, ({ state }) => { + state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }]; state.diffs.mergeRequestDiffs = diffsMockData; state.diffs.targetBranchName = 'target-branch'; state.diffs.mergeRequestDiff = mergeRequestDiff; @@ -567,7 +487,8 @@ describe('diffs/components/app', () => { expect(wrapper.find(CompareVersions).exists()).toBe(true); expect(wrapper.find(CompareVersions).props()).toEqual( expect.objectContaining({ - mergeRequestDiffs: diffsMockData, + isLimitedContainer: false, + diffFilesCountText: null, }), ); }); @@ -635,20 +556,22 @@ describe('diffs/components/app', () => { expect(wrapper.find(DiffFile).exists()).toBe(true); }); - it('should render tree list', () => { + it("doesn't render tree list when no changes exist", () => { createComponent(); - expect(wrapper.find(TreeList).exists()).toBe(true); + expect(wrapper.find(TreeList).exists()).toBe(false); }); - }); - describe('setTreeDisplay', () => { - let setShowTreeList; + it('should render tree list', () => { + createComponent({}, ({ state }) => { + state.diffs.diffFiles = [{ file_hash: '111', file_path: '111.js' }]; + }); - beforeEach(() => { - setShowTreeList = jest.fn(); + expect(wrapper.find(TreeList).exists()).toBe(true); }); + }); + describe('setTreeDisplay', () => { afterEach(() => { localStorage.removeItem('mr_tree_show'); }); @@ -657,14 +580,13 @@ describe('diffs/components/app', () => { createComponent({}, ({ state }) => { state.diffs.diffFiles.push({ sha: '123' }); }); - - wrapper.setMethods({ - setShowTreeList, - }); - + jest.spyOn(store, 'dispatch'); wrapper.vm.setTreeDisplay(); - expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList: false, saving: false }); + expect(store.dispatch).toHaveBeenCalledWith('diffs/setShowTreeList', { + showTreeList: false, + saving: false, + }); }); it('calls setShowTreeList with true when more than 1 file is in diffs array', () => { @@ -672,14 +594,14 @@ describe('diffs/components/app', () => { state.diffs.diffFiles.push({ sha: '123' }); state.diffs.diffFiles.push({ sha: '124' }); }); - - wrapper.setMethods({ - setShowTreeList, - }); + jest.spyOn(store, 'dispatch'); wrapper.vm.setTreeDisplay(); - expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList: true, saving: false }); + expect(store.dispatch).toHaveBeenCalledWith('diffs/setShowTreeList', { + showTreeList: true, + saving: false, + }); }); it.each` @@ -692,14 +614,14 @@ describe('diffs/components/app', () => { createComponent({}, ({ state }) => { state.diffs.diffFiles.push({ sha: '123' }); }); - - wrapper.setMethods({ - setShowTreeList, - }); + jest.spyOn(store, 'dispatch'); wrapper.vm.setTreeDisplay(); - expect(setShowTreeList).toHaveBeenCalledWith({ showTreeList, saving: false }); + expect(store.dispatch).toHaveBeenCalledWith('diffs/setShowTreeList', { + showTreeList, + saving: false, + }); }); }); @@ -710,7 +632,7 @@ describe('diffs/components/app', () => { state.diffs.diffFiles.push({ file_hash: '312' }); }); - await wrapper.vm.$nextTick(); + await nextTick(); expect(wrapper.findAll(DiffFile).length).toBe(1); }); @@ -724,7 +646,7 @@ describe('diffs/components/app', () => { state.diffs.diffFiles.push({ file_hash: '123' }, { file_hash: '312' }); }); - await wrapper.vm.$nextTick(); + await nextTick(); expect(paginator().attributes('prevpage')).toBe(undefined); expect(paginator().attributes('nextpage')).toBe('2'); @@ -736,7 +658,7 @@ describe('diffs/components/app', () => { state.diffs.currentDiffFileId = '312'; }); - await wrapper.vm.$nextTick(); + await nextTick(); expect(paginator().attributes('prevpage')).toBe('1'); expect(paginator().attributes('nextpage')).toBe(undefined); @@ -748,7 +670,7 @@ describe('diffs/components/app', () => { state.diffs.currentDiffFileId = '123'; }); - await wrapper.vm.$nextTick(); + await nextTick(); expect(fileByFileNav().exists()).toBe(false); }); @@ -765,13 +687,13 @@ describe('diffs/components/app', () => { state.diffs.currentDiffFileId = currentDiffFileId; }); - await wrapper.vm.$nextTick(); + await nextTick(); jest.spyOn(wrapper.vm, 'navigateToDiffFileIndex'); paginator().vm.$emit('input', targetFile); - await wrapper.vm.$nextTick(); + await nextTick(); expect(wrapper.vm.navigateToDiffFileIndex).toHaveBeenCalledWith(targetFile - 1); }, @@ -787,10 +709,10 @@ describe('diffs/components/app', () => { 'triggers the action with the new fileByFile setting - $setting - when the event with that setting is received', async ({ setting }) => { createComponent(); - await wrapper.vm.$nextTick(); + await nextTick(); eventHub.$emit(EVT_VIEW_FILE_BY_FILE, { setting }); - await wrapper.vm.$nextTick(); + await nextTick(); expect(store.state.diffs.viewDiffsFileByFile).toBe(setting); }, diff --git a/spec/frontend/diffs/components/commit_item_spec.js b/spec/frontend/diffs/components/commit_item_spec.js index 8a7eb6aaca6..f588f65dafd 100644 --- a/spec/frontend/diffs/components/commit_item_spec.js +++ b/spec/frontend/diffs/components/commit_item_spec.js @@ -37,18 +37,12 @@ describe('diffs/components/commit_item', () => { const getPrevCommitNavElement = () => getCommitNavButtonsElement().find('.btn-group > *:first-child'); - const mountComponent = (propsData, featureFlags = {}) => { + const mountComponent = (propsData) => { wrapper = mount(Component, { propsData: { commit, ...propsData, }, - provide: { - glFeatures: { - mrCommitNeighborNav: true, - ...featureFlags, - }, - }, stubs: { CommitPipelineStatus: true, }, @@ -224,12 +218,6 @@ describe('diffs/components/commit_item', () => { expect(getCommitNavButtonsElement().exists()).toEqual(true); }); - it('does not render the commit navigation buttons if the `mrCommitNeighborNav` feature flag is disabled', () => { - mountComponent({ commit: mrCommit }, { mrCommitNeighborNav: false }); - - expect(getCommitNavButtonsElement().exists()).toEqual(false); - }); - describe('prev commit', () => { const { location } = window; diff --git a/spec/frontend/diffs/components/compare_dropdown_layout_spec.js b/spec/frontend/diffs/components/compare_dropdown_layout_spec.js index 92e4a2d9c62..d99933a1ee9 100644 --- a/spec/frontend/diffs/components/compare_dropdown_layout_spec.js +++ b/spec/frontend/diffs/components/compare_dropdown_layout_spec.js @@ -31,7 +31,7 @@ describe('CompareDropdownLayout', () => { const findListItems = () => wrapper.findAll('li'); const findListItemsData = () => - findListItems().wrappers.map(listItem => ({ + findListItems().wrappers.map((listItem) => ({ href: listItem.find('a').attributes('href'), text: trimText(listItem.text()), createdAt: listItem.findAll(TimeAgo).wrappers[0]?.props('time'), diff --git a/spec/frontend/diffs/components/compare_versions_spec.js b/spec/frontend/diffs/components/compare_versions_spec.js index 09e9669c474..949cc855200 100644 --- a/spec/frontend/diffs/components/compare_versions_spec.js +++ b/spec/frontend/diffs/components/compare_versions_spec.js @@ -11,32 +11,34 @@ localVue.use(Vuex); describe('CompareVersions', () => { let wrapper; + let store; const targetBranchName = 'tmp-wine-dev'; - const createWrapper = props => { - const store = createStore(); - const mergeRequestDiff = diffsMockData[0]; - - store.state.diffs.addedLines = 10; - store.state.diffs.removedLines = 20; - store.state.diffs.diffFiles.push('test'); - store.state.diffs.targetBranchName = targetBranchName; - store.state.diffs.mergeRequestDiff = mergeRequestDiff; - store.state.diffs.mergeRequestDiffs = diffsMockData; - + const createWrapper = (props) => { wrapper = mount(CompareVersionsComponent, { localVue, store, propsData: { mergeRequestDiffs: diffsMockData, - diffFilesCountText: null, + diffFilesCountText: '1', ...props, }, }); }; + const findLimitedContainer = () => wrapper.find('.container-limited.limit-container-width'); + const findCompareSourceDropdown = () => wrapper.find('.mr-version-dropdown'); + const findCompareTargetDropdown = () => wrapper.find('.mr-version-compare-dropdown'); beforeEach(() => { - createWrapper(); + store = createStore(); + const mergeRequestDiff = diffsMockData[0]; + + store.state.diffs.addedLines = 10; + store.state.diffs.removedLines = 20; + store.state.diffs.diffFiles.push('test'); + store.state.diffs.targetBranchName = targetBranchName; + store.state.diffs.mergeRequestDiff = mergeRequestDiff; + store.state.diffs.mergeRequestDiffs = diffsMockData; }); afterEach(() => { @@ -45,6 +47,10 @@ describe('CompareVersions', () => { }); describe('template', () => { + beforeEach(() => { + createWrapper(); + }); + it('should render Tree List toggle button with correct attribute values', () => { const treeListBtn = wrapper.find('.js-toggle-tree-list'); @@ -54,8 +60,8 @@ describe('CompareVersions', () => { }); it('should render comparison dropdowns with correct values', () => { - const sourceDropdown = wrapper.find('.mr-version-dropdown'); - const targetDropdown = wrapper.find('.mr-version-compare-dropdown'); + const sourceDropdown = findCompareSourceDropdown(); + const targetDropdown = findCompareTargetDropdown(); expect(sourceDropdown.exists()).toBe(true); expect(targetDropdown.exists()).toBe(true); @@ -63,16 +69,6 @@ describe('CompareVersions', () => { expect(targetDropdown.find('button').html()).toContain(targetBranchName); }); - it('should not render comparison dropdowns if no mergeRequestDiffs are specified', () => { - createWrapper({ mergeRequestDiffs: [] }); - - const sourceDropdown = wrapper.find('.mr-version-dropdown'); - const targetDropdown = wrapper.find('.mr-version-compare-dropdown'); - - expect(sourceDropdown.exists()).toBe(false); - expect(targetDropdown.exists()).toBe(false); - }); - it('should render view types buttons with correct values', () => { const inlineBtn = wrapper.find('#inline-diff-btn'); const parallelBtn = wrapper.find('#parallel-diff-btn'); @@ -88,22 +84,34 @@ describe('CompareVersions', () => { it('adds container-limiting classes when showFileTree is false with inline diffs', () => { createWrapper({ isLimitedContainer: true }); - const limitedContainer = wrapper.find('.container-limited.limit-container-width'); - - expect(limitedContainer.exists()).toBe(true); + expect(findLimitedContainer().exists()).toBe(true); }); it('does not add container-limiting classes when showFileTree is false with inline diffs', () => { createWrapper({ isLimitedContainer: false }); - const limitedContainer = wrapper.find('.container-limited.limit-container-width'); + expect(findLimitedContainer().exists()).toBe(false); + }); + }); - expect(limitedContainer.exists()).toBe(false); + describe('noChangedFiles', () => { + beforeEach(() => { + store.state.diffs.diffFiles = []; + }); + + it('should not render Tree List toggle button when there are no changes', () => { + createWrapper(); + + const treeListBtn = wrapper.find('.js-toggle-tree-list'); + + expect(treeListBtn.exists()).toBe(false); }); }); describe('setInlineDiffViewType', () => { it('should persist the view type in the url', () => { + createWrapper(); + const viewTypeBtn = wrapper.find('#inline-diff-btn'); viewTypeBtn.trigger('click'); @@ -113,6 +121,7 @@ describe('CompareVersions', () => { describe('setParallelDiffViewType', () => { it('should persist the view type in the url', () => { + createWrapper(); const viewTypeBtn = wrapper.find('#parallel-diff-btn'); viewTypeBtn.trigger('click'); @@ -121,11 +130,14 @@ describe('CompareVersions', () => { }); describe('commit', () => { - beforeEach(done => { - wrapper.vm.$store.state.diffs.commit = getDiffWithCommit().commit; - wrapper.mergeRequestDiffs = []; + beforeEach(() => { + store.state.diffs.commit = getDiffWithCommit().commit; + createWrapper(); + }); - wrapper.vm.$nextTick(done); + it('does not render compare dropdowns', () => { + expect(findCompareSourceDropdown().exists()).toBe(false); + expect(findCompareTargetDropdown().exists()).toBe(false); }); it('renders latest version button', () => { @@ -137,4 +149,16 @@ describe('CompareVersions', () => { expect(wrapper.text()).toContain(wrapper.vm.commit.short_id); }); }); + + describe('with no versions', () => { + beforeEach(() => { + store.state.diffs.mergeRequestDiffs = []; + createWrapper(); + }); + + it('does not render compare dropdowns', () => { + expect(findCompareSourceDropdown().exists()).toBe(false); + expect(findCompareTargetDropdown().exists()).toBe(false); + }); + }); }); diff --git a/spec/frontend/diffs/components/diff_content_spec.js b/spec/frontend/diffs/components/diff_content_spec.js index 43d295ff1b3..c1cf4793c88 100644 --- a/spec/frontend/diffs/components/diff_content_spec.js +++ b/spec/frontend/diffs/components/diff_content_spec.js @@ -102,7 +102,7 @@ describe('DiffContent', () => { describe('with text based files', () => { afterEach(() => { - [isParallelViewGetterMock, isInlineViewGetterMock].forEach(m => m.mockRestore()); + [isParallelViewGetterMock, isInlineViewGetterMock].forEach((m) => m.mockRestore()); }); const textDiffFile = { ...defaultProps.diffFile, viewer: { name: diffViewerModes.text } }; diff --git a/spec/frontend/diffs/components/diff_discussions_spec.js b/spec/frontend/diffs/components/diff_discussions_spec.js index 96b76183cee..5c390054247 100644 --- a/spec/frontend/diffs/components/diff_discussions_spec.js +++ b/spec/frontend/diffs/components/diff_discussions_spec.js @@ -15,7 +15,7 @@ describe('DiffDiscussions', () => { let wrapper; const getDiscussionsMockData = () => [{ ...discussionsMockData }]; - const createComponent = props => { + const createComponent = (props) => { store = createStore(); wrapper = mount(localVue.extend(DiffDiscussions), { store, @@ -91,12 +91,7 @@ describe('DiffDiscussions', () => { const noteableDiscussion = wrapper.find(NoteableDiscussion); expect(noteableDiscussion.find('.badge-pill').exists()).toBe(true); - expect( - noteableDiscussion - .find('.badge-pill') - .text() - .trim(), - ).toBe('1'); + expect(noteableDiscussion.find('.badge-pill').text().trim()).toBe('1'); }); }); }); diff --git a/spec/frontend/diffs/components/diff_expansion_cell_spec.js b/spec/frontend/diffs/components/diff_expansion_cell_spec.js index a3b4b5c3abb..62e85b31f76 100644 --- a/spec/frontend/diffs/components/diff_expansion_cell_spec.js +++ b/spec/frontend/diffs/components/diff_expansion_cell_spec.js @@ -1,6 +1,5 @@ -import Vue from 'vue'; import { cloneDeep } from 'lodash'; -import { createComponentWithStore } from 'helpers/vue_mount_component_helper'; +import { mount } from '@vue/test-utils'; import { getByText } from '@testing-library/dom'; import { createStore } from '~/mr_notes/stores'; import DiffExpansionCell from '~/diffs/components/diff_expansion_cell.vue'; @@ -14,7 +13,7 @@ const lineSources = { [INLINE_DIFF_VIEW_TYPE]: 'highlighted_diff_lines', }; const lineHandlers = { - [INLINE_DIFF_VIEW_TYPE]: line => line, + [INLINE_DIFF_VIEW_TYPE]: (line) => line, }; function makeLoadMoreLinesPayload({ @@ -59,7 +58,6 @@ describe('DiffExpansionCell', () => { let mockFile; let mockLine; let store; - let vm; beforeEach(() => { mockFile = cloneDeep(diffFileMockData); @@ -70,7 +68,6 @@ describe('DiffExpansionCell', () => { }); const createComponent = (options = {}) => { - const cmp = Vue.extend(DiffExpansionCell); const defaults = { fileHash: mockFile.file_hash, contextLinesPath: 'contextLinesPath', @@ -78,46 +75,46 @@ describe('DiffExpansionCell', () => { isTop: false, isBottom: false, }; - const props = { ...defaults, ...options }; + const propsData = { ...defaults, ...options }; - vm = createComponentWithStore(cmp, store, props).$mount(); + return mount(DiffExpansionCell, { store, propsData }); }; - const findExpandUp = () => vm.$el.querySelector(EXPAND_UP_CLASS); - const findExpandDown = () => vm.$el.querySelector(EXPAND_DOWN_CLASS); - const findExpandAll = () => getByText(vm.$el, 'Show all unchanged lines'); + const findExpandUp = (wrapper) => wrapper.find(EXPAND_UP_CLASS); + const findExpandDown = (wrapper) => wrapper.find(EXPAND_DOWN_CLASS); + const findExpandAll = ({ element }) => getByText(element, 'Show all unchanged lines'); describe('top row', () => { it('should have "expand up" and "show all" option', () => { - createComponent({ + const wrapper = createComponent({ isTop: true, }); - expect(findExpandUp()).not.toBe(null); - expect(findExpandDown()).toBe(null); - expect(findExpandAll()).not.toBe(null); + expect(findExpandUp(wrapper).exists()).toBe(true); + expect(findExpandDown(wrapper).exists()).toBe(false); + expect(findExpandAll(wrapper)).not.toBe(null); }); }); describe('middle row', () => { it('should have "expand down", "show all", "expand up" option', () => { - createComponent(); + const wrapper = createComponent(); - expect(findExpandUp()).not.toBe(null); - expect(findExpandDown()).not.toBe(null); - expect(findExpandAll()).not.toBe(null); + expect(findExpandUp(wrapper).exists()).toBe(true); + expect(findExpandDown(wrapper).exists()).toBe(true); + expect(findExpandAll(wrapper)).not.toBe(null); }); }); describe('bottom row', () => { it('should have "expand down" and "show all" option', () => { - createComponent({ + const wrapper = createComponent({ isBottom: true, }); - expect(findExpandUp()).toBe(null); - expect(findExpandDown()).not.toBe(null); - expect(findExpandAll()).not.toBe(null); + expect(findExpandUp(wrapper).exists()).toBe(false); + expect(findExpandDown(wrapper).exists()).toBe(true); + expect(findExpandAll(wrapper)).not.toBe(null); }); }); @@ -144,9 +141,9 @@ describe('DiffExpansionCell', () => { newLineNumber, }); - createComponent(); + const wrapper = createComponent(); - findExpandAll().click(); + findExpandAll(wrapper).click(); expect(store.dispatch).toHaveBeenCalledWith( 'diffs/loadMoreLines', @@ -167,9 +164,9 @@ describe('DiffExpansionCell', () => { const oldLineNumber = mockLine.meta_data.old_pos; const newLineNumber = mockLine.meta_data.new_pos; - createComponent(); + const wrapper = createComponent(); - findExpandUp().click(); + findExpandUp(wrapper).trigger('click'); expect(store.dispatch).toHaveBeenCalledWith( 'diffs/loadMoreLines', @@ -195,9 +192,9 @@ describe('DiffExpansionCell', () => { mockLine.meta_data.old_pos = 200; mockLine.meta_data.new_pos = 200; - createComponent(); + const wrapper = createComponent(); - findExpandDown().click(); + findExpandDown(wrapper).trigger('click'); expect(store.dispatch).toHaveBeenCalledWith('diffs/loadMoreLines', { endpoint: 'contextLinesPath', diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js index 1b41456f2f5..e9a63e861ed 100644 --- a/spec/frontend/diffs/components/diff_file_header_spec.js +++ b/spec/frontend/diffs/components/diff_file_header_spec.js @@ -62,7 +62,7 @@ describe('DiffFileHeader component', () => { diffHasDiscussionsResultMock, diffHasExpandedDiscussionsResultMock, ...Object.values(mockStoreConfig.modules.diffs.actions), - ].forEach(mock => mock.mockReset()); + ].forEach((mock) => mock.mockReset()); wrapper.destroy(); }); @@ -80,7 +80,7 @@ describe('DiffFileHeader component', () => { const findCollapseIcon = () => wrapper.find({ ref: 'collapseIcon' }); const findEditButton = () => wrapper.find({ ref: 'editButton' }); - const createComponent = props => { + const createComponent = (props) => { mockStoreConfig = cloneDeep(defaultMockStoreConfig); const store = new Vuex.Store(mockStoreConfig); @@ -219,7 +219,7 @@ describe('DiffFileHeader component', () => { }); describe('for any file', () => { - const otherModes = Object.keys(diffViewerModes).filter(m => m !== 'mode_changed'); + const otherModes = Object.keys(diffViewerModes).filter((m) => m !== 'mode_changed'); it('for mode_changed file mode displays mode changes', () => { createComponent({ @@ -236,20 +236,23 @@ describe('DiffFileHeader component', () => { expect(findModeChangedLine().text()).toMatch(/old-mode.+new-mode/); }); - it.each(otherModes.map(m => [m]))('for %s file mode does not display mode changes', mode => { - createComponent({ - diffFile: { - ...diffFile, - a_mode: 'old-mode', - b_mode: 'new-mode', - viewer: { - ...diffFile.viewer, - name: diffViewerModes[mode], + it.each(otherModes.map((m) => [m]))( + 'for %s file mode does not display mode changes', + (mode) => { + createComponent({ + diffFile: { + ...diffFile, + a_mode: 'old-mode', + b_mode: 'new-mode', + viewer: { + ...diffFile.viewer, + name: diffViewerModes[mode], + }, }, - }, - }); - expect(findModeChangedLine().exists()).toBeFalsy(); - }); + }); + expect(findModeChangedLine().exists()).toBeFalsy(); + }, + ); it('displays the LFS label for files stored in LFS', () => { createComponent({ diff --git a/spec/frontend/diffs/components/diff_file_spec.js b/spec/frontend/diffs/components/diff_file_spec.js index 71e0ffd176f..c715d779986 100644 --- a/spec/frontend/diffs/components/diff_file_spec.js +++ b/spec/frontend/diffs/components/diff_file_spec.js @@ -1,6 +1,9 @@ import Vuex from 'vuex'; import { shallowMount, createLocalVue } from '@vue/test-utils'; +import MockAdapter from 'axios-mock-adapter'; +import axios from '~/lib/utils/axios_utils'; +import httpStatus from '~/lib/utils/http_status'; import createDiffsStore from '~/diffs/store/modules'; import createNotesStore from '~/notes/stores/modules'; import diffFileMockDataReadable from '../mock_data/diff_file'; @@ -96,13 +99,13 @@ function createComponent({ file, first = false, last = false }) { }; } -const findDiffHeader = wrapper => wrapper.find(DiffFileHeaderComponent); -const findDiffContentArea = wrapper => wrapper.find('[data-testid="content-area"]'); -const findLoader = wrapper => wrapper.find('[data-testid="loader-icon"]'); -const findToggleButton = wrapper => wrapper.find('[data-testid="expand-button"]'); +const findDiffHeader = (wrapper) => wrapper.find(DiffFileHeaderComponent); +const findDiffContentArea = (wrapper) => wrapper.find('[data-testid="content-area"]'); +const findLoader = (wrapper) => wrapper.find('[data-testid="loader-icon"]'); +const findToggleButton = (wrapper) => wrapper.find('[data-testid="expand-button"]'); -const toggleFile = wrapper => findDiffHeader(wrapper).vm.$emit('toggleFile'); -const isDisplayNone = element => element.style.display === 'none'; +const toggleFile = (wrapper) => findDiffHeader(wrapper).vm.$emit('toggleFile'); +const isDisplayNone = (element) => element.style.display === 'none'; const getReadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataReadable)); const getUnreadableFile = () => JSON.parse(JSON.stringify(diffFileMockDataUnreadable)); @@ -118,14 +121,17 @@ const changeViewerType = (store, newType, index = 0) => describe('DiffFile', () => { let wrapper; let store; + let axiosMock; beforeEach(() => { + axiosMock = new MockAdapter(axios); ({ wrapper, store } = createComponent({ file: getReadableFile() })); }); afterEach(() => { wrapper.destroy(); wrapper = null; + axiosMock.restore(); }); describe('bus events', () => { @@ -157,7 +163,7 @@ describe('DiffFile', () => { await wrapper.vm.$nextTick(); expect(eventHub.$emit).toHaveBeenCalledTimes(events.length); - events.forEach(event => { + events.forEach((event) => { expect(eventHub.$emit).toHaveBeenCalledWith(event); }); }, @@ -174,7 +180,7 @@ describe('DiffFile', () => { })); jest.spyOn(wrapper.vm, 'loadCollapsedDiff').mockResolvedValue(getReadableFile()); - jest.spyOn(window, 'requestIdleCallback').mockImplementation(fn => fn()); + jest.spyOn(window, 'requestIdleCallback').mockImplementation((fn) => fn()); makeFileAutomaticallyCollapsed(store); @@ -247,7 +253,7 @@ describe('DiffFile', () => { it('should not have any content at all', async () => { await wrapper.vm.$nextTick(); - Array.from(findDiffContentArea(wrapper).element.children).forEach(child => { + Array.from(findDiffContentArea(wrapper).element.children).forEach((child) => { expect(isDisplayNone(child)).toBe(true); }); }); @@ -353,8 +359,10 @@ describe('DiffFile', () => { describe('loading', () => { it('should have loading icon while loading a collapsed diffs', async () => { + const { load_collapsed_diff_url } = store.state.diffs.diffFiles[0]; + axiosMock.onGet(load_collapsed_diff_url).reply(httpStatus.OK, getReadableFile()); makeFileAutomaticallyCollapsed(store); - wrapper.vm.isLoadingCollapsedDiff = true; + wrapper.vm.requestDiff(); await wrapper.vm.$nextTick(); diff --git a/spec/frontend/diffs/components/diff_gutter_avatars_spec.js b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js index 61e110b345a..5884a9ebd3a 100644 --- a/spec/frontend/diffs/components/diff_gutter_avatars_spec.js +++ b/spec/frontend/diffs/components/diff_gutter_avatars_spec.js @@ -66,9 +66,7 @@ describe('DiffGutterAvatars', () => { }); it('should emit toggleDiscussions event on avatars click', () => { - findUserAvatars() - .at(0) - .trigger('click'); + findUserAvatars().at(0).trigger('click'); return wrapper.vm.$nextTick().then(() => { expect(wrapper.emitted().toggleLineDiscussions).toBeTruthy(); diff --git a/spec/frontend/diffs/components/diff_line_note_form_spec.js b/spec/frontend/diffs/components/diff_line_note_form_spec.js index 75ec5c202af..faa68159c58 100644 --- a/spec/frontend/diffs/components/diff_line_note_form_spec.js +++ b/spec/frontend/diffs/components/diff_line_note_form_spec.js @@ -11,14 +11,16 @@ describe('DiffLineNoteForm', () => { let diffLines; const getDiffFileMock = () => ({ ...diffFileMockData }); - beforeEach(() => { + const createComponent = (args = {}) => { diffFile = getDiffFileMock(); diffLines = diffFile.highlighted_diff_lines; const store = createStore(); store.state.notes.userData.id = 1; store.state.notes.noteableData = noteableDataMock; - wrapper = shallowMount(DiffLineNoteForm, { + store.replaceState({ ...store.state, ...args.state }); + + return shallowMount(DiffLineNoteForm, { store, propsData: { diffFileHash: diffFile.file_hash, @@ -27,9 +29,13 @@ describe('DiffLineNoteForm', () => { noteTargetLine: diffLines[0], }, }); - }); + }; describe('methods', () => { + beforeEach(() => { + wrapper = createComponent(); + }); + describe('handleCancelCommentForm', () => { it('should ask for confirmation when shouldConfirm and isDirty passed as truthy', () => { jest.spyOn(window, 'confirm').mockReturnValue(false); @@ -51,7 +57,7 @@ describe('DiffLineNoteForm', () => { expect(window.confirm).not.toHaveBeenCalled(); }); - it('should call cancelCommentForm with lineCode', done => { + it('should call cancelCommentForm with lineCode', (done) => { jest.spyOn(window, 'confirm').mockImplementation(() => {}); jest.spyOn(wrapper.vm, 'cancelCommentForm').mockImplementation(() => {}); jest.spyOn(wrapper.vm, 'resetAutoSave').mockImplementation(() => {}); @@ -72,7 +78,7 @@ describe('DiffLineNoteForm', () => { }); describe('saveNoteForm', () => { - it('should call saveNote action with proper params', done => { + it('should call saveNote action with proper params', (done) => { const saveDiffDiscussionSpy = jest .spyOn(wrapper.vm, 'saveDiffDiscussion') .mockReturnValue(Promise.resolve()); @@ -114,14 +120,39 @@ describe('DiffLineNoteForm', () => { describe('mounted', () => { it('should init autosave', () => { const key = 'autosave/Note/Issue/98//DiffNote//1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_1_1'; + wrapper = createComponent(); expect(wrapper.vm.autosave).toBeDefined(); expect(wrapper.vm.autosave.key).toEqual(key); }); + + it('should set selectedCommentPosition', () => { + wrapper = createComponent(); + let startLineCode = wrapper.vm.commentLineStart.line_code; + let lineCode = wrapper.vm.line.line_code; + + expect(startLineCode).toEqual(lineCode); + wrapper.destroy(); + + const state = { + notes: { + selectedCommentPosition: { + start: { + line_code: 'test', + }, + }, + }, + }; + wrapper = createComponent({ state }); + startLineCode = wrapper.vm.commentLineStart.line_code; + lineCode = state.notes.selectedCommentPosition.start.line_code; + expect(startLineCode).toEqual(lineCode); + }); }); describe('template', () => { it('should have note form', () => { + wrapper = createComponent(); expect(wrapper.find(NoteForm).exists()).toBe(true); }); }); diff --git a/spec/frontend/diffs/components/diff_row_spec.js b/spec/frontend/diffs/components/diff_row_spec.js index 0ec075c8ad8..c06d8e78316 100644 --- a/spec/frontend/diffs/components/diff_row_spec.js +++ b/spec/frontend/diffs/components/diff_row_spec.js @@ -1,7 +1,10 @@ import { shallowMount, createLocalVue } from '@vue/test-utils'; +import { getByTestId, fireEvent } from '@testing-library/dom'; import Vuex from 'vuex'; import diffsModule from '~/diffs/store/modules'; import DiffRow from '~/diffs/components/diff_row.vue'; +import diffFileMockData from '../mock_data/diff_file'; +import { mapParallel } from '~/diffs/components/diff_row_utils'; describe('DiffRow', () => { const testLines = [ @@ -42,16 +45,16 @@ describe('DiffRow', () => { fileHash: 'abc', filePath: 'abc', line: {}, + index: 0, ...props, }; - return shallowMount(DiffRow, { propsData, localVue, store }); - }; - it('isHighlighted returns true if isCommented is true', () => { - const props = { isCommented: true }; - const wrapper = createWrapper({ props }); - expect(wrapper.vm.isHighlighted).toBe(true); - }); + const provide = { + glFeatures: { dragCommentSelection: true }, + }; + + return shallowMount(DiffRow, { propsData, localVue, store, provide }); + }; it('isHighlighted returns true given line.left', () => { const props = { @@ -124,4 +127,88 @@ describe('DiffRow', () => { const lineNumber = testLines[0].right.new_line; expect(wrapper.find(`[data-linenumber="${lineNumber}"]`).exists()).toBe(true); }); + + describe('drag operations', () => { + let line; + + beforeEach(() => { + line = { ...testLines[0] }; + }); + + it.each` + side + ${'left'} + ${'right'} + `('emits `enterdragging` onDragEnter $side side', ({ side }) => { + const expectation = { ...line[side], index: 0 }; + const wrapper = createWrapper({ props: { line } }); + fireEvent.dragEnter(getByTestId(wrapper.element, `${side}-side`)); + + expect(wrapper.emitted().enterdragging).toBeTruthy(); + expect(wrapper.emitted().enterdragging[0]).toEqual([expectation]); + }); + + it.each` + side + ${'left'} + ${'right'} + `('emits `stopdragging` onDrop $side side', ({ side }) => { + const wrapper = createWrapper({ props: { line } }); + fireEvent.dragEnd(getByTestId(wrapper.element, `${side}-side`)); + + expect(wrapper.emitted().stopdragging).toBeTruthy(); + }); + }); + + describe('sets coverage title and class', () => { + const thisLine = diffFileMockData.parallel_diff_lines[2]; + const rightLine = diffFileMockData.parallel_diff_lines[2].right; + + const mockDiffContent = { + diffFile: diffFileMockData, + shouldRenderDraftRow: jest.fn(), + hasParallelDraftLeft: jest.fn(), + hasParallelDraftRight: jest.fn(), + draftForLine: jest.fn(), + }; + + const applyMap = mapParallel(mockDiffContent); + const props = { + line: applyMap(thisLine), + fileHash: diffFileMockData.file_hash, + filePath: diffFileMockData.file_path, + contextLinesPath: 'contextLinesPath', + isHighlighted: false, + }; + const name = diffFileMockData.file_path; + const line = rightLine.new_line; + + it('for lines with coverage', () => { + const coverageFiles = { files: { [name]: { [line]: 5 } } }; + const wrapper = createWrapper({ props, state: { coverageFiles } }); + const coverage = wrapper.find('.line-coverage.right-side'); + + expect(coverage.attributes('title')).toContain('Test coverage: 5 hits'); + expect(coverage.classes('coverage')).toBeTruthy(); + }); + + it('for lines without coverage', () => { + const coverageFiles = { files: { [name]: { [line]: 0 } } }; + const wrapper = createWrapper({ props, state: { coverageFiles } }); + const coverage = wrapper.find('.line-coverage.right-side'); + + expect(coverage.attributes('title')).toContain('No test coverage'); + expect(coverage.classes('no-coverage')).toBeTruthy(); + }); + + it('for unknown lines', () => { + const coverageFiles = {}; + const wrapper = createWrapper({ props, state: { coverageFiles } }); + const coverage = wrapper.find('.line-coverage.right-side'); + + expect(coverage.attributes('title')).toBeFalsy(); + expect(coverage.classes('coverage')).toBeFalsy(); + expect(coverage.classes('no-coverage')).toBeFalsy(); + }); + }); }); diff --git a/spec/frontend/diffs/components/diff_row_utils_spec.js b/spec/frontend/diffs/components/diff_row_utils_spec.js index c001857fa49..d70d6b609ac 100644 --- a/spec/frontend/diffs/components/diff_row_utils_spec.js +++ b/spec/frontend/diffs/components/diff_row_utils_spec.js @@ -126,14 +126,14 @@ describe('lineCode', () => { describe('classNameMapCell', () => { it.each` - line | hll | loggedIn | hovered | expectation - ${undefined} | ${true} | ${true} | ${true} | ${[]} - ${{ type: 'new' }} | ${false} | ${false} | ${false} | ${['new', { hll: false, 'is-over': false }]} - ${{ type: 'new' }} | ${true} | ${true} | ${false} | ${['new', { hll: true, 'is-over': false }]} - ${{ type: 'new' }} | ${true} | ${false} | ${true} | ${['new', { hll: true, 'is-over': false }]} - ${{ type: 'new' }} | ${true} | ${true} | ${true} | ${['new', { hll: true, 'is-over': true }]} - `('should return $expectation', ({ line, hll, loggedIn, hovered, expectation }) => { - const classes = utils.classNameMapCell(line, hll, loggedIn, hovered); + line | hll | isLoggedIn | isHover | expectation + ${undefined} | ${true} | ${true} | ${true} | ${[]} + ${{ type: 'new' }} | ${false} | ${false} | ${false} | ${['new', { hll: false, 'is-over': false, new_line: true, old_line: false }]} + ${{ type: 'new' }} | ${true} | ${true} | ${false} | ${['new', { hll: true, 'is-over': false, new_line: true, old_line: false }]} + ${{ type: 'new' }} | ${true} | ${false} | ${true} | ${['new', { hll: true, 'is-over': false, new_line: true, old_line: false }]} + ${{ type: 'new' }} | ${true} | ${true} | ${true} | ${['new', { hll: true, 'is-over': true, new_line: true, old_line: false }]} + `('should return $expectation', ({ line, hll, isLoggedIn, isHover, expectation }) => { + const classes = utils.classNameMapCell({ line, hll, isLoggedIn, isHover }); expect(classes).toEqual(expectation); }); }); diff --git a/spec/frontend/diffs/components/diff_stats_spec.js b/spec/frontend/diffs/components/diff_stats_spec.js index 4dcbb3ec332..0aaec027c0a 100644 --- a/spec/frontend/diffs/components/diff_stats_spec.js +++ b/spec/frontend/diffs/components/diff_stats_spec.js @@ -39,7 +39,7 @@ describe('diff_stats', () => { }); describe('line changes', () => { - const findFileLine = name => wrapper.find(name); + const findFileLine = (name) => wrapper.find(name); it('shows the amount of lines added', () => { expect(findFileLine('.js-file-addition-line').text()).toBe(TEST_ADDED_LINES.toString()); @@ -51,10 +51,10 @@ describe('diff_stats', () => { }); describe('files changes', () => { - const findIcon = name => + const findIcon = (name) => wrapper .findAll(GlIcon) - .filter(c => c.attributes('name') === name) + .filter((c) => c.attributes('name') === name) .at(0).element.parentNode; it('shows amount of file changed with plural "files" when 0 files has changed', () => { diff --git a/spec/frontend/diffs/components/diff_view_spec.js b/spec/frontend/diffs/components/diff_view_spec.js index 4d90112d8f6..3d36ebf14a3 100644 --- a/spec/frontend/diffs/components/diff_view_spec.js +++ b/spec/frontend/diffs/components/diff_view_spec.js @@ -1,19 +1,19 @@ -import { shallowMount, createLocalVue } from '@vue/test-utils'; +import Vue from 'vue'; +import { shallowMount } from '@vue/test-utils'; import Vuex from 'vuex'; import DiffView from '~/diffs/components/diff_view.vue'; -// import DraftNote from '~/batch_comments/components/draft_note.vue'; -// import DiffRow from '~/diffs/components/diff_row.vue'; -// import DiffCommentCell from '~/diffs/components/diff_comment_cell.vue'; -// import DiffExpansionCell from '~/diffs/components/diff_expansion_cell.vue'; describe('DiffView', () => { const DiffExpansionCell = { template: `<div/>` }; const DiffRow = { template: `<div/>` }; const DiffCommentCell = { template: `<div/>` }; const DraftNote = { template: `<div/>` }; - const createWrapper = props => { - const localVue = createLocalVue(); - localVue.use(Vuex); + const showCommentForm = jest.fn(); + const setSelectedCommentPosition = jest.fn(); + const getDiffRow = (wrapper) => wrapper.findComponent(DiffRow).vm; + + const createWrapper = (props) => { + Vue.use(Vuex); const batchComments = { getters: { @@ -26,8 +26,13 @@ describe('DiffView', () => { }, namespaced: true, }; - const diffs = { getters: { commitId: () => 'abc123' }, namespaced: true }; + const diffs = { + actions: { showCommentForm }, + getters: { commitId: () => 'abc123' }, + namespaced: true, + }; const notes = { + actions: { setSelectedCommentPosition }, state: { selectedCommentPosition: null, selectedCommentPositionHover: null }, }; @@ -41,7 +46,7 @@ describe('DiffView', () => { ...props, }; const stubs = { DiffExpansionCell, DiffRow, DiffCommentCell, DraftNote }; - return shallowMount(DiffView, { propsData, store, localVue, stubs }); + return shallowMount(DiffView, { propsData, store, stubs }); }; it('renders a match line', () => { @@ -64,12 +69,7 @@ describe('DiffView', () => { inline: type === 'inline', }); expect(wrapper.findAll(DiffCommentCell).length).toBe(total); - expect( - wrapper - .find(container) - .find(DiffCommentCell) - .exists(), - ).toBe(true); + expect(wrapper.find(container).find(DiffCommentCell).exists()).toBe(true); }, ); @@ -79,4 +79,55 @@ describe('DiffView', () => { }); expect(wrapper.find(DraftNote).exists()).toBe(true); }); + + describe('drag operations', () => { + it('sets `dragStart` onStartDragging', () => { + const wrapper = createWrapper({ diffLines: [{}] }); + + wrapper.findComponent(DiffRow).vm.$emit('startdragging', { test: true }); + expect(wrapper.vm.dragStart).toEqual({ test: true }); + }); + + it('does not call `setSelectedCommentPosition` on different chunks onDragOver', () => { + const wrapper = createWrapper({ diffLines: [{}] }); + const diffRow = getDiffRow(wrapper); + + diffRow.$emit('startdragging', { chunk: 0 }); + diffRow.$emit('enterdragging', { chunk: 1 }); + + expect(setSelectedCommentPosition).not.toHaveBeenCalled(); + }); + + it.each` + start | end | expectation + ${1} | ${2} | ${{ start: { index: 1 }, end: { index: 2 } }} + ${2} | ${1} | ${{ start: { index: 1 }, end: { index: 2 } }} + ${1} | ${1} | ${{ start: { index: 1 }, end: { index: 1 } }} + `( + 'calls `setSelectedCommentPosition` with correct `updatedLineRange`', + ({ start, end, expectation }) => { + const wrapper = createWrapper({ diffLines: [{}] }); + const diffRow = getDiffRow(wrapper); + + diffRow.$emit('startdragging', { chunk: 1, index: start }); + diffRow.$emit('enterdragging', { chunk: 1, index: end }); + + const arg = setSelectedCommentPosition.mock.calls[0][1]; + + expect(arg).toMatchObject(expectation); + }, + ); + + it('sets `dragStart` to null onStopDragging', () => { + const wrapper = createWrapper({ diffLines: [{}] }); + const diffRow = getDiffRow(wrapper); + + diffRow.$emit('startdragging', { test: true }); + expect(wrapper.vm.dragStart).toEqual({ test: true }); + + diffRow.$emit('stopdragging'); + expect(wrapper.vm.dragStart).toBeNull(); + expect(showCommentForm).toHaveBeenCalled(); + }); + }); }); diff --git a/spec/frontend/diffs/components/file_row_stats_spec.js b/spec/frontend/diffs/components/file_row_stats_spec.js index 34d85ba10b0..3f5a63c19e5 100644 --- a/spec/frontend/diffs/components/file_row_stats_spec.js +++ b/spec/frontend/diffs/components/file_row_stats_spec.js @@ -1,33 +1,21 @@ -import Vue from 'vue'; -import mountComponent from 'helpers/vue_mount_component_helper'; +import { mount } from '@vue/test-utils'; import FileRowStats from '~/diffs/components/file_row_stats.vue'; describe('Diff file row stats', () => { - let Component; - let vm; - - beforeAll(() => { - Component = Vue.extend(FileRowStats); - }); - - beforeEach(() => { - vm = mountComponent(Component, { + const wrapper = mount(FileRowStats, { + propsData: { file: { addedLines: 20, removedLines: 10, }, - }); - }); - - afterEach(() => { - vm.$destroy(); + }, }); it('renders added lines count', () => { - expect(vm.$el.querySelector('.cgreen').textContent).toContain('+20'); + expect(wrapper.find('.cgreen').text()).toContain('+20'); }); it('renders removed lines count', () => { - expect(vm.$el.querySelector('.cred').textContent).toContain('-10'); + expect(wrapper.find('.cred').text()).toContain('-10'); }); }); diff --git a/spec/frontend/diffs/components/hidden_files_warning_spec.js b/spec/frontend/diffs/components/hidden_files_warning_spec.js index 6fb4e4645f8..3f1f23a40f5 100644 --- a/spec/frontend/diffs/components/hidden_files_warning_spec.js +++ b/spec/frontend/diffs/components/hidden_files_warning_spec.js @@ -26,13 +26,15 @@ describe('HiddenFilesWarning', () => { }); it('has a correct plain diff URL', () => { - const plainDiffLink = wrapper.findAll('a').wrappers.filter(x => x.text() === 'Plain diff')[0]; + const plainDiffLink = wrapper.findAll('a').wrappers.filter((x) => x.text() === 'Plain diff')[0]; expect(plainDiffLink.attributes('href')).toBe(propsData.plainDiffPath); }); it('has a correct email patch URL', () => { - const emailPatchLink = wrapper.findAll('a').wrappers.filter(x => x.text() === 'Email patch')[0]; + const emailPatchLink = wrapper + .findAll('a') + .wrappers.filter((x) => x.text() === 'Email patch')[0]; expect(emailPatchLink.attributes('href')).toBe(propsData.emailPatchPath); }); diff --git a/spec/frontend/diffs/components/image_diff_overlay_spec.js b/spec/frontend/diffs/components/image_diff_overlay_spec.js index 087715111b4..93c9b922fdd 100644 --- a/spec/frontend/diffs/components/image_diff_overlay_spec.js +++ b/spec/frontend/diffs/components/image_diff_overlay_spec.js @@ -21,6 +21,11 @@ describe('Diffs image diff overlay component', () => { wrapper = shallowMount(ImageDiffOverlay, { store, + parentComponent: { + data() { + return dimensions; + }, + }, propsData: { discussions: [...imageDiffDiscussions], fileHash: 'ABC', @@ -28,9 +33,6 @@ describe('Diffs image diff overlay component', () => { renderedHeight: 200, ...props, }, - methods: { - getImageDimensions: jest.fn().mockReturnValue(dimensions), - }, }); } @@ -49,18 +51,8 @@ describe('Diffs image diff overlay component', () => { createComponent(); const imageBadges = getAllImageBadges(); - expect( - imageBadges - .at(0) - .text() - .trim(), - ).toBe('1'); - expect( - imageBadges - .at(1) - .text() - .trim(), - ).toBe('2'); + expect(imageBadges.at(0).text().trim()).toBe('1'); + expect(imageBadges.at(1).text().trim()).toBe('2'); }); it('renders icon when showCommentIcon is true', () => { @@ -124,7 +116,7 @@ describe('Diffs image diff overlay component', () => { describe('comment form', () => { const getCommentIndicator = () => wrapper.find('.comment-indicator'); beforeEach(() => { - createComponent({ canComment: true }, store => { + createComponent({ canComment: true }, (store) => { store.state.diffs.commentForms.push({ fileHash: 'ABC', x: 20, diff --git a/spec/frontend/diffs/components/no_changes_spec.js b/spec/frontend/diffs/components/no_changes_spec.js index 78805a1cddc..df9af51f9cf 100644 --- a/spec/frontend/diffs/components/no_changes_spec.js +++ b/spec/frontend/diffs/components/no_changes_spec.js @@ -1,20 +1,22 @@ -import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { createLocalVue, shallowMount, mount } from '@vue/test-utils'; import Vuex from 'vuex'; import { GlButton } from '@gitlab/ui'; import { createStore } from '~/mr_notes/stores'; import NoChanges from '~/diffs/components/no_changes.vue'; +import diffsMockData from '../mock_data/merge_request_diffs'; -describe('Diff no changes empty state', () => { - let vm; +const localVue = createLocalVue(); +localVue.use(Vuex); - function createComponent(extendStore = () => {}) { - const localVue = createLocalVue(); - localVue.use(Vuex); +const TEST_TARGET_BRANCH = 'foo'; +const TEST_SOURCE_BRANCH = 'dev/update'; - const store = createStore(); - extendStore(store); +describe('Diff no changes empty state', () => { + let wrapper; + let store; - vm = shallowMount(NoChanges, { + function createComponent(mountFn = shallowMount) { + wrapper = mountFn(NoChanges, { localVue, store, propsData: { @@ -23,26 +25,61 @@ describe('Diff no changes empty state', () => { }); } + beforeEach(() => { + store = createStore(); + store.state.diffs.mergeRequestDiff = {}; + store.state.notes.noteableData = { + target_branch: TEST_TARGET_BRANCH, + source_branch: TEST_SOURCE_BRANCH, + }; + store.state.diffs.mergeRequestDiffs = diffsMockData; + }); + afterEach(() => { - vm.destroy(); + wrapper.destroy(); + wrapper = null; }); + const findMessage = () => wrapper.find('[data-testid="no-changes-message"]'); + it('prevents XSS', () => { - createComponent(store => { - // eslint-disable-next-line no-param-reassign - store.state.notes.noteableData = { - source_branch: '<script>alert("test");</script>', - target_branch: '<script>alert("test");</script>', - }; - }); + store.state.notes.noteableData = { + source_branch: '<script>alert("test");</script>', + target_branch: '<script>alert("test");</script>', + }; - expect(vm.find('script').exists()).toBe(false); + createComponent(); + + expect(wrapper.find('script').exists()).toBe(false); }); describe('Renders', () => { it('Show create commit button', () => { createComponent(); - expect(vm.find(GlButton).exists()).toBe(true); + + expect(wrapper.find(GlButton).exists()).toBe(true); }); + + it.each` + expectedText | sourceIndex | targetIndex + ${`No changes between ${TEST_SOURCE_BRANCH} and ${TEST_TARGET_BRANCH}`} | ${null} | ${null} + ${`No changes between ${TEST_SOURCE_BRANCH} and version 1`} | ${diffsMockData[0].version_index} | ${1} + ${`No changes between version 3 and version 2`} | ${3} | ${2} + ${`No changes between version 3 and ${TEST_TARGET_BRANCH}`} | ${3} | ${-1} + `( + 'renders text "$expectedText" (sourceIndex=$sourceIndex and targetIndex=$targetIndex)', + ({ expectedText, targetIndex, sourceIndex }) => { + if (targetIndex !== null) { + store.state.diffs.startVersion = { version_index: targetIndex }; + } + if (sourceIndex !== null) { + store.state.diffs.mergeRequestDiff.version_index = sourceIndex; + } + + createComponent(mount); + + expect(findMessage().text()).toBe(expectedText); + }, + ); }); }); diff --git a/spec/frontend/diffs/components/parallel_diff_table_row_spec.js b/spec/frontend/diffs/components/parallel_diff_table_row_spec.js index 57eff177261..445553706b7 100644 --- a/spec/frontend/diffs/components/parallel_diff_table_row_spec.js +++ b/spec/frontend/diffs/components/parallel_diff_table_row_spec.js @@ -40,7 +40,7 @@ describe('ParallelDiffTableRow', () => { vm = wrapper.vm; }); - it('does not highlight non empty line content when line does not match highlighted row', done => { + it('does not highlight non empty line content when line does not match highlighted row', (done) => { vm.$nextTick() .then(() => { expect(vm.$el.querySelector('.line_content.right-side').classList).not.toContain('hll'); @@ -49,7 +49,7 @@ describe('ParallelDiffTableRow', () => { .catch(done.fail); }); - it('highlights nonempty line content when line is the highlighted row', done => { + it('highlights nonempty line content when line is the highlighted row', (done) => { vm.$nextTick() .then(() => { vm.$store.state.diffs.highlightedRow = rightLine.line_code; @@ -86,7 +86,7 @@ describe('ParallelDiffTableRow', () => { }).$mount(); }); - it('does not highlight either line when line does not match highlighted row', done => { + it('does not highlight either line when line does not match highlighted row', (done) => { vm.$nextTick() .then(() => { expect(vm.$el.querySelector('.line_content.right-side').classList).not.toContain('hll'); @@ -96,7 +96,7 @@ describe('ParallelDiffTableRow', () => { .catch(done.fail); }); - it('adds hll class to lineContent when line is the highlighted row', done => { + it('adds hll class to lineContent when line is the highlighted row', (done) => { vm.$nextTick() .then(() => { vm.$store.state.diffs.highlightedRow = rightLine.line_code; @@ -112,7 +112,7 @@ describe('ParallelDiffTableRow', () => { }); describe('sets coverage title and class', () => { - it('for lines with coverage', done => { + it('for lines with coverage', (done) => { vm.$nextTick() .then(() => { const name = diffFileMockData.file_path; @@ -132,7 +132,7 @@ describe('ParallelDiffTableRow', () => { .catch(done.fail); }); - it('for lines without coverage', done => { + it('for lines without coverage', (done) => { vm.$nextTick() .then(() => { const name = diffFileMockData.file_path; @@ -152,7 +152,7 @@ describe('ParallelDiffTableRow', () => { .catch(done.fail); }); - it('for unknown lines', done => { + it('for unknown lines', (done) => { vm.$nextTick() .then(() => { vm.$store.state.diffs.coverageFiles = {}; diff --git a/spec/frontend/diffs/components/settings_dropdown_spec.js b/spec/frontend/diffs/components/settings_dropdown_spec.js index eb9f9b4db73..fcb627c570a 100644 --- a/spec/frontend/diffs/components/settings_dropdown_spec.js +++ b/spec/frontend/diffs/components/settings_dropdown_spec.js @@ -73,7 +73,7 @@ describe('Diff settings dropdown component', () => { }); it('sets list button as selected when renderTreeList is false', () => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { renderTreeList: false, }); @@ -84,7 +84,7 @@ describe('Diff settings dropdown component', () => { }); it('sets tree button as selected when renderTreeList is true', () => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { renderTreeList: true, }); @@ -97,7 +97,7 @@ describe('Diff settings dropdown component', () => { describe('compare changes', () => { it('sets inline button as selected', () => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { diffViewType: INLINE_DIFF_VIEW_TYPE, }); @@ -108,7 +108,7 @@ describe('Diff settings dropdown component', () => { }); it('sets parallel button as selected', () => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { diffViewType: PARALLEL_DIFF_VIEW_TYPE, }); @@ -137,7 +137,7 @@ describe('Diff settings dropdown component', () => { describe('whitespace toggle', () => { it('does not set as checked when showWhitespace is false', () => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { showWhitespace: false, }); @@ -147,7 +147,7 @@ describe('Diff settings dropdown component', () => { }); it('sets as checked when showWhitespace is true', () => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { showWhitespace: true, }); @@ -183,7 +183,7 @@ describe('Diff settings dropdown component', () => { `( 'sets the checkbox to { checked: $checked } if the fileByFile setting is $fileByFile', async ({ fileByFile, checked }) => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { viewDiffsFileByFile: fileByFile, }); @@ -202,7 +202,7 @@ describe('Diff settings dropdown component', () => { `( 'when the file by file setting starts as $start, toggling the checkbox should emit an event set to $emit', async ({ start, emit }) => { - createComponent(store => { + createComponent((store) => { Object.assign(store.state.diffs, { viewDiffsFileByFile: start, }); diff --git a/spec/frontend/diffs/components/tree_list_spec.js b/spec/frontend/diffs/components/tree_list_spec.js index c89403e4869..4666321e0c2 100644 --- a/spec/frontend/diffs/components/tree_list_spec.js +++ b/spec/frontend/diffs/components/tree_list_spec.js @@ -88,16 +88,8 @@ describe('Diffs tree list component', () => { it('renders tree', () => { expect(getFileRows()).toHaveLength(2); - expect( - getFileRows() - .at(0) - .html(), - ).toContain('index.js'); - expect( - getFileRows() - .at(1) - .html(), - ).toContain('app'); + expect(getFileRows().at(0).html()).toContain('index.js'); + expect(getFileRows().at(1).html()).toContain('app'); }); it('hides file stats', () => { @@ -111,9 +103,7 @@ describe('Diffs tree list component', () => { it('calls toggleTreeOpen when clicking folder', () => { jest.spyOn(wrapper.vm.$store, 'dispatch').mockReturnValue(undefined); - getFileRows() - .at(1) - .trigger('click'); + getFileRows().at(1).trigger('click'); expect(wrapper.vm.$store.dispatch).toHaveBeenCalledWith('diffs/toggleTreeOpen', 'app'); }); diff --git a/spec/frontend/diffs/store/actions_spec.js b/spec/frontend/diffs/store/actions_spec.js index fef7676e795..056ac23fcf7 100644 --- a/spec/frontend/diffs/store/actions_spec.js +++ b/spec/frontend/diffs/store/actions_spec.js @@ -2,7 +2,8 @@ import MockAdapter from 'axios-mock-adapter'; import Cookies from 'js-cookie'; import mockDiffFile from 'jest/diffs/mock_data/diff_file'; import { useLocalStorageSpy } from 'helpers/local_storage_helper'; -import { TEST_HOST } from 'jest/helpers/test_constants'; +import { TEST_HOST } from 'helpers/test_constants'; +import testAction from 'helpers/vuex_action_helper'; import { DIFF_VIEW_COOKIE_NAME, INLINE_DIFF_VIEW_TYPE, @@ -49,11 +50,11 @@ import { setCurrentDiffFileIdFromNote, navigateToDiffFileIndex, setFileByFile, + reviewFile, } from '~/diffs/store/actions'; import eventHub from '~/notes/event_hub'; import * as types from '~/diffs/store/mutation_types'; import axios from '~/lib/utils/axios_utils'; -import testAction from '../../helpers/vuex_action_helper'; import * as utils from '~/diffs/store/utils'; import * as commonUtils from '~/lib/utils/common_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; @@ -77,22 +78,22 @@ describe('DiffsStoreActions', () => { jest.spyOn(commonUtils, 'scrollToElement').mockImplementation(() => null); jest.spyOn(utils, 'convertExpandLines').mockImplementation(() => null); jest.spyOn(utils, 'idleCallback').mockImplementation(() => null); - ['requestAnimationFrame', 'requestIdleCallback'].forEach(method => { - global[method] = cb => { + ['requestAnimationFrame', 'requestIdleCallback'].forEach((method) => { + global[method] = (cb) => { cb(); }; }); }); afterEach(() => { - ['requestAnimationFrame', 'requestIdleCallback'].forEach(method => { + ['requestAnimationFrame', 'requestIdleCallback'].forEach((method) => { global[method] = originalMethods[method]; }); createFlash.mockClear(); }); describe('setBaseConfig', () => { - it('should set given endpoint and project path', done => { + it('should set given endpoint and project path', (done) => { const endpoint = '/diffs/set/endpoint'; const endpointMetadata = '/diffs/set/endpoint/metadata'; const endpointBatch = '/diffs/set/endpoint/batch'; @@ -152,7 +153,7 @@ describe('DiffsStoreActions', () => { mock.restore(); }); - it('should fetch batch diff files', done => { + it('should fetch batch diff files', (done) => { const endpointBatch = '/fetch/diffs_batch'; const res1 = { diff_files: [{ file_hash: 'test' }], pagination: { next_page: 2 } }; const res2 = { diff_files: [{ file_hash: 'test2' }], pagination: {} }; @@ -240,7 +241,7 @@ describe('DiffsStoreActions', () => { mock.onGet(endpointMetadata).reply(200, diffMetadata); }); - it('should fetch diff meta information', done => { + it('should fetch diff meta information', (done) => { testAction( fetchDiffFilesMeta, {}, @@ -270,8 +271,8 @@ describe('DiffsStoreActions', () => { afterEach(() => mock.restore()); - it('should commit SET_COVERAGE_DATA with received response', done => { - const data = { files: { 'app.js': { '1': 0, '2': 1 } } }; + it('should commit SET_COVERAGE_DATA with received response', (done) => { + const data = { files: { 'app.js': { 1: 0, 2: 1 } } }; mock.onGet(endpointCoverage).reply(200, { data }); @@ -285,7 +286,7 @@ describe('DiffsStoreActions', () => { ); }); - it('should show flash on API error', done => { + it('should show flash on API error', (done) => { mock.onGet(endpointCoverage).reply(400); testAction(fetchCoverageFiles, {}, { endpointCoverage }, [], [], () => { @@ -310,7 +311,7 @@ describe('DiffsStoreActions', () => { window.location.hash = ''; }); - it('should merge discussions into diffs', done => { + it('should merge discussions into diffs', (done) => { window.location.hash = 'ABC_123'; const state = { @@ -404,7 +405,7 @@ describe('DiffsStoreActions', () => { ); }); - it('dispatches setCurrentDiffFileIdFromNote with note ID', done => { + it('dispatches setCurrentDiffFileIdFromNote with note ID', (done) => { window.location.hash = 'note_123'; testAction( @@ -419,7 +420,7 @@ describe('DiffsStoreActions', () => { }); describe('removeDiscussionsFromDiff', () => { - it('should remove discussions from diffs', done => { + it('should remove discussions from diffs', (done) => { const state = { diffFiles: [ { @@ -511,7 +512,7 @@ describe('DiffsStoreActions', () => { }); describe('setInlineDiffViewType', () => { - it('should set diff view type to inline and also set the cookie properly', done => { + it('should set diff view type to inline and also set the cookie properly', (done) => { testAction( setInlineDiffViewType, null, @@ -529,7 +530,7 @@ describe('DiffsStoreActions', () => { }); describe('setParallelDiffViewType', () => { - it('should set diff view type to parallel and also set the cookie properly', done => { + it('should set diff view type to parallel and also set the cookie properly', (done) => { testAction( setParallelDiffViewType, null, @@ -547,7 +548,7 @@ describe('DiffsStoreActions', () => { }); describe('showCommentForm', () => { - it('should call mutation to show comment form', done => { + it('should call mutation to show comment form', (done) => { const payload = { lineCode: 'lineCode', fileHash: 'hash' }; testAction( @@ -562,7 +563,7 @@ describe('DiffsStoreActions', () => { }); describe('cancelCommentForm', () => { - it('should call mutation to cancel comment form', done => { + it('should call mutation to cancel comment form', (done) => { const payload = { lineCode: 'lineCode', fileHash: 'hash' }; testAction( @@ -577,7 +578,7 @@ describe('DiffsStoreActions', () => { }); describe('loadMoreLines', () => { - it('should call mutation to show comment form', done => { + it('should call mutation to show comment form', (done) => { const endpoint = '/diffs/load/more/lines'; const params = { since: 6, to: 26 }; const lineNumbers = { oldLineNumber: 3, newLineNumber: 5 }; @@ -610,7 +611,7 @@ describe('DiffsStoreActions', () => { describe('loadCollapsedDiff', () => { const state = { showWhitespace: true }; - it('should fetch data and call mutation with response and the give parameter', done => { + it('should fetch data and call mutation with response and the give parameter', (done) => { const file = { hash: 123, load_collapsed_diff_url: '/load/collapsed/diff/url' }; const data = { hash: 123, parallelDiffLines: [{ lineCode: 1 }] }; const mock = new MockAdapter(axios); @@ -810,7 +811,7 @@ describe('DiffsStoreActions', () => { }); describe('saveDiffDiscussion', () => { - it('dispatches actions', done => { + it('dispatches actions', (done) => { const commitId = 'something'; const formData = { diffFile: { ...mockDiffFile }, @@ -822,7 +823,7 @@ describe('DiffsStoreActions', () => { id: commitId, }, }; - const dispatch = jest.fn(name => { + const dispatch = jest.fn((name) => { switch (name) { case 'saveNote': return Promise.resolve({ @@ -854,7 +855,7 @@ describe('DiffsStoreActions', () => { }); describe('toggleTreeOpen', () => { - it('commits TOGGLE_FOLDER_OPEN', done => { + it('commits TOGGLE_FOLDER_OPEN', (done) => { testAction( toggleTreeOpen, 'path', @@ -903,7 +904,7 @@ describe('DiffsStoreActions', () => { }); describe('setShowTreeList', () => { - it('commits toggle', done => { + it('commits toggle', (done) => { testAction( setShowTreeList, { showTreeList: true }, @@ -991,7 +992,7 @@ describe('DiffsStoreActions', () => { }); describe('setRenderTreeList', () => { - it('commits SET_RENDER_TREE_LIST', done => { + it('commits SET_RENDER_TREE_LIST', (done) => { testAction( setRenderTreeList, true, @@ -1014,7 +1015,7 @@ describe('DiffsStoreActions', () => { jest.spyOn(eventHub, '$emit').mockImplementation(); }); - it('commits SET_SHOW_WHITESPACE', done => { + it('commits SET_SHOW_WHITESPACE', (done) => { testAction( setShowWhitespace, { showWhitespace: true }, @@ -1057,13 +1058,13 @@ describe('DiffsStoreActions', () => { }); describe('setRenderIt', () => { - it('commits RENDER_FILE', done => { + it('commits RENDER_FILE', (done) => { testAction(setRenderIt, 'file', {}, [{ type: types.RENDER_FILE, payload: 'file' }], [], done); }); }); describe('receiveFullDiffError', () => { - it('updates state with the file that did not load', done => { + it('updates state with the file that did not load', (done) => { testAction( receiveFullDiffError, 'file', @@ -1091,7 +1092,7 @@ describe('DiffsStoreActions', () => { mock.onGet(`${TEST_HOST}/context`).replyOnce(200, ['test']); }); - it('commits the success and dispatches an action to expand the new lines', done => { + it('commits the success and dispatches an action to expand the new lines', (done) => { const file = { context_lines_path: `${TEST_HOST}/context`, file_path: 'test', @@ -1113,7 +1114,7 @@ describe('DiffsStoreActions', () => { mock.onGet(`${TEST_HOST}/context`).replyOnce(500); }); - it('dispatches receiveFullDiffError', done => { + it('dispatches receiveFullDiffError', (done) => { testAction( fetchFullDiff, { context_lines_path: `${TEST_HOST}/context`, file_path: 'test', file_hash: 'test' }, @@ -1135,7 +1136,7 @@ describe('DiffsStoreActions', () => { }; }); - it('dispatches fetchFullDiff when file is not expanded', done => { + it('dispatches fetchFullDiff when file is not expanded', (done) => { testAction( toggleFullDiff, 'test', @@ -1211,7 +1212,7 @@ describe('DiffsStoreActions', () => { }); describe('setFileUserCollapsed', () => { - it('commits SET_FILE_COLLAPSED', done => { + it('commits SET_FILE_COLLAPSED', (done) => { testAction( setFileCollapsedByUser, { filePath: 'test', collapsed: true }, @@ -1230,12 +1231,12 @@ describe('DiffsStoreActions', () => { describe('setExpandedDiffLines', () => { beforeEach(() => { - utils.idleCallback.mockImplementation(cb => { + utils.idleCallback.mockImplementation((cb) => { cb({ timeRemaining: () => 50 }); }); }); - it('commits SET_CURRENT_VIEW_DIFF_FILE_LINES when lines less than MAX_RENDERING_DIFF_LINES', done => { + it('commits SET_CURRENT_VIEW_DIFF_FILE_LINES when lines less than MAX_RENDERING_DIFF_LINES', (done) => { utils.convertExpandLines.mockImplementation(() => ['test']); testAction( @@ -1253,7 +1254,7 @@ describe('DiffsStoreActions', () => { ); }); - it('commits ADD_CURRENT_VIEW_DIFF_FILE_LINES when lines more than MAX_RENDERING_DIFF_LINES', done => { + it('commits ADD_CURRENT_VIEW_DIFF_FILE_LINES when lines more than MAX_RENDERING_DIFF_LINES', (done) => { const lines = new Array(501).fill().map((_, i) => `line-${i}`); utils.convertExpandLines.mockReturnValue(lines); @@ -1280,7 +1281,7 @@ describe('DiffsStoreActions', () => { }); describe('setSuggestPopoverDismissed', () => { - it('commits SET_SHOW_SUGGEST_POPOVER', done => { + it('commits SET_SHOW_SUGGEST_POPOVER', (done) => { const state = { dismissEndpoint: `${TEST_HOST}/-/user_callouts` }; const mock = new MockAdapter(axios); mock.onPost(state.dismissEndpoint).reply(200, {}); @@ -1409,7 +1410,7 @@ describe('DiffsStoreActions', () => { const state = { diffFiles: [{ file_hash: '123' }] }; const rootGetters = { getDiscussion: () => ({ diff_file: { file_hash: '123' } }), - notesById: { '1': { discussion_id: '2' } }, + notesById: { 1: { discussion_id: '2' } }, }; setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); @@ -1422,7 +1423,7 @@ describe('DiffsStoreActions', () => { const state = { diffFiles: [{ file_hash: '123' }] }; const rootGetters = { getDiscussion: () => ({ id: '1' }), - notesById: { '1': { discussion_id: '2' } }, + notesById: { 1: { discussion_id: '2' } }, }; setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); @@ -1435,7 +1436,7 @@ describe('DiffsStoreActions', () => { const state = { diffFiles: [{ file_hash: '123' }] }; const rootGetters = { getDiscussion: () => ({ diff_file: { file_hash: '124' } }), - notesById: { '1': { discussion_id: '2' } }, + notesById: { 1: { discussion_id: '2' } }, }; setCurrentDiffFileIdFromNote({ commit, state, rootGetters }, '1'); @@ -1445,7 +1446,7 @@ describe('DiffsStoreActions', () => { }); describe('navigateToDiffFileIndex', () => { - it('commits VIEW_DIFF_FILE', done => { + it('commits VIEW_DIFF_FILE', (done) => { testAction( navigateToDiffFileIndex, 0, @@ -1472,4 +1473,46 @@ describe('DiffsStoreActions', () => { ); }); }); + + describe('reviewFile', () => { + const file = { + id: '123', + file_identifier_hash: 'abc', + load_collapsed_diff_url: 'gitlab-org/gitlab-test/-/merge_requests/1/diffs', + }; + it.each` + reviews | diffFile | reviewed + ${{ abc: ['123'] }} | ${file} | ${true} + ${{}} | ${file} | ${false} + `( + 'sets reviews ($reviews) to localStorage and state for file $file if it is marked reviewed=$reviewed', + ({ reviews, diffFile, reviewed }) => { + const commitSpy = jest.fn(); + const getterSpy = jest.fn().mockReturnValue([]); + + reviewFile( + { + commit: commitSpy, + getters: { + fileReviews: getterSpy, + }, + state: { + mrReviews: { abc: ['123'] }, + }, + }, + { + file: diffFile, + reviewed, + }, + ); + + expect(localStorage.setItem).toHaveBeenCalledTimes(1); + expect(localStorage.setItem).toHaveBeenCalledWith( + 'gitlab-org/gitlab-test/-/merge_requests/1-file-reviews', + JSON.stringify(reviews), + ); + expect(commitSpy).toHaveBeenCalledWith(types.SET_MR_FILE_REVIEWS, reviews); + }, + ); + }); }); diff --git a/spec/frontend/diffs/store/getters_spec.js b/spec/frontend/diffs/store/getters_spec.js index 7e936c561fc..4d7f861ac22 100644 --- a/spec/frontend/diffs/store/getters_spec.js +++ b/spec/frontend/diffs/store/getters_spec.js @@ -251,9 +251,12 @@ describe('Diffs Module Getters', () => { discussionMock.diff_file.file_hash = diffFileMock.file_hash; expect( - getters.getDiffFileDiscussions(localState, {}, {}, { discussions: [discussionMock] })( - diffFileMock, - ).length, + getters.getDiffFileDiscussions( + localState, + {}, + {}, + { discussions: [discussionMock] }, + )(diffFileMock).length, ).toEqual(1); }); @@ -345,7 +348,7 @@ describe('Diffs Module Getters', () => { describe('fileLineCoverage', () => { beforeEach(() => { - Object.assign(localState.coverageFiles, { files: { 'app.js': { '1': 0, '2': 5 } } }); + Object.assign(localState.coverageFiles, { files: { 'app.js': { 1: 0, 2: 5 } } }); }); it('returns empty object when no coverage data is available', () => { @@ -372,4 +375,26 @@ describe('Diffs Module Getters', () => { }); }); }); + + describe('fileReviews', () => { + const file1 = { id: '123', file_identifier_hash: 'abc' }; + const file2 = { id: '098', file_identifier_hash: 'abc' }; + + it.each` + reviews | files | fileReviews + ${{}} | ${[file1, file2]} | ${[false, false]} + ${{ abc: ['123'] }} | ${[file1, file2]} | ${[true, false]} + ${{ abc: ['098'] }} | ${[file1, file2]} | ${[false, true]} + ${{ def: ['123'] }} | ${[file1, file2]} | ${[false, false]} + ${{ abc: ['123'], def: ['098'] }} | ${[]} | ${[]} + `( + 'returns $fileReviews based on the diff files in state and the existing reviews $reviews', + ({ reviews, files, fileReviews }) => { + localState.diffFiles = files; + localState.mrReviews = reviews; + + expect(getters.fileReviews(localState)).toStrictEqual(fileReviews); + }, + ); + }); }); diff --git a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js index 0343ef75732..f7954515422 100644 --- a/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js +++ b/spec/frontend/diffs/store/getters_versions_dropdowns_spec.js @@ -49,7 +49,7 @@ describe('Compare diff version dropdowns', () => { let expectedHeadVersion; const originalLocation = window.location; - const setupTest = includeDiffHeadParam => { + const setupTest = (includeDiffHeadParam) => { const diffHeadParam = includeDiffHeadParam ? '?diff_head=true' : ''; Object.defineProperty(window, 'location', { @@ -81,7 +81,7 @@ describe('Compare diff version dropdowns', () => { }; }; - const assertVersions = targetVersions => { + const assertVersions = (targetVersions) => { // base and head should be the last two versions in that order const targetBaseVersion = targetVersions[targetVersions.length - 2]; const targetHeadVersion = targetVersions[targetVersions.length - 1]; @@ -136,6 +136,7 @@ describe('Compare diff version dropdowns', () => { ...firstDiff, href: firstDiff.version_path, commitsText: `${firstDiff.commits_count} commits,`, + isLatestVersion: true, versionName: 'latest version', selected: true, }; @@ -144,6 +145,9 @@ describe('Compare diff version dropdowns', () => { selectedSourceIndex: expectedShape.version_index, }); expect(sourceVersions[0]).toEqual(expectedShape); - expect(sourceVersions[1].selected).toBe(false); + expect(sourceVersions[1]).toMatchObject({ + selected: false, + isLatestVersion: false, + }); }); }); diff --git a/spec/frontend/diffs/store/mutations_spec.js b/spec/frontend/diffs/store/mutations_spec.js index 13e7cad835d..2c342d8e2a5 100644 --- a/spec/frontend/diffs/store/mutations_spec.js +++ b/spec/frontend/diffs/store/mutations_spec.js @@ -105,7 +105,7 @@ describe('DiffsStoreMutations', () => { describe('SET_COVERAGE_DATA', () => { it('should set coverage data properly', () => { const state = { coverageFiles: {} }; - const coverage = { 'app.js': { '1': 0, '2': 1 } }; + const coverage = { 'app.js': { 1: 0, 2: 1 } }; mutations[types.SET_COVERAGE_DATA](state, coverage); @@ -906,4 +906,19 @@ describe('DiffsStoreMutations', () => { expect(state.viewDiffsFileByFile).toBe(value); }); }); + + describe('SET_MR_FILE_REVIEWS', () => { + it.each` + newReviews | oldReviews + ${{ abc: ['123'] }} | ${{}} + ${{ abc: [] }} | ${{ abc: ['123'] }} + ${{}} | ${{ abc: ['123'] }} + `('sets mrReviews to $newReviews', ({ newReviews, oldReviews }) => { + const state = { mrReviews: oldReviews }; + + mutations[types.SET_MR_FILE_REVIEWS](state, newReviews); + + expect(state.mrReviews).toStrictEqual(newReviews); + }); + }); }); diff --git a/spec/frontend/diffs/store/utils_spec.js b/spec/frontend/diffs/store/utils_spec.js index 7ee97224707..a19e5e91677 100644 --- a/spec/frontend/diffs/store/utils_spec.js +++ b/spec/frontend/diffs/store/utils_spec.js @@ -481,7 +481,7 @@ describe('DiffsStoreUtils', () => { }); it('adds the `.brokenSymlink` property to each diff file', () => { - preparedDiff.diff_files.forEach(file => { + preparedDiff.diff_files.forEach((file) => { expect(file).toEqual(expect.objectContaining({ brokenSymlink: false })); }); }); @@ -492,9 +492,9 @@ describe('DiffsStoreUtils', () => { ...splitInlineDiff.diff_files, ...splitParallelDiff.diff_files, ...completedDiff.diff_files, - ].flatMap(file => [...file[INLINE_DIFF_LINES_KEY]]); + ].flatMap((file) => [...file[INLINE_DIFF_LINES_KEY]]); - lines.forEach(line => { + lines.forEach((line) => { expect(line.commentsDisabled).toBe(false); }); }); @@ -560,7 +560,7 @@ describe('DiffsStoreUtils', () => { }); it('adds the `.brokenSymlink` property to each meta diff file', () => { - preparedDiffFiles.forEach(file => { + preparedDiffFiles.forEach((file) => { expect(file).toMatchObject({ brokenSymlink: false }); }); }); @@ -1119,22 +1119,87 @@ describe('DiffsStoreUtils', () => { }); }); + describe('isConflictMarker', () => { + it.each` + type | expected + ${'conflict_marker_our'} | ${true} + ${'conflict_marker_their'} | ${true} + ${'conflict_their'} | ${false} + ${'conflict_our'} | ${false} + `('returns $expected when type is $type', ({ type, expected }) => { + expect(utils.isConflictMarker({ type })).toBe(expected); + }); + }); + + describe('isConflictOur', () => { + it.each` + type | expected + ${'conflict_marker_our'} | ${false} + ${'conflict_marker_their'} | ${false} + ${'conflict_their'} | ${false} + ${'conflict_our'} | ${true} + `('returns $expected when type is $type', ({ type, expected }) => { + expect(utils.isConflictOur({ type })).toBe(expected); + }); + }); + + describe('isConflictTheir', () => { + it.each` + type | expected + ${'conflict_marker_our'} | ${false} + ${'conflict_marker_their'} | ${false} + ${'conflict_their'} | ${true} + ${'conflict_our'} | ${false} + `('returns $expected when type is $type', ({ type, expected }) => { + expect(utils.isConflictTheir({ type })).toBe(expected); + }); + }); + describe('parallelizeDiffLines', () => { it('converts inline diff lines to parallel diff lines', () => { const file = getDiffFileMock(); - expect(utils.parallelizeDiffLines(file[INLINE_DIFF_LINES_KEY])).toEqual( + expect(utils.parallelizeDiffLines(file[INLINE_DIFF_LINES_KEY])).toMatchObject( file.parallel_diff_lines, ); }); + it('converts conflicted diffs line', () => { + const lines = [ + { type: 'new' }, + { type: 'conflict_marker_our' }, + { type: 'conflict_our' }, + { type: 'conflict_marker' }, + { type: 'conflict_their' }, + { type: 'conflict_marker_their' }, + ]; + + expect(utils.parallelizeDiffLines(lines)).toEqual([ + { + left: null, + right: { + chunk: 0, + type: 'new', + }, + }, + { + left: { chunk: 0, type: 'conflict_marker_our' }, + right: { chunk: 0, type: 'conflict_marker_their' }, + }, + { + left: { chunk: 0, type: 'conflict_our' }, + right: { chunk: 0, type: 'conflict_their' }, + }, + ]); + }); + it('converts inline diff lines', () => { const file = getDiffFileMock(); const files = utils.parallelizeDiffLines(file.highlighted_diff_lines, true); - expect(files[5].left).toEqual(file.parallel_diff_lines[5].left); + expect(files[5].left).toMatchObject(file.parallel_diff_lines[5].left); expect(files[5].right).toBeNull(); - expect(files[6].left).toEqual(file.parallel_diff_lines[5].right); + expect(files[6].left).toMatchObject(file.parallel_diff_lines[5].right); expect(files[6].right).toBeNull(); }); }); diff --git a/spec/frontend/diffs/utils/diff_file_spec.js b/spec/frontend/diffs/utils/diff_file_spec.js index 2e6247b8c07..2de8db28e71 100644 --- a/spec/frontend/diffs/utils/diff_file_spec.js +++ b/spec/frontend/diffs/utils/diff_file_spec.js @@ -1,6 +1,8 @@ import { prepareRawDiffFile } from '~/diffs/utils/diff_file'; function getDiffFiles() { + const loadFull = 'namespace/project/-/merge_requests/12345/diff_for_path?file_identifier=abc'; + return [ { blob: { @@ -8,6 +10,7 @@ function getDiffFiles() { }, file_hash: 'ABC', // This file is just a normal file file_identifier_hash: 'ABC1', + load_collapsed_diff_url: loadFull, }, { blob: { @@ -15,6 +18,7 @@ function getDiffFiles() { }, file_hash: 'DEF', // This file replaces a symlink file_identifier_hash: 'DEF1', + load_collapsed_diff_url: loadFull, a_mode: '0', b_mode: '0755', }, @@ -24,6 +28,7 @@ function getDiffFiles() { }, file_hash: 'DEF', // This symlink is replaced by a file file_identifier_hash: 'DEF2', + load_collapsed_diff_url: loadFull, a_mode: '120000', b_mode: '0', }, @@ -33,6 +38,7 @@ function getDiffFiles() { }, file_hash: 'GHI', // This symlink replaces a file file_identifier_hash: 'GHI1', + load_collapsed_diff_url: loadFull, a_mode: '0', b_mode: '120000', }, @@ -42,6 +48,7 @@ function getDiffFiles() { }, file_hash: 'GHI', // This file is replaced by a symlink file_identifier_hash: 'GHI2', + load_collapsed_diff_url: loadFull, a_mode: '0755', b_mode: '0', }, @@ -86,11 +93,11 @@ describe('diff_file utilities', () => { it.each` fileIndex | id - ${0} | ${'8dcd585e-a421-4dab-a04e-6f88c81b7b4c'} - ${1} | ${'3f178b78-392b-44a4-bd7d-5d6192208a97'} - ${2} | ${'3d9e1354-cddf-4a11-8234-f0413521b2e5'} - ${3} | ${'460f005b-d29d-43c1-9a08-099a7c7f08de'} - ${4} | ${'d8c89733-6ce1-4455-ae3d-f8aad6ee99f9'} + ${0} | ${'68296a4f-f1c7-445a-bd0e-6e3b02c4eec0'} + ${1} | ${'051c9bb8-cdba-4eb7-b8d1-508906e6d8ba'} + ${2} | ${'ed3d53d5-5da0-412d-a3c6-7213f84e88d3'} + ${3} | ${'39d998dc-bc69-4b19-a6af-41e4369c2bd5'} + ${4} | ${'7072d115-ce39-423c-8346-9fcad58cd68e'} `('sets the file id properly { id: $id } on normal diff files', ({ fileIndex, id }) => { const preppedFile = prepareRawDiffFile({ file: files[fileIndex], @@ -122,5 +129,18 @@ describe('diff_file utilities', () => { expect(preppedFile).not.toHaveProp('id'); }); + + it('does not set the id property if the file is missing a `load_collapsed_diff_url` property', () => { + const fileMissingContentSha = { ...files[0] }; + + delete fileMissingContentSha.load_collapsed_diff_url; + + const preppedFile = prepareRawDiffFile({ + file: fileMissingContentSha, + allFiles: files, + }); + + expect(preppedFile).not.toHaveProp('id'); + }); }); }); diff --git a/spec/frontend/diffs/utils/file_reviews_spec.js b/spec/frontend/diffs/utils/file_reviews_spec.js new file mode 100644 index 00000000000..819426ee75f --- /dev/null +++ b/spec/frontend/diffs/utils/file_reviews_spec.js @@ -0,0 +1,146 @@ +import { useLocalStorageSpy } from 'helpers/local_storage_helper'; + +import { + getReviewsForMergeRequest, + setReviewsForMergeRequest, + isFileReviewed, + markFileReview, + reviewable, +} from '~/diffs/utils/file_reviews'; + +function getDefaultReviews() { + return { + abc: ['123', '098'], + }; +} + +describe('File Review(s) utilities', () => { + const mrPath = 'my/fake/mr/42'; + const storageKey = `${mrPath}-file-reviews`; + const file = { id: '123', file_identifier_hash: 'abc' }; + const storedValue = JSON.stringify(getDefaultReviews()); + let reviews; + + useLocalStorageSpy(); + + beforeEach(() => { + reviews = getDefaultReviews(); + localStorage.clear(); + }); + + describe('getReviewsForMergeRequest', () => { + it('fetches the appropriate stored reviews from localStorage', () => { + getReviewsForMergeRequest(mrPath); + + expect(localStorage.getItem).toHaveBeenCalledTimes(1); + expect(localStorage.getItem).toHaveBeenCalledWith(storageKey); + }); + + it('returns an empty object if there have never been stored reviews for this MR', () => { + expect(getReviewsForMergeRequest(mrPath)).toStrictEqual({}); + }); + + it.each` + data + ${'+++'} + ${'{ lookinGood: "yeah!", missingClosingBrace: "yeah :(" '} + `( + "returns an empty object if the stored reviews are corrupted/aren't parseable as JSON (like: $data)", + ({ data }) => { + localStorage.getItem.mockReturnValueOnce(data); + + expect(getReviewsForMergeRequest(mrPath)).toStrictEqual({}); + }, + ); + + it('fetches the reviews for the MR if they exist', () => { + localStorage.setItem(storageKey, storedValue); + + expect(getReviewsForMergeRequest(mrPath)).toStrictEqual(reviews); + }); + }); + + describe('setReviewsForMergeRequest', () => { + it('sets the new value to localStorage', () => { + setReviewsForMergeRequest(mrPath, reviews); + + expect(localStorage.setItem).toHaveBeenCalledTimes(1); + expect(localStorage.setItem).toHaveBeenCalledWith(storageKey, storedValue); + }); + + it('returns the new value for chainability', () => { + expect(setReviewsForMergeRequest(mrPath, reviews)).toStrictEqual(reviews); + }); + }); + + describe('isFileReviewed', () => { + it.each` + description | diffFile | fileReviews + ${'the file does not have an `id`'} | ${{ ...file, id: undefined }} | ${getDefaultReviews()} + ${'there are no reviews for the file'} | ${file} | ${{ ...getDefaultReviews(), abc: undefined }} + `('returns `false` if $description', ({ diffFile, fileReviews }) => { + expect(isFileReviewed(fileReviews, diffFile)).toBe(false); + }); + + it("returns `true` for a file if it's available in the provided reviews", () => { + expect(isFileReviewed(reviews, file)).toBe(true); + }); + }); + + describe('reviewable', () => { + it.each` + response | diffFile | description + ${true} | ${file} | ${'has an `.id` and a `.file_identifier_hash`'} + ${false} | ${{ file_identifier_hash: 'abc' }} | ${'does not have an `.id`'} + ${false} | ${{ ...file, id: undefined }} | ${'has an undefined `.id`'} + ${false} | ${{ ...file, id: null }} | ${'has a null `.id`'} + ${false} | ${{ ...file, id: 0 }} | ${'has an `.id` set to 0'} + ${false} | ${{ ...file, id: false }} | ${'has an `.id` set to false'} + ${false} | ${{ id: '123' }} | ${'does not have a `.file_identifier_hash`'} + ${false} | ${{ ...file, file_identifier_hash: undefined }} | ${'has an undefined `.file_identifier_hash`'} + ${false} | ${{ ...file, file_identifier_hash: null }} | ${'has a null `.file_identifier_hash`'} + ${false} | ${{ ...file, file_identifier_hash: 0 }} | ${'has a `.file_identifier_hash` set to 0'} + ${false} | ${{ ...file, file_identifier_hash: false }} | ${'has a `.file_identifier_hash` set to false'} + `('returns `$response` when the file $description`', ({ response, diffFile }) => { + expect(reviewable(diffFile)).toBe(response); + }); + }); + + describe('markFileReview', () => { + it("adds a review when there's nothing that already exists", () => { + expect(markFileReview(null, file)).toStrictEqual({ abc: ['123'] }); + }); + + it("overwrites an existing review if it's for the same file (identifier hash)", () => { + expect(markFileReview(reviews, file)).toStrictEqual(getDefaultReviews()); + }); + + it('removes a review from the list when `reviewed` is `false`', () => { + expect(markFileReview(reviews, file, false)).toStrictEqual({ abc: ['098'] }); + }); + + it('adds a new review if the file ID is new', () => { + const updatedFile = { ...file, id: '098' }; + const allReviews = markFileReview({ abc: ['123'] }, updatedFile); + + expect(allReviews).toStrictEqual(getDefaultReviews()); + expect(allReviews.abc).toStrictEqual(['123', '098']); + }); + + it.each` + description | diffFile + ${'missing an `.id`'} | ${{ file_identifier_hash: 'abc' }} + ${'missing a `.file_identifier_hash`'} | ${{ id: '123' }} + `("doesn't modify the reviews if the file is $description", ({ diffFile }) => { + expect(markFileReview(reviews, diffFile)).toStrictEqual(getDefaultReviews()); + }); + + it('removes the file key if there are no more reviews for it', () => { + let updated = markFileReview(reviews, file, false); + + updated = markFileReview(updated, { ...file, id: '098' }, false); + + expect(updated).toStrictEqual({}); + }); + }); +}); diff --git a/spec/frontend/diffs/utils/merge_request_spec.js b/spec/frontend/diffs/utils/merge_request_spec.js new file mode 100644 index 00000000000..8c7b1e1f2a5 --- /dev/null +++ b/spec/frontend/diffs/utils/merge_request_spec.js @@ -0,0 +1,31 @@ +import { getDerivedMergeRequestInformation } from '~/diffs/utils/merge_request'; +import { diffMetadata } from '../mock_data/diff_metadata'; + +describe('Merge Request utilities', () => { + const derivedMrInfo = { + mrPath: '/gitlab-org/gitlab-test/-/merge_requests/4', + userOrGroup: 'gitlab-org', + project: 'gitlab-test', + id: '4', + }; + const unparseableEndpoint = { + mrPath: undefined, + userOrGroup: undefined, + project: undefined, + id: undefined, + }; + + describe('getDerivedMergeRequestInformation', () => { + const endpoint = `${diffMetadata.latest_version_path}.json?searchParam=irrelevant`; + + it.each` + argument | response + ${{ endpoint }} | ${derivedMrInfo} + ${{}} | ${unparseableEndpoint} + ${{ endpoint: undefined }} | ${unparseableEndpoint} + ${{ endpoint: null }} | ${unparseableEndpoint} + `('generates the correct derived results based on $argument', ({ argument, response }) => { + expect(getDerivedMergeRequestInformation(argument)).toStrictEqual(response); + }); + }); +}); diff --git a/spec/frontend/diffs/utils/uuids_spec.js b/spec/frontend/diffs/utils/uuids_spec.js index 79d3ebadd4f..8d0a01e8cbd 100644 --- a/spec/frontend/diffs/utils/uuids_spec.js +++ b/spec/frontend/diffs/utils/uuids_spec.js @@ -32,7 +32,7 @@ describe('UUIDs Util', () => { const ids = uuids({ count: 11 }); expect(ids.length).toEqual(11); - expect(ids.every(id => UUIDV4.test(id))).toEqual(true); + expect(ids.every((id) => UUIDV4.test(id))).toEqual(true); }); it.each` |