From 5a9f0ac851aa4ba2e0b6ac7a2dbc5b381ca00f0c Mon Sep 17 00:00:00 2001
From: Marc Shaw <mshaw@gitlab.com>
Date: Tue, 13 Feb 2024 12:32:39 +0900
Subject: [PATCH] Merge when checks pass should not need a pipeline to merge
MR: gitlab.com/gitlab-org/gitlab/-/merge_requests/144553
Changelog: fixed
---
.../merge_when_checks_pass_service.rb | 12 ++++
.../merge_when_checks_pass_service_spec.rb | 65 ++++---------------
...r_merges_only_if_pipeline_succeeds_spec.rb | 23 +++++++
.../user_sees_merge_request_pipelines_spec.rb | 5 +-
4 files changed, 49 insertions(+), 56 deletions(-)
diff --git a/ee/app/services/auto_merge/merge_when_checks_pass_service.rb b/ee/app/services/auto_merge/merge_when_checks_pass_service.rb
index d738219e07039..ffa3caa3e7cf2 100644
--- a/ee/app/services/auto_merge/merge_when_checks_pass_service.rb
+++ b/ee/app/services/auto_merge/merge_when_checks_pass_service.rb
@@ -4,6 +4,17 @@ module AutoMerge
class MergeWhenChecksPassService < AutoMerge::MergeWhenPipelineSucceedsService
extend Gitlab::Utils::Override
+ override :process
+ def process(merge_request)
+ logger.info("Processing Automerge")
+
+ return unless merge_request.mergeable?
+
+ logger.info("Merge request mergeable")
+
+ merge_request.merge_async(merge_request.merge_user_id, merge_request.merge_params)
+ end
+
override :overrideable_available_for_checks
def overrideable_available_for_checks(merge_request)
if Feature.enabled?(:additional_merge_when_checks_ready, merge_request.project)
@@ -27,6 +38,7 @@ def add_system_note(merge_request)
)
end
+ override :check_availability
def check_availability(merge_request)
return false if Feature.disabled?(:merge_when_checks_pass, merge_request.project)
return false unless merge_request.approval_feature_available?
diff --git a/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb b/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb
index 082811fb682e5..e8e36b4ef6dc7 100644
--- a/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb
+++ b/ee/spec/services/auto_merge/merge_when_checks_pass_service_spec.rb
@@ -133,61 +133,22 @@
end
describe "#process" do
- it_behaves_like 'auto_merge service #process' do
- context 'when pipeline has succeeded and approvals are required' do
- let_it_be(:approver1) { create(:user) }
- let_it_be(:approver2) { create(:user) }
- let(:approvals_required) { 2 }
- let(:triggering_pipeline) do
- create(:ci_pipeline, :success, project: project, ref: merge_request_ref,
- sha: merge_request_head,
- head_pipeline_of: mr_merge_if_green_enabled)
- end
-
- before do
- allow(mr_merge_if_green_enabled)
- .to receive_messages(head_pipeline: triggering_pipeline, actual_head_pipeline: triggering_pipeline)
- approval_rule.users += [approver1, approver2]
- end
-
- context 'when all required approvals are given' do
- before do
- create(:approval, merge_request: mr_merge_if_green_enabled, user: approver1)
- create(:approval, merge_request: mr_merge_if_green_enabled, user: approver2)
- end
-
- context 'when mr is not draft' do
- it 'merges the merge request' do
- expect(MergeWorker).to receive(:perform_async)
-
- service.process(mr_merge_if_green_enabled)
- end
- end
+ context 'when the merge request is mergable' do
+ it 'calls the merge worker' do
+ expect(mr_merge_if_green_enabled)
+ .to receive(:merge_async)
+ .with(mr_merge_if_green_enabled.merge_user_id, mr_merge_if_green_enabled.merge_params)
- context 'when mr is draft' do
- before do
- mr_merge_if_green_enabled.update!(title: 'Draft: check')
- end
-
- it 'does not merge request' do
- expect(MergeWorker).not_to receive(:perform_async)
-
- service.process(mr_merge_if_green_enabled)
- end
- end
- end
-
- context 'when some approvals are missing' do
- before do
- create(:approval, merge_request: mr_merge_if_green_enabled, user: approver1)
- end
+ service.process(mr_merge_if_green_enabled)
+ end
+ end
- it 'does not merge request' do
- expect(MergeWorker).not_to receive(:perform_async)
+ context 'when the merge request is not mergeable' do
+ it 'does not call the merge worker' do
+ expect(mr_merge_if_green_enabled).to receive(:mergeable?).and_return(false)
+ expect(mr_merge_if_green_enabled).not_to receive(:merge_async)
- service.process(mr_merge_if_green_enabled)
- end
- end
+ service.process(mr_merge_if_green_enabled)
end
end
end
diff --git a/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb b/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb
index 1a7b72e680987..2809772697fbc 100644
--- a/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb
+++ b/spec/features/merge_request/user_merges_only_if_pipeline_succeeds_spec.rb
@@ -21,6 +21,29 @@
expect(page).to have_button 'Merge'
end
end
+
+ context 'when an active pipeline running' do
+ let!(:pipeline) do
+ create(
+ :ci_empty_pipeline,
+ project: project,
+ sha: merge_request.diff_head_sha,
+ ref: merge_request.source_branch,
+ status: :running,
+ head_pipeline_of: merge_request
+ )
+ end
+
+ it 'allows MR to be merged' do
+ visit project_merge_request_path(project, merge_request)
+
+ wait_for_requests
+
+ page.within('.mr-state-widget') do
+ expect(page).to have_button 'Set to auto-merge'
+ end
+ end
+ end
end
context 'when project has CI enabled' do
diff --git a/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb b/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb
index b5c0c163f98c5..e88e5d915d7a2 100644
--- a/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb
+++ b/spec/features/merge_request/user_sees_merge_request_pipelines_spec.rb
@@ -32,6 +32,7 @@
stub_licensed_features(merge_request_approvers: true) if Gitlab.ee?
# rubocop:enable RSpec/AvoidConditionalStatements
+ project.update!(only_allow_merge_if_pipeline_succeeds: true)
stub_application_setting(auto_devops_enabled: false)
stub_ci_pipeline_yaml_file(YAML.dump(config))
project.add_maintainer(user)
@@ -381,10 +382,6 @@ def mr_widget_title
end
context 'when the parent project enables pipeline must succeed' do
- before do
- project.update!(only_allow_merge_if_pipeline_succeeds: true)
- end
-
it 'shows Set to auto-merge button' do
visit project_merge_request_path(project, merge_request)
--
GitLab