Test composite statusesruby-composite-status

3 files changed, 70 insertions, 7 deletions

diff --git a/app/models/concerns/has_status.rb b/app/models/concerns/has_status.rb
index 12034a34cef..0d06985c245 100644
--- a/app/models/concerns/has_status.rb
+++ b/app/models/concerns/has_status.rb
@@ -10,8 +10,8 @@ module HasStatus
   ACTIVE_STATUSES = %w[preparing pending running].freeze
   COMPLETED_STATUSES = %w[success failed canceled skipped].freeze
   ORDERED_STATUSES = %w[failed preparing pending running manual scheduled canceled success skipped created].freeze
-  WARNING_IF_ALLOW_FAILURE_STATUSES = %w[manual failed canceled].to_set.freeze
-  IGNORED_IF_ALLOW_FAILURE_STATUSES = %w[failed canceled].to_set.freeze
+  PASSED_WITH_WARNINGS_STATUSES = %w[failed canceled].to_set.freeze
+  EXCLUDE_IGNORED_STATUSES = %w[manual failed canceled].to_set.freeze
   STATUSES_ENUM = { created: 0, pending: 1, running: 2, success: 3,
                     failed: 4, canceled: 5, skipped: 6, manual: 7,
                     scheduled: 8, preparing: 9 }.freeze
diff --git a/lib/gitlab/ci/status/composite_status.rb b/lib/gitlab/ci/status/composite_status.rb
index fd960fa8663..ec360082112 100644
--- a/lib/gitlab/ci/status/composite_status.rb
+++ b/lib/gitlab/ci/status/composite_status.rb
@@ -7,6 +7,7 @@ module Gitlab
         attr_reader :warnings
 
         def initialize(all_statuses)
+          @count = 0
           @warnings = 0
           @status_set = Set.new
 
@@ -15,6 +16,8 @@ module Gitlab
 
         def status
           case
+          when @count.zero?
+            nil
           when none? || only_of?(:skipped)
             warnings? ? :success : :skipped
           when only_of?(:success, :skipped)
@@ -63,13 +66,18 @@ module Gitlab
 
         def build_status_set(all_statuses)
           all_statuses.each do |status|
-            if status[:allow_failure] && HasStatus::WARNING_IF_ALLOW_FAILURE_STATUSES.include?(status[:status])
-              @warnings += 1
-            end
+            @count += 1
+            if status[:allow_failure]
+              if HasStatus::PASSED_WITH_WARNINGS_STATUSES.include?(status[:status])
+                @warnings += 1
+              end
 
-            if !status[:allow_failure] || !HasStatus::IGNORED_IF_ALLOW_FAILURE_STATUSES.include?(status[:status])
-              @status_set.add(status[:status].to_sym)
+              if HasStatus::EXCLUDE_IGNORED_STATUSES.include?(status[:status])
+                next
+              end
             end
+
+            @status_set.add(status[:status].to_sym)
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/status/composite_status_spec.rb b/spec/lib/gitlab/ci/status/composite_status_spec.rb
new file mode 100644
index 00000000000..799bef666c3
--- /dev/null
+++ b/spec/lib/gitlab/ci/status/composite_status_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Status::CompositeStatus do
+  set(:pipeline) { create(:ci_pipeline) }
+
+  let(:composite_status) { described_class.new(all_statuses) }
+
+  before(:all) do
+    @statuses = HasStatus::STATUSES_ENUM.map do |status, idx|
+      [status, create(:ci_build, pipeline: pipeline, status: status, importing: true)]
+    end.to_h
+
+    @statuses_with_allow_failure = HasStatus::STATUSES_ENUM.map do |status, idx|
+      [status, create(:ci_build, pipeline: pipeline, status: status, allow_failure: true, importing: true)]
+    end.to_h
+  end
+
+  describe '#status' do
+    subject { composite_status.status.to_s }
+
+    shared_examples 'compares composite with SQL status' do
+      it 'returns exactly the same result' do
+        is_expected.to eq(Ci::Build.where(id: all_statuses).legacy_status.to_s)
+      end
+    end
+
+    shared_examples 'validate all combinations' do |perms|
+      HasStatus::STATUSES_ENUM.keys.combination(perms).each do |statuses|
+        context "with #{statuses.join(",")}" do
+          it_behaves_like 'compares composite with SQL status' do
+            let(:all_statuses) do
+              statuses.map { |status| @statuses[status] }
+            end
+          end
+
+          HasStatus::STATUSES_ENUM.each do |allow_failure_status, _|
+            context "and allow_failure #{allow_failure_status}" do
+              it_behaves_like 'compares composite with SQL status' do
+                let(:all_statuses) do
+                  statuses.map { |status| @statuses[status] } +
+                    [@statuses_with_allow_failure[allow_failure_status]]
+                end
+              end
+            end
+          end
+        end
+      end
+    end
+
+    it_behaves_like 'validate all combinations', 0
+    it_behaves_like 'validate all combinations', 1
+    it_behaves_like 'validate all combinations', 2
+    #it_behaves_like 'validate all combinations', 3
+  end
+end