From 68f274e76e21065ce48b28d73a2235af249a03b2 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Thu, 22 Dec 2022 19:28:20 -0800 Subject: [PATCH 1/7] Refresh pipelines when target branch changes When the target branch of an MR changes, trigger a refresh of pipelines Changelog: added --- app/services/merge_requests/base_service.rb | 4 ++ .../merge_requests/refresh_service.rb | 4 -- app/services/merge_requests/update_service.rb | 2 + .../merge_requests/update_service_spec.rb | 39 +++++++++++++++++++ 4 files changed, 45 insertions(+), 4 deletions(-) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 5f00b4d85c6b18..4781f52e5f88bd 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -94,6 +94,10 @@ def merge_request_activity_counter private + def refresh_pipelines_on_merge_requests(merge_request) + create_pipeline_for(merge_request, current_user, async: true) + end + def enqueue_jira_connect_messages_for(merge_request) return unless project.jira_subscription_exists? diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 533d0052fb8874..ce49d5dd43caec 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -162,10 +162,6 @@ def outdate_service @outdate_service ||= Suggestions::OutdateService.new end - def refresh_pipelines_on_merge_requests(merge_request) - create_pipeline_for(merge_request, current_user, async: true) - end - def abort_auto_merges(merge_request) abort_auto_merge(merge_request, 'source branch was updated') end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 745647b727c0ee..500df22a686018 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -169,6 +169,8 @@ def handle_target_branch_change(merge_request) merge_request.target_branch ) + refresh_pipelines_on_merge_requests(merge_request) + abort_auto_merge(merge_request, 'target branch was changed') end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 64d723f8f6c1c9..67b6fb01ff112a 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -479,6 +479,33 @@ def update_merge_request(opts) end end + shared_examples_for "creates a new pipeline" do + context "with configured CI", :sidekiq_inline do + before do + stub_ci_pipeline_yaml_file(config) + end + + let(:config) do + YAML.dump({ + test: { + stage: 'test', + script: 'echo', + only: ['merge_requests'] + } + }) + end + + it "creates a new pipeline" do + expect_next_instance_of(MergeRequests::UpdateService) do |mrus| + expect(mrus).to receive(:create_pipeline_for).and_call_original + end + update_merge_request(target_branch: new_target_branch) + # expect { update_merge_request(target_branch: new_target_branch) } + # .to change { merge_request.pipelines_for_merge_request.count }.by(1) + end + end + end + shared_examples_for 'correct merge behavior' do let(:opts) do { @@ -799,6 +826,10 @@ def update_merge_request(opts) update_merge_request({ target_branch: "target" }) end + + it_behaves_like "creates a new pipeline" do + let(:new_target_branch) {"target"} + end end context 'when auto merge is enabled and target branch changed' do @@ -813,6 +844,10 @@ def update_merge_request(opts) update_merge_request({ target_branch: 'target' }) end + + it_behaves_like "creates a new pipeline" do + let(:new_target_branch) {"target"} + end end end @@ -1237,6 +1272,10 @@ def update_merge_request(opts) expect { update_merge_request(target_branch: 'master', target_branch_was_deleted: true) } .to change { merge_request.reload.target_branch }.from('mr-a').to('master') end + + it_behaves_like "creates a new pipeline" do + let(:new_target_branch) {"target"} + end end it_behaves_like 'issuable record that supports quick actions' do -- GitLab From 9a3ef563244688424acd2951bc3164db47a62bac Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Thu, 22 Dec 2022 20:32:12 -0800 Subject: [PATCH 2/7] Fix minor rubocop violation --- spec/services/merge_requests/update_service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 67b6fb01ff112a..307e4688c11cfb 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -828,7 +828,7 @@ def update_merge_request(opts) end it_behaves_like "creates a new pipeline" do - let(:new_target_branch) {"target"} + let(:new_target_branch) { "target" } end end @@ -846,7 +846,7 @@ def update_merge_request(opts) end it_behaves_like "creates a new pipeline" do - let(:new_target_branch) {"target"} + let(:new_target_branch) { "target" } end end end @@ -1274,7 +1274,7 @@ def update_merge_request(opts) end it_behaves_like "creates a new pipeline" do - let(:new_target_branch) {"target"} + let(:new_target_branch) { "target" } end end -- GitLab From 1727adde8dea6cf0dac20bcd4f7d6de8ec35b150 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Fri, 23 Dec 2022 08:01:26 -0800 Subject: [PATCH 3/7] Remove extraneous CI creation --- .../merge_requests/update_service_spec.rb | 26 +++---------------- 1 file changed, 4 insertions(+), 22 deletions(-) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 307e4688c11cfb..b7be89d10a9965 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -480,29 +480,11 @@ def update_merge_request(opts) end shared_examples_for "creates a new pipeline" do - context "with configured CI", :sidekiq_inline do - before do - stub_ci_pipeline_yaml_file(config) - end - - let(:config) do - YAML.dump({ - test: { - stage: 'test', - script: 'echo', - only: ['merge_requests'] - } - }) - end - - it "creates a new pipeline" do - expect_next_instance_of(MergeRequests::UpdateService) do |mrus| - expect(mrus).to receive(:create_pipeline_for).and_call_original - end - update_merge_request(target_branch: new_target_branch) - # expect { update_merge_request(target_branch: new_target_branch) } - # .to change { merge_request.pipelines_for_merge_request.count }.by(1) + it "creates a new pipeline" do + expect_next_instance_of(MergeRequests::UpdateService) do |mrus| + expect(mrus).to receive(:create_pipeline_for).and_call_original end + update_merge_request(target_branch: new_target_branch) end end -- GitLab From 7299a66606f85888393627307607e91aee193107 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Fri, 23 Dec 2022 08:02:43 -0800 Subject: [PATCH 4/7] Add load-bearing whitespace --- spec/services/merge_requests/update_service_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index b7be89d10a9965..47580e1c0b7239 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -484,6 +484,7 @@ def update_merge_request(opts) expect_next_instance_of(MergeRequests::UpdateService) do |mrus| expect(mrus).to receive(:create_pipeline_for).and_call_original end + update_merge_request(target_branch: new_target_branch) end end -- GitLab From 6063103541353072621e9bff81ba7b8d1720d9b3 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Wed, 28 Dec 2022 09:23:41 -0800 Subject: [PATCH 5/7] Test CreatePipelineWorker receives calls --- spec/services/merge_requests/update_service_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 47580e1c0b7239..15c806d1d43c2c 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -481,9 +481,9 @@ def update_merge_request(opts) shared_examples_for "creates a new pipeline" do it "creates a new pipeline" do - expect_next_instance_of(MergeRequests::UpdateService) do |mrus| - expect(mrus).to receive(:create_pipeline_for).and_call_original - end + expect(MergeRequests::CreatePipelineWorker) + .to receive(:perform_async) + .with(project.id, user.id, merge_request.id, {}) update_merge_request(target_branch: new_target_branch) end -- GitLab From a80eefb6f999a889b31053a98909565619c84dd5 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Fri, 6 Jan 2023 19:47:21 -0800 Subject: [PATCH 6/7] Update spec to account for :allow_duplicate --- spec/services/merge_requests/update_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 15c806d1d43c2c..bf227810d79a97 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -483,7 +483,7 @@ def update_merge_request(opts) it "creates a new pipeline" do expect(MergeRequests::CreatePipelineWorker) .to receive(:perform_async) - .with(project.id, user.id, merge_request.id, {}) + .with(project.id, user.id, merge_request.id, { "allow_duplicate" => false }) update_merge_request(target_branch: new_target_branch) end -- GitLab From 0060963dad1e8fb93f50ffb55f87563dd8095f07 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Fri, 6 Jan 2023 21:32:55 -0800 Subject: [PATCH 7/7] Sets :allow_duplicate to true --- app/services/merge_requests/base_service.rb | 4 ++-- app/services/merge_requests/update_service.rb | 2 +- spec/services/merge_requests/update_service_spec.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 4781f52e5f88bd..f6cbe889128623 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -94,8 +94,8 @@ def merge_request_activity_counter private - def refresh_pipelines_on_merge_requests(merge_request) - create_pipeline_for(merge_request, current_user, async: true) + def refresh_pipelines_on_merge_requests(merge_request, allow_duplicate: false) + create_pipeline_for(merge_request, current_user, async: true, allow_duplicate: allow_duplicate) end def enqueue_jira_connect_messages_for(merge_request) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 500df22a686018..a273b853c0d271 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -169,7 +169,7 @@ def handle_target_branch_change(merge_request) merge_request.target_branch ) - refresh_pipelines_on_merge_requests(merge_request) + refresh_pipelines_on_merge_requests(merge_request, allow_duplicate: true) abort_auto_merge(merge_request, 'target branch was changed') end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index bf227810d79a97..6fc12ea6363347 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -483,7 +483,7 @@ def update_merge_request(opts) it "creates a new pipeline" do expect(MergeRequests::CreatePipelineWorker) .to receive(:perform_async) - .with(project.id, user.id, merge_request.id, { "allow_duplicate" => false }) + .with(project.id, user.id, merge_request.id, { "allow_duplicate" => true }) update_merge_request(target_branch: new_target_branch) end -- GitLab