Fix failures

app/models/ci/legacy_stage.rb

@@ -30,7 +30,11 @@ module Ci
     end
 
     def status
-      @status ||= statuses.latest.status
+      @status ||=
+        Gitlab::Ci::Status::GroupedStatuses
+          .new(statuses.latest)
+          .one[:status]
+
     end
 
     def detailed_status(current_user)
@@ -52,8 +56,11 @@ module Ci
     end
 
     def has_warnings?
-      if @warnings.is_a?(Integer)
+      case @warnings
+      when Integer
         @warnings > 0
+      when TrueClass, FalseClass
+        @warnings
       else
         statuses.latest.failed_but_allowed.any?
       end

app/models/commit_status.rb

@@ -29,6 +29,14 @@ class CommitStatus < ApplicationRecord
     where(allow_failure: true, status: [:failed, :canceled])
   end
 
+  scope :exclude_ignored, -> do
+    # 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
+
   scope :latest, -> { where(retried: [false, nil]) }
   scope :retried, -> { where(retried: true) }
   scope :ordered, -> { order(:name) }

app/models/concerns/has_status.rb

@@ -58,6 +58,12 @@ module HasStatus
       all.pluck(legacy_status_sql).first
     end
 
+    def slow_composite_status
+      Gitlab::Ci::Status::GroupedStatuses
+        .new(all)
+        .one[:status]
+    end
+
     def started_at
       all.minimum(:started_at)
     end

lib/gitlab/ci/status/composite_status.rb

@@ -4,27 +4,25 @@ module Gitlab
   module Ci
     module Status
       class CompositeStatus
+        attr_reader :warnings
+
         def initialize(all_statuses)
-          @warnings = false
+          @warnings = 0
           @status_set = Set.new
-          
+
           build_status_set(all_statuses)
         end
 
         def status
           case
-          when only_of?(:skipped) && warnings?
-            :success
-          when only_of?(:skipped)
-            :skipped
-          when only_of?(:success)
+          when none? || only_of?(:skipped)
+            warnings? ? :success : :skipped
+          when only_of?(:success, :skipped)
             :success
           when only_of?(:created)
             :created
           when only_of?(:preparing)
             :preparing
-          when only_of?(:success, :skipped)
-            :success
           when only_of?(:success, :skipped, :canceled)
             :canceled
           when only_of?(:created, :skipped, :pending)
@@ -45,11 +43,15 @@ module Gitlab
         end
 
         def warnings?
-          @warnings
+          @warnings > 0
         end
 
         private
 
+        def none?
+          @status_set.empty?
+        end
+
         def any_of?(*names)
           names.any? { |name| @status_set.include?(name) }
         end
@@ -62,7 +64,7 @@ module Gitlab
         def build_status_set(all_statuses)
           all_statuses.each do |status|
             if status[:allow_failure] && HasStatus::WARNING_STATUSES.include?(status[:status])
-              @warnings = true
+              @warnings += 1
             else
               @status_set.add(status[:status].to_sym)
             end

lib/gitlab/ci/status/grouped_statuses.rb

@@ -70,7 +70,8 @@ module Gitlab
 
           key.merge(
             status: composite_status.status.to_s,
-            warnings: composite_status.warnings?)
+            warnings: composite_status.warnings?
+          )
         end
       end
     end

spec/models/ci/legacy_stage_spec.rb

@@ -216,7 +216,7 @@ describe Ci::LegacyStage do
     context 'when stage has warnings' do
       context 'when using memoized warnings flag' do
         context 'when there are warnings' do
-          let(:stage) { build(:ci_stage, warnings: 2) }
+          let(:stage) { build(:ci_stage, warnings: true) }
 
           it 'returns true using memoized value' do
             expect(stage).not_to receive(:statuses)
@@ -225,7 +225,7 @@ describe Ci::LegacyStage do
         end
 
         context 'when there are no warnings' do
-          let(:stage) { build(:ci_stage, warnings: 0) }
+          let(:stage) { build(:ci_stage, warnings: false) }
 
           it 'returns false using memoized value' do
             expect(stage).not_to receive(:statuses)
@@ -234,7 +234,7 @@ describe Ci::LegacyStage do
         end
 
         context 'when number of warnings is not a valid value' do
-          let(:stage) { build(:ci_stage, warnings: true) }
+          let(:stage) { build(:ci_stage, warnings: 'aa') }
 
           it 'calculates statuses using database queries' do
             expect(stage).to receive(:statuses).and_call_original

spec/models/ci/pipeline_spec.rb

@@ -2301,36 +2301,38 @@ describe Ci::Pipeline, :mailer do
   describe '#update_status' do
     context 'when pipeline is empty' do
       it 'updates does not change pipeline status' do
-        expect(pipeline.statuses.latest.status).to be_nil
+        expect(pipeline.statuses.latest.slow_composite_status).to be_nil
 
         expect { pipeline.update_status }
-          .to change { pipeline.reload.status }.to 'skipped'
+          .to change { pipeline.reload.status }
+          .from('created')
+          .to('skipped')
       end
     end
 
     context 'when updating status to pending' do
       before do
-        allow(pipeline)
-          .to receive_message_chain(:statuses, :latest, :status)
-          .and_return(:running)
+        create(:ci_build, pipeline: pipeline, status: :running)
       end
 
       it 'updates pipeline status to running' do
         expect { pipeline.update_status }
-          .to change { pipeline.reload.status }.to 'running'
+          .to change { pipeline.reload.status }
+          .from('created')
+          .to('running')
       end
     end
 
     context 'when updating status to scheduled' do
       before do
-        allow(pipeline)
-          .to receive_message_chain(:statuses, :latest, :status)
-          .and_return(:scheduled)
+        create(:ci_build, pipeline: pipeline, status: :scheduled)
       end
 
       it 'updates pipeline status to scheduled' do
         expect { pipeline.update_status }
-          .to change { pipeline.reload.status }.to 'scheduled'
+          .to change { pipeline.reload.status }
+          .from('created')
+          .to('scheduled')
       end
     end
 

spec/models/ci/stage_spec.rb

@@ -130,7 +130,7 @@ describe Ci::Stage, :models do
     context 'when statuses status was not recognized' do
       before do
         allow(stage)
-          .to receive_message_chain(:statuses, :latest, :status)
+          .to receive(:latest_stage_status)
           .and_return(:unknown)
       end
 

spec/models/commit_status_spec.rb

@@ -298,7 +298,7 @@ describe CommitStatus do
       end
 
       it 'returns a correct compound status' do
-        expect(described_class.all.status).to eq 'running'
+        expect(described_class.all.slow_composite_status).to eq 'running'
       end
     end
 
@@ -308,7 +308,7 @@ describe CommitStatus do
       end
 
       it 'returns status that indicates success' do
-        expect(described_class.all.status).to eq 'success'
+        expect(described_class.all.slow_composite_status).to eq 'success'
       end
     end
 
@@ -319,7 +319,7 @@ describe CommitStatus do
       end
 
       it 'returns status according to the scope' do
-        expect(described_class.latest.status).to eq 'success'
+        expect(described_class.latest.slow_composite_status).to eq 'success'
       end
     end
   end