Merge branch 'mark-update-head-pipeline-for-merge-request-worker-as-idempotent' into 'master'

Mark UpdateHeadPipelineForMergeRequestWorker as idempotent

See merge request gitlab-org/gitlab!30863

app/workers/all_queues.yml

@@ -849,7 +849,7 @@
   :urgency: :high
   :resource_boundary: :cpu
   :weight: 5
-  :idempotent: 
+  :idempotent: true
 - :name: repository_check:repository_check_batch
   :feature_category: :source_code_management
   :has_external_dependencies: 

app/workers/update_head_pipeline_for_merge_request_worker.rb

 # frozen_string_literal: true
 
-class UpdateHeadPipelineForMergeRequestWorker # rubocop:disable Scalability/IdempotentWorker
+class UpdateHeadPipelineForMergeRequestWorker
   include ApplicationWorker
   include PipelineQueue
 
@@ -9,6 +9,8 @@ class UpdateHeadPipelineForMergeRequestWorker # rubocop:disable Scalability/Idem
   urgency :high
   worker_resource_boundary :cpu
 
+  idempotent!
+
   def perform(merge_request_id)
     MergeRequest.find_by_id(merge_request_id).try do |merge_request|
       merge_request.update_head_pipeline

spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb

@@ -4,18 +4,27 @@ require 'spec_helper'
 
 describe UpdateHeadPipelineForMergeRequestWorker do
   describe '#perform' do
-    let(:user) { create(:user) }
-    let(:project) { create(:project, :repository) }
-    let(:merge_request) { create(:merge_request, source_project: project) }
-    let(:latest_sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
+    let_it_be(:user) { create(:user) }
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:merge_request) { create(:merge_request, source_project: project) }
+    let_it_be(:latest_sha) { 'b83d6e391c22777fca1ed3012fce84f633d7fed0' }
 
     context 'when pipeline exists for the source project and branch' do
-      before do
-        create(:ci_empty_pipeline, project: project, ref: merge_request.source_branch, sha: latest_sha)
-      end
+      let_it_be(:pipeline) { create(:ci_empty_pipeline, project: project, ref: merge_request.source_branch, sha: latest_sha) }
 
       it 'updates the head_pipeline_id of the merge_request' do
-        expect { subject.perform(merge_request.id) }.to change { merge_request.reload.head_pipeline_id }
+        expect { subject.perform(merge_request.id) }
+          .to change { merge_request.reload.head_pipeline_id }.from(nil).to(pipeline.id)
+      end
+
+      it_behaves_like 'an idempotent worker' do
+        let(:job_args) { merge_request.id }
+
+        it 'sets the pipeline as the head pipeline when run multiple times' do
+          subject
+
+          expect(merge_request.reload.head_pipeline_id).to eq(pipeline.id)
+        end
       end
 
       context 'when merge request sha does not equal pipeline sha' do
@@ -27,6 +36,15 @@ describe UpdateHeadPipelineForMergeRequestWorker do
           expect { subject.perform(merge_request.id) }
             .not_to change { merge_request.reload.head_pipeline_id }
         end
+
+        it_behaves_like 'an idempotent worker' do
+          let(:job_args) { merge_request.id }
+
+          it 'does not update the head_pipeline_id when run multiple times' do
+            expect { subject }
+              .not_to change { merge_request.reload.head_pipeline_id }
+          end
+        end
       end
     end
 
@@ -35,10 +53,19 @@ describe UpdateHeadPipelineForMergeRequestWorker do
         expect { subject.perform(merge_request.id) }
           .not_to change { merge_request.reload.head_pipeline_id }
       end
+
+      it_behaves_like 'an idempotent worker' do
+        let(:job_args) { merge_request.id }
+
+        it 'does not update the head_pipeline_id when run multiple times' do
+          expect { subject }
+            .not_to change { merge_request.reload.head_pipeline_id }
+        end
+      end
     end
 
     context 'when a merge request pipeline exists' do
-      let!(:merge_request_pipeline) do
+      let_it_be(:merge_request_pipeline) do
         create(:ci_pipeline,
                project: project,
                source: :merge_request_event,
@@ -52,6 +79,16 @@ describe UpdateHeadPipelineForMergeRequestWorker do
           .from(nil).to(merge_request_pipeline.id)
       end
 
+      it_behaves_like 'an idempotent worker' do
+        let(:job_args) { merge_request.id }
+
+        it 'sets the merge request pipeline as the head pipeline when run multiple times' do
+          subject
+
+          expect(merge_request.reload.head_pipeline_id).to eq(merge_request_pipeline.id)
+        end
+      end
+
       context 'when branch pipeline exists' do
         let!(:branch_pipeline) do
           create(:ci_pipeline, project: project, source: :push, sha: latest_sha)
@@ -62,6 +99,16 @@ describe UpdateHeadPipelineForMergeRequestWorker do
             .to change { merge_request.reload.head_pipeline_id }
             .from(nil).to(merge_request_pipeline.id)
         end
+
+        it_behaves_like 'an idempotent worker' do
+          let(:job_args) { merge_request.id }
+
+          it 'sets the merge request pipeline as the head pipeline when run multiple times' do
+            subject
+
+            expect(merge_request.reload.head_pipeline_id).to eq(merge_request_pipeline.id)
+          end
+        end
       end
     end
   end