Allow to defined per-job allow_failure parameterallow-per-job-failure

It allows to ignore status of specific job when computed for commit

14 files changed, 151 insertions, 81 deletions

diff --git a/CHANGELOG b/CHANGELOG
index e64281d..516e9ba 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -9,6 +9,7 @@ v7.13.0
   - Redirect back after authorization
   - Change favicon
   - Refactoring: Get rid of private_token usage in the frontend.
+  - Allow to specify allow_failure for job
 
 v7.12.2
   - Revert: Runner without tag should pick builds without tag only
diff --git a/app/models/build.rb b/app/models/build.rb
index 725da75..7a01ecf 100644
--- a/app/models/build.rb
+++ b/app/models/build.rb
@@ -2,22 +2,23 @@
 #
 # Table name: builds
 #
-#  id          :integer          not null, primary key
-#  project_id  :integer
-#  status      :string(255)
-#  finished_at :datetime
-#  trace       :text
-#  created_at  :datetime
-#  updated_at  :datetime
-#  started_at  :datetime
-#  runner_id   :integer
-#  commit_id   :integer
-#  coverage    :float
-#  commands    :text
-#  options     :text
-#  job_id      :integer
-#  name        :string(255)
-#  deploy      :boolean          default(FALSE)
+#  id            :integer          not null, primary key
+#  project_id    :integer
+#  status        :string(255)
+#  finished_at   :datetime
+#  trace         :text
+#  created_at    :datetime
+#  updated_at    :datetime
+#  started_at    :datetime
+#  runner_id     :integer
+#  commit_id     :integer
+#  coverage      :float
+#  commands      :text
+#  job_id        :integer
+#  name          :string(255)
+#  deploy        :boolean          default(FALSE)
+#  options       :text
+#  allow_failure :boolean          default(FALSE), not null
 #
 
 class Build < ActiveRecord::Base
@@ -75,6 +76,7 @@ class Build < ActiveRecord::Base
       new_build.commit_id = build.commit_id
       new_build.project_id = build.project_id
       new_build.name = build.name
+      new_build.allow_failure = build.allow_failure
       new_build.save
       new_build
     end
@@ -153,6 +155,10 @@ class Build < ActiveRecord::Base
     canceled? || success? || failed?
   end
 
+  def ignored?
+    failed? && allow_failure?
+  end
+
   def timeout
     project.timeout
   end
diff --git a/app/models/commit.rb b/app/models/commit.rb
index d9774d3..3680220 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -111,7 +111,8 @@ class Commit < ActiveRecord::Base
         name: build_attrs[:name],
         commands: build_attrs[:script],
         tag_list: build_attrs[:tags],
-        options: build_attrs[:options]
+        options: build_attrs[:options],
+        allow_failure: build_attrs[:allow_failure]
       })
     end
   end
@@ -149,6 +150,7 @@ class Commit < ActiveRecord::Base
         commands: build_attrs[:script],
         tag_list: build_attrs[:tags],
         options: build_attrs[:options],
+        allow_failure: build_attrs[:allow_failure],
         deploy: true
       })
     end
@@ -186,7 +188,7 @@ class Commit < ActiveRecord::Base
 
   def success?
     builds_without_retry.all? do |build|
-      build.success?
+      build.success? || build.ignored?
     end
   end
 
diff --git a/app/views/builds/_build.html.haml b/app/views/builds/_build.html.haml
index b71111b..f2a340d 100644
--- a/app/views/builds/_build.html.haml
+++ b/app/views/builds/_build.html.haml
@@ -17,11 +17,13 @@
       #{build.short_sha}
   %td
     = build.name
-    - if build.tags.any?
-      - build.tag_list.each do |tag|
-        %span.label.label-primary
-          = tag
-
+    .pull-right
+      - if build.tags.any?
+        - build.tag_list.each do |tag|
+          %span.label.label-primary
+            = tag
+      - if build.allow_failure
+        %span.label.label-danger allowed to fail
 
   %td.duration
     - if build.duration
diff --git a/db/migrate/20150707134456_add_allow_failure_to_builds.rb b/db/migrate/20150707134456_add_allow_failure_to_builds.rb
new file mode 100644
index 0000000..cc3da34
--- /dev/null
+++ b/db/migrate/20150707134456_add_allow_failure_to_builds.rb
@@ -0,0 +1,5 @@
+class AddAllowFailureToBuilds < ActiveRecord::Migration
+  def change
+    add_column :builds, :allow_failure, :boolean, default: false, null: false
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index c5e59b2..6b88c7f 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20150706103229) do
+ActiveRecord::Schema.define(version: 20150707134456) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -30,8 +30,9 @@ ActiveRecord::Schema.define(version: 20150706103229) do
     t.text     "commands"
     t.integer  "job_id"
     t.string   "name"
-    t.boolean  "deploy",      default: false
+    t.boolean  "deploy",        default: false
     t.text     "options"
+    t.boolean  "allow_failure", default: false, null: false
   end
 
   add_index "builds", ["commit_id"], name: "index_builds_on_commit_id", using: :btree
diff --git a/lib/gitlab_ci_yaml_processor.rb b/lib/gitlab_ci_yaml_processor.rb
index 00fceda..b055d81 100644
--- a/lib/gitlab_ci_yaml_processor.rb
+++ b/lib/gitlab_ci_yaml_processor.rb
@@ -84,6 +84,7 @@ class GitlabCiYamlProcessor
       name: name,
       only: job[:only],
       except: job[:except],
+      allow_failure: job[:allow_failure] || false,
       options: {
         image: job[:image] || @image,
         services: job[:services] || @services
@@ -133,7 +134,7 @@ class GitlabCiYamlProcessor
 
   def validate_job!(name, job)
     job.keys.each do |key|
-      unless [:tags, :script, :only, :except, :type, :image, :services].include? key
+      unless [:tags, :script, :only, :except, :type, :image, :services, :allow_failure].include? key
         raise ValidationError, "#{name}: unknown parameter #{key}"
       end
     end
@@ -159,5 +160,9 @@ class GitlabCiYamlProcessor
     if job[:except] && !job[:except].is_a?(Array)
       raise ValidationError, "#{name}: except parameter should be an array"
     end
+
+    if job[:allow_failure] && !job[:allow_failure].in?([true, false])
+      raise ValidationError, "#{name}: allow_failure parameter should be an boolean"
+    end
   end
 end
diff --git a/spec/factories/builds.rb b/spec/factories/builds.rb
index bddf6eb..af63bbd 100644
--- a/spec/factories/builds.rb
+++ b/spec/factories/builds.rb
@@ -2,22 +2,23 @@
 #
 # Table name: builds
 #
-#  id          :integer          not null, primary key
-#  project_id  :integer
-#  status      :string(255)
-#  finished_at :datetime
-#  trace       :text
-#  created_at  :datetime
-#  updated_at  :datetime
-#  started_at  :datetime
-#  runner_id   :integer
-#  commit_id   :integer
-#  coverage    :float
-#  commands    :text
-#  options     :text
-#  job_id      :integer
-#  name        :string(255)
-#  deploy      :boolean          default(FALSE)
+#  id            :integer          not null, primary key
+#  project_id    :integer
+#  status        :string(255)
+#  finished_at   :datetime
+#  trace         :text
+#  created_at    :datetime
+#  updated_at    :datetime
+#  started_at    :datetime
+#  runner_id     :integer
+#  commit_id     :integer
+#  coverage      :float
+#  commands      :text
+#  job_id        :integer
+#  name          :string(255)
+#  deploy        :boolean          default(FALSE)
+#  options       :text
+#  allow_failure :boolean          default(FALSE), not null
 #
 
 # Read about factories at https://github.com/thoughtbot/factory_girl
diff --git a/spec/factories/commits.rb b/spec/factories/commits.rb
index 1f4a383..4a411ee 100644
--- a/spec/factories/commits.rb
+++ b/spec/factories/commits.rb
@@ -49,5 +49,17 @@ FactoryGirl.define do
         ci_yaml_file: File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
       }
     end
+
+    factory :commit_with_one_job do
+      after(:create) do |commit, evaluator|
+        commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { script: "ls" }})
+      end
+    end
+
+    factory :commit_with_two_jobs do
+      after(:create) do |commit, evaluator|
+        commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { script: "ls" }, spinach: { script: "ls" }})
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab_ci_yaml_processor_spec.rb b/spec/lib/gitlab_ci_yaml_processor_spec.rb
index 9c9fde9..5f202b2 100644
--- a/spec/lib/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/gitlab_ci_yaml_processor_spec.rb
@@ -18,7 +18,8 @@ describe GitlabCiYamlProcessor do
         only: nil,
         script: "pwd\nrspec",
         tags: [],
-        options: {}
+        options: {},
+        allow_failure: false
       }
     end
 
@@ -71,7 +72,7 @@ describe GitlabCiYamlProcessor do
     it "returns builds if no branch specified" do
       config = YAML.dump({
         before_script: ["pwd"],
-        rspec: {script: "rspec", type: "deploy"}
+        rspec: {script: "rspec", type: "deploy", allow_failure: true}
       })
 
       config_processor = GitlabCiYamlProcessor.new(config)
@@ -83,7 +84,8 @@ describe GitlabCiYamlProcessor do
         only: nil,
         script: "pwd\nrspec",
         tags: [],
-        options: {}
+        options: {},
+        allow_failure: true
       }
     end
 
@@ -153,7 +155,8 @@ describe GitlabCiYamlProcessor do
         options: {
           image: "ruby:2.1",
           services: ["mysql"]
-        }
+        },
+        allow_failure: false
       }
     end
 
@@ -177,7 +180,8 @@ describe GitlabCiYamlProcessor do
         options: {
           image: "ruby:2.5",
           services: ["postgresql"]
-        }
+        },
+        allow_failure: false
       }
     end
   end
@@ -256,5 +260,12 @@ describe GitlabCiYamlProcessor do
         GitlabCiYamlProcessor.new(config)
       end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Please define at least one job")
     end
+
+    it "returns errors if job allow_failure parameter is not an boolean" do
+      config = YAML.dump({rspec: {script: "test", allow_failure: "string"}})
+      expect do
+        GitlabCiYamlProcessor.new(config)
+      end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: allow_failure parameter should be an boolean")
+    end
   end
 end
\ No newline at end of file
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index 8bc9807..7e1c7e9 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -2,22 +2,23 @@
 #
 # Table name: builds
 #
-#  id          :integer          not null, primary key
-#  project_id  :integer
-#  status      :string(255)
-#  finished_at :datetime
-#  trace       :text
-#  created_at  :datetime
-#  updated_at  :datetime
-#  started_at  :datetime
-#  runner_id   :integer
-#  commit_id   :integer
-#  coverage    :float
-#  commands    :text
-#  options     :text
-#  job_id      :integer
-#  name        :string(255)
-#  deploy      :boolean          default(FALSE)
+#  id            :integer          not null, primary key
+#  project_id    :integer
+#  status        :string(255)
+#  finished_at   :datetime
+#  trace         :text
+#  created_at    :datetime
+#  updated_at    :datetime
+#  started_at    :datetime
+#  runner_id     :integer
+#  commit_id     :integer
+#  coverage      :float
+#  commands      :text
+#  job_id        :integer
+#  name          :string(255)
+#  deploy        :boolean          default(FALSE)
+#  options       :text
+#  allow_failure :boolean          default(FALSE), not null
 #
 
 require 'spec_helper'
@@ -126,6 +127,42 @@ describe Build do
     end
   end
 
+  describe :ignored? do
+    subject { build.ignored? }
+
+    context 'if build is not allowed to fail' do
+      before { build.allow_failure = false }
+
+      context 'and build.status is success' do
+        before { build.status = 'success' }
+
+        it { should be_false }
+      end
+
+      context 'and build.status is failed' do
+        before { build.status = 'failed' }
+
+        it { should be_false }
+      end
+    end
+
+    context 'if build is allowed to fail' do
+      before { build.allow_failure = true }
+
+      context 'and build.status is success' do
+        before { build.status = 'success' }
+
+        it { should be_false }
+      end
+
+      context 'and build.status is failed' do
+        before { build.status = 'failed' }
+
+        it { should be_true }
+      end
+    end
+  end
+
   describe :trace do
     subject { build.trace_html }
 
diff --git a/spec/models/project_services/hip_chat_message_spec.rb b/spec/models/project_services/hip_chat_message_spec.rb
index 1afe3ed..f1ad875 100644
--- a/spec/models/project_services/hip_chat_message_spec.rb
+++ b/spec/models/project_services/hip_chat_message_spec.rb
@@ -6,12 +6,7 @@ describe HipChatMessage do
   let(:project) { FactoryGirl.create(:project) }
 
   context "One build" do
-    let(:commit) do
-      commit = FactoryGirl.create(:commit, project: project)
-      commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { script: 'pwd' }})
-      commit.save
-      commit
-    end
+    let(:commit) { FactoryGirl.create(:commit_with_one_job, project: project) }
 
     let(:build) do 
       commit.create_builds
@@ -42,7 +37,7 @@ describe HipChatMessage do
   end
 
   context "Several builds" do
-    let(:commit) {commit = FactoryGirl.create(:commit, project: project)}
+    let(:commit) { FactoryGirl.create(:commit_with_two_jobs, project: project) }
 
     let(:build) do 
       commit.builds.first
diff --git a/spec/models/project_services/slack_message_spec.rb b/spec/models/project_services/slack_message_spec.rb
index f60f89c..88e0f37 100644
--- a/spec/models/project_services/slack_message_spec.rb
+++ b/spec/models/project_services/slack_message_spec.rb
@@ -6,12 +6,7 @@ describe SlackMessage do
   let(:project) { FactoryGirl.create :project }
 
   context "One build" do
-    let(:commit) do
-      commit = FactoryGirl.create(:commit, project: project)
-      commit.push_data[:ci_yaml_file] = YAML.dump({rspec: { script: "ls" }})
-      commit.save
-      commit
-    end
+    let(:commit) { FactoryGirl.create(:commit_with_one_job, project: project) }
 
     let(:build) do 
       commit.create_builds
@@ -48,11 +43,7 @@ describe SlackMessage do
   end
 
   context "Several builds" do
-    let(:commit) {commit = FactoryGirl.create(:commit, project: project)}
-
-    let(:build) do 
-      commit.builds.first
-    end
+    let(:commit) { FactoryGirl.create(:commit_with_two_jobs, project: project) }
 
     context 'when all matrix builds succeeded' do
       let(:color) { 'good' }
diff --git a/spec/support/gitlab_stubs/gitlab_ci.yml b/spec/support/gitlab_stubs/gitlab_ci.yml
index 4f0f106..4095d6a 100644
--- a/spec/support/gitlab_stubs/gitlab_ci.yml
+++ b/spec/support/gitlab_stubs/gitlab_ci.yml
@@ -17,6 +17,7 @@ rspec:
 
 spinach:
   script: "rake spinach"
+  allow_failure: true
   tags: 
     - ruby
     - mysql