From b82de0f0087a91165a7306fa9fe11a0cbc48fcd5 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 21 Mar 2018 12:12:45 +0100
Subject: [PATCH] Reduce stage seeds coupling between pipeline and YAML

This is a first step to decouple pipeline from YAML processing. It
reduces the coupling by removing some methods that introduce coupling
and by moving logic into separate chain element that is being used to
populate pipelines with stages and builds.
---
 app/models/ci/pipeline.rb                     |  8 ------
 lib/gitlab/ci/pipeline/chain/populate.rb      |  8 ++++--
 .../ci/pipeline/chain/validate/config.rb      |  4 ---
 .../gitlab/ci/pipeline/chain/populate_spec.rb | 26 +++++++++++++++++++
 .../ci/pipeline/chain/validate/config_spec.rb | 22 ----------------
 spec/models/ci/pipeline_spec.rb               | 22 ----------------
 6 files changed, 32 insertions(+), 58 deletions(-)

diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 8889195ec24..6b38c2eff98 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -365,18 +365,10 @@ module Ci
       @stage_seeds ||= config_processor.stage_seeds(self)
     end
 
-    def seeds_size
-      @seeds_size ||= stage_seeds.sum(&:size)
-    end
-
     def has_kubernetes_active?
       project.deployment_platform&.active?
     end
 
-    def has_stage_seeds?
-      stage_seeds.any?
-    end
-
     def has_warnings?
       builds.latest.failed_but_allowed.any?
     end
diff --git a/lib/gitlab/ci/pipeline/chain/populate.rb b/lib/gitlab/ci/pipeline/chain/populate.rb
index 94d9b44b257..989cb8312ba 100644
--- a/lib/gitlab/ci/pipeline/chain/populate.rb
+++ b/lib/gitlab/ci/pipeline/chain/populate.rb
@@ -21,15 +21,19 @@ module Gitlab
               pipeline.stages << seed.to_resource
             end
 
+            if pipeline.stages.none?
+              return error('No stages / jobs for this pipeline.')
+            end
+
             if pipeline.invalid?
-              error('Failed to build the pipeline!')
+              return error('Failed to build the pipeline!')
             end
 
             raise Populate::PopulateError if pipeline.persisted?
           end
 
           def break?
-            pipeline.invalid?
+            pipeline.errors.any?
           end
         end
       end
diff --git a/lib/gitlab/ci/pipeline/chain/validate/config.rb b/lib/gitlab/ci/pipeline/chain/validate/config.rb
index 075504bcce5..655a6d0b3e7 100644
--- a/lib/gitlab/ci/pipeline/chain/validate/config.rb
+++ b/lib/gitlab/ci/pipeline/chain/validate/config.rb
@@ -18,10 +18,6 @@ module Gitlab
 
                 return error(@pipeline.yaml_errors)
               end
-
-              unless @pipeline.has_stage_seeds?
-                return error('No stages / jobs for this pipeline.')
-              end
             end
 
             def break?
diff --git a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
index f1fc7acb969..115c76aff2d 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/populate_spec.rb
@@ -42,6 +42,32 @@ describe Gitlab::Ci::Pipeline::Chain::Populate do
     end
   end
 
+  context 'when pipeline is empty' do
+    let(:config) do
+      { rspec: {
+          script: 'ls',
+          only: ['something']
+      } }
+    end
+
+    let(:pipeline) do
+      build(:ci_pipeline, project: project, config: config)
+    end
+
+    before do
+      step.perform!
+    end
+
+    it 'breaks the chain' do
+      expect(step.break?).to be true
+    end
+
+    it 'appends an error about missing stages' do
+      expect(pipeline.errors.to_a)
+        .to include 'No stages / jobs for this pipeline.'
+    end
+  end
+
   context 'when pipeline has validation errors' do
     let(:pipeline) do
       build(:ci_pipeline, project: project, ref: nil)
diff --git a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb
index 5c12c6e6392..c53294d091c 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/validate/config_spec.rb
@@ -76,28 +76,6 @@ describe Gitlab::Ci::Pipeline::Chain::Validate::Config do
     end
   end
 
-  context 'when pipeline has no stages / jobs' do
-    let(:config) do
-      { rspec: {
-          script: 'ls',
-          only: ['something']
-      } }
-    end
-
-    let(:pipeline) do
-      build(:ci_pipeline, project: project, config: config)
-    end
-
-    it 'appends an error about missing stages' do
-      expect(pipeline.errors.to_a)
-        .to include 'No stages / jobs for this pipeline.'
-    end
-
-    it 'breaks the chain' do
-      expect(step.break?).to be true
-    end
-  end
-
   context 'when pipeline contains configuration validation errors' do
     let(:config) { { rspec: {} } }
 
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 0ceb25153ab..3d8047ca3ac 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -253,14 +253,6 @@ describe Ci::Pipeline, :mailer do
       end
     end
 
-    describe '#seeds_size' do
-      let(:pipeline) { build(:ci_pipeline_with_one_job) }
-
-      it 'returns number of jobs in stage seeds' do
-        expect(pipeline.seeds_size).to eq 1
-      end
-    end
-
     describe '#legacy_stages' do
       subject { pipeline.legacy_stages }
 
@@ -590,20 +582,6 @@ describe Ci::Pipeline, :mailer do
     end
   end
 
-  describe '#has_stage_seeds?' do
-    context 'when pipeline has stage seeds' do
-      subject { build(:ci_pipeline_with_one_job) }
-
-      it { is_expected.to have_stage_seeds }
-    end
-
-    context 'when pipeline does not have stage seeds' do
-      subject { create(:ci_pipeline_without_jobs) }
-
-      it { is_expected.not_to have_stage_seeds }
-    end
-  end
-
   describe '#has_warnings?' do
     subject { pipeline.has_warnings? }