Merge branch 'fix_bug_in_merge_when_checks_pass' into 'master'

Fix bug in Merge When Checks Pass needing a pipeline

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144553



Merged-by: Kerri Miller <kerrizor@kerrizor.com>
Approved-by: Patrick Bajao <ebajao@gitlab.com>
Approved-by: Jay McCure <jmccure@gitlab.com>
Approved-by: Kerri Miller <kerrizor@kerrizor.com>
Reviewed-by: Patrick Bajao <ebajao@gitlab.com>
Co-authored-by: Marc Shaw <mshaw@gitlab.com>

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?

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

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

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)