Merge branch 'broken-yaml-errors' into 'master'

Fix broken yaml errors

It fixes two errors:
1. invalid `.gitlab-ci.yml` did not trigger yaml_errors, thus making the commit success
2. skipped status were used for invalid `.gitlab-ci.yml`

Link to issue: https://gitlab.com/gitlab-org/gitlab-ci/issues/261

/cc @vsizov 

See merge request !228

3 files changed, 35 insertions, 8 deletions

diff --git a/CHANGELOG b/CHANGELOG
index adee745..a7c9289 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -10,6 +10,7 @@ v7.14.0 (unreleased)
   - Fix project API listing returning empty list when first projects are not added to CI
   - Allow to define variables from YAML
   - Added support for CI skipped status
+  - Fix broken yaml error saving
   - Rename type(s) to stage(s)
   
 v7.13.1
diff --git a/app/models/commit.rb b/app/models/commit.rb
index c7e2d8b..68057b7 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -161,15 +161,13 @@ class Commit < ActiveRecord::Base
   end
 
   def status
-    if skip_ci? || builds.none?
+    if skip_ci?
       return 'skipped'
-    end
-
-    if yaml_errors.present?
+    elsif yaml_errors.present?
       return 'failed'
-    end
-
-    if success?
+    elsif builds.none?
+      return 'skipped'
+    elsif success?
       'success'
     elsif pending?
       'pending'
@@ -250,7 +248,7 @@ class Commit < ActiveRecord::Base
   private
 
   def save_yaml_error(error)
-    return unless self.yaml_errors?
+    return if self.yaml_errors?
     self.yaml_errors = error
     save
   end
diff --git a/spec/services/create_commit_service_spec.rb b/spec/services/create_commit_service_spec.rb
index c72ba6b..34e00d5 100644
--- a/spec/services/create_commit_service_spec.rb
+++ b/spec/services/create_commit_service_spec.rb
@@ -76,6 +76,19 @@ describe CreateCommitService do
         
         commit.builds.first.name.should == "staging"
       end
+
+      it "skips builds creation if there is [ci skip] tag in commit message and yaml is invalid" do
+        commits = [{message: "some message[ci skip]"}]
+        commit = service.execute(project,
+                                 ref: 'refs/tags/0_1',
+                                 before: '00000000',
+                                 after: '31das312',
+                                 commits: commits,
+                                 ci_yaml_file: "invalid: file"
+        )
+        commit.builds.any?.should be_false
+        commit.status.should == "skipped"
+      end
     end
 
     it "skips build creation if there are already builds" do
@@ -98,5 +111,20 @@ describe CreateCommitService do
       )
       commit.builds.count(:all).should == 2
     end
+
+    it "creates commit with failed status if yaml is invalid" do
+      commits = [{message: "some message"}]
+
+      commit = service.execute(project,
+                               ref: 'refs/tags/0_1',
+                               before: '00000000',
+                               after: '31das312',
+                               commits: commits,
+                               ci_yaml_file: "invalid: file"
+      )
+
+      commit.status.should == "failed"
+      commit.builds.any?.should be_false
+    end
   end
 end