diff options
Diffstat (limited to 'spec')
39 files changed, 950 insertions, 229 deletions
diff --git a/spec/features/incidents/incident_timeline_events_spec.rb b/spec/features/incidents/incident_timeline_events_spec.rb index e39f348013c..f78a49759df 100644 --- a/spec/features/incidents/incident_timeline_events_spec.rb +++ b/spec/features/incidents/incident_timeline_events_spec.rb @@ -41,7 +41,39 @@ RSpec.describe 'Incident timeline events', :js do end end - context 'when delete event is clicked' do + context 'when edit is clicked' do + before do + click_button 'Add new timeline event' + fill_in 'Description', with: 'Event note to edit' + click_button 'Save' + end + + it 'shows the confirmation modal and edits the event' do + click_button 'More actions' + + page.within '.gl-new-dropdown-contents' do + expect(page).to have_content('Edit') + page.find('.gl-new-dropdown-item-text-primary', text: 'Edit').click + end + + expect(page).to have_selector('.common-note-form') + + fill_in 'Description', with: 'Event note goes here' + fill_in 'timeline-input-hours', with: '07' + fill_in 'timeline-input-minutes', with: '25' + + click_button 'Save' + + wait_for_requests + + page.within '.timeline-event-note' do + expect(page).to have_content('Event note goes here') + expect(page).to have_content('07:25') + end + end + end + + context 'when delete is clicked' do before do click_button 'Add new timeline event' fill_in 'Description', with: 'Event note to delete' @@ -51,7 +83,7 @@ RSpec.describe 'Incident timeline events', :js do it 'shows the confirmation modal and deletes the event' do click_button 'More actions' - page.within '.gl-new-dropdown-item-text-wrapper' do + page.within '.gl-new-dropdown-contents' do expect(page).to have_content('Delete') page.find('.gl-new-dropdown-item-text-primary', text: 'Delete').click end diff --git a/spec/frontend/ide/components/preview/clientside_spec.js b/spec/frontend/ide/components/preview/clientside_spec.js index cf768114e70..51e6a9d9034 100644 --- a/spec/frontend/ide/components/preview/clientside_spec.js +++ b/spec/frontend/ide/components/preview/clientside_spec.js @@ -2,15 +2,15 @@ import { GlLoadingIcon } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; import Vue, { nextTick } from 'vue'; import { dispatch } from 'codesandbox-api'; -import smooshpack from 'smooshpack'; +import { SandpackClient } from '@codesandbox/sandpack-client'; import Vuex from 'vuex'; import waitForPromises from 'helpers/wait_for_promises'; import Clientside from '~/ide/components/preview/clientside.vue'; import { PING_USAGE_PREVIEW_KEY, PING_USAGE_PREVIEW_SUCCESS_KEY } from '~/ide/constants'; import eventHub from '~/ide/eventhub'; -jest.mock('smooshpack', () => ({ - Manager: jest.fn(), +jest.mock('@codesandbox/sandpack-client', () => ({ + SandpackClient: jest.fn(), })); Vue.use(Vuex); @@ -78,8 +78,8 @@ describe('IDE clientside preview', () => { // eslint-disable-next-line no-restricted-syntax wrapper.setData({ sandpackReady: true, - manager: { - listener: jest.fn(), + client: { + cleanup: jest.fn(), updatePreview: jest.fn(), }, }); @@ -90,9 +90,9 @@ describe('IDE clientside preview', () => { }); describe('without main entry', () => { - it('creates sandpack manager', () => { + it('creates sandpack client', () => { createComponent(); - expect(smooshpack.Manager).not.toHaveBeenCalled(); + expect(SandpackClient).not.toHaveBeenCalled(); }); }); describe('with main entry', () => { @@ -102,8 +102,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith( + it('creates sandpack client', () => { + expect(SandpackClient).toHaveBeenCalledWith( '#ide-preview', expectedSandpackOptions(), expectedSandpackSettings(), @@ -141,8 +141,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager with bundlerURL', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith('#ide-preview', expectedSandpackOptions(), { + it('creates sandpack client with bundlerURL', () => { + expect(SandpackClient).toHaveBeenCalledWith('#ide-preview', expectedSandpackOptions(), { ...expectedSandpackSettings(), bundlerURL: TEST_BUNDLER_URL, }); @@ -156,8 +156,8 @@ describe('IDE clientside preview', () => { return waitForPromises(); }); - it('creates sandpack manager', () => { - expect(smooshpack.Manager).toHaveBeenCalledWith( + it('creates sandpack client', () => { + expect(SandpackClient).toHaveBeenCalledWith( '#ide-preview', { files: {}, @@ -332,7 +332,7 @@ describe('IDE clientside preview', () => { }); describe('update', () => { - it('initializes manager if manager is empty', () => { + it('initializes client if client is empty', () => { createComponent({ getters: { packageJson: dummyPackageJson } }); // setData usage is discouraged. See https://gitlab.com/groups/gitlab-org/-/epics/7330 for details // eslint-disable-next-line no-restricted-syntax @@ -340,7 +340,7 @@ describe('IDE clientside preview', () => { wrapper.vm.update(); return waitForPromises().then(() => { - expect(smooshpack.Manager).toHaveBeenCalled(); + expect(SandpackClient).toHaveBeenCalled(); }); }); @@ -349,7 +349,7 @@ describe('IDE clientside preview', () => { wrapper.vm.update(); - expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); + expect(wrapper.vm.client.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); }); }); @@ -361,7 +361,7 @@ describe('IDE clientside preview', () => { }); it('calls updatePreview', () => { - expect(wrapper.vm.manager.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); + expect(wrapper.vm.client.updatePreview).toHaveBeenCalledWith(wrapper.vm.sandboxOpts); }); }); }); @@ -405,7 +405,7 @@ describe('IDE clientside preview', () => { beforeEach(() => { createInitializedComponent(); - spy = wrapper.vm.manager.updatePreview; + spy = wrapper.vm.client.updatePreview; wrapper.destroy(); }); diff --git a/spec/frontend/ide/components/preview/navigator_spec.js b/spec/frontend/ide/components/preview/navigator_spec.js index 9c4f825ccf5..532cb6e795c 100644 --- a/spec/frontend/ide/components/preview/navigator_spec.js +++ b/spec/frontend/ide/components/preview/navigator_spec.js @@ -11,7 +11,7 @@ jest.mock('codesandbox-api', () => ({ describe('IDE clientside preview navigator', () => { let wrapper; - let manager; + let client; let listenHandler; const findBackButton = () => wrapper.findAll('button').at(0); @@ -20,9 +20,9 @@ describe('IDE clientside preview navigator', () => { beforeEach(() => { listen.mockClear(); - manager = { bundlerURL: TEST_HOST, iframe: { src: '' } }; + client = { bundlerURL: TEST_HOST, iframe: { src: '' } }; - wrapper = shallowMount(ClientsideNavigator, { propsData: { manager } }); + wrapper = shallowMount(ClientsideNavigator, { propsData: { client } }); [[listenHandler]] = listen.mock.calls; }); @@ -31,7 +31,7 @@ describe('IDE clientside preview navigator', () => { }); it('renders readonly URL bar', async () => { - listenHandler({ type: 'urlchange', url: manager.bundlerURL }); + listenHandler({ type: 'urlchange', url: client.bundlerURL }); await nextTick(); expect(wrapper.find('input[readonly]').element.value).toBe('/'); }); @@ -89,13 +89,13 @@ describe('IDE clientside preview navigator', () => { expect(findBackButton().attributes('disabled')).toBe('disabled'); }); - it('updates manager iframe src', async () => { + it('updates client iframe src', async () => { listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url1` }); listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url2` }); await nextTick(); findBackButton().trigger('click'); - expect(manager.iframe.src).toBe(`${TEST_HOST}/url1`); + expect(client.iframe.src).toBe(`${TEST_HOST}/url1`); }); }); @@ -133,13 +133,13 @@ describe('IDE clientside preview navigator', () => { expect(findForwardButton().attributes('disabled')).toBe('disabled'); }); - it('updates manager iframe src', async () => { + it('updates client iframe src', async () => { listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url1` }); listenHandler({ type: 'urlchange', url: `${TEST_HOST}/url2` }); await nextTick(); findBackButton().trigger('click'); - expect(manager.iframe.src).toBe(`${TEST_HOST}/url1`); + expect(client.iframe.src).toBe(`${TEST_HOST}/url1`); }); }); @@ -152,10 +152,10 @@ describe('IDE clientside preview navigator', () => { }); it('calls refresh with current path', () => { - manager.iframe.src = 'something-other'; + client.iframe.src = 'something-other'; findRefreshButton().trigger('click'); - expect(manager.iframe.src).toBe(url); + expect(client.iframe.src).toBe(url); }); }); }); diff --git a/spec/frontend/issues/show/components/incidents/create_timeline_events_form_spec.js b/spec/frontend/issues/show/components/incidents/create_timeline_events_form_spec.js index 3ab2bb3460b..a277dd70764 100644 --- a/spec/frontend/issues/show/components/incidents/create_timeline_events_form_spec.js +++ b/spec/frontend/issues/show/components/incidents/create_timeline_events_form_spec.js @@ -42,17 +42,15 @@ describe('Create Timeline events', () => { const findDatePicker = () => wrapper.findComponent(GlDatepicker); const findNoteInput = () => wrapper.findByTestId('input-note'); const setNoteInput = () => { - const textarea = findNoteInput().element; - textarea.value = mockInputData.note; - textarea.dispatchEvent(new Event('input')); + findNoteInput().setValue(mockInputData.note); }; const findHourInput = () => wrapper.findByTestId('input-hours'); const findMinuteInput = () => wrapper.findByTestId('input-minutes'); const setDatetime = () => { const inputDate = new Date(mockInputData.occurredAt); findDatePicker().vm.$emit('input', inputDate); - findHourInput().vm.$emit('input', inputDate.getHours()); - findMinuteInput().vm.$emit('input', inputDate.getMinutes()); + findHourInput().setValue(inputDate.getHours()); + findMinuteInput().setValue(inputDate.getMinutes()); }; const fillForm = () => { setDatetime(); diff --git a/spec/frontend/issues/show/components/incidents/edit_timeline_event_spec.js b/spec/frontend/issues/show/components/incidents/edit_timeline_event_spec.js new file mode 100644 index 00000000000..4c1638a9147 --- /dev/null +++ b/spec/frontend/issues/show/components/incidents/edit_timeline_event_spec.js @@ -0,0 +1,44 @@ +import EditTimelineEvent from '~/issues/show/components/incidents/edit_timeline_event.vue'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import TimelineEventsForm from '~/issues/show/components/incidents/timeline_events_form.vue'; + +import { mockEvents, fakeEventData, mockInputData } from './mock_data'; + +describe('Edit Timeline events', () => { + let wrapper; + + const mountComponent = () => { + wrapper = mountExtended(EditTimelineEvent, { + propsData: { + event: mockEvents[0], + editTimelineEventActive: false, + }, + }); + }; + + beforeEach(() => { + mountComponent(); + }); + + const findTimelineEventsForm = () => wrapper.findComponent(TimelineEventsForm); + + const mockSaveData = { ...fakeEventData, ...mockInputData }; + + describe('editTimelineEvent', () => { + const saveEventEvent = { 'handle-save-edit': [[mockSaveData, false]] }; + + it('should call the mutation with the right variables', async () => { + await findTimelineEventsForm().vm.$emit('save-event', mockSaveData, false); + + expect(wrapper.emitted()).toEqual(saveEventEvent); + }); + + it('should close the form on cancel', async () => { + const cancelEvent = { 'hide-edit': [[]] }; + + await findTimelineEventsForm().vm.$emit('cancel'); + + expect(wrapper.emitted()).toEqual(cancelEvent); + }); + }); +}); diff --git a/spec/frontend/issues/show/components/incidents/mock_data.js b/spec/frontend/issues/show/components/incidents/mock_data.js index fd26fd86ced..adea2b6df59 100644 --- a/spec/frontend/issues/show/components/incidents/mock_data.js +++ b/spec/frontend/issues/show/components/incidents/mock_data.js @@ -49,6 +49,15 @@ export const mockEvents = [ }, ]; +const mockUpdatedEvent = { + id: 'gid://gitlab/IncidentManagement::TimelineEvent/8', + note: 'another one23', + noteHtml: '<p>another one23</p>', + action: 'comment', + occurredAt: '2022-07-01T12:47:00Z', + createdAt: '2022-07-20T12:47:40Z', +}; + export const timelineEventsQueryListResponse = { data: { project: { @@ -93,6 +102,29 @@ export const timelineEventsCreateEventError = { }, }; +export const timelineEventsEditEventResponse = { + data: { + timelineEventUpdate: { + timelineEvent: { + ...mockUpdatedEvent, + }, + errors: [], + __typename: 'TimelineEventUpdatePayload', + }, + }, +}; + +export const timelineEventsEditEventError = { + data: { + timelineEventUpdate: { + timelineEvent: { + ...mockUpdatedEvent, + }, + errors: ['Create error'], + }, + }, +}; + const timelineEventDeleteData = (errors = []) => { return { data: { @@ -128,5 +160,10 @@ export const mockGetTimelineData = { export const fakeDate = '2020-07-08T00:00:00.000Z'; +export const mockInputData = { + note: 'test', + occurredAt: '2020-08-10T02:30:00.000Z', +}; + const { id, note, occurredAt } = mockEvents[0]; export const fakeEventData = { id, note, occurredAt }; diff --git a/spec/frontend/issues/show/components/incidents/timeline_events_form_spec.js b/spec/frontend/issues/show/components/incidents/timeline_events_form_spec.js index abe42dc3f28..d04d2965401 100644 --- a/spec/frontend/issues/show/components/incidents/timeline_events_form_spec.js +++ b/spec/frontend/issues/show/components/incidents/timeline_events_form_spec.js @@ -23,7 +23,7 @@ describe('Timeline events form', () => { const mountComponent = ({ mountMethod = shallowMountExtended }) => { wrapper = mountMethod(TimelineEventsForm, { propsData: { - hasTimelineEvents: true, + showSaveAndAdd: true, isEventProcessed: false, }, }); @@ -42,8 +42,8 @@ describe('Timeline events form', () => { const findMinuteInput = () => wrapper.findByTestId('input-minutes'); const setDatetime = () => { findDatePicker().vm.$emit('input', mockInputDate); - findHourInput().vm.$emit('input', 5); - findMinuteInput().vm.$emit('input', 45); + findHourInput().setValue(5); + findMinuteInput().setValue(45); }; const submitForm = async () => { diff --git a/spec/frontend/issues/show/components/incidents/timeline_events_item_spec.js b/spec/frontend/issues/show/components/incidents/timeline_events_item_spec.js index 90e55003ab3..9f1f6aff57e 100644 --- a/spec/frontend/issues/show/components/incidents/timeline_events_item_spec.js +++ b/spec/frontend/issues/show/components/incidents/timeline_events_item_spec.js @@ -15,7 +15,6 @@ describe('IncidentTimelineEventList', () => { action, noteHtml, occurredAt, - isLastItem: false, ...propsData, }, provide: { @@ -26,7 +25,6 @@ describe('IncidentTimelineEventList', () => { }; const findCommentIcon = () => wrapper.findComponent(GlIcon); - const findTextContainer = () => wrapper.findByTestId('event-text-container'); const findEventTime = () => wrapper.findByTestId('event-time'); const findDropdown = () => wrapper.findComponent(GlDropdown); const findDeleteButton = () => wrapper.findByText('Delete'); @@ -50,20 +48,6 @@ describe('IncidentTimelineEventList', () => { expect(findEventTime().text()).toBe('15:59 UTC'); }); - describe('last item in list', () => { - it('shows a bottom border when not the last item', () => { - mountComponent(); - - expect(findTextContainer().classes()).toContain('gl-border-1'); - }); - - it('does not show a bottom border when the last item', () => { - mountComponent({ propsData: { isLastItem: true } }); - - expect(wrapper.classes()).not.toContain('gl-border-1'); - }); - }); - describe.each` timezone ${'Europe/London'} diff --git a/spec/frontend/issues/show/components/incidents/timeline_events_list_spec.js b/spec/frontend/issues/show/components/incidents/timeline_events_list_spec.js index fe182016924..18bd91f0465 100644 --- a/spec/frontend/issues/show/components/incidents/timeline_events_list_spec.js +++ b/spec/frontend/issues/show/components/incidents/timeline_events_list_spec.js @@ -4,10 +4,11 @@ import Vue from 'vue'; import { confirmAction } from '~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal'; import IncidentTimelineEventList from '~/issues/show/components/incidents/timeline_events_list.vue'; import IncidentTimelineEventItem from '~/issues/show/components/incidents/timeline_events_item.vue'; +import EditTimelineEvent from '~/issues/show/components/incidents/edit_timeline_event.vue'; import { mountExtended } from 'helpers/vue_test_utils_helper'; import waitForPromises from 'helpers/wait_for_promises'; import deleteTimelineEventMutation from '~/issues/show/components/incidents/graphql/queries/delete_timeline_event.mutation.graphql'; -import getTimelineEvents from '~/issues/show/components/incidents/graphql/queries/get_timeline_events.query.graphql'; +import editTimelineEventMutation from '~/issues/show/components/incidents/graphql/queries/edit_timeline_event.mutation.graphql'; import createMockApollo from 'helpers/mock_apollo_helper'; import { useFakeDate } from 'helpers/fake_date'; import { createAlert } from '~/flash'; @@ -15,9 +16,11 @@ import { mockEvents, timelineEventsDeleteEventResponse, timelineEventsDeleteEventError, + timelineEventsEditEventResponse, + timelineEventsEditEventError, fakeDate, fakeEventData, - timelineEventsQueryListResponse, + mockInputData, } from './mock_data'; Vue.use(VueApollo); @@ -32,20 +35,15 @@ const mockConfirmAction = ({ confirmed }) => { describe('IncidentTimelineEventList', () => { useFakeDate(fakeDate); let wrapper; - const responseSpy = jest.fn().mockResolvedValue(timelineEventsDeleteEventResponse); + const deleteResponseSpy = jest.fn().mockResolvedValue(timelineEventsDeleteEventResponse); + const editResponseSpy = jest.fn().mockResolvedValue(timelineEventsEditEventResponse); - const requestHandlers = [[deleteTimelineEventMutation, responseSpy]]; + const requestHandlers = [ + [deleteTimelineEventMutation, deleteResponseSpy], + [editTimelineEventMutation, editResponseSpy], + ]; const apolloProvider = createMockApollo(requestHandlers); - apolloProvider.clients.defaultClient.cache.writeQuery({ - query: getTimelineEvents, - data: timelineEventsQueryListResponse.data, - variables: { - fullPath: 'group/project', - incidentId: 'gid://gitlab/Issue/1', - }, - }); - const mountComponent = () => { wrapper = mountExtended(IncidentTimelineEventList, { propsData: { @@ -70,6 +68,10 @@ describe('IncidentTimelineEventList', () => { await waitForPromises(); }; + const clickFirstEditButton = async () => { + findItems().at(0).vm.$emit('edit'); + await waitForPromises(); + }; beforeEach(() => { mountComponent(); }); @@ -86,12 +88,6 @@ describe('IncidentTimelineEventList', () => { expect(findItems(findSecondTimelineEventGroup())).toHaveLength(2); }); - it('sets the isLastItem prop correctly', () => { - expect(findItems().at(0).props('isLastItem')).toBe(false); - expect(findItems().at(1).props('isLastItem')).toBe(false); - expect(findItems().at(2).props('isLastItem')).toBe(true); - }); - it('sets the event props correctly', () => { expect(findItems().at(1).props('occurredAt')).toBe(mockEvents[1].occurredAt); expect(findItems().at(1).props('action')).toBe(mockEvents[1].action); @@ -133,7 +129,7 @@ describe('IncidentTimelineEventList', () => { const expectedVars = { input: { id: mockEvents[0].id } }; await clickFirstDeleteButton(); - expect(responseSpy).toHaveBeenCalledWith(expectedVars); + expect(deleteResponseSpy).toHaveBeenCalledWith(expectedVars); }); it('should show an error when delete returns an error', async () => { @@ -141,7 +137,7 @@ describe('IncidentTimelineEventList', () => { message: 'Error deleting incident timeline event: Item does not exist', }; - responseSpy.mockResolvedValue(timelineEventsDeleteEventError); + deleteResponseSpy.mockResolvedValue(timelineEventsDeleteEventError); await clickFirstDeleteButton(); @@ -154,7 +150,7 @@ describe('IncidentTimelineEventList', () => { error: new Error(), message: 'Something went wrong while deleting the incident timeline event.', }; - responseSpy.mockRejectedValueOnce(); + deleteResponseSpy.mockRejectedValueOnce(); await clickFirstDeleteButton(); @@ -162,4 +158,76 @@ describe('IncidentTimelineEventList', () => { }); }); }); + + describe('Edit Functionality', () => { + beforeEach(() => { + mountComponent(); + clickFirstEditButton(); + }); + + const findEditEvent = () => wrapper.findComponent(EditTimelineEvent); + const mockSaveData = { ...fakeEventData, ...mockInputData }; + + describe('editTimelineEvent', () => { + it('should call the mutation with the right variables', async () => { + await findEditEvent().vm.$emit('handle-save-edit', mockSaveData); + await waitForPromises(); + + expect(editResponseSpy).toHaveBeenCalledWith({ + input: mockSaveData, + }); + }); + + it('should close the form on successful addition', async () => { + await findEditEvent().vm.$emit('handle-save-edit', mockSaveData); + await waitForPromises(); + + expect(findEditEvent().exists()).toBe(false); + }); + + it('should close the form on cancel', async () => { + await findEditEvent().vm.$emit('hide-edit'); + await waitForPromises(); + + expect(findEditEvent().exists()).toBe(false); + }); + }); + + describe('error handling', () => { + it('should show an error when submission returns an error', async () => { + const expectedAlertArgs = { + message: `Error updating incident timeline event: ${timelineEventsEditEventError.data.timelineEventUpdate.errors[0]}`, + }; + editResponseSpy.mockResolvedValueOnce(timelineEventsEditEventError); + + await findEditEvent().vm.$emit('handle-save-edit', mockSaveData); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith(expectedAlertArgs); + }); + + it('should show an error when submission fails', async () => { + const expectedAlertArgs = { + captureError: true, + error: new Error(), + message: 'Something went wrong while updating the incident timeline event.', + }; + editResponseSpy.mockRejectedValueOnce(); + + await findEditEvent().vm.$emit('handle-save-edit', mockSaveData); + await waitForPromises(); + + expect(createAlert).toHaveBeenCalledWith(expectedAlertArgs); + }); + + it('should keep the form open on failed addition', async () => { + editResponseSpy.mockResolvedValueOnce(timelineEventsEditEventError); + + await findEditEvent().vm.$emit('handle-save-edit', mockSaveData); + await waitForPromises(); + + expect(findEditEvent().exists()).toBe(true); + }); + }); + }); }); diff --git a/spec/frontend/issues/show/components/incidents/timeline_events_tab_spec.js b/spec/frontend/issues/show/components/incidents/timeline_events_tab_spec.js index 2cdb971395d..f260b803b7a 100644 --- a/spec/frontend/issues/show/components/incidents/timeline_events_tab_spec.js +++ b/spec/frontend/issues/show/components/incidents/timeline_events_tab_spec.js @@ -143,22 +143,13 @@ describe('TimelineEventsTab', () => { }); it('should not show a form by default', () => { - expect(findCreateTimelineEvent().isVisible()).toBe(false); + expect(findCreateTimelineEvent().exists()).toBe(false); }); it('should show a form when button is clicked', async () => { await findAddEventButton().trigger('click'); - expect(findCreateTimelineEvent().isVisible()).toBe(true); - }); - - it('should clear the form when button is clicked', async () => { - const mockClear = jest.fn(); - wrapper.vm.$refs.createEventForm.clearForm = mockClear; - - await findAddEventButton().trigger('click'); - - expect(mockClear).toHaveBeenCalled(); + expect(findCreateTimelineEvent().exists()).toBe(true); }); it('should hide the form when the hide event is emitted', async () => { @@ -167,7 +158,7 @@ describe('TimelineEventsTab', () => { await findCreateTimelineEvent().vm.$emit('hide-new-timeline-events-form'); - expect(findCreateTimelineEvent().isVisible()).toBe(false); + expect(findCreateTimelineEvent().exists()).toBe(false); }); }); }); diff --git a/spec/frontend/issues/show/components/incidents/utils_spec.js b/spec/frontend/issues/show/components/incidents/utils_spec.js index d3a86680f14..f0494591e95 100644 --- a/spec/frontend/issues/show/components/incidents/utils_spec.js +++ b/spec/frontend/issues/show/components/incidents/utils_spec.js @@ -2,7 +2,7 @@ import timezoneMock from 'timezone-mock'; import { displayAndLogError, getEventIcon, - getUtcShiftedDateNow, + getUtcShiftedDate, } from '~/issues/show/components/incidents/utils'; import { createAlert } from '~/flash'; @@ -34,7 +34,7 @@ describe('incident utils', () => { }); }); - describe('getUtcShiftedDateNow', () => { + describe('getUtcShiftedDate', () => { beforeEach(() => { timezoneMock.register('US/Pacific'); }); @@ -46,7 +46,7 @@ describe('incident utils', () => { it('should shift the date by the timezone offset', () => { const date = new Date(); - const shiftedDate = getUtcShiftedDateNow(); + const shiftedDate = getUtcShiftedDate(); expect(shiftedDate > date).toBe(true); }); diff --git a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js index 70c7f56b62f..296d01ddd99 100644 --- a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js +++ b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js @@ -38,7 +38,7 @@ export default [ '</tr>\n', '</table>', ].join(''), - output: '<table>', + output: '<table data-myattr="XSS">', }, ], // Note: style is sanitized out @@ -98,7 +98,7 @@ export default [ '</svg>', ].join(), output: - '<svg xmlns="http://www.w3.org/2000/svg" width="388.84pt" version="1.0" id="svg2" height="115.02pt">', + '<svg height="115.02pt" id="svg2" version="1.0" width="388.84pt" xmlns="http://www.w3.org/2000/svg">', }, ], ]; diff --git a/spec/frontend/notebook/cells/output/index_spec.js b/spec/frontend/notebook/cells/output/index_spec.js index 4d1d03e5e34..97a7e22be60 100644 --- a/spec/frontend/notebook/cells/output/index_spec.js +++ b/spec/frontend/notebook/cells/output/index_spec.js @@ -49,15 +49,17 @@ describe('Output component', () => { const htmlType = json.cells[4]; createComponent(htmlType.outputs[0]); - expect(wrapper.findAll('p')).toHaveLength(1); - expect(wrapper.text()).toContain('test'); + const iframe = wrapper.find('iframe'); + expect(iframe.exists()).toBe(true); + expect(iframe.element.getAttribute('sandbox')).toBe(''); + expect(iframe.element.getAttribute('srcdoc')).toBe('<p>test</p>'); }); it('renders multiple raw HTML outputs', () => { const htmlType = json.cells[4]; createComponent([htmlType.outputs[0], htmlType.outputs[0]]); - expect(wrapper.findAll('p')).toHaveLength(2); + expect(wrapper.findAll('iframe')).toHaveLength(2); }); }); @@ -84,7 +86,11 @@ describe('Output component', () => { }); it('renders as an svg', () => { - expect(wrapper.find('svg').exists()).toBe(true); + const iframe = wrapper.find('iframe'); + + expect(iframe.exists()).toBe(true); + expect(iframe.element.getAttribute('sandbox')).toBe(''); + expect(iframe.element.getAttribute('srcdoc')).toBe('<svg></svg>'); }); }); diff --git a/spec/helpers/commits_helper_spec.rb b/spec/helpers/commits_helper_spec.rb index d2d2e226f2a..0cc53da98b2 100644 --- a/spec/helpers/commits_helper_spec.rb +++ b/spec/helpers/commits_helper_spec.rb @@ -320,7 +320,7 @@ RSpec.describe CommitsHelper do let(:current_path) { "test" } before do - expect(commit).to receive(:status_for).with(ref).and_return(commit_status) + expect(commit).to receive(:detailed_status_for).with(ref).and_return(commit_status) assign(:path, current_path) end diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 5efa88a2a7d..90366d7772c 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -112,6 +112,14 @@ RSpec.describe LabelsHelper do end end + describe 'render_label_text' do + it 'html escapes the bg_color correctly' do + xss_payload = '"><img src=x onerror=prompt(1)>' + label_text = render_label_text('xss', bg_color: xss_payload) + expect(label_text).to include(html_escape(xss_payload)) + end + end + describe 'text_color_for_bg' do it 'uses light text on dark backgrounds' do expect(text_color_for_bg('#222E2E')).to be_color('#FFFFFF') diff --git a/spec/initializers/rack_VULNDB-255039_patch_spec.rb b/spec/initializers/rack_VULNDB-255039_patch_spec.rb new file mode 100644 index 00000000000..754ff2f10e0 --- /dev/null +++ b/spec/initializers/rack_VULNDB-255039_patch_spec.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Rack VULNDB-255039' do + context 'when handling query params in GET requests' do + it 'does not treat semicolons as query delimiters' do + env = ::Rack::MockRequest.env_for('http://gitlab.com?a=b;c=1') + + query_hash = ::Rack::Request.new(env).GET + + # Prior to this patch, this was splitting around the semicolon, which + # would return {"a"=>"b", "c"=>"1"} + expect(query_hash).to eq({ "a" => "b;c=1" }) + end + end +end diff --git a/spec/initializers/sawyer_patch_spec.rb b/spec/initializers/sawyer_patch_spec.rb new file mode 100644 index 00000000000..dc922654d7d --- /dev/null +++ b/spec/initializers/sawyer_patch_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true +require 'fast_spec_helper' +require 'sawyer' + +require_relative '../../config/initializers/sawyer_patch' + +RSpec.describe 'sawyer_patch' do + it 'raises error when acessing a method that overlaps a Ruby method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + to_s: 'Overriding method', + user: { to_s: 'Overriding method', name: 'User name' } + } + ) + + error_message = 'Sawyer method "to_s" overlaps Ruby method. Convert to a hash to access the attribute.' + expect { sawyer_resource.to_s }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.to_s? }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.to_s = 'new value' }.to raise_error(Sawyer::Error, error_message) + expect { sawyer_resource.user.to_s }.to raise_error(Sawyer::Error, error_message) + expect(sawyer_resource.user.name).to eq('User name') + end + + it 'raises error when acessing a boolean method that overlaps a Ruby method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + nil?: 'value' + } + ) + + expect { sawyer_resource.nil? }.to raise_error(Sawyer::Error) + end + + it 'raises error when acessing a method that expects an argument' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + 'user': 'value', + 'user=': 'value', + '==': 'value', + '!=': 'value', + '+': 'value' + } + ) + + expect(sawyer_resource.user).to eq('value') + expect { sawyer_resource.user = 'New user' }.to raise_error(ArgumentError) + expect { sawyer_resource == true }.to raise_error(ArgumentError) + expect { sawyer_resource != true }.to raise_error(ArgumentError) + expect { sawyer_resource + 1 }.to raise_error(ArgumentError) + end + + it 'does not raise error if is not an overlapping method' do + sawyer_resource = Sawyer::Resource.new( + Sawyer::Agent.new(''), + { + count_total: 1, + user: { name: 'User name' } + } + ) + + expect(sawyer_resource.count_total).to eq(1) + expect(sawyer_resource.count_total?).to eq(true) + expect(sawyer_resource.count_total + 1).to eq(2) + expect(sawyer_resource.user.name).to eq('User name') + end +end diff --git a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb index 38f9bda57e6..2bdf702083a 100644 --- a/spec/lib/banzai/filter/commit_trailers_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_trailers_filter_spec.rb @@ -18,10 +18,20 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do context 'detects' do let(:email) { FFaker::Internet.email } - it 'trailers in the form of *-by and replace users with links' do - doc = filter(commit_message_html) + context 'trailers in the form of *-by' do + where(:commit_trailer) do + ["#{FFaker::Lorem.word}-by:", "#{FFaker::Lorem.word}-BY:", "#{FFaker::Lorem.word}-By:"] + end - expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + with_them do + let(:trailer) { commit_trailer } + + it 'replaces users with links' do + doc = filter(commit_message_html) + + expect_to_have_user_link_with_avatar(doc, user: user, trailer: trailer) + end + end end it 'trailers prefixed with whitespaces' do @@ -121,7 +131,7 @@ RSpec.describe Banzai::Filter::CommitTrailersFilter do context "ignores" do it 'commit messages without trailers' do - exp = message = commit_html(FFaker::Lorem.sentence) + exp = message = commit_html(Array.new(5) { FFaker::Lorem.sentence }.join("\n")) doc = filter(message) expect(doc.to_html).to match Regexp.escape(exp) diff --git a/spec/lib/banzai/filter/image_link_filter_spec.rb b/spec/lib/banzai/filter/image_link_filter_spec.rb index 6326d894b08..78d68697ac7 100644 --- a/spec/lib/banzai/filter/image_link_filter_spec.rb +++ b/spec/lib/banzai/filter/image_link_filter_spec.rb @@ -92,5 +92,50 @@ RSpec.describe Banzai::Filter::ImageLinkFilter do expect(doc.at_css('a')['class']).to match(%r{with-attachment-icon}) end + + context 'when link attributes contain malicious code' do + let(:malicious_code) do + # rubocop:disable Layout/LineLength + %q(<a class='fixed-top fixed-bottom' data-create-path=/malicious-url><style> .tab-content>.tab-pane{display: block !important}</style>) + # rubocop:enable Layout/LineLength + end + + context 'when image alt contains malicious code' do + it 'ignores image alt and uses image path as the link text', :aggregate_failures do + doc = filter(image(path, alt: malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*>#{path}</a>$}) + expect(doc.at_css('a')['href']).to eq(path) + end + end + + context 'when image src contains malicious code' do + it 'ignores image src and does not use it as the link text' do + doc = filter(image(malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*></a>$}) + end + + it 'keeps image src unchanged, malicious code does not execute as part of url' do + doc = filter(image(malicious_code), context) + + expect(doc.at_css('a')['href']).to eq(malicious_code) + end + end + + context 'when image data-src contains malicious code' do + it 'ignores data-src and uses image path as the link text', :aggregate_failures do + doc = filter(image(path, data_src: malicious_code), context) + + expect(doc.to_html).to match(%r{^<a[^>]*>#{path}</a>$}) + end + + it 'uses image data-src, malicious code does not execute as part of url' do + doc = filter(image(path, data_src: malicious_code), context) + + expect(doc.at_css('a')['href']).to eq(malicious_code) + end + end + end end end diff --git a/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb b/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb new file mode 100644 index 00000000000..e0a07d1ea77 --- /dev/null +++ b/spec/lib/banzai/filter/pathological_markdown_filter_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Banzai::Filter::PathologicalMarkdownFilter do + include FilterSpecHelper + + let_it_be(:short_text) { '![a' * 5 } + let_it_be(:long_text) { ([short_text] * 10).join(' ') } + let_it_be(:with_images_text) { " #{'and\n' * 200} " } + + it 'detects a significat number of unclosed image links' do + msg = <<~TEXT + _Unable to render markdown - too many unclosed markdown image links detected._ + TEXT + + expect(filter(long_text)).to eq(msg.strip) + end + + it 'does nothing when there are only a few unclosed image links' do + expect(filter(short_text)).to eq(short_text) + end + + it 'does nothing when there are only a few unclosed image links and images' do + expect(filter(with_images_text)).to eq(with_images_text) + end +end diff --git a/spec/lib/banzai/pipeline/full_pipeline_spec.rb b/spec/lib/banzai/pipeline/full_pipeline_spec.rb index 376edfb99fc..c07f99dc9fc 100644 --- a/spec/lib/banzai/pipeline/full_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/full_pipeline_spec.rb @@ -167,4 +167,16 @@ RSpec.describe Banzai::Pipeline::FullPipeline do expect(output).to include('<em>@test_</em>') end end + + describe 'unclosed image links' do + it 'detects a significat number of unclosed image links' do + markdown = '![a ' * 30 + msg = <<~TEXT + Unable to render markdown - too many unclosed markdown image links detected. + TEXT + output = described_class.to_html(markdown, project: nil) + + expect(output).to include(msg.strip) + end + end end diff --git a/spec/lib/gitlab/git/tree_spec.rb b/spec/lib/gitlab/git/tree_spec.rb index b520de03929..2e4520cd3a0 100644 --- a/spec/lib/gitlab/git/tree_spec.rb +++ b/spec/lib/gitlab/git/tree_spec.rb @@ -9,12 +9,13 @@ RSpec.describe Gitlab::Git::Tree do let(:repository) { project.repository.raw } shared_examples :repo do - subject(:tree) { Gitlab::Git::Tree.where(repository, sha, path, recursive, pagination_params) } + subject(:tree) { Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, pagination_params) } let(:sha) { SeedRepo::Commit::ID } let(:path) { nil } let(:recursive) { false } let(:pagination_params) { nil } + let(:skip_flat_paths) { false } let(:entries) { tree.first } let(:cursor) { tree.second } @@ -107,6 +108,12 @@ RSpec.describe Gitlab::Git::Tree do end it { expect(subdir_file.flat_path).to eq('files/flat/path/correct') } + + context 'when skip_flat_paths is true' do + let(:skip_flat_paths) { true } + + it { expect(subdir_file.flat_path).to be_blank } + end end end @@ -162,7 +169,7 @@ RSpec.describe Gitlab::Git::Tree do allow(instance).to receive(:lookup).with(SeedRepo::Commit::ID) end - described_class.where(repository, SeedRepo::Commit::ID, 'files', false) + described_class.where(repository, SeedRepo::Commit::ID, 'files', false, false) end it_behaves_like :repo do @@ -180,7 +187,7 @@ RSpec.describe Gitlab::Git::Tree do let(:entries_count) { entries.count } it 'returns all entries without a cursor' do - result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, { limit: entries_count, page_token: nil }) + result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: entries_count, page_token: nil }) expect(cursor).to be_nil expect(result.entries.count).to eq(entries_count) @@ -209,7 +216,7 @@ RSpec.describe Gitlab::Git::Tree do let(:entries_count) { entries.count } it 'returns all entries' do - result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, { limit: -1, page_token: nil }) + result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: -1, page_token: nil }) expect(result.count).to eq(entries_count) expect(cursor).to be_nil @@ -220,7 +227,7 @@ RSpec.describe Gitlab::Git::Tree do let(:token) { entries.second.id } it 'returns all entries after token' do - result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, { limit: -1, page_token: token }) + result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: -1, page_token: token }) expect(result.count).to eq(entries.count - 2) expect(cursor).to be_nil @@ -252,7 +259,7 @@ RSpec.describe Gitlab::Git::Tree do expected_entries = entries loop do - result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, { limit: 5, page_token: token }) + result, cursor = Gitlab::Git::Tree.where(repository, sha, path, recursive, skip_flat_paths, { limit: 5, page_token: token }) collected_entries += result.entries token = cursor&.next_cursor diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 0d591fe6c43..ed6a87cda6f 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -150,16 +150,18 @@ RSpec.describe Gitlab::GitalyClient::CommitService do end describe '#tree_entries' do - subject { client.tree_entries(repository, revision, path, recursive, pagination_params) } + subject { client.tree_entries(repository, revision, path, recursive, skip_flat_paths, pagination_params) } let(:path) { '/' } let(:recursive) { false } let(:pagination_params) { nil } + let(:skip_flat_paths) { false } - it 'sends a get_tree_entries message' do + it 'sends a get_tree_entries message with default limit' do + expected_pagination_params = Gitaly::PaginationParameter.new(limit: Gitlab::GitalyClient::CommitService::TREE_ENTRIES_DEFAULT_LIMIT) expect_any_instance_of(Gitaly::CommitService::Stub) .to receive(:get_tree_entries) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash)) .and_return([]) is_expected.to eq([[], nil]) @@ -189,9 +191,10 @@ RSpec.describe Gitlab::GitalyClient::CommitService do pagination_cursor: pagination_cursor ) + expected_pagination_params = Gitaly::PaginationParameter.new(limit: 3) expect_any_instance_of(Gitaly::CommitService::Stub) .to receive(:get_tree_entries) - .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash)) + .with(gitaly_request_with_params({ pagination_params: expected_pagination_params }), kind_of(Hash)) .and_return([response]) is_expected.to eq([[], pagination_cursor]) diff --git a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb index f405b2ad86e..207ac1c0eaa 100644 --- a/spec/lib/gitlab/reactive_cache_set_cache_spec.rb +++ b/spec/lib/gitlab/reactive_cache_set_cache_spec.rb @@ -72,4 +72,18 @@ RSpec.describe Gitlab::ReactiveCacheSetCache, :clean_gitlab_redis_cache do it { is_expected.to be(true) } end end + + describe 'count' do + subject { cache.count(cache_prefix) } + + it { is_expected.to be(0) } + + context 'item added' do + before do + cache.write(cache_prefix, 'test_item') + end + + it { is_expected.to be(1) } + end + end end diff --git a/spec/lib/gitlab/zentao/client_spec.rb b/spec/lib/gitlab/zentao/client_spec.rb index 135f13e6265..b17ad867f0d 100644 --- a/spec/lib/gitlab/zentao/client_spec.rb +++ b/spec/lib/gitlab/zentao/client_spec.rb @@ -2,17 +2,21 @@ require 'spec_helper' -RSpec.describe Gitlab::Zentao::Client do - subject(:integration) { described_class.new(zentao_integration) } +RSpec.describe Gitlab::Zentao::Client, :clean_gitlab_redis_cache do + subject(:client) { described_class.new(zentao_integration) } let(:zentao_integration) { create(:zentao_integration) } def mock_get_products_url - integration.send(:url, "products/#{zentao_integration.zentao_product_xid}") + client.send(:url, "products/#{zentao_integration.zentao_product_xid}") + end + + def mock_fetch_issues_url + client.send(:url, "products/#{zentao_integration.zentao_product_xid}/issues") end def mock_fetch_issue_url(issue_id) - integration.send(:url, "issues/#{issue_id}") + client.send(:url, "issues/#{issue_id}") end let(:mock_headers) do @@ -29,13 +33,13 @@ RSpec.describe Gitlab::Zentao::Client do let(:zentao_integration) { nil } it 'raises ConfigError' do - expect { integration }.to raise_error(described_class::ConfigError) + expect { client }.to raise_error(described_class::ConfigError) end end context 'integration is provided' do it 'is initialized successfully' do - expect { integration }.not_to raise_error + expect { client }.not_to raise_error end end end @@ -50,7 +54,7 @@ RSpec.describe Gitlab::Zentao::Client do end it 'fetches the product' do - expect(integration.fetch_product(zentao_integration.zentao_product_xid)).to eq mock_response + expect(client.fetch_product(zentao_integration.zentao_product_xid)).to eq mock_response end end @@ -62,8 +66,8 @@ RSpec.describe Gitlab::Zentao::Client do it 'fetches the empty product' do expect do - integration.fetch_product(zentao_integration.zentao_product_xid) - end.to raise_error(Gitlab::Zentao::Client::Error, 'request error') + client.fetch_product(zentao_integration.zentao_product_xid) + end.to raise_error(Gitlab::Zentao::Client::RequestError) end end @@ -75,7 +79,7 @@ RSpec.describe Gitlab::Zentao::Client do it 'fetches the empty product' do expect do - integration.fetch_product(zentao_integration.zentao_product_xid) + client.fetch_product(zentao_integration.zentao_product_xid) end.to raise_error(Gitlab::Zentao::Client::Error, 'invalid response format') end end @@ -89,7 +93,7 @@ RSpec.describe Gitlab::Zentao::Client do end it 'responds with success' do - expect(integration.ping[:success]).to eq true + expect(client.ping[:success]).to eq true end end @@ -100,7 +104,69 @@ RSpec.describe Gitlab::Zentao::Client do end it 'responds with unsuccess' do - expect(integration.ping[:success]).to eq false + expect(client.ping[:success]).to eq false + end + end + end + + describe '#fetch_issues' do + let(:mock_response) { { 'issues' => [{ 'id' => 'story-1' }, { 'id' => 'bug-11' }] } } + + before do + WebMock.stub_request(:get, mock_fetch_issues_url) + .with(mock_headers).to_return(status: 200, body: mock_response.to_json) + end + + it 'returns the response' do + expect(client.fetch_issues).to eq(mock_response) + end + + describe 'marking the issues as seen in the product' do + let(:cache) { ::Gitlab::SetCache.new } + let(:cache_key) do + [ + :zentao_product_issues, + OpenSSL::Digest::SHA256.hexdigest(zentao_integration.client_url), + zentao_integration.zentao_product_xid + ].join(':') + end + + it 'adds issue ids to the cache' do + expect { client.fetch_issues }.to change { cache.read(cache_key) } + .from(be_empty) + .to match_array(%w[bug-11 story-1]) + end + + it 'does not add issue ids to the cache if max set size has been reached' do + cache.write(cache_key, %w[foo bar]) + stub_const("#{described_class}::CACHE_MAX_SET_SIZE", 1) + + client.fetch_issues + + expect(cache.read(cache_key)).to match_array(%w[foo bar]) + end + + it 'does not duplicate issue ids in the cache' do + client.fetch_issues + client.fetch_issues + + expect(cache.read(cache_key)).to match_array(%w[bug-11 story-1]) + end + + it 'touches the cache ttl every time issues are fetched' do + fresh_ttl = 1.month.to_i + + freeze_time do + client.fetch_issues + + expect(cache.ttl(cache_key)).to eq(fresh_ttl) + end + + travel_to(1.minute.from_now) do + client.fetch_issues + + expect(cache.ttl(cache_key)).to eq(fresh_ttl) + end end end end @@ -109,9 +175,9 @@ RSpec.describe Gitlab::Zentao::Client do context 'with invalid id' do let(:invalid_ids) { ['story', 'story-', '-', '123', ''] } - it 'returns empty object' do + it 'raises Error' do invalid_ids.each do |id| - expect { integration.fetch_issue(id) } + expect { client.fetch_issue(id) } .to raise_error(Gitlab::Zentao::Client::Error, 'invalid issue id') end end @@ -120,12 +186,31 @@ RSpec.describe Gitlab::Zentao::Client do context 'with valid id' do let(:valid_ids) { %w[story-1 bug-23] } - it 'fetches current issue' do - valid_ids.each do |id| - WebMock.stub_request(:get, mock_fetch_issue_url(id)) - .with(mock_headers).to_return(status: 200, body: { issue: { id: id } }.to_json) + context 'when issue has been seen on the index' do + before do + issues_body = { issues: valid_ids.map { { id: _1 } } }.to_json + + WebMock.stub_request(:get, mock_fetch_issues_url) + .with(mock_headers).to_return(status: 200, body: issues_body) + + client.fetch_issues + end + + it 'fetches the issue' do + valid_ids.each do |id| + WebMock.stub_request(:get, mock_fetch_issue_url(id)) + .with(mock_headers).to_return(status: 200, body: { issue: { id: id } }.to_json) + + expect(client.fetch_issue(id).dig('issue', 'id')).to eq id + end + end + end - expect(integration.fetch_issue(id).dig('issue', 'id')).to eq id + context 'when issue has not been seen on the index' do + it 'raises RequestError' do + valid_ids.each do |id| + expect { client.fetch_issue(id) }.to raise_error(Gitlab::Zentao::Client::RequestError) + end end end end @@ -135,7 +220,7 @@ RSpec.describe Gitlab::Zentao::Client do context 'api url' do shared_examples 'joins api_url correctly' do it 'verify url' do - expect(integration.send(:url, "products/1").to_s) + expect(client.send(:url, "products/1").to_s) .to eq("https://jihudemo.zentao.net/zentao/api.php/v1/products/1") end end @@ -157,7 +242,7 @@ RSpec.describe Gitlab::Zentao::Client do let(:zentao_integration) { create(:zentao_integration, url: 'https://jihudemo.zentao.net') } it 'joins url correctly' do - expect(integration.send(:url, "products/1").to_s) + expect(client.send(:url, "products/1").to_s) .to eq("https://jihudemo.zentao.net/api.php/v1/products/1") end end diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb index 4ef977ba3d2..1a32453819d 100644 --- a/spec/models/integrations/zentao_spec.rb +++ b/spec/models/integrations/zentao_spec.rb @@ -81,4 +81,24 @@ RSpec.describe Integrations::Zentao do expect(zentao_integration.help).not_to be_empty end end + + describe '#client_url' do + subject(:integration) { build(:zentao_integration, api_url: api_url, url: 'url').client_url } + + context 'when api_url is set' do + let(:api_url) { 'api_url' } + + it 'returns the api_url' do + is_expected.to eq(api_url) + end + end + + context 'when api_url is not set' do + let(:api_url) { '' } + + it 'returns the url' do + is_expected.to eq('url') + end + end + end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 4ca76ae7dd2..17c3cd17364 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -831,14 +831,22 @@ RSpec.describe Issue do end describe '#to_branch_name exists ending with -index' do - before do + it 'returns #to_branch_name ending with max index + 1' do allow(repository).to receive(:branch_exists?).and_return(true) allow(repository).to receive(:branch_exists?).with("#{subject.to_branch_name}-3").and_return(false) - end - it 'returns #to_branch_name ending with max index + 1' do expect(subject.suggested_branch_name).to eq("#{subject.to_branch_name}-3") end + + context 'when branch name still exists after 5 attempts' do + it 'returns #to_branch_name ending with random characters' do + allow(repository).to receive(:branch_exists?).with(subject.to_branch_name).and_return(true) + allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\d/).and_return(true) + allow(repository).to receive(:branch_exists?).with(/#{subject.to_branch_name}-\h{8}/).and_return(false) + + expect(subject.suggested_branch_name).to match(/#{subject.to_branch_name}-\h{8}/) + end + end end end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 530b03714b4..47532ed1216 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -2625,7 +2625,7 @@ RSpec.describe Repository do end shared_examples '#tree' do - subject { repository.tree(sha, path, recursive: recursive, pagination_params: pagination_params) } + subject { repository.tree(sha, path, recursive: recursive, skip_flat_paths: false, pagination_params: pagination_params) } let(:sha) { :head } let(:path) { nil } diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 38bd189f6f4..da1f2653676 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -91,6 +91,45 @@ RSpec.describe Snippet do end end end + + context 'description validations' do + let_it_be(:invalid_description) { 'a' * (described_class::DESCRIPTION_LENGTH_MAX * 2) } + + context 'with existing snippets' do + let(:snippet) { create(:personal_snippet, description: 'This is a valid content at the time of creation') } + + it 'does not raise a validation error if the description is not changed' do + snippet.title = 'new title' + + expect(snippet).to be_valid + end + + it 'raises and error if the description is changed and the size is bigger than limit' do + expect(snippet).to be_valid + + snippet.description = invalid_description + + expect(snippet).not_to be_valid + end + end + + context 'with new snippets' do + it 'is valid when description is smaller than the limit' do + snippet = build(:personal_snippet, description: 'Valid Desc') + + expect(snippet).to be_valid + end + + it 'raises error when description is bigger than setting limit' do + snippet = build(:personal_snippet, description: invalid_description) + + aggregate_failures do + expect(snippet).not_to be_valid + expect(snippet.errors.messages_for(:description)).to include("is too long (2 MB). The maximum size is 1 MB.") + end + end + end + end end describe 'callbacks' do diff --git a/spec/presenters/commit_presenter_spec.rb b/spec/presenters/commit_presenter_spec.rb index b221c9ca8f7..df3ee69621b 100644 --- a/spec/presenters/commit_presenter_spec.rb +++ b/spec/presenters/commit_presenter_spec.rb @@ -12,29 +12,51 @@ RSpec.describe CommitPresenter do it { expect(presenter.web_path).to eq("/#{project.full_path}/-/commit/#{commit.sha}") } end - describe '#status_for' do - subject { presenter.status_for('ref') } + describe '#detailed_status_for' do + using RSpec::Parameterized::TableSyntax + + let(:pipeline) { create(:ci_pipeline, :success, project: project, sha: commit.sha, ref: 'ref') } - context 'when user can read_commit_status' do + subject { presenter.detailed_status_for('ref')&.text } + + where(:read_commit_status, :read_pipeline, :expected_result) do + true | true | 'passed' + true | false | nil + false | true | nil + false | false | nil + end + + with_them do before do - allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(true) + allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(read_commit_status) + allow(presenter).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(read_pipeline) end - it 'returns commit status for ref' do - pipeline = double - status = double + it { is_expected.to eq expected_result } + end + end - expect(commit).to receive(:latest_pipeline).with('ref').and_return(pipeline) - expect(pipeline).to receive(:detailed_status).with(user).and_return(status) + describe '#status_for' do + using RSpec::Parameterized::TableSyntax - expect(subject).to eq(status) - end + let(:pipeline) { create(:ci_pipeline, :success, project: project, sha: commit.sha) } + + subject { presenter.status_for } + + where(:read_commit_status, :read_pipeline, :expected_result) do + true | true | 'success' + true | false | nil + false | true | nil + false | false | nil end - context 'when user can not read_commit_status' do - it 'is nil' do - is_expected.to eq(nil) + with_them do + before do + allow(presenter).to receive(:can?).with(user, :read_commit_status, project).and_return(read_commit_status) + allow(presenter).to receive(:can?).with(user, :read_pipeline, pipeline).and_return(read_pipeline) end + + it { is_expected.to eq expected_result } end end diff --git a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb index 31fef75f679..bcbb1f11d43 100644 --- a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb +++ b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb @@ -6,11 +6,16 @@ RSpec.describe 'getting incident timeline events' do include GraphqlHelpers let_it_be(:project) { create(:project) } + let_it_be(:private_project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: private_project) } let_it_be(:current_user) { create(:user) } let_it_be(:updated_by_user) { create(:user) } let_it_be(:incident) { create(:incident, project: project) } let_it_be(:another_incident) { create(:incident, project: project) } let_it_be(:promoted_from_note) { create(:note, project: project, noteable: incident) } + let_it_be(:issue_url) { project_issue_url(private_project, issue) } + let_it_be(:issue_ref) { "#{private_project.full_path}##{issue.iid}" } + let_it_be(:issue_link) { %Q(<a href="#{issue_url}">#{issue_url}</a>) } let_it_be(:timeline_event) do create( @@ -18,7 +23,8 @@ RSpec.describe 'getting incident timeline events' do incident: incident, project: project, updated_by_user: updated_by_user, - promoted_from_note: promoted_from_note + promoted_from_note: promoted_from_note, + note: "Referencing #{issue.to_reference(full: true)} - Full URL #{issue_url}" ) end @@ -89,7 +95,7 @@ RSpec.describe 'getting incident timeline events' do 'title' => incident.title }, 'note' => timeline_event.note, - 'noteHtml' => timeline_event.note_html, + 'noteHtml' => "<p>Referencing #{issue_ref} - Full URL #{issue_link}</p>", 'promotedFromNote' => { 'id' => promoted_from_note.to_global_id.to_s, 'body' => promoted_from_note.note diff --git a/spec/requests/api/search_spec.rb b/spec/requests/api/search_spec.rb index 66b78829e0d..6034d26f1d2 100644 --- a/spec/requests/api/search_spec.rb +++ b/spec/requests/api/search_spec.rb @@ -763,6 +763,96 @@ RSpec.describe API::Search do it_behaves_like 'pagination', scope: :commits, search: 'merge' it_behaves_like 'ping counters', scope: :commits + + describe 'pipeline visibility' do + shared_examples 'pipeline information visible' do + it 'contains status and last_pipeline' do + request + + expect(json_response[0]['status']).to eq 'success' + expect(json_response[0]['last_pipeline']).not_to be_nil + end + end + + shared_examples 'pipeline information not visible' do + it 'does not contain status and last_pipeline' do + request + + expect(json_response[0]['status']).to be_nil + expect(json_response[0]['last_pipeline']).to be_nil + end + end + + let(:request) { get api(endpoint, user), params: { scope: 'commits', search: repo_project.commit.sha } } + + before do + create(:ci_pipeline, :success, project: repo_project, sha: repo_project.commit.sha) + end + + context 'with non public pipeline' do + let_it_be(:repo_project) do + create(:project, :public, :repository, public_builds: false, group: group) + end + + context 'user is project member with reporter role or above' do + before do + repo_project.add_reporter(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is project member with guest role' do + before do + repo_project.add_guest(user) + end + + it_behaves_like 'pipeline information not visible' + end + + context 'user is not project member' do + let_it_be(:user) { create(:user) } + + it_behaves_like 'pipeline information not visible' + end + end + + context 'with public pipeline' do + let_it_be(:repo_project) do + create(:project, :public, :repository, public_builds: true, group: group) + end + + context 'user is project member with reporter role or above' do + before do + repo_project.add_reporter(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is project member with guest role' do + before do + repo_project.add_guest(user) + end + + it_behaves_like 'pipeline information visible' + end + + context 'user is not project member' do + let_it_be(:user) { create(:user) } + + it_behaves_like 'pipeline information visible' + + context 'when CI/CD is set to only project members' do + before do + repo_project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE) + end + + it_behaves_like 'pipeline information not visible' + end + end + end + end end context 'for commits scope with project path as id' do diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 5eda50f7e96..81e923983ab 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -643,17 +643,17 @@ RSpec.describe 'Git HTTP requests' do end context 'when username and password are provided' do - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects the push attempt with personal access token error message' do + it 'rejects the push attempt with generic error message' do upload(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -750,17 +750,17 @@ RSpec.describe 'Git HTTP requests' do allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } end - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects pushes with personal access token error message' do + it 'rejects pushes with generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end @@ -771,10 +771,10 @@ RSpec.describe 'Git HTTP requests' do .to receive(:login).and_return(nil) end - it 'does not display the personal access token error message' do + it 'displays the generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -1300,17 +1300,18 @@ RSpec.describe 'Git HTTP requests' do end context 'when username and password are provided' do - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects the push attempt with personal access token error message' do + it 'rejects the push attempt with generic error message' do upload(path, user: user.username, password: user.password) do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end @@ -1381,17 +1382,17 @@ RSpec.describe 'Git HTTP requests' do allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false } end - it 'rejects pulls with personal access token error message' do + it 'rejects pulls with generic error message' do download(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end - it 'rejects pushes with personal access token error message' do + it 'rejects pushes with generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end @@ -1402,10 +1403,10 @@ RSpec.describe 'Git HTTP requests' do .to receive(:login).and_return(nil) end - it 'does not display the personal access token error message' do + it 'returns a generic error message' do upload(path, user: 'foo', password: 'bar') do |response| expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP') + expect(response.body).to eq('HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/topics/git/troubleshooting_git#error-on-git-fetch-http-basic-access-denied') end end end diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb index 7427ca11431..81c4f5d8188 100644 --- a/spec/requests/jwt_controller_spec.rb +++ b/spec/requests/jwt_controller_spec.rb @@ -37,6 +37,22 @@ RSpec.describe JwtController do end end + shared_examples "with invalid credentials" do + it "returns a generic error message" do + subject + + expect(response).to have_gitlab_http_status(:unauthorized) + expect(json_response).to eq( + { + "errors" => [{ + "code" => "UNAUTHORIZED", + "message" => "HTTP Basic: Access denied. The provided password or token is incorrect or your account has 2FA enabled and you must use a personal access token instead of a password. See http://www.example.com/help/user/profile/account/two_factor_authentication#troubleshooting" + }] + } + ) + end + end + context 'authenticating against container registry' do context 'existing service' do subject! { get '/jwt/auth', params: parameters } @@ -55,10 +71,7 @@ RSpec.describe JwtController do context 'with blocked user' do let(:user) { create(:user, :blocked) } - it 'rejects the request as unauthorized' do - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('HTTP Basic: Access denied') - end + it_behaves_like 'with invalid credentials' end end @@ -158,10 +171,7 @@ RSpec.describe JwtController do let(:user) { create(:user, :two_factor) } context 'without personal token' do - it 'rejects the authorization attempt' do - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end context 'with personal token' do @@ -185,14 +195,10 @@ RSpec.describe JwtController do context 'using invalid login' do let(:headers) { { authorization: credentials('invalid', 'password') } } + let(:subject) { get '/jwt/auth', params: parameters, headers: headers } context 'when internal auth is enabled' do - it 'rejects the authorization attempt' do - get '/jwt/auth', params: parameters, headers: headers - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).not_to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end context 'when internal auth is disabled' do @@ -200,12 +206,7 @@ RSpec.describe JwtController do stub_application_setting(password_authentication_enabled_for_git: false) end - it 'rejects the authorization attempt with personal access token message' do - get '/jwt/auth', params: parameters, headers: headers - - expect(response).to have_gitlab_http_status(:unauthorized) - expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP') - end + it_behaves_like 'with invalid credentials' end end end diff --git a/spec/scripts/lib/glfm/shared_spec.rb b/spec/scripts/lib/glfm/shared_spec.rb index f6792b93718..3ce9d44ba3d 100644 --- a/spec/scripts/lib/glfm/shared_spec.rb +++ b/spec/scripts/lib/glfm/shared_spec.rb @@ -9,6 +9,16 @@ RSpec.describe Glfm::Shared do end.new end + describe '#write_file' do + it 'works' do + filename = Dir::Tmpname.create('basename') do |path| + instance.write_file(path, 'test') + end + + expect(File.read(filename)).to eq 'test' + end + end + describe '#run_external_cmd' do it 'works' do expect(instance.run_external_cmd('echo "hello"')).to eq("hello\n") @@ -24,6 +34,14 @@ RSpec.describe Glfm::Shared do end end + describe '#dump_yaml_with_formatting' do + it 'works' do + hash = { a: 'b' } + yaml = instance.dump_yaml_with_formatting(hash, literal_scalars: true) + expect(yaml).to eq("---\na: |-\n b\n") + end + end + describe '#output' do # NOTE: The #output method is normally always mocked, to prevent output while the specs are # running. However, in order to provide code coverage for the method, we have to invoke diff --git a/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb b/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb index 57d31be8792..8401ab65418 100644 --- a/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb +++ b/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb @@ -43,10 +43,9 @@ RSpec.shared_context 'with API::Markdown Snapshot shared context' do |glfm_speci post api_url, params: { text: markdown, gfm: true } expect(response).to be_successful - response_body = Gitlab::Json.parse(response.body) - # Some requests have the HTML in the `html` key, others in the `body` key. - response_html = response_body['body'] ? response_body.fetch('body') : response_body.fetch('html') - # noinspection RubyResolve + parsed_response = Gitlab::Json.parse(response.body, symbolize_names: true) + # Some responses have the HTML in the `html` key, others in the `body` key. + response_html = parsed_response[:body] || parsed_response[:html] normalized_response_html = normalize_html(response_html, normalizations) expect(normalized_response_html).to eq(normalized_html) diff --git a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb index 1a248bb04e7..ba8311bf0be 100644 --- a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb @@ -170,6 +170,17 @@ RSpec.shared_examples 'PyPI package download' do |user_type, status, add_member end end +RSpec.shared_examples 'rejected package download' do |user_type, status, add_member = true| + context "for user type #{user_type}" do + before do + project.send("add_#{user_type}", user) if add_member && user_type != :anonymous + group.send("add_#{user_type}", user) if add_member && user_type != :anonymous + end + + it_behaves_like 'returning response status', status + end +end + RSpec.shared_examples 'process PyPI api request' do |user_type, status, add_member = true| context "for user type #{user_type}" do before do @@ -330,25 +341,25 @@ RSpec.shared_examples 'pypi file download endpoint' do using RSpec::Parameterized::TableSyntax context 'with valid project' do - where(:visibility_level, :user_role, :member, :user_token) do - :public | :developer | true | true - :public | :guest | true | true - :public | :developer | true | false - :public | :guest | true | false - :public | :developer | false | true - :public | :guest | false | true - :public | :developer | false | false - :public | :guest | false | false - :public | :anonymous | false | true - :private | :developer | true | true - :private | :guest | true | true - :private | :developer | true | false - :private | :guest | true | false - :private | :developer | false | true - :private | :guest | false | true - :private | :developer | false | false - :private | :guest | false | false - :private | :anonymous | false | true + where(:visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + :public | :developer | true | true | 'PyPI package download' | :success + :public | :guest | true | true | 'PyPI package download' | :success + :public | :developer | true | false | 'PyPI package download' | :success + :public | :guest | true | false | 'PyPI package download' | :success + :public | :developer | false | true | 'PyPI package download' | :success + :public | :guest | false | true | 'PyPI package download' | :success + :public | :developer | false | false | 'PyPI package download' | :success + :public | :guest | false | false | 'PyPI package download' | :success + :public | :anonymous | false | true | 'PyPI package download' | :success + :private | :developer | true | true | 'PyPI package download' | :success + :private | :guest | true | true | 'rejected package download' | :forbidden + :private | :developer | true | false | 'rejected package download' | :unauthorized + :private | :guest | true | false | 'rejected package download' | :unauthorized + :private | :developer | false | true | 'rejected package download' | :not_found + :private | :guest | false | true | 'rejected package download' | :not_found + :private | :developer | false | false | 'rejected package download' | :unauthorized + :private | :guest | false | false | 'rejected package download' | :unauthorized + :private | :anonymous | false | true | 'rejected package download' | :unauthorized end with_them do @@ -360,7 +371,7 @@ RSpec.shared_examples 'pypi file download endpoint' do group.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility_level.to_s)) end - it_behaves_like 'PyPI package download', params[:user_role], :success, params[:member] + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] end end diff --git a/spec/validators/bytesize_validator_spec.rb b/spec/validators/bytesize_validator_spec.rb new file mode 100644 index 00000000000..1914ccedd87 --- /dev/null +++ b/spec/validators/bytesize_validator_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe BytesizeValidator do + let(:model) do + Class.new do + include ActiveModel::Model + include ActiveModel::Validations + + attr_accessor :content + alias_method :content_before_type_cast, :content + + validates :content, bytesize: { maximum: -> { 7 } } + end.new + end + + using RSpec::Parameterized::TableSyntax + + where(:content, :validity, :errors) do + 'short' | true | {} + 'very long' | false | { content: ['is too long (9 Bytes). The maximum size is 7 Bytes.'] } + 'short😁' | false | { content: ['is too long (9 Bytes). The maximum size is 7 Bytes.'] } + 'short⇏' | false | { content: ['is too long (8 Bytes). The maximum size is 7 Bytes.'] } + end + + with_them do + before do + model.content = content + model.validate + end + + it { expect(model.valid?).to eq(validity) } + it { expect(model.errors.messages).to eq(errors) } + end +end diff --git a/spec/views/projects/commits/_commit.html.haml_spec.rb b/spec/views/projects/commits/_commit.html.haml_spec.rb index 2ca23d4cb2d..4e8a680b6de 100644 --- a/spec/views/projects/commits/_commit.html.haml_spec.rb +++ b/spec/views/projects/commits/_commit.html.haml_spec.rb @@ -47,13 +47,12 @@ RSpec.describe 'projects/commits/_commit.html.haml' do context 'with ci status' do let(:ref) { 'master' } - let(:user) { create(:user) } + + let_it_be(:user) { create(:user) } before do allow(view).to receive(:current_user).and_return(user) - project.add_developer(user) - create( :ci_empty_pipeline, ref: 'master', @@ -80,18 +79,32 @@ RSpec.describe 'projects/commits/_commit.html.haml' do end context 'when pipelines are enabled' do - before do - allow(project).to receive(:builds_enabled?).and_return(true) + context 'when user has access' do + before do + project.add_developer(user) + end + + it 'displays a ci status icon' do + render partial: template, formats: :html, locals: { + project: project, + ref: ref, + commit: commit + } + + expect(rendered).to have_css('.ci-status-link') + end end - it 'does display a ci status icon when pipelines are enabled' do - render partial: template, formats: :html, locals: { - project: project, - ref: ref, - commit: commit - } + context 'when user does not have access' do + it 'does not display a ci status icon' do + render partial: template, formats: :html, locals: { + project: project, + ref: ref, + commit: commit + } - expect(rendered).to have_css('.ci-status-link') + expect(rendered).not_to have_css('.ci-status-link') + end end end end |