Make rubocop happymake-before-after-overridable

2 files changed, 23 insertions, 20 deletions

diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 88d6ce01b03..ff9887cba1e 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -160,6 +160,7 @@ module Ci
       validate_job_name!(name)
       validate_job_keys!(name, job)
       validate_job_types!(name, job)
+      validate_job_script!(name, job)
 
       validate_job_stage!(name, job) if job[:stage]
       validate_job_variables!(name, job) if job[:variables]
@@ -183,18 +184,6 @@ module Ci
     end
 
     def validate_job_types!(name, job)
-      if !validate_string(job[:script]) && !validate_array_of_strings(job[:script])
-        raise ValidationError, "#{name} job: script should be a string or an array of a strings"
-      end
-
-      if job[:before_script] && !validate_array_of_strings(job[:before_script])
-        raise ValidationError, "#{name} job: before_script should be an array of strings"
-      end
-
-      if job[:after_script] && !validate_array_of_strings(job[:after_script])
-        raise ValidationError, "#{name} job: after_script should be an array of strings"
-      end
-
       if job[:image] && !validate_string(job[:image])
         raise ValidationError, "#{name} job: image should be a string"
       end
@@ -224,6 +213,20 @@ module Ci
       end
     end
 
+    def validate_job_script!(name, job)
+      if !validate_string(job[:script]) && !validate_array_of_strings(job[:script])
+        raise ValidationError, "#{name} job: script should be a string or an array of a strings"
+      end
+
+      if job[:before_script] && !validate_array_of_strings(job[:before_script])
+        raise ValidationError, "#{name} job: before_script should be an array of strings"
+      end
+
+      if job[:after_script] && !validate_array_of_strings(job[:after_script])
+        raise ValidationError, "#{name} job: after_script should be an array of strings"
+      end
+    end
+
     def validate_job_stage!(name, job)
       unless job[:stage].is_a?(String) && job[:stage].in?(stages)
         raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index a20b9ead559..643acf0303c 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -295,12 +295,12 @@ module Ci
       
       describe "before_script" do
         context "in global context" do
-          let(:config) {
+          let(:config) do
             {
               before_script: ["global script"],
               test: { script: ["script"] }
             }
-          }
+          end
           
           it "return commands with scripts concencaced" do
             expect(subject[:commands]).to eq("global script\nscript")
@@ -308,12 +308,12 @@ module Ci
         end
  
         context "overwritten in local context" do
-          let(:config) {
+          let(:config) do
             {
               before_script: ["global script"],
               test: { before_script: ["local script"], script: ["script"] }
             }
-          }
+          end
 
           it "return commands with scripts concencaced" do
             expect(subject[:commands]).to eq("local script\nscript")
@@ -322,11 +322,11 @@ module Ci
       end
 
       describe "script" do
-        let(:config) {
+        let(:config) do
           {
             test: { script: ["script"] }
           }
-        }
+        end
 
         it "return commands with scripts concencaced" do
           expect(subject[:commands]).to eq("script")
@@ -348,12 +348,12 @@ module Ci
         end
 
         context "overwritten in local context" do
-          let(:config) {
+          let(:config) do
             {
               after_script: ["local after_script"],
               test: { after_script: ["local after_script"], script: ["script"] }
             }
-          }
+          end
 
           it "return after_script in options" do
             expect(subject[:options][:after_script]).to eq(["local after_script"])