Ensure pippeline corresponds with the sha of an MR

5 files changed, 115 insertions, 23 deletions

diff --git a/spec/features/merge_requests/pipelines_spec.rb b/spec/features/merge_requests/pipelines_spec.rb
index a3fcc27cab0..307c860eac4 100644
--- a/spec/features/merge_requests/pipelines_spec.rb
+++ b/spec/features/merge_requests/pipelines_spec.rb
@@ -20,10 +20,14 @@ feature 'Pipelines for Merge Requests', :js do
       end
 
       before do
-        visit project_merge_request_path(project, merge_request)
+        merge_request.update_attribute(:head_pipeline_id, pipeline.id)
       end
 
       scenario 'user visits merge request pipelines tab' do
+        visit project_merge_request_path(project, merge_request)
+
+        expect(page.find('.ci-widget')).to have_content('pending')
+
         page.within('.merge-request-tabs') do
           click_link('Pipelines')
         end
@@ -31,6 +35,15 @@ feature 'Pipelines for Merge Requests', :js do
 
         expect(page).to have_selector('.stage-cell')
       end
+
+      scenario 'pipeline sha does not equal last commit sha' do
+        pipeline.update_attribute(:sha, '19e2e9b4ef76b422ce1154af39a91323ccc57434')
+        visit project_merge_request_path(project, merge_request)
+        wait_for_requests
+
+        expect(page.find('.ci-widget')).to have_content(
+          'Could not connect to the CI server. Please check your settings and try again')
+      end
     end
 
     context 'without pipelines' do
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 728028746d8..e10f375d42b 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -828,20 +828,28 @@ describe MergeRequest do
   end
 
   describe '#head_pipeline' do
-    describe 'when the source project exists' do
-      it 'returns the latest pipeline' do
-        pipeline = create(:ci_empty_pipeline, project: subject.source_project, ref: 'master', status: 'running', sha: "123abc", head_pipeline_of: subject)
+    before do
+      allow(subject).to receive(:diff_head_sha).and_return('lastsha')
+    end
 
-        expect(subject.head_pipeline).to eq(pipeline)
-      end
+    it 'returns nil for MR without head_pipeline_id' do
+      subject.update_attribute(:head_pipeline_id, nil)
+
+      expect(subject.head_pipeline).to be_nil
     end
 
-    describe 'when the source project does not exist' do
-      it 'returns nil' do
-        allow(subject).to receive(:source_project).and_return(nil)
+    it 'returns nil for MR with old pipeline' do
+      pipeline = create(:ci_empty_pipeline, sha: 'notlatestsha')
+      subject.update_attribute(:head_pipeline_id, pipeline.id)
 
-        expect(subject.head_pipeline).to be_nil
-      end
+      expect(subject.head_pipeline).to be_nil
+    end
+
+    it 'returns the pipeline for MR with recent pipeline' do
+      pipeline = create(:ci_empty_pipeline, sha: 'lastsha')
+      subject.update_attribute(:head_pipeline_id, pipeline.id)
+
+      expect(subject.head_pipeline).to eq(pipeline)
     end
   end
 
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index befd0faf1b6..b0de8d447a2 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -57,19 +57,39 @@ describe Ci::CreatePipelineService do
       end
 
       context 'when merge requests already exist for this source branch' do
-        it 'updates head pipeline of each merge request' do
-          merge_request_1 = create(:merge_request, source_branch: 'master',
-                                                   target_branch: "branch_1",
-                                                   source_project: project)
+        let(:merge_request_1) do
+          create(:merge_request, source_branch: 'master', target_branch: "branch_1", source_project: project)
+        end
+        let(:merge_request_2) do
+          create(:merge_request, source_branch: 'master', target_branch: "branch_2", source_project: project)
+        end
 
-          merge_request_2 = create(:merge_request, source_branch: 'master',
-                                                   target_branch: "branch_2",
-                                                   source_project: project)
+        context 'when the head pipeline sha equals merge request sha' do
+          it 'updates head pipeline of each merge request' do
+            merge_request_1
+            merge_request_2
 
-          head_pipeline = execute_service
+            head_pipeline = execute_service
 
-          expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline)
-          expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline)
+            expect(merge_request_1.reload.head_pipeline).to eq(head_pipeline)
+            expect(merge_request_2.reload.head_pipeline).to eq(head_pipeline)
+          end
+        end
+
+        context 'when the head pipeline sha does not equal merge request sha' do
+          it 'raises the ArgumentError error from worker and does not update the head piepeline of MRs' do
+            merge_request_1
+            merge_request_2
+
+            allow_any_instance_of(Ci::Pipeline).to receive(:latest?).and_return(true)
+
+            expect { execute_service(after: 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f') }.to raise_error(ArgumentError)
+
+            last_pipeline = Ci::Pipeline.last
+
+            expect(merge_request_1.reload.head_pipeline).not_to eq(last_pipeline)
+            expect(merge_request_2.reload.head_pipeline).not_to eq(last_pipeline)
+          end
         end
 
         context 'when there is no pipeline for source branch' do
@@ -106,8 +126,7 @@ describe Ci::CreatePipelineService do
                                                    target_branch: "branch_1",
                                                    source_project: project)
 
-            allow_any_instance_of(Ci::Pipeline)
-              .to receive(:latest?).and_return(false)
+            allow_any_instance_of(Ci::Pipeline).to receive(:latest?).and_return(false)
 
             execute_service
 
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index a2c05761f6b..61ec4709c59 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -74,6 +74,20 @@ describe MergeRequests::RefreshService do
       end
     end
 
+    context 'when pipeline exists for the source branch' do
+      let!(:pipeline) { create(:ci_empty_pipeline, ref: @merge_request.source_branch, project: @project, sha: @commits.first.sha)}
+
+      subject { service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') }
+
+      it 'updates the head_pipeline_id for @merge_request' do
+        expect { subject }.to change { @merge_request.reload.head_pipeline_id }.from(nil).to(pipeline.id)
+      end
+
+      it 'does not update the head_pipeline_id for @fork_merge_request' do
+        expect { subject }.not_to change { @fork_merge_request.reload.head_pipeline_id }
+      end
+    end
+
     context 'push to origin repo source branch when an MR was reopened' do
       let(:refresh_service) { service.new(@project, @user) }
 
diff --git a/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb b/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb
new file mode 100644
index 00000000000..522e1566271
--- /dev/null
+++ b/spec/workers/update_head_pipeline_for_merge_request_worker_spec.rb
@@ -0,0 +1,38 @@
+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' }
+
+    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
+
+      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 }
+      end
+
+      context 'when merge request sha does not equal pipeline sha' do
+        before do
+          merge_request.merge_request_diff.update(head_commit_sha: 'different_sha')
+        end
+
+        it 'does not update head_pipeline_id' do
+          expect { subject.perform(merge_request.id) }.to raise_error(ArgumentError)
+
+          expect(merge_request.reload.head_pipeline_id).to eq(nil)
+        end
+      end
+    end
+
+    context 'when pipeline does not exist for the source project and branch' do
+      it 'does not update the head_pipeline_id of the merge_request' do
+        expect { subject.perform(merge_request.id) }.not_to change { merge_request.reload.head_pipeline_id }
+      end
+    end
+  end
+end