From 65724301e6129440d0fba9cf9779297bc702c95b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 18 Nov 2016 14:14:41 +0100
Subject: Make CI job script a required configuration entry

---
 lib/gitlab/ci/config/entry/job.rb            | 32 ++++++++++++++--------------
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb |  4 ++--
 spec/lib/gitlab/ci/config/entry/job_spec.rb  | 14 +++++++++---
 3 files changed, 29 insertions(+), 21 deletions(-)

diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index ab4ef333629..20dcc024b4e 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -13,12 +13,10 @@ module Gitlab
                             type stage when artifacts cache dependencies before_script
                             after_script variables environment]
 
-          attributes :tags, :allow_failure, :when, :dependencies
-
           validations do
             validates :config, allowed_keys: ALLOWED_KEYS
-
             validates :config, presence: true
+            validates :script, presence: true
             validates :name, presence: true
             validates :name, type: Symbol
 
@@ -77,6 +75,8 @@ module Gitlab
                   :cache, :image, :services, :only, :except, :variables,
                   :artifacts, :commands, :environment
 
+          attributes :script, :tags, :allow_failure, :when, :dependencies
+
           def compose!(deps = nil)
             super do
               if type_defined? && !stage_defined?
@@ -118,20 +118,20 @@ module Gitlab
 
           def to_hash
             { name: name,
-              before_script: before_script,
-              script: script,
+              before_script: before_script_value,
+              script: script_value,
               commands: commands,
-              image: image,
-              services: services,
-              stage: stage,
-              cache: cache,
-              only: only,
-              except: except,
-              variables: variables_defined? ? variables : nil,
-              environment: environment_defined? ? environment : nil,
-              environment_name: environment_defined? ? environment[:name] : nil,
-              artifacts: artifacts,
-              after_script: after_script }
+              image: image_value,
+              services: services_value,
+              stage: stage_value,
+              cache: cache_value,
+              only: only_value,
+              except: except_value,
+              variables: variables_defined? ? variables_value : nil,
+              environment: environment_defined? ? environment_value : nil,
+              environment_name: environment_defined? ? environment_value[:name] : nil,
+              artifacts: artifacts_value,
+              after_script: after_script_value }
           end
         end
       end
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 84f21631719..ff5dcc06ab3 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -1124,8 +1124,8 @@ EOT
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:extra config should be a hash")
       end
 
-      it "returns errors if there are unknown parameters that are hashes, but doesn't have a script" do
-        config = YAML.dump({ extra: { services: "test" } })
+      it "returns errors if services configuration is not correct" do
+        config = YAML.dump({ extra: { script: 'rspec', services: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:extra:services config should be an array of strings")
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index c05711b6338..fc9b8b86dc4 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -19,8 +19,7 @@ describe Gitlab::Ci::Config::Entry::Job do
         let(:entry) { described_class.new(config, name: ''.to_sym) }
 
         it 'reports error' do
-          expect(entry.errors)
-            .to include "job name can't be blank"
+          expect(entry.errors).to include "job name can't be blank"
         end
       end
     end
@@ -56,6 +55,15 @@ describe Gitlab::Ci::Config::Entry::Job do
           end
         end
       end
+
+      context 'when script is not provided' do
+        let(:config) { { stage: 'test' } }
+
+        it 'returns error about missing script entry' do
+          expect(entry).not_to be_valid
+          expect(entry.errors).to include "job script can't be blank"
+        end
+      end
     end
   end
 
@@ -78,7 +86,7 @@ describe Gitlab::Ci::Config::Entry::Job do
       before { entry.compose!(deps) }
 
       let(:config) do
-        { image: 'some_image', cache: { key: 'test' } }
+        { script: 'rspec', image: 'some_image', cache: { key: 'test' } }
       end
 
       it 'overrides global config' do
-- 
cgit v1.2.1