From 2480701436bf84281e4afd65eb0d4c2d642754b9 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 9 Jul 2016 18:43:26 +0200
Subject: [PATCH] Extend CI job entries fabrication and validation

---
 lib/gitlab/ci/config/node/global.rb           |  1 +
 lib/gitlab/ci/config/node/hidden_job.rb       |  1 +
 lib/gitlab/ci/config/node/job.rb              |  4 +++
 lib/gitlab/ci/config/node/jobs.rb             | 18 ++++++-----
 spec/lib/gitlab/ci/config/node/global_spec.rb |  2 +-
 .../gitlab/ci/config/node/hidden_job_spec.rb  | 10 +++++++
 spec/lib/gitlab/ci/config/node/job_spec.rb    | 10 +++++++
 spec/lib/gitlab/ci/config/node/jobs_spec.rb   | 30 +++++++++----------
 8 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index f59a967b1ef..f5500e27439 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -38,6 +38,7 @@ module Gitlab
 
           def initialize(*)
             super
+
             @global = self
           end
 
diff --git a/lib/gitlab/ci/config/node/hidden_job.rb b/lib/gitlab/ci/config/node/hidden_job.rb
index 6a559ee8c04..073044b66f8 100644
--- a/lib/gitlab/ci/config/node/hidden_job.rb
+++ b/lib/gitlab/ci/config/node/hidden_job.rb
@@ -10,6 +10,7 @@ module Gitlab
 
           validations do
             validates :config, type: Hash
+            validates :config, presence: true
           end
 
           def relevant?
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 4a9cc28d763..28a3f407605 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -8,6 +8,10 @@ module Gitlab
         class Job < Entry
           include Configurable
 
+          validations do
+            validates :config, presence: true
+          end
+
           node :stage, Stage,
             description: 'Pipeline stage this job will be executed into.'
 
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index f6acc25e4fb..7a164b69aff 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -30,17 +30,19 @@ module Gitlab
           private
 
           def create(name, config)
-            job_node(name).new(config, job_attributes(name))
+            Node::Factory.new(job_node(name))
+              .value(config || {})
+              .with(key: name, parent: self, global: @global)
+              .with(description: "#{name} job definition.")
+              .create!
           end
 
           def job_node(name)
-            name.to_s.start_with?('.') ? Node::HiddenJob : Node::Job
-          end
-
-          def job_attributes(name)
-            @attributes.merge(key: name,
-                              parent: self,
-                              description: "#{name} job definition.")
+            if name.to_s.start_with?('.')
+              Node::HiddenJob
+            else
+              Node::Job
+            end
           end
         end
       end
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 10e5f05a2d5..3e1c197fe61 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -137,7 +137,7 @@ describe Gitlab::Ci::Config::Node::Global do
     end
 
     context 'when most of entires not defined' do
-      let(:hash) { { cache: { key: 'a' }, rspec: {} } }
+      let(:hash) { { cache: { key: 'a' }, rspec: { script: %w[ls] } } }
       before { global.process! }
 
       describe '#nodes' do
diff --git a/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
index ab865c3522e..cc44e2cc054 100644
--- a/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb
@@ -31,6 +31,16 @@ describe Gitlab::Ci::Config::Node::HiddenJob do
           end
         end
       end
+
+      context 'when config is empty' do
+        let(:config) { {} }
+
+        describe '#valid' do
+          it 'is invalid' do
+            expect(entry).not_to be_valid
+          end
+        end
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 2a4296448fb..f841936ee6b 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -39,6 +39,16 @@ describe Gitlab::Ci::Config::Node::Job do
           end
         end
       end
+
+      context 'when config is empty' do
+        let(:config) { {} }
+
+        describe '#valid' do
+          it 'is invalid' do
+            expect(entry).not_to be_valid
+          end
+        end
+      end
     end
   end
 
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 52018958dcf..b0171174157 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -4,6 +4,11 @@ describe Gitlab::Ci::Config::Node::Jobs do
   let(:entry) { described_class.new(config, global: spy) }
 
   describe 'validations' do
+    before do
+      entry.process!
+      entry.validate!
+    end
+
     context 'when entry config value is correct' do
       let(:config) { { rspec: { script: 'rspec' } } }
 
@@ -25,25 +30,20 @@ describe Gitlab::Ci::Config::Node::Jobs do
           end
         end
 
-        context 'when no visible jobs present' do
-          let(:config) { { '.hidden'.to_sym => {} } }
+        context 'when job is unspecified' do
+          let(:config) { { rspec: nil } }
 
-          context 'when not processed' do
-            it 'is valid' do
-              expect(entry.errors).to be_empty
-            end
+          it 'is not valid' do
+            expect(entry).not_to be_valid
           end
+        end
 
-          context 'when processed' do
-            before do
-              entry.process!
-              entry.validate!
-            end
+        context 'when no visible jobs present' do
+          let(:config) { { '.hidden'.to_sym => { script: [] } } }
 
-            it 'returns error about no visible jobs defined' do
-              expect(entry.errors)
-                .to include 'jobs config should contain at least one visible job'
-            end
+          it 'returns error about no visible jobs defined' do
+            expect(entry.errors)
+              .to include 'jobs config should contain at least one visible job'
           end
         end
       end
-- 
GitLab