diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb index 3c27ad56ebb486592d605eefbfb7fb71ff309cfc..91f1425160819db475c591a84599dd6f998456d8 100644 --- a/app/services/git/branch_push_service.rb +++ b/app/services/git/branch_push_service.rb @@ -39,6 +39,7 @@ def execute def enqueue_update_mrs return if params[:merge_request_branches]&.exclude?(branch_name) + # TODO: pass params[:push_options] to worker UpdateMergeRequestsWorker.perform_async( project.id, current_user.id, diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 30ad549db915bbeedc802c1406b8d5b50f0c052f..74a8d9d6732e215971c9983f4d50f03fdcaf9705 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -185,9 +185,12 @@ def create_reviewer_note(merge_request, old_reviewers) def create_pipeline_for(merge_request, user, async: false) if async + # TODO: pass push_options to worker MergeRequests::CreatePipelineWorker.perform_async(project.id, user.id, merge_request.id) else - MergeRequests::CreatePipelineService.new(project: project, current_user: user).execute(merge_request) + MergeRequests::CreatePipelineService + .new(project: project, current_user: user, params: params.slice(:push_options)) + .execute(merge_request) end end diff --git a/app/services/merge_requests/create_pipeline_service.rb b/app/services/merge_requests/create_pipeline_service.rb index 9d7f8393ba52cb430cd1074b3b2454bed7471e9d..37c734613e791d098042f7b0cea4457f0882e536 100644 --- a/app/services/merge_requests/create_pipeline_service.rb +++ b/app/services/merge_requests/create_pipeline_service.rb @@ -9,9 +9,11 @@ def execute(merge_request) end def create_detached_merge_request_pipeline(merge_request) - Ci::CreatePipelineService.new(pipeline_project(merge_request), - current_user, - ref: pipeline_ref_for_detached_merge_request_pipeline(merge_request)) + Ci::CreatePipelineService + .new(pipeline_project(merge_request), + current_user, + ref: pipeline_ref_for_detached_merge_request_pipeline(merge_request), + push_options: params[:push_options]) .execute(:merge_request_event, merge_request: merge_request) end diff --git a/app/workers/merge_requests/create_pipeline_worker.rb b/app/workers/merge_requests/create_pipeline_worker.rb index ee42a3dee08ac975c7eeba9bf188b3d49675b9b6..b40408cf647af2d364eab4ae9f6551157749a10d 100644 --- a/app/workers/merge_requests/create_pipeline_worker.rb +++ b/app/workers/merge_requests/create_pipeline_worker.rb @@ -15,7 +15,7 @@ class CreatePipelineWorker worker_resource_boundary :cpu idempotent! - def perform(project_id, user_id, merge_request_id) + def perform(project_id, user_id, merge_request_id, params = {}) project = Project.find_by_id(project_id) return unless project @@ -25,7 +25,12 @@ def perform(project_id, user_id, merge_request_id) merge_request = MergeRequest.find_by_id(merge_request_id) return unless merge_request - MergeRequests::CreatePipelineService.new(project: project, current_user: user).execute(merge_request) + push_options = params.with_indifferent_access[:push_options] + + MergeRequests::CreatePipelineService + .new(project: project, current_user: user, params: { push_options: push_options }) + .execute(merge_request) + merge_request.update_head_pipeline end end diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index 5c96257cb632ece2e73d02a2b5b86ea3ffbfbde0..eb69c0eaba68a566f72a1281a1c5c9d4ae1fb6ed 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -13,13 +13,17 @@ class UpdateMergeRequestsWorker # rubocop:disable Scalability/IdempotentWorker weight 3 loggable_arguments 2, 3, 4 - def perform(project_id, user_id, oldrev, newrev, ref) + def perform(project_id, user_id, oldrev, newrev, ref, params = {}) project = Project.find_by_id(project_id) return unless project user = User.find_by_id(user_id) return unless user - MergeRequests::RefreshService.new(project: project, current_user: user).execute(oldrev, newrev, ref) + push_options = params.with_indifferent_access[:push_options] + + MergeRequests::RefreshService + .new(project: project, current_user: user, params: { push_options: push_options }) + .execute(oldrev, newrev, ref) end end diff --git a/ee/app/services/ee/merge_requests/create_pipeline_service.rb b/ee/app/services/ee/merge_requests/create_pipeline_service.rb index 1c2127cc34169f6deb32a6ba3f30f2e44c84e7e1..1d6324aadd107b917c66c1c2b1502725102ad41c 100644 --- a/ee/app/services/ee/merge_requests/create_pipeline_service.rb +++ b/ee/app/services/ee/merge_requests/create_pipeline_service.rb @@ -22,11 +22,13 @@ def create_merged_result_pipeline_for(merge_request) if result.success? && merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true) ref_payload = result.payload.fetch(:merge_ref_head) - ::Ci::CreatePipelineService.new(merge_request.target_project, current_user, - ref: merge_request.merge_ref_path, - checkout_sha: ref_payload[:commit_id], - target_sha: ref_payload[:target_id], - source_sha: ref_payload[:source_id]) + ::Ci::CreatePipelineService + .new(merge_request.target_project, current_user, + ref: merge_request.merge_ref_path, + checkout_sha: ref_payload[:commit_id], + target_sha: ref_payload[:target_id], + source_sha: ref_payload[:source_id], + push_options: params[:push_options]) .execute(:merge_request_event, merge_request: merge_request) else cannot_create_pipeline_error diff --git a/ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb b/ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb index e6196e7fa59c339a77661169610c39a5234af72f..c91b69ad1c8369ee8edb75d34bb7bc595c8dcdfc 100644 --- a/ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb +++ b/ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb @@ -8,9 +8,10 @@ describe '#execute' do subject { service.execute(merge_request) } - let(:service) { described_class.new(project: source_project, current_user: user) } + let(:service) { described_class.new(project: source_project, current_user: user, params: params) } let(:project) { create(:project, :repository) } let(:user) { create(:user) } + let(:params) { {} } let(:title) { 'Awesome merge request' } let(:merge_pipelines_enabled) { true } let(:merge_pipelines_license) { true } @@ -72,6 +73,20 @@ expect(subject.payload).to eq(Ci::Pipeline.last) end + context 'when push options contain ci.skip' do + let(:params) { { push_options: { ci: { skip: true } } } } + + it 'creates a skipped pipeline' do + subject + + expect(merge_request.all_pipelines.count).to eq(1) + pipeline = merge_request.all_pipelines.last + expect(pipeline).to be_merged_result_pipeline + expect(pipeline.builds).to be_empty + expect(pipeline).to be_skipped + end + end + context 'when merge request is WIP' do before do merge_request.update!(title: merge_request.wip_title) diff --git a/spec/services/merge_requests/create_pipeline_service_spec.rb b/spec/services/merge_requests/create_pipeline_service_spec.rb index d84ce8d15b4b42b00e1bd459abe248ca5f10806c..08ad05b54da9bf89f0bc7c68fc94d62662dfa953 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 @@ 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/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 6e6b4a91e0db2c67730cc17cb4c3360b184a7596..7560f0d5e240badd7008478eaa7ad61f005c384b 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -228,6 +228,21 @@ 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 } diff --git a/spec/workers/merge_requests/create_pipeline_worker_spec.rb b/spec/workers/merge_requests/create_pipeline_worker_spec.rb index 06d44c45706e7ea46e76b04afe5234cc5fb5c311..441d765221964c8ccf37f3740f0fb1d02eed32aa 100644 --- a/spec/workers/merge_requests/create_pipeline_worker_spec.rb +++ b/spec/workers/merge_requests/create_pipeline_worker_spec.rb @@ -3,24 +3,50 @@ require 'spec_helper' RSpec.describe MergeRequests::CreatePipelineWorker do - subject(:worker) { described_class.new } - describe '#perform' do let(:user) { create(:user) } let(:project) { create(:project) } let(:merge_request) { create(:merge_request) } + let(:worker) { described_class.new } + + subject { worker.perform(project.id, user.id, merge_request.id) } context 'when the objects exist' do it 'calls the merge request create pipeline service and calls update head pipeline' do aggregate_failures do - expect_next_instance_of(MergeRequests::CreatePipelineService, project: project, current_user: user) do |service| + expect_next_instance_of(MergeRequests::CreatePipelineService, + project: project, + current_user: user, + params: { push_options: nil }) do |service| expect(service).to receive(:execute).with(merge_request) end expect(MergeRequest).to receive(:find_by_id).with(merge_request.id).and_return(merge_request) expect(merge_request).to receive(:update_head_pipeline) - subject.perform(project.id, user.id, merge_request.id) + subject + end + end + + context 'when push options are passed as Hash to the worker' do + let(:extra_params) { { 'push_options' => { 'ci' => { 'skip' => true } } } } + + subject { worker.perform(project.id, user.id, merge_request.id, extra_params) } + + it 'calls the merge request create pipeline service and calls update head pipeline' do + aggregate_failures do + expect_next_instance_of(MergeRequests::CreatePipelineService, + project: project, + current_user: user, + params: { push_options: { ci: { skip: true } } }) do |service| + expect(service).to receive(:execute).with(merge_request) + end + + expect(MergeRequest).to receive(:find_by_id).with(merge_request.id).and_return(merge_request) + expect(merge_request).to receive(:update_head_pipeline) + + subject + end end end end @@ -29,8 +55,7 @@ it 'does not call the create pipeline service' do expect(MergeRequests::CreatePipelineService).not_to receive(:new) - expect { subject.perform(project.id, user.id, merge_request.id) } - .not_to raise_exception + expect { subject }.not_to raise_exception end end diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb index bd0dc2f9ef4ccf53feb2da292ecafdfe224bc0b7..64fcc2bd388040968a2fc213ede7fb5403ea7266 100644 --- a/spec/workers/update_merge_requests_worker_spec.rb +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -3,28 +3,47 @@ require 'spec_helper' RSpec.describe UpdateMergeRequestsWorker do - include RepoHelpers + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let_it_be(:oldrev) { "123456" } + let_it_be(:newrev) { "789012" } + let_it_be(:ref) { "refs/heads/test" } - let(:project) { create(:project, :repository) } - let(:user) { create(:user) } - - subject { described_class.new } + let(:worker) { described_class.new } describe '#perform' do - let(:oldrev) { "123456" } - let(:newrev) { "789012" } - let(:ref) { "refs/heads/test" } - - def perform - subject.perform(project.id, user.id, oldrev, newrev, ref) - end + subject { worker.perform(project.id, user.id, oldrev, newrev, ref) } it 'executes MergeRequests::RefreshService with expected values' do - expect_next_instance_of(MergeRequests::RefreshService, project: project, current_user: user) do |refresh_service| - expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref) + expect_next_instance_of(MergeRequests::RefreshService, + project: project, + current_user: user, + params: { push_options: nil }) do |service| + expect(service) + .to receive(:execute) + .with(oldrev, newrev, ref) end - perform + subject + end + + context 'when push options are passed as Hash' do + let(:extra_params) { { 'push_options' => { 'ci' => { 'skip' => true } } } } + + subject { worker.perform(project.id, user.id, oldrev, newrev, ref, extra_params) } + + it 'executes MergeRequests::RefreshService with expected values' do + expect_next_instance_of(MergeRequests::RefreshService, + project: project, + current_user: user, + params: { push_options: { ci: { skip: true } } }) do |service| + expect(service) + .to receive(:execute) + .with(oldrev, newrev, ref) + end + + subject + end end end end