diff options
Diffstat (limited to 'spec')
15 files changed, 293 insertions, 291 deletions
diff --git a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js index b04bfa65e37..98f190bc33a 100644 --- a/spec/frontend/design_management/components/design_notes/design_discussion_spec.js +++ b/spec/frontend/design_management/components/design_notes/design_discussion_spec.js @@ -32,7 +32,6 @@ describe('Design discussions component', () => { const mutationVariables = { mutation: createNoteMutation, - update: expect.anything(), variables: { input: { noteableId: 'noteable-id', @@ -41,7 +40,7 @@ describe('Design discussions component', () => { }, }, }; - const mutate = jest.fn(() => Promise.resolve()); + const mutate = jest.fn().mockResolvedValue({ data: { createNote: { errors: [] } } }); const $apollo = { mutate, }; @@ -227,7 +226,7 @@ describe('Design discussions component', () => { }); }); - it('calls mutation on submitting form and closes the form', () => { + it('calls mutation on submitting form and closes the form', async () => { createComponent( { discussionWithOpenForm: defaultMockDiscussion.id }, { discussionComment: 'test', isFormRendered: true }, @@ -236,13 +235,10 @@ describe('Design discussions component', () => { findReplyForm().vm.$emit('submitForm'); expect(mutate).toHaveBeenCalledWith(mutationVariables); - return mutate() - .then(() => { - return wrapper.vm.$nextTick(); - }) - .then(() => { - expect(findReplyForm().exists()).toBe(false); - }); + await mutate(); + await wrapper.vm.$nextTick(); + + expect(findReplyForm().exists()).toBe(false); }); it('clears the discussion comment on closing comment form', () => { diff --git a/spec/frontend/design_management/utils/cache_update_spec.js b/spec/frontend/design_management/utils/cache_update_spec.js index e8a5cf3949d..6c859e8c3e8 100644 --- a/spec/frontend/design_management/utils/cache_update_spec.js +++ b/spec/frontend/design_management/utils/cache_update_spec.js @@ -1,14 +1,12 @@ import { InMemoryCache } from 'apollo-cache-inmemory'; import { updateStoreAfterDesignsDelete, - updateStoreAfterAddDiscussionComment, updateStoreAfterAddImageDiffNote, updateStoreAfterUploadDesign, updateStoreAfterUpdateImageDiffNote, } from '~/design_management/utils/cache_update'; import { designDeletionError, - ADD_DISCUSSION_COMMENT_ERROR, ADD_IMAGE_DIFF_NOTE_ERROR, UPDATE_IMAGE_DIFF_NOTE_ERROR, } from '~/design_management/utils/error_messages'; @@ -28,12 +26,11 @@ describe('Design Management cache update', () => { describe('error handling', () => { it.each` - fnName | subject | errorMessage | extraArgs - ${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]} - ${'updateStoreAfterAddDiscussionComment'} | ${updateStoreAfterAddDiscussionComment} | ${ADD_DISCUSSION_COMMENT_ERROR} | ${[]} - ${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]} - ${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]} - ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]} + fnName | subject | errorMessage | extraArgs + ${'updateStoreAfterDesignsDelete'} | ${updateStoreAfterDesignsDelete} | ${designDeletionError({ singular: true })} | ${[[design]]} + ${'updateStoreAfterAddImageDiffNote'} | ${updateStoreAfterAddImageDiffNote} | ${ADD_IMAGE_DIFF_NOTE_ERROR} | ${[]} + ${'updateStoreAfterUploadDesign'} | ${updateStoreAfterUploadDesign} | ${mockErrors[0]} | ${[]} + ${'updateStoreAfterUpdateImageDiffNote'} | ${updateStoreAfterUpdateImageDiffNote} | ${UPDATE_IMAGE_DIFF_NOTE_ERROR} | ${[]} `('$fnName handles errors in response', ({ subject, extraArgs, errorMessage }) => { expect(createFlash).not.toHaveBeenCalled(); expect(() => subject(mockStore, { errors: mockErrors }, {}, ...extraArgs)).toThrow(); diff --git a/spec/frontend/ide/components/jobs/detail_spec.js b/spec/frontend/ide/components/jobs/detail_spec.js index acd30dee718..496d8284fdd 100644 --- a/spec/frontend/ide/components/jobs/detail_spec.js +++ b/spec/frontend/ide/components/jobs/detail_spec.js @@ -24,7 +24,7 @@ describe('IDE jobs detail view', () => { beforeEach(() => { vm = createComponent(); - jest.spyOn(vm, 'fetchJobTrace').mockResolvedValue(); + jest.spyOn(vm, 'fetchJobLogs').mockResolvedValue(); }); afterEach(() => { @@ -36,8 +36,8 @@ describe('IDE jobs detail view', () => { vm = vm.$mount(); }); - it('calls fetchJobTrace', () => { - expect(vm.fetchJobTrace).toHaveBeenCalled(); + it('calls fetchJobLogs', () => { + expect(vm.fetchJobLogs).toHaveBeenCalled(); }); it('scrolls to bottom', () => { @@ -96,7 +96,7 @@ describe('IDE jobs detail view', () => { describe('scroll buttons', () => { beforeEach(() => { vm = createComponent(); - jest.spyOn(vm, 'fetchJobTrace').mockResolvedValue(); + jest.spyOn(vm, 'fetchJobLogs').mockResolvedValue(); }); afterEach(() => { diff --git a/spec/frontend/ide/stores/modules/pipelines/actions_spec.js b/spec/frontend/ide/stores/modules/pipelines/actions_spec.js index 71918e7e2c2..8511843cc92 100644 --- a/spec/frontend/ide/stores/modules/pipelines/actions_spec.js +++ b/spec/frontend/ide/stores/modules/pipelines/actions_spec.js @@ -15,10 +15,10 @@ import { fetchJobs, toggleStageCollapsed, setDetailJob, - requestJobTrace, - receiveJobTraceError, - receiveJobTraceSuccess, - fetchJobTrace, + requestJobLogs, + receiveJobLogsError, + receiveJobLogsSuccess, + fetchJobLogs, resetLatestPipeline, } from '~/ide/stores/modules/pipelines/actions'; import state from '~/ide/stores/modules/pipelines/state'; @@ -324,24 +324,24 @@ describe('IDE pipelines actions', () => { }); }); - describe('requestJobTrace', () => { + describe('requestJobLogs', () => { it('commits request', done => { - testAction(requestJobTrace, null, mockedState, [{ type: types.REQUEST_JOB_TRACE }], [], done); + testAction(requestJobLogs, null, mockedState, [{ type: types.REQUEST_JOB_LOGS }], [], done); }); }); - describe('receiveJobTraceError', () => { + describe('receiveJobLogsError', () => { it('commits error', done => { testAction( - receiveJobTraceError, + receiveJobLogsError, null, mockedState, - [{ type: types.RECEIVE_JOB_TRACE_ERROR }], + [{ type: types.RECEIVE_JOB_LOGS_ERROR }], [ { type: 'setErrorMessage', payload: { - text: 'An error occurred while fetching the job trace.', + text: 'An error occurred while fetching the job logs.', action: expect.any(Function), actionText: 'Please try again', actionPayload: null, @@ -353,20 +353,20 @@ describe('IDE pipelines actions', () => { }); }); - describe('receiveJobTraceSuccess', () => { + describe('receiveJobLogsSuccess', () => { it('commits data', done => { testAction( - receiveJobTraceSuccess, + receiveJobLogsSuccess, 'data', mockedState, - [{ type: types.RECEIVE_JOB_TRACE_SUCCESS, payload: 'data' }], + [{ type: types.RECEIVE_JOB_LOGS_SUCCESS, payload: 'data' }], [], done, ); }); }); - describe('fetchJobTrace', () => { + describe('fetchJobLogs', () => { beforeEach(() => { mockedState.detailJob = { path: `${TEST_HOST}/project/builds` }; }); @@ -379,20 +379,20 @@ describe('IDE pipelines actions', () => { it('dispatches request', done => { testAction( - fetchJobTrace, + fetchJobLogs, null, mockedState, [], [ - { type: 'requestJobTrace' }, - { type: 'receiveJobTraceSuccess', payload: { html: 'html' } }, + { type: 'requestJobLogs' }, + { type: 'receiveJobLogsSuccess', payload: { html: 'html' } }, ], done, ); }); it('sends get request to correct URL', () => { - fetchJobTrace({ + fetchJobLogs({ state: mockedState, dispatch() {}, @@ -410,11 +410,11 @@ describe('IDE pipelines actions', () => { it('dispatches error', done => { testAction( - fetchJobTrace, + fetchJobLogs, null, mockedState, [], - [{ type: 'requestJobTrace' }, { type: 'receiveJobTraceError' }], + [{ type: 'requestJobLogs' }, { type: 'receiveJobLogsError' }], done, ); }); diff --git a/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js b/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js index 3b7f92cfa74..7d2f5d5d710 100644 --- a/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js +++ b/spec/frontend/ide/stores/modules/pipelines/mutations_spec.js @@ -175,37 +175,37 @@ describe('IDE pipelines mutations', () => { }); }); - describe('REQUEST_JOB_TRACE', () => { + describe('REQUEST_JOB_LOGS', () => { beforeEach(() => { mockedState.detailJob = { ...jobs[0] }; }); it('sets loading on detail job', () => { - mutations[types.REQUEST_JOB_TRACE](mockedState); + mutations[types.REQUEST_JOB_LOGS](mockedState); expect(mockedState.detailJob.isLoading).toBe(true); }); }); - describe('RECEIVE_JOB_TRACE_ERROR', () => { + describe('RECEIVE_JOB_LOGS_ERROR', () => { beforeEach(() => { mockedState.detailJob = { ...jobs[0], isLoading: true }; }); it('sets loading to false on detail job', () => { - mutations[types.RECEIVE_JOB_TRACE_ERROR](mockedState); + mutations[types.RECEIVE_JOB_LOGS_ERROR](mockedState); expect(mockedState.detailJob.isLoading).toBe(false); }); }); - describe('RECEIVE_JOB_TRACE_SUCCESS', () => { + describe('RECEIVE_JOB_LOGS_SUCCESS', () => { beforeEach(() => { mockedState.detailJob = { ...jobs[0], isLoading: true }; }); it('sets output on detail job', () => { - mutations[types.RECEIVE_JOB_TRACE_SUCCESS](mockedState, { html: 'html' }); + mutations[types.RECEIVE_JOB_LOGS_SUCCESS](mockedState, { html: 'html' }); expect(mockedState.detailJob.output).toBe('html'); expect(mockedState.detailJob.isLoading).toBe(false); }); diff --git a/spec/frontend/vue_shared/components/confirm_modal_spec.js b/spec/frontend/vue_shared/components/confirm_modal_spec.js index 7bccd6f1a64..5d92af64de0 100644 --- a/spec/frontend/vue_shared/components/confirm_modal_spec.js +++ b/spec/frontend/vue_shared/components/confirm_modal_spec.js @@ -1,5 +1,4 @@ import { shallowMount } from '@vue/test-utils'; -import { GlModal } from '@gitlab/ui'; import { TEST_HOST } from 'helpers/test_constants'; import ConfirmModal from '~/vue_shared/components/confirm_modal.vue'; @@ -21,9 +20,14 @@ describe('vue_shared/components/confirm_modal', () => { selector: '.test-button', }; - const actionSpies = { - openModal: jest.fn(), - closeModal: jest.fn(), + const popupMethods = { + hide: jest.fn(), + show: jest.fn(), + }; + + const GlModalStub = { + template: '<div><slot></slot></div>', + methods: popupMethods, }; let wrapper; @@ -34,8 +38,8 @@ describe('vue_shared/components/confirm_modal', () => { ...defaultProps, ...props, }, - methods: { - ...actionSpies, + stubs: { + GlModal: GlModalStub, }, }); }; @@ -44,7 +48,7 @@ describe('vue_shared/components/confirm_modal', () => { wrapper.destroy(); }); - const findModal = () => wrapper.find(GlModal); + const findModal = () => wrapper.find(GlModalStub); const findForm = () => wrapper.find('form'); const findFormData = () => findForm() @@ -103,7 +107,7 @@ describe('vue_shared/components/confirm_modal', () => { }); it('does not close modal', () => { - expect(actionSpies.closeModal).not.toHaveBeenCalled(); + expect(popupMethods.hide).not.toHaveBeenCalled(); }); describe('when modal closed', () => { @@ -112,7 +116,7 @@ describe('vue_shared/components/confirm_modal', () => { }); it('closes modal', () => { - expect(actionSpies.closeModal).toHaveBeenCalled(); + expect(popupMethods.hide).toHaveBeenCalled(); }); }); }); diff --git a/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js b/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js index e0e982f4e11..e91e6577aaf 100644 --- a/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js +++ b/spec/frontend/vue_shared/components/diff_viewer/viewers/renamed_spec.js @@ -14,19 +14,13 @@ import { const localVue = createLocalVue(); localVue.use(Vuex); -function createRenamedComponent({ - props = {}, - methods = {}, - store = new Vuex.Store({}), - deep = false, -}) { +function createRenamedComponent({ props = {}, store = new Vuex.Store({}), deep = false }) { const mnt = deep ? mount : shallowMount; return mnt(Renamed, { propsData: { ...props }, localVue, store, - methods, }); } @@ -258,25 +252,17 @@ describe('Renamed Diff Viewer', () => { 'includes a link to the full file for alternate viewer type "$altType"', ({ altType, linkText }) => { const file = { ...diffFile }; - const clickMock = jest.fn().mockImplementation(() => {}); file.alternate_viewer.name = altType; wrapper = createRenamedComponent({ deep: true, props: { diffFile: file }, - methods: { - clickLink: clickMock, - }, }); const link = wrapper.find('a'); expect(link.text()).toEqual(linkText); expect(link.attributes('href')).toEqual(DIFF_FILE_VIEW_PATH); - - link.vm.$emit('click'); - - expect(clickMock).toHaveBeenCalled(); }, ); }); diff --git a/spec/lib/gitlab/ci/templates/templates_spec.rb b/spec/lib/gitlab/ci/templates/templates_spec.rb index 685243d6315..768256ee6b3 100644 --- a/spec/lib/gitlab/ci/templates/templates_spec.rb +++ b/spec/lib/gitlab/ci/templates/templates_spec.rb @@ -7,17 +7,17 @@ RSpec.describe 'CI YML Templates' do let(:all_templates) { Gitlab::Template::GitlabCiYmlTemplate.all.map(&:full_name) } - let(:disabled_templates) do - Gitlab::Template::GitlabCiYmlTemplate.disabled_templates.map do |template| - template + Gitlab::Template::GitlabCiYmlTemplate.extension + let(:excluded_templates) do + all_templates.select do |name| + Gitlab::Template::GitlabCiYmlTemplate.excluded_patterns.any? { |pattern| pattern.match?(name) } end end - context 'included in a CI YAML configuration' do + context 'when including available templates in a CI YAML configuration' do using RSpec::Parameterized::TableSyntax where(:template_name) do - all_templates - disabled_templates + all_templates - excluded_templates end with_them do @@ -41,4 +41,29 @@ RSpec.describe 'CI YML Templates' do end end end + + context 'when including unavailable templates in a CI YAML configuration' do + using RSpec::Parameterized::TableSyntax + + where(:template_name) do + excluded_templates + end + + with_them do + let(:content) do + <<~EOS + include: + - template: #{template_name} + + concrete_build_implemented_by_a_user: + stage: test + script: do something + EOS + end + + it 'is not valid' do + expect(subject).not_to be_valid + end + end + end end diff --git a/spec/lib/gitlab/template/finders/global_template_finder_spec.rb b/spec/lib/gitlab/template/finders/global_template_finder_spec.rb index e776284b3e8..e2751d194d3 100644 --- a/spec/lib/gitlab/template/finders/global_template_finder_spec.rb +++ b/spec/lib/gitlab/template/finders/global_template_finder_spec.rb @@ -15,9 +15,9 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do FileUtils.rm_rf(base_dir) end - subject(:finder) { described_class.new(base_dir, '', { 'General' => '', 'Bar' => 'Bar' }, exclusions: exclusions) } + subject(:finder) { described_class.new(base_dir, '', { 'General' => '', 'Bar' => 'Bar' }, excluded_patterns: excluded_patterns) } - let(:exclusions) { [] } + let(:excluded_patterns) { [] } describe '.find' do context 'with a non-prefixed General template' do @@ -38,7 +38,7 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do end context 'while listed as an exclusion' do - let(:exclusions) { %w[test-template] } + let(:excluded_patterns) { [%r{^test-template$}] } it 'does not find the template without a prefix' do expect(finder.find('test-template')).to be_nil @@ -77,7 +77,7 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do end context 'while listed as an exclusion' do - let(:exclusions) { %w[Bar/test-template] } + let(:excluded_patterns) { [%r{^Bar/test-template$}] } it 'does not find the template with a prefix' do expect(finder.find('Bar/test-template')).to be_nil @@ -96,6 +96,17 @@ RSpec.describe Gitlab::Template::Finders::GlobalTemplateFinder do expect(finder.find('Bar/test-template')).to be_nil end end + + context 'while listed as an exclusion' do + let(:excluded_patterns) { [%r{\.latest$}] } + + it 'excludes the template matched the pattern' do + create_template!('test-template.latest') + + expect(finder.find('test-template')).to be_present + expect(finder.find('test-template.latest')).to be_nil + end + end end end end diff --git a/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb b/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb index 55444114d39..26c83ed6793 100644 --- a/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb +++ b/spec/lib/gitlab/template/gitlab_ci_yml_template_spec.rb @@ -13,6 +13,12 @@ RSpec.describe Gitlab::Template::GitlabCiYmlTemplate do expect(all).to include('Docker') expect(all).to include('Ruby') end + + it 'does not include Browser-Performance template in FOSS' do + all = subject.all.map(&:name) + + expect(all).not_to include('Browser-Performance') unless Gitlab.ee? + end end describe '#content' do diff --git a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb index 7b9650a966c..6da33a83330 100644 --- a/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb +++ b/spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb @@ -62,65 +62,81 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s end describe '.track_event' do - it "raise error if metrics don't have same aggregation" do - expect { described_class.track_event(entity1, different_aggregation, Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownAggregation) - end + context 'when usage_ping is disabled' do + it 'does not track the event' do + stub_application_setting(usage_ping_enabled: false) - it 'raise error if metrics of unknown aggregation' do - expect { described_class.track_event(entity1, 'unknown', Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent) + described_class.track_event(entity1, weekly_event, Date.current) + + expect(Gitlab::Redis::HLL).not_to receive(:add) + end end - context 'for weekly events' do - it 'sets the keys in Redis to expire automatically after the given expiry time' do - described_class.track_event(entity1, "g_analytics_contribution") + context 'when usage_ping is enabled' do + before do + stub_application_setting(usage_ping_enabled: true) + end - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "g_{analytics}_contribution-*").to_a - expect(keys).not_to be_empty + it "raise error if metrics don't have same aggregation" do + expect { described_class.track_event(entity1, different_aggregation, Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownAggregation) + end - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(12.weeks) + it 'raise error if metrics of unknown aggregation' do + expect { described_class.track_event(entity1, 'unknown', Date.current) } .to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::UnknownEvent) + end + + context 'for weekly events' do + it 'sets the keys in Redis to expire automatically after the given expiry time' do + described_class.track_event(entity1, "g_analytics_contribution") + + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "g_{analytics}_contribution-*").to_a + expect(keys).not_to be_empty + + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(12.weeks) + end end end - end - it 'sets the keys in Redis to expire automatically after 6 weeks by default' do - described_class.track_event(entity1, "g_compliance_dashboard") + it 'sets the keys in Redis to expire automatically after 6 weeks by default' do + described_class.track_event(entity1, "g_compliance_dashboard") - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "g_{compliance}_dashboard-*").to_a - expect(keys).not_to be_empty + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "g_{compliance}_dashboard-*").to_a + expect(keys).not_to be_empty - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(6.weeks) + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(6.weeks) + end end end end - end - context 'for daily events' do - it 'sets the keys in Redis to expire after the given expiry time' do - described_class.track_event(entity1, "g_analytics_search") + context 'for daily events' do + it 'sets the keys in Redis to expire after the given expiry time' do + described_class.track_event(entity1, "g_analytics_search") - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "*-g_{analytics}_search").to_a - expect(keys).not_to be_empty + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "*-g_{analytics}_search").to_a + expect(keys).not_to be_empty - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(84.days) + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(84.days) + end end end - end - it 'sets the keys in Redis to expire after 29 days by default' do - described_class.track_event(entity1, "no_slot") + it 'sets the keys in Redis to expire after 29 days by default' do + described_class.track_event(entity1, "no_slot") - Gitlab::Redis::SharedState.with do |redis| - keys = redis.scan_each(match: "*-{no_slot}").to_a - expect(keys).not_to be_empty + Gitlab::Redis::SharedState.with do |redis| + keys = redis.scan_each(match: "*-{no_slot}").to_a + expect(keys).not_to be_empty - keys.each do |key| - expect(redis.ttl(key)).to be_within(5.seconds).of(29.days) + keys.each do |key| + expect(redis.ttl(key)).to be_within(5.seconds).of(29.days) + end end end end diff --git a/spec/requests/api/project_snippets_spec.rb b/spec/requests/api/project_snippets_spec.rb index 66dcb40d2ec..0746dee3e51 100644 --- a/spec/requests/api/project_snippets_spec.rb +++ b/spec/requests/api/project_snippets_spec.rb @@ -304,30 +304,9 @@ RSpec.describe API::ProjectSnippets do let(:visibility_level) { Snippet::PUBLIC } let(:snippet) { create(:project_snippet, :repository, author: admin, visibility_level: visibility_level, project: project) } - it 'updates snippet' do - new_content = 'New content' - new_description = 'New description' - - update_snippet(params: { content: new_content, description: new_description, visibility: 'private' }) - - expect(response).to have_gitlab_http_status(:ok) - snippet.reload - expect(snippet.content).to eq(new_content) - expect(snippet.description).to eq(new_description) - expect(snippet.visibility).to eq('private') - end - - it 'updates snippet with content parameter' do - new_content = 'New content' - new_description = 'New description' - - update_snippet(params: { content: new_content, description: new_description }) - - expect(response).to have_gitlab_http_status(:ok) - snippet.reload - expect(snippet.content).to eq(new_content) - expect(snippet.description).to eq(new_description) - end + it_behaves_like 'snippet file updates' + it_behaves_like 'snippet non-file updates' + it_behaves_like 'invalid snippet updates' it 'updates snippet with visibility parameter' do expect { update_snippet(params: { visibility: 'private' }) } @@ -336,33 +315,6 @@ RSpec.describe API::ProjectSnippets do expect(snippet.visibility).to eq('private') end - it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: non_existing_record_id, params: { title: 'foo' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it 'returns 400 for missing parameters' do - update_snippet - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title, file_name, content, visibility are missing, at least one parameter must be provided' - end - - it 'returns 400 if content is blank' do - update_snippet(params: { content: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 if title is blank' do - update_snippet(params: { title: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title is empty' - end - it_behaves_like 'update with repository actions' do let(:snippet_without_repo) { create(:project_snippet, author: admin, project: project, visibility_level: visibility_level) } end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 508a39ce227..5bba308a2d3 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -368,7 +368,7 @@ RSpec.describe API::Snippets do context 'when the snippet is public' do let(:extra_params) { { visibility: 'public' } } - it 'rejects the shippet' do + it 'rejects the snippet' do expect { subject }.not_to change { Snippet.count } expect(response).to have_gitlab_http_status(:bad_request) @@ -391,98 +391,17 @@ RSpec.describe API::Snippets do create(:personal_snippet, :repository, author: user, visibility_level: visibility_level) end - let(:create_action) { { action: 'create', file_path: 'foo.txt', content: 'bar' } } - let(:update_action) { { action: 'update', file_path: 'CHANGELOG', content: 'bar' } } - let(:move_action) { { action: 'move', file_path: '.old-gitattributes', previous_path: '.gitattributes' } } - let(:delete_action) { { action: 'delete', file_path: 'CONTRIBUTING.md' } } - let(:bad_file_path) { { action: 'create', file_path: '../../etc/passwd', content: 'bar' } } - let(:bad_previous_path) { { action: 'create', previous_path: '../../etc/passwd', file_path: 'CHANGELOG', content: 'bar' } } - let(:invalid_move) { { action: 'move', file_path: 'missing_previous_path.txt' } } - - context 'with snippet file changes' do - using RSpec::Parameterized::TableSyntax - - where(:is_multi_file, :file_name, :content, :files, :status) do - true | nil | nil | [create_action] | :success - true | nil | nil | [update_action] | :success - true | nil | nil | [move_action] | :success - true | nil | nil | [delete_action] | :success - true | nil | nil | [create_action, update_action] | :success - true | 'foo.txt' | 'bar' | [create_action] | :bad_request - true | 'foo.txt' | 'bar' | nil | :bad_request - true | nil | nil | nil | :bad_request - true | 'foo.txt' | nil | [create_action] | :bad_request - true | nil | 'bar' | [create_action] | :bad_request - true | '' | nil | [create_action] | :bad_request - true | nil | '' | [create_action] | :bad_request - true | nil | nil | [bad_file_path] | :bad_request - true | nil | nil | [bad_previous_path] | :bad_request - true | nil | nil | [invalid_move] | :unprocessable_entity - - false | 'foo.txt' | 'bar' | nil | :success - false | 'foo.txt' | nil | nil | :success - false | nil | 'bar' | nil | :success - false | 'foo.txt' | 'bar' | [create_action] | :bad_request - false | nil | nil | nil | :bad_request - false | nil | '' | nil | :bad_request - false | nil | nil | [bad_file_path] | :bad_request - false | nil | nil | [bad_previous_path] | :bad_request - end - - with_them do - before do - allow_any_instance_of(Snippet).to receive(:multiple_files?).and_return(is_multi_file) - end - - it 'has the correct response' do - update_params = {}.tap do |params| - params[:files] = files if files - params[:file_name] = file_name if file_name - params[:content] = content if content - end - - update_snippet(params: update_params) - - expect(response).to have_gitlab_http_status(status) - end - end - - context 'when save fails due to a repository commit error' do - before do - allow_next_instance_of(Repository) do |instance| - allow(instance).to receive(:multi_action).and_raise(Gitlab::Git::CommitError) - end - - update_snippet(params: { files: [create_action] }) - end - - it 'returns a bad request response' do - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end - - shared_examples 'snippet non-file updates' do - it 'updates a snippet non-file attributes' do - new_description = 'New description' - new_title = 'New title' - new_visibility = 'internal' - - update_snippet(params: { title: new_title, description: new_description, visibility: new_visibility }) + it_behaves_like 'snippet file updates' + it_behaves_like 'snippet non-file updates' + it_behaves_like 'invalid snippet updates' - snippet.reload + it "returns 404 for another user's snippet" do + update_snippet(requester: other_user, params: { title: 'foobar' }) - aggregate_failures do - expect(response).to have_gitlab_http_status(:ok) - expect(snippet.description).to eq(new_description) - expect(snippet.visibility).to eq(new_visibility) - expect(snippet.title).to eq(new_title) - end - end + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Snippet Not Found') end - it_behaves_like 'snippet non-file updates' - context 'with restricted visibility settings' do before do stub_application_setting(restricted_visibility_levels: @@ -493,33 +412,6 @@ RSpec.describe API::Snippets do it_behaves_like 'snippet non-file updates' end - it 'returns 404 for invalid snippet id' do - update_snippet(snippet_id: non_existing_record_id, params: { title: 'Foo' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it "returns 404 for another user's snippet" do - update_snippet(requester: other_user, params: { title: 'foobar' }) - - expect(response).to have_gitlab_http_status(:not_found) - expect(json_response['message']).to eq('404 Snippet Not Found') - end - - it 'returns 400 for missing parameters' do - update_snippet - - expect(response).to have_gitlab_http_status(:bad_request) - end - - it 'returns 400 if title is blank' do - update_snippet(params: { title: '' }) - - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to eq 'title is empty' - end - it_behaves_like 'update with repository actions' do let(:snippet_without_repo) { create(:personal_snippet, author: user, visibility_level: visibility_level) } end @@ -543,7 +435,7 @@ RSpec.describe API::Snippets do context 'when the snippet is public' do let(:visibility_level) { Snippet::PUBLIC } - it 'rejects the shippet' do + it 'rejects the snippet' do expect { update_snippet(params: { title: 'Foo' }) } .not_to change { snippet.reload.title } diff --git a/spec/support/helpers/stubbed_feature.rb b/spec/support/helpers/stubbed_feature.rb index e78efcf6b75..d4e9af7a031 100644 --- a/spec/support/helpers/stubbed_feature.rb +++ b/spec/support/helpers/stubbed_feature.rb @@ -37,10 +37,7 @@ module StubbedFeature # We do `m.call` as we want to validate the execution of method arguments # and a feature flag state if it is not persisted unless Feature.persisted_name?(args.first) - # TODO: this is hack to support `promo_feature_available?` - # We enable all feature flags by default unless they are `promo_` - # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/218667 - feature_flag = true unless args.first.to_s.start_with?('promo_') + feature_flag = true end feature_flag diff --git a/spec/support/shared_examples/requests/api/snippets_shared_examples.rb b/spec/support/shared_examples/requests/api/snippets_shared_examples.rb index cfbb84dd099..97f85977a20 100644 --- a/spec/support/shared_examples/requests/api/snippets_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/snippets_shared_examples.rb @@ -77,3 +77,123 @@ RSpec.shared_examples 'raw snippet files' do end end end + +RSpec.shared_examples 'snippet file updates' do + let(:create_action) { { action: 'create', file_path: 'foo.txt', content: 'bar' } } + let(:update_action) { { action: 'update', file_path: 'CHANGELOG', content: 'bar' } } + let(:move_action) { { action: 'move', file_path: '.old-gitattributes', previous_path: '.gitattributes' } } + let(:delete_action) { { action: 'delete', file_path: 'CONTRIBUTING.md' } } + let(:bad_file_path) { { action: 'create', file_path: '../../etc/passwd', content: 'bar' } } + let(:bad_previous_path) { { action: 'create', previous_path: '../../etc/passwd', file_path: 'CHANGELOG', content: 'bar' } } + let(:invalid_move) { { action: 'move', file_path: 'missing_previous_path.txt' } } + + context 'with various snippet file changes' do + using RSpec::Parameterized::TableSyntax + + where(:is_multi_file, :file_name, :content, :files, :status) do + true | nil | nil | [create_action] | :success + true | nil | nil | [update_action] | :success + true | nil | nil | [move_action] | :success + true | nil | nil | [delete_action] | :success + true | nil | nil | [create_action, update_action] | :success + true | 'foo.txt' | 'bar' | [create_action] | :bad_request + true | 'foo.txt' | 'bar' | nil | :bad_request + true | nil | nil | nil | :bad_request + true | 'foo.txt' | nil | [create_action] | :bad_request + true | nil | 'bar' | [create_action] | :bad_request + true | '' | nil | [create_action] | :bad_request + true | nil | '' | [create_action] | :bad_request + true | nil | nil | [bad_file_path] | :bad_request + true | nil | nil | [bad_previous_path] | :bad_request + true | nil | nil | [invalid_move] | :unprocessable_entity + + false | 'foo.txt' | 'bar' | nil | :success + false | 'foo.txt' | nil | nil | :success + false | nil | 'bar' | nil | :success + false | 'foo.txt' | 'bar' | [create_action] | :bad_request + false | nil | nil | nil | :bad_request + false | nil | '' | nil | :bad_request + false | nil | nil | [bad_file_path] | :bad_request + false | nil | nil | [bad_previous_path] | :bad_request + end + + with_them do + before do + allow_any_instance_of(Snippet).to receive(:multiple_files?).and_return(is_multi_file) + end + + it 'has the correct response' do + update_params = {}.tap do |params| + params[:files] = files if files + params[:file_name] = file_name if file_name + params[:content] = content if content + end + + update_snippet(params: update_params) + + expect(response).to have_gitlab_http_status(status) + end + end + + context 'when save fails due to a repository commit error' do + before do + allow_next_instance_of(Repository) do |instance| + allow(instance).to receive(:multi_action).and_raise(Gitlab::Git::CommitError) + end + + update_snippet(params: { files: [create_action] }) + end + + it 'returns a bad request response' do + expect(response).to have_gitlab_http_status(:bad_request) + end + end + end +end + +RSpec.shared_examples 'snippet non-file updates' do + it 'updates a snippet non-file attributes' do + new_description = 'New description' + new_title = 'New title' + new_visibility = 'internal' + + update_snippet(params: { title: new_title, description: new_description, visibility: new_visibility }) + + snippet.reload + + aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) + expect(snippet.description).to eq(new_description) + expect(snippet.visibility).to eq(new_visibility) + expect(snippet.title).to eq(new_title) + end + end +end + +RSpec.shared_examples 'invalid snippet updates' do + it 'returns 404 for invalid snippet id' do + update_snippet(snippet_id: non_existing_record_id, params: { title: 'foo' }) + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['message']).to eq('404 Snippet Not Found') + end + + it 'returns 400 for missing parameters' do + update_snippet + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 if content is blank' do + update_snippet(params: { content: '' }) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 if title is blank' do + update_snippet(params: { title: '' }) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq 'title is empty' + end +end |