From f4421817de474cda3598eac8cad0752d324608e1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 29 Jun 2016 09:39:04 +0200
Subject: [PATCH] Add global cache config entry to new CI config

---
 lib/gitlab/ci/config/node/cache.rb            |  3 ++-
 lib/gitlab/ci/config/node/configurable.rb     |  5 +++-
 lib/gitlab/ci/config/node/entry.rb            |  4 +++
 lib/gitlab/ci/config/node/global.rb           |  7 +++--
 lib/gitlab/ci/config/node/validator.rb        |  6 +++--
 spec/lib/ci/gitlab_ci_yaml_processor_spec.rb  | 26 +++++++++----------
 .../lib/gitlab/ci/config/node/boolean_spec.rb |  2 +-
 spec/lib/gitlab/ci/config/node/cache_spec.rb  |  6 ++---
 spec/lib/gitlab/ci/config/node/global_spec.rb | 26 ++++++++++++++-----
 spec/lib/gitlab/ci/config/node/image_spec.rb  |  2 +-
 spec/lib/gitlab/ci/config/node/key_spec.rb    |  2 +-
 spec/lib/gitlab/ci/config/node/paths_spec.rb  |  2 +-
 spec/lib/gitlab/ci/config/node/script_spec.rb |  2 +-
 .../gitlab/ci/config/node/services_spec.rb    |  2 +-
 spec/lib/gitlab/ci/config/node/stages_spec.rb |  2 +-
 .../gitlab/ci/config/node/validator_spec.rb   |  3 ++-
 16 files changed, 64 insertions(+), 36 deletions(-)

diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
index 251d7aa9097..01a9ef511ee 100644
--- a/lib/gitlab/ci/config/node/cache.rb
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -27,7 +27,8 @@ module Gitlab
 
             def keys
               if unknown_keys.any?
-                errors.add(:config, "contains unknown keys #{unknown_keys}")
+                unknown_list = unknown_keys.join(', ')
+                errors.add(:config, "contains unknown keys: #{unknown_list}")
               end
             end
           end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 46a473ad092..4889a21a234 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -26,7 +26,10 @@ module Gitlab
           private
 
           def create_node(key, factory)
-            factory.with(value: @config[key], key: key)
+            factory.with(value: @config[key])
+            factory.with(parent: self)
+            factory.with(key: key)
+
             factory.create!
           end
 
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 17d04fbdfec..985d1705191 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -34,6 +34,10 @@ module Gitlab
             self.class.nodes.none?
           end
 
+          def ancestors
+            @parent ? @parent.ancestors + [@parent] : []
+          end
+
           def valid?
             errors.none?
           end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 4ca379712c6..65919ef1eeb 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -30,8 +30,11 @@ module Gitlab
           node :types, Stages,
             description: 'Stages for this pipeline (deprecated key).'
 
-          helpers :before_script, :image, :services, :after_script, :variables,
-                  :stages, :types
+          node :cache, Cache,
+            description: 'Configure caching between build jobs.'
+
+          helpers :before_script, :image, :services, :after_script,
+                  :variables, :stages, :types, :cache
 
           def stages
             stages_defined? ? stages_value : types_value
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index d898d521548..94a8af4d080 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -13,7 +13,7 @@ module Gitlab
 
           def messages
             errors.full_messages.map do |error|
-              "#{location} #{error}".humanize
+              "#{location} #{error}".downcase
             end
           end
 
@@ -24,7 +24,9 @@ module Gitlab
           private
 
           def location
-            key || @node.class.name.demodulize.underscore
+            predecessors = ancestors.map(&:key).compact
+            current = key || @node.class.name.demodulize.underscore
+            predecessors.append(current).join(':')
           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 147301b3128..262a91fedff 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -600,7 +600,7 @@ module Ci
 
           expect { GitlabCiYamlProcessor.new(config) }.to raise_error(
             GitlabCiYamlProcessor::ValidationError,
-            'Cache config has unknown parameter: invalid'
+            'cache config contains unknown keys: invalid'
           )
         end
       end
@@ -964,7 +964,7 @@ EOT
         config = YAML.dump({ before_script: "bundle update", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Before script config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script config should be an array of strings")
       end
 
       it "returns errors if job before_script parameter is not an array of strings" do
@@ -978,7 +978,7 @@ EOT
         config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "After script config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script config should be an array of strings")
       end
 
       it "returns errors if job after_script parameter is not an array of strings" do
@@ -992,7 +992,7 @@ EOT
         config = YAML.dump({ image: ["test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Image config should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image config should be a string")
       end
 
       it "returns errors if job name is blank" do
@@ -1020,14 +1020,14 @@ EOT
         config = YAML.dump({ services: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Services config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
       end
 
       it "returns errors if services parameter is not an array of strings" do
         config = YAML.dump({ services: [10, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Services config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
       end
 
       it "returns errors if job services parameter is not an array" do
@@ -1097,28 +1097,28 @@ EOT
         config = YAML.dump({ stages: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Stages config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
       end
 
       it "returns errors if stages is not an array of strings" do
         config = YAML.dump({ stages: [true, "test"], rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Stages config should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
       end
 
       it "returns errors if variables is not a map" do
         config = YAML.dump({ variables: "test", rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Variables config should be a hash of key value pairs")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config should be a hash of key value pairs")
       end
 
       it "returns errors if variables is not a map of key-value strings" do
         config = YAML.dump({ variables: { test: false }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config, path)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Variables config should be a hash of key value pairs")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config should be a hash of key value pairs")
       end
 
       it "returns errors if job when is not on_success, on_failure or always" do
@@ -1174,21 +1174,21 @@ EOT
         config = YAML.dump({ cache: { untracked: "string" }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked parameter should be an boolean")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked config should be a boolean value")
       end
 
       it "returns errors if cache:paths is not an array of strings" do
         config = YAML.dump({ cache: { paths: "string" }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths parameter should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths config should be an array of strings")
       end
 
       it "returns errors if cache:key is not a string" do
         config = YAML.dump({ cache: { key: 1 }, rspec: { script: "test" } })
         expect do
           GitlabCiYamlProcessor.new(config)
-        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key parameter should be a string")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key config should be a string or symbol")
       end
 
       it "returns errors if job cache:key is not an a string" do
diff --git a/spec/lib/gitlab/ci/config/node/boolean_spec.rb b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
index 97f13b2d5fc..32639296e6d 100644
--- a/spec/lib/gitlab/ci/config/node/boolean_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Boolean do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Boolean config should be a boolean value'
+            .to include 'boolean config should be a boolean value'
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/cache_spec.rb b/spec/lib/gitlab/ci/config/node/cache_spec.rb
index d6428f6b996..50f619ce26e 100644
--- a/spec/lib/gitlab/ci/config/node/cache_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/cache_spec.rb
@@ -33,7 +33,7 @@ describe Gitlab::Ci::Config::Node::Cache do
 
           it 'reports errors with config value' do
             expect(entry.errors)
-              .to include 'Cache config should be a hash'
+              .to include 'cache config should be a hash'
           end
         end
 
@@ -42,7 +42,7 @@ describe Gitlab::Ci::Config::Node::Cache do
 
           it 'reports error with descendants' do
             expect(entry.errors)
-              .to include 'Key config should be a string or symbol'
+              .to include 'key config should be a string or symbol'
           end
         end
 
@@ -51,7 +51,7 @@ describe Gitlab::Ci::Config::Node::Cache do
 
           it 'reports error with descendants' do
             expect(entry.errors)
-              .to include 'Cache config contains unknown keys [:invalid]'
+              .to include 'cache config contains unknown keys: invalid'
           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 cf7ab13c8b1..c87c9e97bc8 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -21,7 +21,8 @@ describe Gitlab::Ci::Config::Node::Global do
           services: ['postgres:9.1', 'mysql:5.5'],
           variables: { VAR: 'value' },
           after_script: ['make clean'],
-          stages: ['build', 'pages'] }
+          stages: ['build', 'pages'],
+          cache: { key: 'k', untracked: true, paths: ['public/'] } }
       end
 
       describe '#process!' do
@@ -32,7 +33,7 @@ describe Gitlab::Ci::Config::Node::Global do
         end
 
         it 'creates node object for each entry' do
-          expect(global.nodes.count).to eq 7
+          expect(global.nodes.count).to eq 8
         end
 
         it 'creates node object using valid class' do
@@ -112,20 +113,27 @@ describe Gitlab::Ci::Config::Node::Global do
             end
           end
         end
+
+        describe '#cache' do
+          it 'returns cache configuration' do
+            expect(global.cache)
+              .to eq(key: 'k', untracked: true, paths: ['public/'])
+          end
+        end
       end
     end
 
     context 'when most of entires not defined' do
-      let(:hash) { { rspec: {} } }
+      let(:hash) { { cache: { key: 'a' }, rspec: {} } }
       before { global.process! }
 
       describe '#nodes' do
         it 'instantizes all nodes' do
-          expect(global.nodes.count).to eq 7
+          expect(global.nodes.count).to eq 8
         end
 
         it 'contains undefined nodes' do
-          expect(global.nodes.last)
+          expect(global.nodes.first)
             .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
         end
       end
@@ -141,6 +149,12 @@ describe Gitlab::Ci::Config::Node::Global do
           expect(global.stages).to eq %w[build test deploy]
         end
       end
+
+      describe '#cache' do
+        it 'returns correct cache definition' do
+          expect(global.cache).to eq(key: 'a')
+        end
+      end
     end
 
     ##
@@ -177,7 +191,7 @@ describe Gitlab::Ci::Config::Node::Global do
     describe '#errors' do
       it 'reports errors from child nodes' do
         expect(global.errors)
-          .to include 'Before script config should be an array of strings'
+          .to include 'before_script config should be an array of strings'
       end
     end
 
diff --git a/spec/lib/gitlab/ci/config/node/image_spec.rb b/spec/lib/gitlab/ci/config/node/image_spec.rb
index 0b0821ca558..d11bb39f328 100644
--- a/spec/lib/gitlab/ci/config/node/image_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/image_spec.rb
@@ -32,7 +32,7 @@ describe Gitlab::Ci::Config::Node::Image do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Image config should be a string'
+            .to include 'image config should be a string'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/key_spec.rb b/spec/lib/gitlab/ci/config/node/key_spec.rb
index 23e7fc46201..8cda43173fe 100644
--- a/spec/lib/gitlab/ci/config/node/key_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/key_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Key do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Key config should be a string or symbol'
+            .to include 'key config should be a string or symbol'
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/paths_spec.rb b/spec/lib/gitlab/ci/config/node/paths_spec.rb
index 0d95ad8abda..6fd744b3975 100644
--- a/spec/lib/gitlab/ci/config/node/paths_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/paths_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Paths do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Paths config should be an array of strings'
+            .to include 'paths config should be an array of strings'
         end
       end
     end
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index abd43aa1ee9..ee7395362a9 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -34,7 +34,7 @@ describe Gitlab::Ci::Config::Node::Script do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Script config should be an array of strings'
+            .to include 'script config should be an array of strings'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/services_spec.rb b/spec/lib/gitlab/ci/config/node/services_spec.rb
index e38f6f6923f..be0fe46befd 100644
--- a/spec/lib/gitlab/ci/config/node/services_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/services_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Services do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Services config should be an array of strings'
+            .to include 'services config should be an array of strings'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/stages_spec.rb b/spec/lib/gitlab/ci/config/node/stages_spec.rb
index dbf2eb8993d..1a3818d8997 100644
--- a/spec/lib/gitlab/ci/config/node/stages_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/stages_spec.rb
@@ -26,7 +26,7 @@ describe Gitlab::Ci::Config::Node::Stages do
       describe '#errors' do
         it 'saves errors' do
           expect(entry.errors)
-            .to include 'Stages config should be an array of strings'
+            .to include 'stages config should be an array of strings'
         end
       end
 
diff --git a/spec/lib/gitlab/ci/config/node/validator_spec.rb b/spec/lib/gitlab/ci/config/node/validator_spec.rb
index 87a1bbf55c0..090fd63b844 100644
--- a/spec/lib/gitlab/ci/config/node/validator_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/validator_spec.rb
@@ -7,6 +7,7 @@ describe Gitlab::Ci::Config::Node::Validator do
 
   before do
     allow(node).to receive(:key).and_return('node')
+    allow(node).to receive(:ancestors).and_return([])
   end
 
   describe 'delegated validator' do
@@ -47,7 +48,7 @@ describe Gitlab::Ci::Config::Node::Validator do
         validator_instance.validate
 
         expect(validator_instance.messages)
-          .to include "Node test attribute can't be blank"
+          .to include "node test attribute can't be blank"
       end
     end
   end
-- 
GitLab