Merge branch 'all-skipped-equals-success' into 'master'

Skipped jobs should be considered successful

## What does this MR do?

If all jobs in previous stage are all skipped, the next stage should consider previous stage succeeded.

## Why was this MR needed?

Since for now we consider all manual jobs if skipped, should not block the next stage from running.

Closes #22598 

See also #20342 (because this merge request conflicts with it)

See merge request !6604

3 files changed, 62 insertions, 40 deletions

diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index e7864b7ad33..ae185de9ca3 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -39,8 +39,8 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe '#ignored?' do
-    subject { build.ignored? }
+  describe '#failed_but_allowed?' do
+    subject { build.failed_but_allowed? }
 
     context 'when build is not allowed to fail' do
       before do
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index 2f1baff5d66..80c2a1bc7a9 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -7,7 +7,11 @@ describe CommitStatus, models: true do
     create(:ci_pipeline, project: project, sha: project.commit.id)
   end
 
-  let(:commit_status) { create(:commit_status, pipeline: pipeline) }
+  let(:commit_status) { create_status }
+
+  def create_status(args = {})
+    create(:commit_status, args.merge(pipeline: pipeline))
+  end
 
   it { is_expected.to belong_to(:pipeline) }
   it { is_expected.to belong_to(:user) }
@@ -125,32 +129,53 @@ describe CommitStatus, models: true do
   describe '.latest' do
     subject { CommitStatus.latest.order(:id) }
 
-    before do
-      @commit1 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'running'
-      @commit2 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'cc', status: 'pending'
-      @commit3 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'cc', status: 'success'
-      @commit4 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'bb', status: 'success'
-      @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'success'
+    let(:statuses) do
+      [create_status(name: 'aa', ref: 'bb', status: 'running'),
+       create_status(name: 'cc', ref: 'cc', status: 'pending'),
+       create_status(name: 'aa', ref: 'cc', status: 'success'),
+       create_status(name: 'cc', ref: 'bb', status: 'success'),
+       create_status(name: 'aa', ref: 'bb', status: 'success')]
     end
 
     it 'returns unique statuses' do
-      is_expected.to eq([@commit4, @commit5])
+      is_expected.to eq(statuses.values_at(3, 4))
     end
   end
 
   describe '.running_or_pending' do
     subject { CommitStatus.running_or_pending.order(:id) }
 
-    before do
-      @commit1 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: 'bb', status: 'running'
-      @commit2 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'cc', ref: 'cc', status: 'pending'
-      @commit3 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'aa', ref: nil, status: 'success'
-      @commit4 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'dd', ref: nil, status: 'failed'
-      @commit5 = FactoryGirl.create :commit_status, pipeline: pipeline, name: 'ee', ref: nil, status: 'canceled'
+    let(:statuses) do
+      [create_status(name: 'aa', ref: 'bb', status: 'running'),
+       create_status(name: 'cc', ref: 'cc', status: 'pending'),
+       create_status(name: 'aa', ref: nil, status: 'success'),
+       create_status(name: 'dd', ref: nil, status: 'failed'),
+       create_status(name: 'ee', ref: nil, status: 'canceled')]
     end
 
     it 'returns statuses that are running or pending' do
-      is_expected.to eq([@commit1, @commit2])
+      is_expected.to eq(statuses.values_at(0, 1))
+    end
+  end
+
+  describe '.exclude_ignored' do
+    subject { CommitStatus.exclude_ignored.order(:id) }
+
+    let(:statuses) do
+      [create_status(when: 'manual', status: 'skipped'),
+       create_status(when: 'manual', status: 'success'),
+       create_status(when: 'manual', status: 'failed'),
+       create_status(when: 'on_failure', status: 'skipped'),
+       create_status(when: 'on_failure', status: 'success'),
+       create_status(when: 'on_failure', status: 'failed'),
+       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')]
+    end
+
+    it 'returns statuses without what we want to ignore' do
+      is_expected.to eq(statuses.values_at(1, 2, 4, 5, 6, 8, 9))
     end
   end
 
diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb
index e118432d098..87bffbdc54e 100644
--- a/spec/models/concerns/has_status_spec.rb
+++ b/spec/models/concerns/has_status_spec.rb
@@ -1,26 +1,17 @@
 require 'spec_helper'
 
 describe HasStatus do
-  before do
-    @object = Object.new
-    @object.extend(HasStatus::ClassMethods)
-  end
-
   describe '.status' do
-    before do
-      allow(@object).to receive(:all).and_return(CommitStatus.where(id: statuses))
-    end
-
-    subject { @object.status }
+    subject { CommitStatus.status }
 
     shared_examples 'build status summary' do
       context 'all successful' do
-        let(:statuses) { Array.new(2) { create(type, status: :success) } }
+        let!(:statuses) { Array.new(2) { create(type, status: :success) } }
         it { is_expected.to eq 'success' }
       end
 
       context 'at least one failed' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :success), create(type, status: :failed)]
         end
 
@@ -28,7 +19,7 @@ describe HasStatus do
       end
 
       context 'at least one running' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :success), create(type, status: :running)]
         end
 
@@ -36,7 +27,7 @@ describe HasStatus do
       end
 
       context 'at least one pending' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :success), create(type, status: :pending)]
         end
 
@@ -44,7 +35,7 @@ describe HasStatus do
       end
 
       context 'success and failed but allowed to fail' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :success),
            create(type, status: :failed, allow_failure: true)]
         end
@@ -53,12 +44,15 @@ describe HasStatus do
       end
 
       context 'one failed but allowed to fail' do
-        let(:statuses) { [create(type, status: :failed, allow_failure: true)] }
+        let!(:statuses) do
+          [create(type, status: :failed, allow_failure: true)]
+        end
+
         it { is_expected.to eq 'success' }
       end
 
       context 'success and canceled' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :success), create(type, status: :canceled)]
         end
 
@@ -66,7 +60,7 @@ describe HasStatus do
       end
 
       context 'one failed and one canceled' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :failed), create(type, status: :canceled)]
         end
 
@@ -74,7 +68,7 @@ describe HasStatus do
       end
 
       context 'one failed but allowed to fail and one canceled' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :failed, allow_failure: true),
            create(type, status: :canceled)]
         end
@@ -83,7 +77,7 @@ describe HasStatus do
       end
 
       context 'one running one canceled' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :running), create(type, status: :canceled)]
         end
 
@@ -91,14 +85,15 @@ describe HasStatus do
       end
 
       context 'all canceled' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :canceled), create(type, status: :canceled)]
         end
+
         it { is_expected.to eq 'canceled' }
       end
 
       context 'success and canceled but allowed to fail' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :success),
            create(type, status: :canceled, allow_failure: true)]
         end
@@ -107,7 +102,7 @@ describe HasStatus do
       end
 
       context 'one finished and second running but allowed to fail' do
-        let(:statuses) do
+        let!(:statuses) do
           [create(type, status: :success),
            create(type, status: :running, allow_failure: true)]
         end
@@ -118,11 +113,13 @@ describe HasStatus do
 
     context 'ci build statuses' do
       let(:type) { :ci_build }
+
       it_behaves_like 'build status summary'
     end
 
     context 'generic commit statuses' do
       let(:type) { :generic_commit_status }
+
       it_behaves_like 'build status summary'
     end
   end