From c4d5b231ed5888367912db000ebd8e2856709bc0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 6 Mar 2017 14:43:32 +0100
Subject: [PATCH] Ignore optional  actions when calculating warnings

---
 app/models/commit_status.rb       |  6 ++++--
 spec/models/ci/build_spec.rb      | 10 +++++++++-
 spec/models/commit_status_spec.rb | 19 +++++++++++++++++++
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 44b2c1fb1db..7e23e14794f 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -25,11 +25,13 @@ class CommitStatus < ActiveRecord::Base
   end
 
   scope :failed_but_allowed, -> do
-    where(allow_failure: true, status: [:failed, :canceled, :manual])
+    where(allow_failure: true, status: [:failed, :canceled])
   end
 
   scope :exclude_ignored, -> do
-    # We want to ignore failed but allowed to fail jobs
+    # We want to ignore failed but allowed to fail jobs.
+    #
+    # TODO, we also skip ignored optional manual actions.
     where("allow_failure = ? OR status IN (?)",
       false, all_state_names - [:failed, :canceled, :manual])
   end
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 30870873440..900fd69e32f 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -611,13 +611,21 @@ describe Ci::Build, :models do
         it { is_expected.to be_falsey }
       end
 
-      context 'and build.status is failed' do
+      context 'and build status is failed' do
         before do
           build.status = 'failed'
         end
 
         it { is_expected.to be_truthy }
       end
+
+      context 'when build is a manual action' do
+        before do
+          build.status = 'manual'
+        end
+
+        it { is_expected.to be_falsey }
+      end
     end
   end
 
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index da339b143a8..fee3c001e22 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -195,6 +195,25 @@ describe CommitStatus, :models do
     end
   end
 
+  describe '.failed_but_allowed' do
+    subject { described_class.failed_but_allowed.order(:id) }
+
+    let(:statuses) do
+       [create_status(allow_failure: true, status: 'success'),
+        create_status(allow_failure: true, status: 'failed'),
+        create_status(allow_failure: false, status: 'success'),
+        create_status(allow_failure: false, status: 'failed'),
+        create_status(allow_failure: true, status: 'canceled'),
+        create_status(allow_failure: false, status: 'canceled'),
+        create_status(allow_failure: true, status: 'manual'),
+        create_status(allow_failure: false, status: 'manual')]
+    end
+
+    it 'returns statuses without what we want to ignore' do
+      is_expected.to eq(statuses.values_at(1, 4))
+    end
+  end
+
   describe '#before_sha' do
     subject { commit_status.before_sha }