diff options
Diffstat (limited to 'spec')
37 files changed, 587 insertions, 434 deletions
diff --git a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb index d232408b775..fdef9bc5638 100644 --- a/spec/controllers/projects/environments/prometheus_api_controller_spec.rb +++ b/spec/controllers/projects/environments/prometheus_api_controller_spec.rb @@ -85,12 +85,12 @@ describe Projects::Environments::PrometheusApiController do context 'with nil result' do let(:service_result) { nil } - it 'returns 202 accepted' do + it 'returns 204 no_content' do get :proxy, params: environment_params expect(json_response['status']).to eq('processing') expect(json_response['message']).to eq('Not ready yet. Try again later.') - expect(response).to have_gitlab_http_status(:accepted) + expect(response).to have_gitlab_http_status(:no_content) end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index f8c0ab55eb4..34cbf0c8723 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -412,7 +412,7 @@ describe Projects::MergeRequestsController do end end - context 'when the pipeline succeeds is passed' do + context 'when merge when pipeline succeeds option is passed' do let!(:head_pipeline) do create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request) end @@ -462,6 +462,30 @@ describe Projects::MergeRequestsController do expect(json_response).to eq('status' => 'merge_when_pipeline_succeeds') end end + + context 'when auto merge has not been enabled yet' do + it 'calls AutoMergeService#execute' do + expect_next_instance_of(AutoMergeService) do |service| + expect(service).to receive(:execute).with(merge_request, 'merge_when_pipeline_succeeds') + end + + merge_when_pipeline_succeeds + end + end + + context 'when auto merge has already been enabled' do + before do + merge_request.update!(auto_merge_enabled: true, merge_user: user) + end + + it 'calls AutoMergeService#update' do + expect_next_instance_of(AutoMergeService) do |service| + expect(service).to receive(:update).with(merge_request) + end + + merge_when_pipeline_succeeds + end + end end describe 'only_allow_merge_if_all_discussions_are_resolved? setting' do diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 5ee9425c491..5bdd9113b06 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -210,7 +210,7 @@ describe 'Issues' do let(:issue) { @issue } it 'allows filtering by issues with no specified assignee' do - visit project_issues_path(project, assignee_id: IssuableFinder::NONE) + visit project_issues_path(project, assignee_id: IssuableFinder::FILTER_NONE) expect(page).to have_content 'foobar' expect(page).not_to have_content 'barbaz' diff --git a/spec/features/merge_requests/user_lists_merge_requests_spec.rb b/spec/features/merge_requests/user_lists_merge_requests_spec.rb index bd91fae1453..2dee0e26954 100644 --- a/spec/features/merge_requests/user_lists_merge_requests_spec.rb +++ b/spec/features/merge_requests/user_lists_merge_requests_spec.rb @@ -33,7 +33,7 @@ describe 'Merge requests > User lists merge requests' do end it 'filters on no assignee' do - visit_merge_requests(project, assignee_id: IssuableFinder::NONE) + visit_merge_requests(project, assignee_id: IssuableFinder::FILTER_NONE) expect(current_path).to eq(project_merge_requests_path(project)) expect(page).to have_content 'merge-test' diff --git a/spec/finders/issues_finder_spec.rb b/spec/finders/issues_finder_spec.rb index 89fdaceaa9f..bf38d083ca6 100644 --- a/spec/finders/issues_finder_spec.rb +++ b/spec/finders/issues_finder_spec.rb @@ -241,14 +241,6 @@ describe IssuesFinder do end end - context 'filtering by legacy No+Label' do - let(:params) { { label_name: Label::NONE } } - - it 'returns issues with no labels' do - expect(issues).to contain_exactly(issue1, issue3, issue4) - end - end - context 'filtering by any label' do let(:params) { { label_name: described_class::FILTER_ANY } } diff --git a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json index 2d0af57ec2c..33393805464 100644 --- a/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json +++ b/spec/fixtures/lib/gitlab/metrics/dashboard/schemas/metrics.json @@ -2,7 +2,8 @@ "type": "object", "required": [ "unit", - "label" + "label", + "prometheus_endpoint_path" ], "oneOf": [ { "required": ["query"] }, @@ -14,7 +15,8 @@ "query": { "type": "string" }, "unit": { "type": "string" }, "label": { "type": "string" }, - "track": { "type": "string" } + "track": { "type": "string" }, + "prometheus_endpoint_path": { "type": "string" } }, "additionalProperties": false } diff --git a/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap b/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap index 1b4564303e4..86bfde1a28c 100644 --- a/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap +++ b/spec/frontend/repository/components/table/__snapshots__/row_spec.js.snap @@ -22,6 +22,8 @@ exports[`Repository table row component renders table row 1`] = ` </a> <!----> + + <!----> </td> <td diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index a70dc7bb866..90a502966ad 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -1,4 +1,5 @@ import { shallowMount, RouterLinkStub } from '@vue/test-utils'; +import { GlBadge } from '@gitlab/ui'; import TableRow from '~/repository/components/table/row.vue'; let vm; @@ -98,4 +99,16 @@ describe('Repository table row component', () => { expect(vm.find('a').attributes('href')).toEqual('https://test.com'); }); + + it('renders LFS badge', () => { + factory({ + id: '1', + path: 'test', + type: 'commit', + currentPath: '/', + lfsOid: '1', + }); + + expect(vm.find(GlBadge).exists()).toBe(true); + }); }); diff --git a/spec/graphql/types/tree/blob_type_spec.rb b/spec/graphql/types/tree/blob_type_spec.rb index b12e214ca84..22c11aff90a 100644 --- a/spec/graphql/types/tree/blob_type_spec.rb +++ b/spec/graphql/types/tree/blob_type_spec.rb @@ -5,5 +5,5 @@ require 'spec_helper' describe Types::Tree::BlobType do it { expect(described_class.graphql_name).to eq('Blob') } - it { expect(described_class).to have_graphql_fields(:id, :name, :type, :path, :flat_path, :web_url) } + it { expect(described_class).to have_graphql_fields(:id, :name, :type, :path, :flat_path, :web_url, :lfs_oid) } end diff --git a/spec/javascripts/monitoring/dashboard_spec.js b/spec/javascripts/monitoring/dashboard_spec.js index 1a371c3adaf..f9c3122088e 100644 --- a/spec/javascripts/monitoring/dashboard_spec.js +++ b/spec/javascripts/monitoring/dashboard_spec.js @@ -314,9 +314,7 @@ describe('Dashboard', () => { }); setTimeout(() => { - const selectedTimeWindow = component.$el.querySelector( - '.js-time-window-dropdown [active="true"]', - ); + const selectedTimeWindow = component.$el.querySelector('.js-time-window-dropdown .active'); expect(selectedTimeWindow.textContent.trim()).toEqual('30 minutes'); done(); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merge_when_pipeline_succeeds_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_auto_merge_enabled_spec.js index 8e0415b813b..2ea8c169add 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_merge_when_pipeline_succeeds_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_auto_merge_enabled_spec.js @@ -1,17 +1,19 @@ import Vue from 'vue'; -import mwpsComponent from '~/vue_merge_request_widget/components/states/mr_widget_merge_when_pipeline_succeeds.vue'; +import autoMergeEnabledComponent from '~/vue_merge_request_widget/components/states/mr_widget_auto_merge_enabled.vue'; import MRWidgetService from '~/vue_merge_request_widget/services/mr_widget_service'; import eventHub from '~/vue_merge_request_widget/event_hub'; import mountComponent from 'spec/helpers/vue_mount_component_helper'; +import { trimText } from 'spec/helpers/text_helper'; +import { MWPS_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants'; -describe('MRWidgetMergeWhenPipelineSucceeds', () => { +describe('MRWidgetAutoMergeEnabled', () => { let vm; const targetBranchPath = '/foo/bar'; const targetBranch = 'foo'; const sha = '1EA2EZ34'; beforeEach(() => { - const Component = Vue.extend(mwpsComponent); + const Component = Vue.extend(autoMergeEnabledComponent); spyOn(eventHub, '$emit'); vm = mountComponent(Component, { @@ -25,6 +27,7 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { sha, targetBranchPath, targetBranch, + autoMergeStrategy: MWPS_MERGE_STRATEGY, }, service: new MRWidgetService({}), }); @@ -66,6 +69,32 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { expect(vm.canRemoveSourceBranch).toBeFalsy(); }); }); + + describe('statusTextBeforeAuthor', () => { + it('should return "Set by" if the MWPS is selected', () => { + Vue.set(vm.mr, 'autoMergeStrategy', MWPS_MERGE_STRATEGY); + + expect(vm.statusTextBeforeAuthor).toBe('Set by'); + }); + }); + + describe('statusTextAfterAuthor', () => { + it('should return "to be merged automatically..." if MWPS is selected', () => { + Vue.set(vm.mr, 'autoMergeStrategy', MWPS_MERGE_STRATEGY); + + expect(vm.statusTextAfterAuthor).toBe( + 'to be merged automatically when the pipeline succeeds', + ); + }); + }); + + describe('cancelButtonText', () => { + it('should return "Cancel automatic merge" if MWPS is selected', () => { + Vue.set(vm.mr, 'autoMergeStrategy', MWPS_MERGE_STRATEGY); + + expect(vm.cancelButtonText).toBe('Cancel automatic merge'); + }); + }); }); describe('methods', () => { @@ -96,7 +125,7 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { spyOn(vm.service, 'merge').and.returnValue( Promise.resolve({ data: { - status: 'merge_when_pipeline_succeeds', + status: MWPS_MERGE_STRATEGY, }, }), ); @@ -106,7 +135,7 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested'); expect(vm.service.merge).toHaveBeenCalledWith({ sha, - auto_merge_strategy: 'merge_when_pipeline_succeeds', + auto_merge_strategy: MWPS_MERGE_STRATEGY, should_remove_source_branch: true, }); done(); @@ -119,6 +148,7 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { it('should have correct elements', () => { expect(vm.$el.classList.contains('mr-widget-body')).toBeTruthy(); expect(vm.$el.innerText).toContain('to be merged automatically when the pipeline succeeds'); + expect(vm.$el.innerText).toContain('The changes will be merged into'); expect(vm.$el.innerText).toContain(targetBranch); expect(vm.$el.innerText).toContain('The source branch will not be deleted'); @@ -174,5 +204,27 @@ describe('MRWidgetMergeWhenPipelineSucceeds', () => { done(); }); }); + + it('should render the status text as "...to merged automatically" if MWPS is selected', done => { + Vue.set(vm.mr, 'autoMergeStrategy', MWPS_MERGE_STRATEGY); + + Vue.nextTick(() => { + const statusText = trimText(vm.$el.querySelector('.js-status-text-after-author').innerText); + + expect(statusText).toBe('to be merged automatically when the pipeline succeeds'); + done(); + }); + }); + + it('should render the cancel button as "Cancel automatic merge" if MWPS is selected', done => { + Vue.set(vm.mr, 'autoMergeStrategy', MWPS_MERGE_STRATEGY); + + Vue.nextTick(() => { + const cancelButtonText = trimText(vm.$el.querySelector('.js-cancel-auto-merge').innerText); + + expect(cancelButtonText).toBe('Cancel automatic merge'); + done(); + }); + }); }); }); diff --git a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js index 3ae773b6ccb..bb76616be56 100644 --- a/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js +++ b/spec/javascripts/vue_mr_widget/components/states/mr_widget_ready_to_merge_spec.js @@ -6,6 +6,7 @@ import CommitEdit from '~/vue_merge_request_widget/components/states/commit_edit import CommitMessageDropdown from '~/vue_merge_request_widget/components/states/commit_message_dropdown.vue'; import eventHub from '~/vue_merge_request_widget/event_hub'; import { createLocalVue, shallowMount } from '@vue/test-utils'; +import { MWPS_MERGE_STRATEGY, ATMTWPS_MERGE_STRATEGY } from '~/vue_merge_request_widget/constants'; const commitMessage = 'This is the commit message'; const squashCommitMessage = 'This is the squash commit message'; @@ -29,6 +30,8 @@ const createTestMr = customConfig => { shouldRemoveSourceBranch: true, canRemoveSourceBranch: false, targetBranch: 'master', + preferredAutoMergeStrategy: MWPS_MERGE_STRATEGY, + availableAutoMergeStrategies: [MWPS_MERGE_STRATEGY], }; Object.assign(mr, customConfig.mr); @@ -80,7 +83,6 @@ describe('ReadyToMerge', () => { it('should have default data', () => { expect(vm.mergeWhenBuildSucceeds).toBeFalsy(); expect(vm.useCommitMessageWithDescription).toBeFalsy(); - expect(vm.autoMergeStrategy).toBeUndefined(); expect(vm.showCommitMessageEditor).toBeFalsy(); expect(vm.isMakingRequest).toBeFalsy(); expect(vm.isMergingImmediately).toBeFalsy(); @@ -91,47 +93,51 @@ describe('ReadyToMerge', () => { }); describe('computed', () => { - describe('shouldShowAutoMergeText', () => { - it('should return true with active pipeline', () => { - vm.mr.isPipelineActive = true; + describe('isAutoMergeAvailable', () => { + it('should return true when at least one merge strategy is available', () => { + vm.mr.availableAutoMergeStrategies = [MWPS_MERGE_STRATEGY]; - expect(vm.shouldShowAutoMergeText).toBeTruthy(); + expect(vm.isAutoMergeAvailable).toBe(true); }); - it('should return false with inactive pipeline', () => { - vm.mr.isPipelineActive = false; + it('should return false when no merge strategies are available', () => { + vm.mr.availableAutoMergeStrategies = []; - expect(vm.shouldShowAutoMergeText).toBeFalsy(); + expect(vm.isAutoMergeAvailable).toBe(false); }); }); describe('status', () => { it('defaults to success', () => { - vm.mr.pipeline = true; + Vue.set(vm.mr, 'pipeline', true); + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); expect(vm.status).toEqual('success'); }); it('returns failed when MR has CI but also has an unknown status', () => { - vm.mr.hasCI = true; + Vue.set(vm.mr, 'hasCI', true); expect(vm.status).toEqual('failed'); }); it('returns default when MR has no pipeline', () => { + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); + expect(vm.status).toEqual('success'); }); it('returns pending when pipeline is active', () => { - vm.mr.pipeline = {}; - vm.mr.isPipelineActive = true; + Vue.set(vm.mr, 'pipeline', {}); + Vue.set(vm.mr, 'isPipelineActive', true); expect(vm.status).toEqual('pending'); }); it('returns failed when pipeline is failed', () => { - vm.mr.pipeline = {}; - vm.mr.isPipelineFailed = true; + Vue.set(vm.mr, 'pipeline', {}); + Vue.set(vm.mr, 'isPipelineFailed', true); + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); expect(vm.status).toEqual('failed'); }); @@ -143,18 +149,20 @@ describe('ReadyToMerge', () => { const inActionClass = `${defaultClass} btn-info`; it('defaults to success class', () => { + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); + expect(vm.mergeButtonClass).toEqual(defaultClass); }); it('returns success class for success status', () => { - vm.mr.pipeline = true; + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); + Vue.set(vm.mr, 'pipeline', true); expect(vm.mergeButtonClass).toEqual(defaultClass); }); it('returns info class for pending status', () => { - vm.mr.pipeline = {}; - vm.mr.isPipelineActive = true; + Vue.set(vm.mr, 'availableAutoMergeStrategies', [ATMTWPS_MERGE_STRATEGY]); expect(vm.mergeButtonClass).toEqual(inActionClass); }); @@ -198,69 +206,82 @@ describe('ReadyToMerge', () => { }); describe('mergeButtonText', () => { - it('should return Merge', () => { + it('should return "Merge" when no auto merge strategies are available', () => { + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); + expect(vm.mergeButtonText).toEqual('Merge'); }); - it('should return Merge in progress', () => { - vm.isMergingImmediately = true; + it('should return "Merge in progress"', () => { + Vue.set(vm, 'isMergingImmediately', true); expect(vm.mergeButtonText).toEqual('Merge in progress'); }); - it('should return Merge when pipeline succeeds', () => { - vm.isMergingImmediately = false; - vm.mr.isPipelineActive = true; + it('should return "Merge when pipeline succeeds" when the MWPS auto merge strategy is available', () => { + Vue.set(vm, 'isMergingImmediately', false); + Vue.set(vm.mr, 'preferredAutoMergeStrategy', MWPS_MERGE_STRATEGY); expect(vm.mergeButtonText).toEqual('Merge when pipeline succeeds'); }); }); + describe('autoMergeText', () => { + it('should return Merge when pipeline succeeds', () => { + Vue.set(vm.mr, 'preferredAutoMergeStrategy', MWPS_MERGE_STRATEGY); + + expect(vm.autoMergeText).toEqual('Merge when pipeline succeeds'); + }); + }); + describe('shouldShowMergeOptionsDropdown', () => { - it('should return false with initial data', () => { - expect(vm.shouldShowMergeOptionsDropdown).toBeFalsy(); + it('should return false when no auto merge strategies are available', () => { + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); + + expect(vm.shouldShowMergeOptionsDropdown).toBe(false); }); - it('should return true when pipeline active', () => { - vm.mr.isPipelineActive = true; + it('should return true when at least one auto merge strategy is available', () => { + Vue.set(vm.mr, 'availableAutoMergeStrategies', [ATMTWPS_MERGE_STRATEGY]); - expect(vm.shouldShowMergeOptionsDropdown).toBeTruthy(); + expect(vm.shouldShowMergeOptionsDropdown).toBe(true); }); it('should return false when pipeline active but only merge when pipeline succeeds set in project options', () => { - vm.mr.isPipelineActive = true; - vm.mr.onlyAllowMergeIfPipelineSucceeds = true; + Vue.set(vm.mr, 'availableAutoMergeStrategies', [ATMTWPS_MERGE_STRATEGY]); + Vue.set(vm.mr, 'onlyAllowMergeIfPipelineSucceeds', true); - expect(vm.shouldShowMergeOptionsDropdown).toBeFalsy(); + expect(vm.shouldShowMergeOptionsDropdown).toBe(false); }); }); describe('isMergeButtonDisabled', () => { it('should return false with initial data', () => { - vm.mr.isMergeAllowed = true; + Vue.set(vm.mr, 'isMergeAllowed', true); - expect(vm.isMergeButtonDisabled).toBeFalsy(); + expect(vm.isMergeButtonDisabled).toBe(false); }); it('should return true when there is no commit message', () => { - vm.mr.isMergeAllowed = true; - vm.commitMessage = ''; + Vue.set(vm.mr, 'isMergeAllowed', true); + Vue.set(vm, 'commitMessage', ''); - expect(vm.isMergeButtonDisabled).toBeTruthy(); + expect(vm.isMergeButtonDisabled).toBe(true); }); it('should return true if merge is not allowed', () => { - vm.mr.isMergeAllowed = false; - vm.mr.onlyAllowMergeIfPipelineSucceeds = true; + Vue.set(vm.mr, 'isMergeAllowed', false); + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); + Vue.set(vm.mr, 'onlyAllowMergeIfPipelineSucceeds', true); - expect(vm.isMergeButtonDisabled).toBeTruthy(); + expect(vm.isMergeButtonDisabled).toBe(true); }); it('should return true when the vm instance is making request', () => { - vm.mr.isMergeAllowed = true; - vm.isMakingRequest = true; + Vue.set(vm.mr, 'isMergeAllowed', true); + Vue.set(vm, 'isMakingRequest', true); - expect(vm.isMergeButtonDisabled).toBeTruthy(); + expect(vm.isMergeButtonDisabled).toBe(true); }); }); }); @@ -268,31 +289,31 @@ describe('ReadyToMerge', () => { describe('methods', () => { describe('shouldShowMergeControls', () => { it('should return false when an external pipeline is running and required to succeed', () => { - vm.mr.isMergeAllowed = false; - vm.mr.isPipelineActive = false; + Vue.set(vm.mr, 'isMergeAllowed', false); + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); - expect(vm.shouldShowMergeControls).toBeFalsy(); + expect(vm.shouldShowMergeControls).toBe(false); }); it('should return true when the build succeeded or build not required to succeed', () => { - vm.mr.isMergeAllowed = true; - vm.mr.isPipelineActive = false; + Vue.set(vm.mr, 'isMergeAllowed', true); + Vue.set(vm.mr, 'availableAutoMergeStrategies', []); - expect(vm.shouldShowMergeControls).toBeTruthy(); + expect(vm.shouldShowMergeControls).toBe(true); }); it('should return true when showing the MWPS button and a pipeline is running that needs to be successful', () => { - vm.mr.isMergeAllowed = false; - vm.mr.isPipelineActive = true; + Vue.set(vm.mr, 'isMergeAllowed', false); + Vue.set(vm.mr, 'availableAutoMergeStrategies', [MWPS_MERGE_STRATEGY]); - expect(vm.shouldShowMergeControls).toBeTruthy(); + expect(vm.shouldShowMergeControls).toBe(true); }); it('should return true when showing the MWPS button but not required for the pipeline to succeed', () => { - vm.mr.isMergeAllowed = true; - vm.mr.isPipelineActive = true; + Vue.set(vm.mr, 'isMergeAllowed', true); + Vue.set(vm.mr, 'availableAutoMergeStrategies', [MWPS_MERGE_STRATEGY]); - expect(vm.shouldShowMergeControls).toBeTruthy(); + expect(vm.shouldShowMergeControls).toBe(true); }); }); @@ -325,7 +346,6 @@ describe('ReadyToMerge', () => { vm.handleMergeButtonClick(true); setTimeout(() => { - expect(vm.autoMergeStrategy).toBe('merge_when_pipeline_succeeds'); expect(vm.isMakingRequest).toBeTruthy(); expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested'); @@ -349,14 +369,13 @@ describe('ReadyToMerge', () => { vm.handleMergeButtonClick(false, true); setTimeout(() => { - expect(vm.autoMergeStrategy).toBeUndefined(); expect(vm.isMakingRequest).toBeTruthy(); expect(eventHub.$emit).toHaveBeenCalledWith('FailedToMerge', undefined); const params = vm.service.merge.calls.argsFor(0)[0]; expect(params.should_remove_source_branch).toBeTruthy(); - expect(params.merge_when_pipeline_succeeds).toBeFalsy(); + expect(params.auto_merge_strategy).toBeUndefined(); done(); }, 333); }); @@ -367,14 +386,13 @@ describe('ReadyToMerge', () => { vm.handleMergeButtonClick(); setTimeout(() => { - expect(vm.autoMergeStrategy).toBeUndefined(); expect(vm.isMakingRequest).toBeTruthy(); expect(vm.initiateMergePolling).toHaveBeenCalled(); const params = vm.service.merge.calls.argsFor(0)[0]; expect(params.should_remove_source_branch).toBeTruthy(); - expect(params.merge_when_pipeline_succeeds).toBeFalsy(); + expect(params.auto_merge_strategy).toBeUndefined(); done(); }, 333); }); diff --git a/spec/javascripts/vue_mr_widget/mock_data.js b/spec/javascripts/vue_mr_widget/mock_data.js index edbd0d54151..3c9a5cece90 100644 --- a/spec/javascripts/vue_mr_widget/mock_data.js +++ b/spec/javascripts/vue_mr_widget/mock_data.js @@ -25,7 +25,6 @@ export default { }, merge_status: 'can_be_merged', merge_user_id: null, - merge_when_pipeline_succeeds: false, source_branch: 'daaaa', source_branch_link: 'daaaa', source_project_id: 19, @@ -210,8 +209,7 @@ export default { source_branch_path: '/root/acets-app/branches/daaaa', conflict_resolution_ui_path: '/root/acets-app/merge_requests/22/conflicts', remove_wip_path: '/root/acets-app/merge_requests/22/remove_wip', - cancel_merge_when_pipeline_succeeds_path: - '/root/acets-app/merge_requests/22/cancel_merge_when_pipeline_succeeds', + cancel_auto_merge_path: '/root/acets-app/merge_requests/22/cancel_auto_merge', create_issue_to_resolve_discussions_path: '/root/acets-app/issues/new?merge_request_to_resolve_discussions_of=22', merge_path: '/root/acets-app/merge_requests/22/merge', @@ -237,6 +235,9 @@ export default { merge_request_pipelines_docs_path: '/help/ci/merge_request_pipelines/index.md', squash: true, visual_review_app_available: true, + merge_trains_enabled: true, + merge_trains_count: 3, + merge_train_index: 1, }; export const mockStore = { diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 918717c4547..08f7a17515e 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -222,60 +222,6 @@ describe('mrWidgetOptions', () => { }); }); }); - - describe('showTargetBranchAdvancedError', () => { - describe(`when the pipeline's target_sha property doesn't exist`, () => { - beforeEach(done => { - Vue.set(vm.mr, 'isOpen', true); - Vue.set(vm.mr.pipeline, 'target_sha', undefined); - Vue.set(vm.mr, 'targetBranchSha', 'abcd'); - vm.$nextTick(done); - }); - - it('should be false', () => { - expect(vm.showTargetBranchAdvancedError).toEqual(false); - }); - }); - - describe(`when the pipeline's target_sha matches the target branch's sha`, () => { - beforeEach(done => { - Vue.set(vm.mr, 'isOpen', true); - Vue.set(vm.mr.pipeline, 'target_sha', 'abcd'); - Vue.set(vm.mr, 'targetBranchSha', 'abcd'); - vm.$nextTick(done); - }); - - it('should be false', () => { - expect(vm.showTargetBranchAdvancedError).toEqual(false); - }); - }); - - describe(`when the merge request is not open`, () => { - beforeEach(done => { - Vue.set(vm.mr, 'isOpen', false); - Vue.set(vm.mr.pipeline, 'target_sha', 'abcd'); - Vue.set(vm.mr, 'targetBranchSha', 'bcde'); - vm.$nextTick(done); - }); - - it('should be false', () => { - expect(vm.showTargetBranchAdvancedError).toEqual(false); - }); - }); - - describe(`when the pipeline's target_sha does not match the target branch's sha`, () => { - beforeEach(done => { - Vue.set(vm.mr, 'isOpen', true); - Vue.set(vm.mr.pipeline, 'target_sha', 'abcd'); - Vue.set(vm.mr, 'targetBranchSha', 'bcde'); - vm.$nextTick(done); - }); - - it('should be true', () => { - expect(vm.showTargetBranchAdvancedError).toEqual(true); - }); - }); - }); }); describe('methods', () => { diff --git a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js index e2cd0f084fd..e27a506f426 100644 --- a/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js +++ b/spec/javascripts/vue_mr_widget/stores/mr_widget_store_spec.js @@ -82,5 +82,47 @@ describe('MergeRequestStore', () => { expect(store.isNothingToMergeState).toEqual(false); }); }); + + describe('mergePipelinesEnabled', () => { + it('should set mergePipelinesEnabled = true when merge_pipelines_enabled is true', () => { + store.setData({ ...mockData, merge_pipelines_enabled: true }); + + expect(store.mergePipelinesEnabled).toBe(true); + }); + + it('should set mergePipelinesEnabled = false when merge_pipelines_enabled is not provided', () => { + store.setData({ ...mockData, merge_pipelines_enabled: undefined }); + + expect(store.mergePipelinesEnabled).toBe(false); + }); + }); + + describe('mergeTrainsCount', () => { + it('should set mergeTrainsCount when merge_trains_count is provided', () => { + store.setData({ ...mockData, merge_trains_count: 3 }); + + expect(store.mergeTrainsCount).toBe(3); + }); + + it('should set mergeTrainsCount = 0 when merge_trains_count is not provided', () => { + store.setData({ ...mockData, merge_trains_count: undefined }); + + expect(store.mergeTrainsCount).toBe(0); + }); + }); + + describe('mergeTrainIndex', () => { + it('should set mergeTrainIndex when merge_train_index is provided', () => { + store.setData({ ...mockData, merge_train_index: 3 }); + + expect(store.mergeTrainIndex).toBe(3); + }); + + it('should not set mergeTrainIndex when merge_train_index is not provided', () => { + store.setData({ ...mockData, merge_train_index: undefined }); + + expect(store.mergeTrainIndex).toBeUndefined(); + }); + }); }); }); diff --git a/spec/javascripts/vue_shared/components/pagination/graphql_pagination_spec.js b/spec/javascripts/vue_shared/components/pagination/graphql_pagination_spec.js new file mode 100644 index 00000000000..7445da6cdee --- /dev/null +++ b/spec/javascripts/vue_shared/components/pagination/graphql_pagination_spec.js @@ -0,0 +1,70 @@ +import { shallowMount } from '@vue/test-utils'; +import GraphqlPagination from '~/vue_shared/components/pagination/graphql_pagination.vue'; + +describe('Graphql Pagination component', () => { + let wrapper; + function factory({ hasNextPage = true, hasPreviousPage = true }) { + wrapper = shallowMount(GraphqlPagination, { + propsData: { + hasNextPage, + hasPreviousPage, + }, + }); + } + + afterEach(() => { + wrapper.destroy(); + }); + + describe('without previous page', () => { + beforeEach(() => { + factory({ hasPreviousPage: false }); + }); + + it('renders disabled previous button', () => { + expect(wrapper.find('.js-prev-btn').attributes().disabled).toEqual('true'); + }); + }); + + describe('with previous page', () => { + beforeEach(() => { + factory({ hasPreviousPage: true }); + }); + + it('renders enabled previous button', () => { + expect(wrapper.find('.js-prev-btn').attributes().disabled).toEqual(undefined); + }); + + it('emits previousClicked on click', () => { + wrapper.find('.js-prev-btn').vm.$emit('click'); + + expect(wrapper.emitted().previousClicked.length).toBe(1); + }); + }); + + describe('without next page', () => { + beforeEach(() => { + factory({ hasNextPage: false }); + }); + + it('renders disabled next button', () => { + expect(wrapper.find('.js-next-btn').attributes().disabled).toEqual('true'); + }); + }); + + describe('with next page', () => { + beforeEach(() => { + factory({ hasNextPage: true }); + }); + + it('renders enabled next button', () => { + expect(wrapper.find('.js-next-btn').attributes().disabled).toEqual(undefined); + }); + + it('emits nextClicked on click', () => { + wrapper.find('.js-next-btn').vm.$emit('click'); + + expect(wrapper.emitted().nextClicked.length).toBe(1); + }); + }); +}); diff --git a/spec/lib/gitlab/graphql/loaders/batch_lfs_oid_loader_spec.rb b/spec/lib/gitlab/graphql/loaders/batch_lfs_oid_loader_spec.rb new file mode 100644 index 00000000000..46dd1777285 --- /dev/null +++ b/spec/lib/gitlab/graphql/loaders/batch_lfs_oid_loader_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe Gitlab::Graphql::Loaders::BatchLfsOidLoader do + include GraphqlHelpers + + set(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:blob) { Gitlab::Graphql::Representation::TreeEntry.new(repository.blob_at('master', 'files/lfs/lfs_object.iso'), repository) } + let(:otherblob) { Gitlab::Graphql::Representation::TreeEntry.new(repository.blob_at('master', 'README'), repository) } + + describe '#find' do + it 'batch-resolves LFS blob IDs' do + expect(Gitlab::Git::Blob).to receive(:batch_lfs_pointers).once.and_call_original + + result = batch do + [blob, otherblob].map { |b| described_class.new(repository, b.id).find } + end + + expect(result.first).to eq(blob.lfs_oid) + expect(result.last).to eq(nil) + end + end +end diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index fb7bddb386c..6512fe80a3b 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -6223,7 +6223,10 @@ "erased_by_id": null, "erased_at": null, "type": "Ci::Build", - "token": "abcd" + "token": "abcd", + "artifacts_file_store": 1, + "artifacts_metadata_store": 1, + "artifacts_size": 10 }, { "id": 72, diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index e88eb140b35..bdcb5914575 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi include MetricsDashboardHelpers set(:project) { build(:project) } - set(:environment) { build(:environment, project: project) } + set(:environment) { create(:environment, project: project) } let(:system_dashboard_path) { Gitlab::Metrics::Dashboard::SystemDashboardService::SYSTEM_DASHBOARD_PATH} describe '.find' do @@ -27,6 +27,13 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi it_behaves_like 'misconfigured dashboard service response', :unprocessable_entity end + context 'when the dashboard contains a metric without a query' do + let(:dashboard) { { 'panel_groups' => [{ 'panels' => [{ 'metrics' => [{ 'id' => 'mock' }] }] }] } } + let(:project) { project_with_dashboard(dashboard_path, dashboard.to_yaml) } + + it_behaves_like 'misconfigured dashboard service response', :unprocessable_entity + end + context 'when the system dashboard is specified' do let(:dashboard_path) { system_dashboard_path } diff --git a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb index be3c1095bd7..797d4daabe3 100644 --- a/spec/lib/gitlab/metrics/dashboard/processor_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/processor_spec.rb @@ -4,13 +4,19 @@ require 'spec_helper' describe Gitlab::Metrics::Dashboard::Processor do let(:project) { build(:project) } - let(:environment) { build(:environment, project: project) } + let(:environment) { create(:environment, project: project) } let(:dashboard_yml) { YAML.load_file('spec/fixtures/lib/gitlab/metrics/dashboard/sample_dashboard.yml') } describe 'process' do let(:process_params) { [project, environment, dashboard_yml] } let(:dashboard) { described_class.new(*process_params).process(insert_project_metrics: true) } + it 'includes a path for the prometheus endpoint with each metric' do + expect(all_metrics).to satisfy_all do |metric| + metric[:prometheus_endpoint_path] == prometheus_path(metric[:query_range]) + end + end + context 'when dashboard config corresponds to common metrics' do let!(:common_metric) { create(:prometheus_metric, :common, identifier: 'metric_a1') } @@ -61,7 +67,7 @@ describe Gitlab::Metrics::Dashboard::Processor do shared_examples_for 'errors with message' do |expected_message| it 'raises a DashboardLayoutError' do - error_class = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardLayoutError + error_class = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardProcessingError expect { dashboard }.to raise_error(error_class, expected_message) end @@ -84,6 +90,12 @@ describe Gitlab::Metrics::Dashboard::Processor do it_behaves_like 'errors with message', 'Each "panel" must define an array :metrics' end + + context 'when the dashboard contains a metric which is missing a query' do + let(:dashboard_yml) { { panel_groups: [{ panels: [{ metrics: [{}] }] }] } } + + it_behaves_like 'errors with message', 'Each "metric" must define one of :query or :query_range' + end end private @@ -99,7 +111,17 @@ describe Gitlab::Metrics::Dashboard::Processor do query_range: metric.query, unit: metric.unit, label: metric.legend, - metric_id: metric.id + metric_id: metric.id, + prometheus_endpoint_path: prometheus_path(metric.query) } end + + def prometheus_path(query) + Gitlab::Routing.url_helpers.prometheus_api_project_environment_path( + project, + environment, + proxy_path: :query_range, + query: query + ) + end end diff --git a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb index 162beb0268a..794ac5f109b 100644 --- a/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/project_dashboard_service_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::Metrics::Dashboard::ProjectDashboardService, :use_clean_rails_m set(:user) { build(:user) } set(:project) { build(:project) } - set(:environment) { build(:environment, project: project) } + set(:environment) { create(:environment, project: project) } before do project.add_maintainer(user) diff --git a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb index e71ce2481a3..2648fe990de 100644 --- a/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/system_dashboard_service_spec.rb @@ -6,7 +6,7 @@ describe Gitlab::Metrics::Dashboard::SystemDashboardService, :use_clean_rails_me include MetricsDashboardHelpers set(:project) { build(:project) } - set(:environment) { build(:environment, project: project) } + set(:environment) { create(:environment, project: project) } describe 'get_dashboard' do let(:dashboard_path) { described_class::SYSTEM_DASHBOARD_PATH } diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 3ab013ddc0e..4d53e4aad8a 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -88,13 +88,6 @@ describe BroadcastMessage do expect(Rails.cache).not_to receive(:delete).with(described_class::CACHE_KEY) expect(described_class.current.length).to eq(0) end - - it 'clears the legacy cache key' do - create(:broadcast_message, :future) - - expect(Rails.cache).to receive(:delete).with(described_class::LEGACY_CACHE_KEY) - expect(described_class.current.length).to eq(0) - end end describe '#attributes' do @@ -164,7 +157,6 @@ describe BroadcastMessage do message = create(:broadcast_message) expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY) - expect(Rails.cache).to receive(:delete).with(described_class::LEGACY_CACHE_KEY) message.flush_redis_cache end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 53df9e0bc05..7faa196623f 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -232,4 +232,17 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do end end end + + describe 'default options' do + let(:cached_class) { Class.new { include ReactiveCaching } } + + subject { cached_class.new } + + it { expect(subject.reactive_cache_lease_timeout).to be_a(ActiveSupport::Duration) } + it { expect(subject.reactive_cache_refresh_interval).to be_a(ActiveSupport::Duration) } + it { expect(subject.reactive_cache_lifetime).to be_a(ActiveSupport::Duration) } + + it { expect(subject.reactive_cache_key).to respond_to(:call) } + it { expect(subject.reactive_cache_worker_finder).to respond_to(:call) } + end end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index fc28c216b21..c6251326c22 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1996,6 +1996,57 @@ describe MergeRequest do end end + describe '#check_if_can_be_merged' do + let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } + + shared_examples 'checking if can be merged' do + context 'when it is not broken and has no conflicts' do + before do + allow(subject).to receive(:broken?) { false } + allow(project.repository).to receive(:can_be_merged?).and_return(true) + end + + it 'is marked as mergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') + end + end + + context 'when broken' do + before do + allow(subject).to receive(:broken?) { true } + allow(project.repository).to receive(:can_be_merged?).and_return(false) + end + + it 'becomes unmergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') + end + end + + context 'when it has conflicts' do + before do + allow(subject).to receive(:broken?) { false } + allow(project.repository).to receive(:can_be_merged?).and_return(false) + end + + it 'becomes unmergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') + end + end + end + + context 'when merge_status is unchecked' do + subject { create(:merge_request, source_project: project, merge_status: :unchecked) } + + it_behaves_like 'checking if can be merged' + end + + context 'when merge_status is unchecked' do + subject { create(:merge_request, source_project: project, merge_status: :cannot_be_merged_recheck) } + + it_behaves_like 'checking if can be merged' + end + end + describe '#mergeable?' do let(:project) { create(:project) } @@ -2009,7 +2060,7 @@ describe MergeRequest do it 'return true if #mergeable_state? is true and the MR #can_be_merged? is true' do allow(subject).to receive(:mergeable_state?) { true } - expect(subject).to receive(:check_mergeability) + expect(subject).to receive(:check_if_can_be_merged) expect(subject).to receive(:can_be_merged?) { true } expect(subject.mergeable?).to be_truthy @@ -2023,7 +2074,7 @@ describe MergeRequest do it 'checks if merge request can be merged' do allow(subject).to receive(:mergeable_ci_state?) { true } - expect(subject).to receive(:check_mergeability) + expect(subject).to receive(:check_if_can_be_merged) subject.mergeable? end @@ -3091,6 +3142,38 @@ describe MergeRequest do end end + describe '#mergeable_to_ref?' do + it 'returns true when merge request is mergeable' do + subject = create(:merge_request) + + expect(subject.mergeable_to_ref?).to be(true) + end + + it 'returns false when merge request is already merged' do + subject = create(:merge_request, :merged) + + expect(subject.mergeable_to_ref?).to be(false) + end + + it 'returns false when merge request is closed' do + subject = create(:merge_request, :closed) + + expect(subject.mergeable_to_ref?).to be(false) + end + + it 'returns false when merge request is work in progress' do + subject = create(:merge_request, title: 'WIP: The feature') + + expect(subject.mergeable_to_ref?).to be(false) + end + + it 'returns false when merge request has no commits' do + subject = create(:merge_request, source_branch: 'empty-branch', target_branch: 'master') + + expect(subject.mergeable_to_ref?).to be(false) + end + end + describe '#merge_participants' do it 'contains author' do expect(subject.merge_participants).to eq([subject.author]) diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 8075fcade5f..ed0e82ef179 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -66,7 +66,7 @@ describe ProjectPolicy do %i[ change_namespace change_visibility_level rename_project remove_project archive_project remove_fork_project destroy_merge_request destroy_issue - set_issue_iid set_issue_created_at set_note_created_at + set_issue_iid set_issue_created_at set_issue_updated_at set_note_created_at ] end diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index 9b9cc778fb3..f32ffd1c77b 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -276,14 +276,6 @@ describe API::Issues do it 'returns issues with no assignee' do issue2 = create(:issue, author: user2, project: project) - get api('/issues', user), params: { assignee_id: 0, scope: 'all' } - - expect_paginated_array_response(issue2.id) - end - - it 'returns issues with no assignee' do - issue2 = create(:issue, author: user2, project: project) - get api('/issues', user), params: { assignee_id: 'None', scope: 'all' } expect_paginated_array_response(issue2.id) @@ -496,18 +488,6 @@ describe API::Issues do expect_paginated_array_response(closed_issue.id) end - - it 'returns an array of issues with no label when using the legacy No+Label filter' do - get api('/issues', user), params: { labels: 'No Label' } - - expect_paginated_array_response(closed_issue.id) - end - - it 'returns an array of issues with no label when using the legacy No+Label filter with labels param as array' do - get api('/issues', user), params: { labels: ['No Label'] } - - expect_paginated_array_response(closed_issue.id) - end end it 'returns an empty array if no issue matches milestone' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 4cb4fcc890d..9f9180bc8c9 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -1546,65 +1546,52 @@ describe API::MergeRequests do end end - describe "GET /projects/:id/merge_requests/:merge_request_iid/merge_ref" do - before do - merge_request.mark_as_unchecked! - end - - let(:merge_request_iid) { merge_request.iid } - + describe "PUT /projects/:id/merge_requests/:merge_request_iid/merge_to_ref" do + let(:pipeline) { create(:ci_pipeline_without_jobs) } let(:url) do - "/projects/#{project.id}/merge_requests/#{merge_request_iid}/merge_ref" + "/projects/#{project.id}/merge_requests/#{merge_request.iid}/merge_to_ref" end it 'returns the generated ID from the merge service in case of success' do - get api(url, user) + put api(url, user), params: { merge_commit_message: 'Custom message' } + + commit = project.commit(json_response['commit_id']) expect(response).to have_gitlab_http_status(200) - expect(json_response['commit_id']).to eq(merge_request.merge_ref_head.sha) + expect(json_response['commit_id']).to be_present + expect(commit.message).to eq('Custom message') end it "returns 400 if branch can't be merged" do - merge_request.update!(merge_status: 'cannot_be_merged') + merge_request.update!(state: 'merged') - get api(url, user) + put api(url, user) expect(response).to have_gitlab_http_status(400) - expect(json_response['message']).to eq('Merge request is not mergeable') + expect(json_response['message']) + .to eq("Merge request is not mergeable to #{merge_request.merge_ref_path}") end - context 'when user has no access to the MR' do - let(:project) { create(:project, :private) } - let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } - - it 'returns 404' do - project.add_guest(user) + it 'returns 403 if user has no permissions to merge to the ref' do + user2 = create(:user) + project.add_reporter(user2) - get api(url, user) + put api(url, user2) - expect(response).to have_gitlab_http_status(404) - expect(json_response['message']).to eq('404 Not found') - end + expect(response).to have_gitlab_http_status(403) + expect(json_response['message']).to eq('403 Forbidden') end - context 'when invalid merge request IID' do - let(:merge_request_iid) { '12345' } - - it 'returns 404' do - get api(url, user) + it 'returns 404 for an invalid merge request IID' do + put api("/projects/#{project.id}/merge_requests/12345/merge_to_ref", user) - expect(response).to have_gitlab_http_status(404) - end + expect(response).to have_gitlab_http_status(404) end - context 'when merge request ID is used instead IID' do - let(:merge_request_iid) { merge_request.id } - - it 'returns 404' do - get api(url, user) + it "returns 404 if the merge request id is used instead of iid" do + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) - expect(response).to have_gitlab_http_status(404) - end + expect(response).to have_gitlab_http_status(404) end end diff --git a/spec/services/auto_merge/base_service_spec.rb b/spec/services/auto_merge/base_service_spec.rb index 197fa16961d..cd08e0b6f32 100644 --- a/spec/services/auto_merge/base_service_spec.rb +++ b/spec/services/auto_merge/base_service_spec.rb @@ -12,6 +12,10 @@ describe AutoMerge::BaseService do describe '#execute' do subject { service.execute(merge_request) } + before do + allow(AutoMergeProcessWorker).to receive(:perform_async) {} + end + it 'sets properies to the merge request' do subject @@ -65,6 +69,12 @@ describe AutoMerge::BaseService do it 'returns activated strategy name' do is_expected.to eq(AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS.to_sym) end + + it 'calls AutoMergeProcessWorker' do + expect(AutoMergeProcessWorker).to receive(:perform_async).with(merge_request.id).once + + subject + end end context 'when failed to save' do @@ -82,6 +92,30 @@ describe AutoMerge::BaseService do end end + describe '#update' do + subject { service.update(merge_request) } + + let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + + context 'when merge params are specified' do + let(:params) do + { + 'commit_message' => "Merge branch 'patch-12' into 'master'", + 'sha' => "200fcc9c260f7219eaf0daba87d818f0922c5b18", + 'should_remove_source_branch' => false, + 'squash' => false, + 'squash_commit_message' => "Update README.md" + } + end + + it 'updates merge params' do + expect { subject }.to change { + merge_request.reload.merge_params.slice(*params.keys) + }.from({}).to(params) + end + end + end + describe '#cancel' do subject { service.cancel(merge_request) } diff --git a/spec/services/auto_merge_service_spec.rb b/spec/services/auto_merge_service_spec.rb index d0eefed3150..7e9c412adb3 100644 --- a/spec/services/auto_merge_service_spec.rb +++ b/spec/services/auto_merge_service_spec.rb @@ -92,6 +92,30 @@ describe AutoMergeService do end end + describe '#update' do + subject { service.update(merge_request) } + + context 'when auto merge is enabled' do + let(:merge_request) { create(:merge_request, :merge_when_pipeline_succeeds) } + + it 'delegates to a relevant service instance' do + expect_next_instance_of(AutoMerge::MergeWhenPipelineSucceedsService) do |service| + expect(service).to receive(:update).with(merge_request) + end + + subject + end + end + + context 'when auto merge is not enabled' do + let(:merge_request) { create(:merge_request) } + + it 'returns failed' do + is_expected.to eq(:failed) + end + end + end + describe '#process' do subject { service.process(merge_request) } diff --git a/spec/services/ci/pipeline_schedule_service_spec.rb b/spec/services/ci/pipeline_schedule_service_spec.rb index f2ac53cb25a..867ed0acc0d 100644 --- a/spec/services/ci/pipeline_schedule_service_spec.rb +++ b/spec/services/ci/pipeline_schedule_service_spec.rb @@ -24,5 +24,13 @@ describe Ci::PipelineScheduleService do subject end + + context 'when owner is nil' do + let(:schedule) { create(:ci_pipeline_schedule, project: project, owner: nil) } + + it 'does not raise an error' do + expect { subject }.not_to raise_error + end + end end end diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index 5d492e4b013..0ac23050caf 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -32,8 +32,10 @@ describe MergeRequests::MergeToRefService do expect(result[:status]).to eq(:success) expect(result[:commit_id]).to be_present + expect(result[:source_id]).to eq(merge_request.source_branch_sha) + expect(result[:target_id]).to eq(merge_request.target_branch_sha) expect(repository.ref_exists?(target_ref)).to be(true) - expect(ref_head.sha).to eq(result[:commit_id]) + expect(ref_head.id).to eq(result[:commit_id]) end end @@ -70,6 +72,10 @@ describe MergeRequests::MergeToRefService do let(:merge_request) { create(:merge_request, :simple) } let(:project) { merge_request.project } + before do + project.add_maintainer(user) + end + describe '#execute' do let(:service) do described_class.new(project, user, commit_message: 'Awesome message', @@ -86,12 +92,6 @@ describe MergeRequests::MergeToRefService do it_behaves_like 'successfully evaluates pre-condition checks' context 'commit history comparison with regular MergeService' do - before do - # The merge service needs an authorized user while merge-to-ref - # doesn't. - project.add_maintainer(user) - end - let(:merge_ref_service) do described_class.new(project, user, {}) end @@ -136,9 +136,9 @@ describe MergeRequests::MergeToRefService do let(:merge_method) { :merge } it 'returns error' do - allow(project).to receive_message_chain(:repository, :merge_to_ref) { nil } + allow(merge_request).to receive(:mergeable_to_ref?) { false } - error_message = 'Conflicts detected during merge' + error_message = "Merge request is not mergeable to #{merge_request.merge_ref_path}" result = service.execute(merge_request) @@ -170,5 +170,28 @@ describe MergeRequests::MergeToRefService do it { expect(todo).not_to be_done } end + + context 'when merge request is WIP state' do + it 'fails to merge' do + merge_request = create(:merge_request, title: 'WIP: The feature') + + result = service.execute(merge_request) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq("Merge request is not mergeable to #{merge_request.merge_ref_path}") + end + end + + it 'returns error when user has no authorization to admin the merge request' do + unauthorized_user = create(:user) + project.add_reporter(unauthorized_user) + + service = described_class.new(project, unauthorized_user) + + result = service.execute(merge_request) + + expect(result[:status]).to eq(:error) + expect(result[:message]).to eq('You are not allowed to merge to this ref') + end end end diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb deleted file mode 100644 index aa0485467ed..00000000000 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ /dev/null @@ -1,187 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe MergeRequests::MergeabilityCheckService do - shared_examples_for 'unmergeable merge request' do - it 'updates or keeps merge status as cannot_be_merged' do - subject - - expect(merge_request.merge_status).to eq('cannot_be_merged') - end - - it 'does not change the merge ref HEAD' do - expect { subject }.not_to change(merge_request, :merge_ref_head) - end - - it 'returns ServiceResponse.error' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result).to be_error - end - end - - shared_examples_for 'mergeable merge request' do - it 'updates or keeps merge status as can_be_merged' do - subject - - expect(merge_request.merge_status).to eq('can_be_merged') - end - - it 'updates the merge ref' do - expect { subject }.to change(merge_request, :merge_ref_head).from(nil) - end - - it 'returns ServiceResponse.success' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result).to be_success - end - - it 'ServiceResponse has merge_ref_head payload' do - result = subject - - expect(result.payload.keys).to contain_exactly(:merge_ref_head) - expect(result.payload[:merge_ref_head].keys) - .to contain_exactly(:commit_id, :target_id, :source_id) - end - end - - describe '#execute' do - let(:project) { create(:project, :repository) } - let(:merge_request) { create(:merge_request, merge_status: :unchecked, source_project: project, target_project: project) } - let(:repo) { project.repository } - - subject { described_class.new(merge_request).execute } - - before do - project.add_developer(merge_request.author) - end - - it_behaves_like 'mergeable merge request' - - context 'when multiple calls to the service' do - it 'returns success' do - subject - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.success?).to be(true) - end - - it 'second call does not change the merge-ref' do - expect { subject }.to change(merge_request, :merge_ref_head).from(nil) - expect { subject }.not_to change(merge_request, :merge_ref_head) - end - end - - context 'when broken' do - before do - allow(merge_request).to receive(:broken?) { true } - allow(project.repository).to receive(:can_be_merged?) { false } - end - - it_behaves_like 'unmergeable merge request' - - it 'returns ServiceResponse.error' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.error?).to be(true) - expect(result.message).to eq('Merge request is not mergeable') - end - end - - context 'when it has conflicts' do - before do - allow(merge_request).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?) { false } - end - - it_behaves_like 'unmergeable merge request' - - it 'returns ServiceResponse.error' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.error?).to be(true) - expect(result.message).to eq('Merge request is not mergeable') - end - end - - context 'when MR cannot be merged and has no merge ref' do - before do - merge_request.mark_as_unmergeable! - end - - it_behaves_like 'unmergeable merge request' - - it 'returns ServiceResponse.error' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.error?).to be(true) - expect(result.message).to eq('Merge request is not mergeable') - end - end - - context 'when MR cannot be merged and has outdated merge ref' do - before do - MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) - merge_request.mark_as_unmergeable! - end - - it_behaves_like 'unmergeable merge request' - - it 'returns ServiceResponse.error' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.error?).to be(true) - expect(result.message).to eq('Merge request is not mergeable') - end - end - - context 'when merge request is not given' do - subject { described_class.new(nil).execute } - - it 'returns ServiceResponse.error' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.message).to eq('Invalid argument') - end - end - - context 'when read only DB' do - it 'returns ServiceResponse.error' do - allow(Gitlab::Database).to receive(:read_only?) { true } - - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.message).to eq('Unsupported operation') - end - end - - context 'when MR is mergeable but merge-ref does not exists' do - before do - merge_request.mark_as_mergeable! - end - - it 'keeps merge status as can_be_merged' do - expect { subject }.not_to change(merge_request, :merge_status).from('can_be_merged') - end - - it 'returns ServiceResponse.error' do - result = subject - - expect(result).to be_a(ServiceResponse) - expect(result.error?).to be(true) - expect(result.message).to eq('Merge ref was not found') - end - end - end -end diff --git a/spec/services/service_response_spec.rb b/spec/services/service_response_spec.rb index e790d272e61..30bd4d6820b 100644 --- a/spec/services/service_response_spec.rb +++ b/spec/services/service_response_spec.rb @@ -16,13 +16,6 @@ describe ServiceResponse do expect(response).to be_success expect(response.message).to eq('Good orange') end - - it 'creates a successful response with payload' do - response = described_class.success(payload: { good: 'orange' }) - - expect(response).to be_success - expect(response.payload).to eq(good: 'orange') - end end describe '.error' do @@ -40,15 +33,6 @@ describe ServiceResponse do expect(response.message).to eq('Bad apple') expect(response.http_status).to eq(400) end - - it 'creates a failed response with payload' do - response = described_class.error(message: 'Bad apple', - payload: { bad: 'apple' }) - - expect(response).to be_error - expect(response.message).to eq('Bad apple') - expect(response.payload).to eq(bad: 'apple') - end end describe '#success?' do diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb index 875a9a76e12..14ce3c32e77 100644 --- a/spec/support/capybara.rb +++ b/spec/support/capybara.rb @@ -42,6 +42,9 @@ Capybara.register_driver :chrome do |app| # Disable /dev/shm use in CI. See https://gitlab.com/gitlab-org/gitlab-ee/issues/4252 options.add_argument("disable-dev-shm-usage") if ENV['CI'] || ENV['CI_SERVER'] + # Explicitly set user-data-dir to prevent crashes. See https://gitlab.com/gitlab-org/gitlab-ce/issues/58882#note_179811508 + options.add_argument("user-data-dir=/tmp/chrome") if ENV['CI'] || ENV['CI_SERVER'] + Capybara::Selenium::Driver.new( app, browser: :chrome, diff --git a/spec/support/shared_contexts/policies/project_policy_shared_context.rb b/spec/support/shared_contexts/policies/project_policy_shared_context.rb index 54d9f5b15f2..1aa40dcde3d 100644 --- a/spec/support/shared_contexts/policies/project_policy_shared_context.rb +++ b/spec/support/shared_contexts/policies/project_policy_shared_context.rb @@ -64,7 +64,7 @@ RSpec.shared_context 'ProjectPolicy context' do %i[ change_namespace change_visibility_level rename_project remove_project archive_project remove_fork_project destroy_merge_request destroy_issue - set_issue_iid set_issue_created_at set_note_created_at + set_issue_iid set_issue_created_at set_issue_updated_at set_note_created_at ] end diff --git a/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb b/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb index 782a2d97746..a931c4df99f 100644 --- a/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb +++ b/spec/support/shared_examples/finders/assignees_filter_shared_examples.rb @@ -20,12 +20,6 @@ shared_examples 'no assignee filter' do end it 'returns issuables not assigned to any assignee' do - params[:assignee_id] = 0 - - expect(issuables).to contain_exactly(*expected_issuables) - end - - it 'returns issuables not assigned to any assignee' do params[:assignee_id] = 'none' expect(issuables).to contain_exactly(*expected_issuables) |