diff options
Diffstat (limited to 'spec/services/merge_requests')
7 files changed, 164 insertions, 50 deletions
diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index ab3d9880d29..3c9d2271ddc 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -79,6 +79,15 @@ RSpec.describe MergeRequests::BuildService do end end + shared_examples 'with a Default.md template' do + let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } + let(:project) { create(:project, :custom_repo, files: files ) } + + it 'the template description is preferred' do + expect(merge_request.description).to eq('Default template contents') + end + end + describe '#execute' do it 'calls the compare service with the correct arguments' do allow_any_instance_of(described_class).to receive(:projects_and_branches_valid?).and_return(true) @@ -221,6 +230,7 @@ RSpec.describe MergeRequests::BuildService do end it_behaves_like 'allows the merge request to be created' + it_behaves_like 'with a Default.md template' it 'uses the title of the commit as the title of the merge request' do expect(merge_request.title).to eq(commit_2.safe_message.split("\n").first) @@ -241,6 +251,8 @@ RSpec.describe MergeRequests::BuildService do context 'commit has no description' do let(:commits) { Commit.decorate([commit_3], project) } + it_behaves_like 'with a Default.md template' + it 'uses the title of the commit as the title of the merge request' do expect(merge_request.title).to eq(commit_3.safe_message) end @@ -279,6 +291,17 @@ RSpec.describe MergeRequests::BuildService do expect(merge_request.description).to eq(expected_description) end + + context 'a Default.md template is defined' do + let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } + let(:project) { create(:project, :custom_repo, files: files ) } + + it 'appends the closing description to a Default.md template' do + expected_description = ['Default template contents', closing_message].compact.join("\n\n") + + expect(merge_request.description).to eq(expected_description) + end + end end context 'when the source branch matches an internal issue' do @@ -332,6 +355,7 @@ RSpec.describe MergeRequests::BuildService do end it_behaves_like 'allows the merge request to be created' + it_behaves_like 'with a Default.md template' it 'uses the title of the branch as the merge request title' do expect(merge_request.title).to eq('Feature branch') @@ -347,6 +371,15 @@ RSpec.describe MergeRequests::BuildService do it 'keeps the description from the initial params' do expect(merge_request.description).to eq(description) end + + context 'a Default.md template is defined' do + let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } + let(:project) { create(:project, :custom_repo, files: files ) } + + it 'keeps the description from the initial params' do + expect(merge_request.description).to eq(description) + end + end end context 'when the source branch matches an issue' do @@ -377,6 +410,17 @@ RSpec.describe MergeRequests::BuildService do it 'sets the closing description' do expect(merge_request.description).to eq(closing_message) end + + context 'a Default.md template is defined' do + let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } + let(:project) { create(:project, :custom_repo, files: files ) } + + it 'appends the closing description to a Default.md template' do + expected_description = ['Default template contents', closing_message].compact.join("\n\n") + + expect(merge_request.description).to eq(expected_description) + end + end end end end @@ -389,6 +433,7 @@ RSpec.describe MergeRequests::BuildService do end it_behaves_like 'allows the merge request to be created' + it_behaves_like 'with a Default.md template' it 'uses the first line of the first multi-line commit message as the title' do expect(merge_request.title).to eq('Closes #1234 Second commit') @@ -426,6 +471,17 @@ RSpec.describe MergeRequests::BuildService do it 'sets the closing description' do expect(merge_request.description).to eq("Create the app#{closing_message ? "\n\n" + closing_message : ''}") end + + context 'a Default.md template is defined' do + let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } + let(:project) { create(:project, :custom_repo, files: files ) } + + it 'appends the closing description to a Default.md template' do + expected_description = ['Default template contents', closing_message].compact.join("\n\n") + + expect(merge_request.description).to eq(expected_description) + end + end end end @@ -626,4 +682,52 @@ RSpec.describe MergeRequests::BuildService do end end end + + describe '#assign_description_from_repository_template' do + subject { service.send(:assign_description_from_repository_template) } + + it 'performs no action if the merge request description is not blank' do + merge_request.description = 'foo' + subject + expect(merge_request.description).to eq 'foo' + end + + context 'when a Default template is not found' do + it 'does not modify the merge request description' do + merge_request.description = nil + subject + expect(merge_request.description).to be_nil + end + end + + context 'when a Default template is found' do + context 'when its contents cannot be retrieved' do + let(:files) { { '.gitlab/merge_request_templates/OtherTemplate.md' => 'Other template contents' } } + let(:project) { create(:project, :custom_repo, files: files ) } + + it 'does not modify the merge request description' do + allow(TemplateFinder).to receive(:all_template_names).and_return({ + merge_requests: [ + { name: 'Default', id: 'default', key: 'default', project_id: project.id } + ] + }) + + merge_request.description = nil + subject + expect(merge_request.description).to be_nil + end + end + + context 'when its contents can be retrieved' do + let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } + let(:project) { create(:project, :custom_repo, files: files ) } + + it 'modifies the merge request description' do + merge_request.description = nil + subject + expect(merge_request.description).to eq 'Default template contents' + end + end + end + end end diff --git a/spec/services/merge_requests/create_pipeline_service_spec.rb b/spec/services/merge_requests/create_pipeline_service_spec.rb index d84ce8d15b4..08ad05b54da 100644 --- a/spec/services/merge_requests/create_pipeline_service_spec.rb +++ b/spec/services/merge_requests/create_pipeline_service_spec.rb @@ -50,6 +50,19 @@ RSpec.describe MergeRequests::CreatePipelineService do expect(response.payload.source).to eq('merge_request_event') end + context 'when push options contain ci.skip' do + let(:params) { { push_options: { ci: { skip: true } } } } + + it 'creates a skipped pipeline' do + expect { response }.to change { Ci::Pipeline.count }.by(1) + + expect(response).to be_success + expect(response.payload).to be_persisted + expect(response.payload.builds).to be_empty + expect(response.payload).to be_skipped + end + end + context 'with fork merge request' do let_it_be(:forked_project) { fork_project(project, nil, repository: true, target_project: create(:project, :private, :repository)) } diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 49f691e97e2..c0c56a72192 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -32,7 +32,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it 'creates an MR' do expect(merge_request).to be_valid - expect(merge_request.work_in_progress?).to be(false) + expect(merge_request.draft?).to be(false) expect(merge_request.title).to eq('Awesome merge_request') expect(merge_request.assignees).to be_empty expect(merge_request.merge_params['force_remove_source_branch']).to eq('1') @@ -74,7 +74,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it 'sets MR to draft' do - expect(merge_request.work_in_progress?).to be(true) + expect(merge_request.draft?).to be(true) end end @@ -90,7 +90,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it 'sets MR to draft' do - expect(merge_request.work_in_progress?).to be(true) + expect(merge_request.draft?).to be(true) end end end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 78deab64b1c..a2d73d8c9b1 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -161,7 +161,7 @@ RSpec.describe MergeRequests::MergeService do end context 'with Jira integration' do - include JiraServiceHelper + include JiraIntegrationHelpers let(:jira_tracker) { project.create_jira_integration } let(:jira_issue) { ExternalIssue.new('JIRA-123', project) } @@ -263,10 +263,13 @@ RSpec.describe MergeRequests::MergeService do merge_request.update_attribute(:merge_params, { 'force_remove_source_branch' => '1' }) end + # Not a real use case. When a merger merges a MR , merge param 'should_remove_source_branch' is defined it 'removes the source branch using the author user' do expect(::MergeRequests::DeleteSourceBranchWorker).to receive(:perform_async).with(merge_request.id, merge_request.source_branch_sha, merge_request.author.id) service.execute(merge_request) + + expect(merge_request.reload.should_remove_source_branch?).to be nil end context 'when the merger set the source branch not to be removed' do @@ -276,6 +279,8 @@ RSpec.describe MergeRequests::MergeService do expect(::MergeRequests::DeleteSourceBranchWorker).not_to receive(:perform_async) service.execute(merge_request) + + expect(merge_request.reload.should_remove_source_branch?).to be false end end end @@ -289,6 +294,8 @@ RSpec.describe MergeRequests::MergeService do expect(::MergeRequests::DeleteSourceBranchWorker).to receive(:perform_async).with(merge_request.id, merge_request.source_branch_sha, user.id) service.execute(merge_request) + + expect(merge_request.reload.should_remove_source_branch?).to be true end end end diff --git a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb index d4ee4afd71d..2bb7dc3eef7 100644 --- a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb +++ b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb @@ -7,12 +7,6 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do let_it_be(:merge_request) { create(:merge_request) } - describe '#CHECKS' do - it 'contains every subclass of the base checks service', :eager_load do - expect(described_class::CHECKS).to contain_exactly(*MergeRequests::Mergeability::CheckBaseService.subclasses) - end - end - describe '#execute' do subject(:execute) { run_checks.execute } @@ -22,8 +16,8 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do context 'when every check is skipped', :eager_load do before do MergeRequests::Mergeability::CheckBaseService.subclasses.each do |subclass| - expect_next_instance_of(subclass) do |service| - expect(service).to receive(:skip?).and_return(true) + allow_next_instance_of(subclass) do |service| + allow(service).to receive(:skip?).and_return(true) end end end @@ -35,7 +29,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do context 'when a check is skipped' do it 'does not execute the check' do - described_class::CHECKS.each do |check| + merge_request.mergeability_checks.each do |check| allow_next_instance_of(check) do |service| allow(service).to receive(:skip?).and_return(false) allow(service).to receive(:execute).and_return(success_result) @@ -47,7 +41,13 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do expect(service).not_to receive(:execute) end - expect(execute).to match_array([success_result, success_result, success_result, success_result]) + # Since we're only marking one check to be skipped, we expect to receive + # `# of checks - 1` success result objects in return + # + check_count = merge_request.mergeability_checks.count - 1 + success_array = (1..check_count).each_with_object([]) { |_, array| array << success_result } + + expect(execute).to match_array(success_array) end end @@ -56,7 +56,7 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do let(:merge_check) { instance_double(MergeRequests::Mergeability::CheckCiStatusService) } before do - described_class::CHECKS.each do |check| + merge_request.mergeability_checks.each do |check| allow_next_instance_of(check) do |service| allow(service).to receive(:skip?).and_return(true) end diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index f0885365f96..e486daae15e 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -106,32 +106,6 @@ RSpec.describe MergeRequests::PostMergeService do expect(merge_request.reload).to be_merged end end - - context 'when async_mr_close_issue feature flag is disabled' do - before do - stub_feature_flags(async_mr_close_issue: false) - end - - it 'executes Issues::CloseService' do - expect_next_instance_of(Issues::CloseService) do |close_service| - expect(close_service).to receive(:execute).with(issue, commit: merge_request) - end - - subject - - expect(merge_request.reload).to be_merged - end - - it 'marks MR as merged regardless of errors when closing issues' do - expect_next_instance_of(Issues::CloseService) do |close_service| - allow(close_service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError) - end - - expect { subject }.to raise_error(RuntimeError) - - expect(merge_request.reload).to be_merged - end - end end context 'when the merge request has review apps' do diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 6e6b4a91e0d..eecf7c21cba 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -228,6 +228,21 @@ RSpec.describe MergeRequests::RefreshService do expect(@another_merge_request.has_commits?).to be_falsy end + context 'when "push_options: nil" is passed' do + let(:service_instance) { service.new(project: project, current_user: @user, params: { push_options: nil }) } + + subject { service_instance.execute(@oldrev, @newrev, ref) } + + it 'creates a detached merge request pipeline with commits' do + expect { subject } + .to change { @merge_request.pipelines_for_merge_request.count }.by(1) + .and change { @another_merge_request.pipelines_for_merge_request.count }.by(0) + + expect(@merge_request.has_commits?).to be_truthy + expect(@another_merge_request.has_commits?).to be_falsy + end + end + it 'does not create detached merge request pipeline for forked project' do expect { subject } .not_to change { @fork_merge_request.pipelines_for_merge_request.count } @@ -741,47 +756,48 @@ RSpec.describe MergeRequests::RefreshService do refresh_service.execute(oldrev, newrev, 'refs/heads/wip') fixup_merge_request.reload - expect(fixup_merge_request.work_in_progress?).to eq(true) + expect(fixup_merge_request.draft?).to eq(true) expect(fixup_merge_request.notes.last.note).to match( /marked this merge request as \*\*draft\*\* from #{Commit.reference_pattern}/ ) end it 'references the commit that caused the draft status' do - wip_merge_request = create(:merge_request, + draft_merge_request = create(:merge_request, source_project: @project, source_branch: 'wip', target_branch: 'master', target_project: @project) - commits = wip_merge_request.commits + commits = draft_merge_request.commits oldrev = commits.last.id newrev = commits.first.id - wip_commit = wip_merge_request.commits.find(&:work_in_progress?) + draft_commit = draft_merge_request.commits.find(&:draft?) refresh_service.execute(oldrev, newrev, 'refs/heads/wip') - expect(wip_merge_request.reload.notes.last.note).to eq( - "marked this merge request as **draft** from #{wip_commit.id}" + expect(draft_merge_request.reload.notes.last.note).to eq( + "marked this merge request as **draft** from #{draft_commit.id}" ) end it 'does not mark as draft based on commits that do not belong to an MR' do allow(refresh_service).to receive(:find_new_commits) + refresh_service.instance_variable_set("@commits", [ double( id: 'aaaaaaa', sha: 'aaaaaaa', short_id: 'aaaaaaa', title: 'Fix issue', - work_in_progress?: false + draft?: false ), double( id: 'bbbbbbb', sha: 'bbbbbbbb', short_id: 'bbbbbbb', title: 'fixup! Fix issue', - work_in_progress?: true, + draft?: true, to_reference: 'bbbbbbb' ) ]) @@ -789,7 +805,7 @@ RSpec.describe MergeRequests::RefreshService do refresh_service.execute(@oldrev, @newrev, 'refs/heads/master') reload_mrs - expect(@merge_request.work_in_progress?).to be_falsey + expect(@merge_request.draft?).to be_falsey end end |