Add spec for invalid ci yaml files

Document current behavior

ee/spec/services/ee/merge_requests/create_pipeline_service_spec.rb

@@ -27,13 +27,13 @@ describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_state
     end
 
     let(:ci_yaml) do
-      {
+      YAML.dump({
         test: {
           stage: 'test',
           script: 'echo',
           only: ['merge_requests']
         }
-      }
+      })
     end
 
     before do
@@ -41,7 +41,7 @@ describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_state
       target_project.add_developer(user)
       source_project.merge_pipelines_enabled = merge_pipelines_enabled
       stub_licensed_features(merge_pipelines: merge_pipelines_license)
-      stub_ci_pipeline_yaml_file(YAML.dump(ci_yaml))
+      stub_ci_pipeline_yaml_file(ci_yaml)
     end
 
     shared_examples_for 'detached merge request pipeline' do
@@ -132,5 +132,15 @@ describe MergeRequests::CreatePipelineService, :clean_gitlab_redis_shared_state
         it_behaves_like 'detached merge request pipeline'
       end
     end
+
+    context 'when .gitlab-ci.yml is invalid' do
+      let(:ci_yaml) { 'invalid yaml file' }
+
+      it 'persists a pipeline with a config error' do
+        expect { subject }.to change { Ci::Pipeline.count }.by(1)
+        expect(merge_request.all_pipelines.last).to be_failed
+        expect(merge_request.all_pipelines.last).to be_config_error
+      end
+    end
   end
 end

spec/requests/api/merge_requests_spec.rb

@@ -1150,12 +1150,16 @@ describe API::MergeRequests do
 
   describe 'POST /projects/:id/merge_requests/:merge_request_iid/pipelines' do
     before do
-      stub_ci_pipeline_yaml_file(YAML.dump({
+      stub_ci_pipeline_yaml_file(ci_yaml)
+    end
+
+    let(:ci_yaml) do
+      YAML.dump({
         rspec: {
           script: 'ls',
           only: ['merge_requests']
         }
-      }))
+      })
     end
 
     let(:project) do
@@ -1208,6 +1212,18 @@ describe API::MergeRequests do
         expect(response).to have_gitlab_http_status(:not_found)
       end
     end
+
+    context 'when the .gitlab-ci.yml file is invalid' do
+      let(:ci_yaml) { 'invalid yaml file' }
+
+      it 'creates a failed pipeline' do
+        expect { request }.to change(Ci::Pipeline, :count).by(1)
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to be_a Hash
+        expect(merge_request.pipelines_for_merge_request.last).to be_failed
+        expect(merge_request.pipelines_for_merge_request.last).to be_config_error
+      end
+    end
   end
 
   describe 'POST /projects/:id/merge_requests' do

spec/services/git/branch_push_service_spec.rb

@@ -129,6 +129,21 @@ describe Git::BranchPushService, services: true do
         end
       end
     end
+
+    context 'when .gitlab-ci.yml file is invalid' do
+      before do
+        stub_ci_pipeline_yaml_file('invalid yaml file')
+      end
+
+      it 'persists an error pipeline' do
+        expect { subject }.to change { Ci::Pipeline.count }
+
+        pipeline = Ci::Pipeline.last
+        expect(pipeline).to be_push
+        expect(pipeline).to be_failed
+        expect(pipeline).to be_config_error
+      end
+    end
   end
 
   describe "Updates merge requests" do

spec/services/merge_requests/create_service_spec.rb

@@ -177,18 +177,18 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
 
       describe 'Pipelines for merge requests' do
         before do
-          stub_ci_pipeline_yaml_file(YAML.dump(config))
+          stub_ci_pipeline_yaml_file(config)
         end
 
         context "when .gitlab-ci.yml has merge_requests keywords" do
           let(:config) do
-            {
+            YAML.dump({
               test: {
                 stage: 'test',
                 script: 'echo',
                 only: ['merge_requests']
               }
-            }
+            })
           end
 
           it 'creates a detached merge request pipeline and sets it as a head pipeline' do
@@ -269,12 +269,12 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
 
         context "when .gitlab-ci.yml does not have merge_requests keywords" do
           let(:config) do
-            {
+            YAML.dump({
               test: {
                 stage: 'test',
                 script: 'echo'
               }
-            }
+            })
           end
 
           it 'does not create a detached merge request pipeline' do
@@ -284,6 +284,19 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do
             expect(merge_request.pipelines_for_merge_request.count).to eq(0)
           end
         end
+
+        context 'when .gitlab-ci.yml is invalid' do
+          let(:config) { 'invalid yaml file' }
+
+          it 'persists a pipeline with config error' do
+            expect(merge_request).to be_persisted
+
+            merge_request.reload
+            expect(merge_request.pipelines_for_merge_request.count).to eq(1)
+            expect(merge_request.pipelines_for_merge_request.last).to be_failed
+            expect(merge_request.pipelines_for_merge_request.last).to be_config_error
+          end
+        end
       end
 
       it 'increments the usage data counter of create event' do

spec/services/merge_requests/refresh_service_spec.rb

@@ -148,7 +148,7 @@ describe MergeRequests::RefreshService do
 
     describe 'Pipelines for merge requests' do
       before do
-        stub_ci_pipeline_yaml_file(YAML.dump(config))
+        stub_ci_pipeline_yaml_file(config)
       end
 
       subject { service.new(project, @user).execute(@oldrev, @newrev, ref) }
@@ -158,13 +158,13 @@ describe MergeRequests::RefreshService do
 
       context "when .gitlab-ci.yml has merge_requests keywords" do
         let(:config) do
-          {
+          YAML.dump({
             test: {
               stage: 'test',
               script: 'echo',
               only: ['merge_requests']
             }
-          }
+          })
         end
 
         it 'create detached merge request pipeline with commits' do
@@ -255,16 +255,28 @@ describe MergeRequests::RefreshService do
             end.not_to change { @merge_request.pipelines_for_merge_request.count }
           end
         end
+
+        context 'when the pipeline should be skipped' do
+          it 'saves a skipped detached merge request pipeline' do
+            project.repository.create_file(@user, 'new-file.txt', 'A new file',
+                                           message: '[skip ci] This is a test',
+                                           branch_name: 'master')
+
+            expect { subject }
+              .to change { @merge_request.pipelines_for_merge_request.count }.by(1)
+            expect(@merge_request.pipelines_for_merge_request.last).to be_skipped
+          end
+        end
       end
 
       context "when .gitlab-ci.yml does not have merge_requests keywords" do
         let(:config) do
-          {
+          YAML.dump({
             test: {
               stage: 'test',
               script: 'echo'
             }
-          }
+          })
         end
 
         it 'does not create a detached merge request pipeline' do
@@ -272,6 +284,40 @@ describe MergeRequests::RefreshService do
             .not_to change { @merge_request.pipelines_for_merge_request.count }
         end
       end
+
+      context 'when .gitlab-ci.yml is invalid' do
+        let(:config) { 'invalid yaml file' }
+
+        it 'persists a pipeline with config error' do
+          expect { subject }
+            .to change { @merge_request.pipelines_for_merge_request.count }.by(1)
+          expect(@merge_request.pipelines_for_merge_request.last).to be_failed
+          expect(@merge_request.pipelines_for_merge_request.last).to be_config_error
+        end
+      end
+
+      context 'when .gitlab-ci.yml file is valid but has a logical error' do
+        let(:config) do
+          YAML.dump({
+            build: {
+              script: 'echo "Valid yaml syntax, but..."',
+              only: ['master']
+            },
+            test: {
+              script: 'echo "... I depend on build, which does not run."',
+              only: ['merge_request'],
+              needs: ['build']
+            }
+          })
+        end
+
+        it 'persists a pipeline with config error' do
+          expect { subject }
+            .to change { @merge_request.pipelines_for_merge_request.count }.by(1)
+          expect(@merge_request.pipelines_for_merge_request.last).to be_failed
+          expect(@merge_request.pipelines_for_merge_request.last).to be_config_error
+        end
+      end
     end
 
     context 'push to origin repo source branch' do