Add class that encapsulates error in new Ci config

lib/gitlab/ci/config.rb

@@ -4,8 +4,6 @@ module Gitlab
     # Base GitLab CI Configuration facade
     #
     class Config
-      delegate :valid?, :errors, to: :@global
-
       ##
       # Temporary delegations that should be removed after refactoring
       #
@@ -18,6 +16,14 @@ module Gitlab
         @global.process!
       end
 
+      def valid?
+        errors.none?
+      end
+
+      def errors
+        @global.errors.map(&:to_s)
+      end
+
       def to_hash
         @config
       end

lib/gitlab/ci/config/node/configurable.rb

@@ -24,12 +24,12 @@ module Gitlab
 
           def prevalidate!
             unless @value.is_a?(Hash)
-              @errors << 'should be a configuration entry with hash value'
+              add_error('should be a configuration entry with hash value')
             end
           end
 
           def create_node(key, factory)
-            factory.with(value: @value[key])
+            factory.with(value: @value[key], key: key)
             factory.nullify! unless @value.has_key?(key)
             factory.create!
           end

lib/gitlab/ci/config/node/entry.rb

@@ -8,6 +8,7 @@ module Gitlab
         class Entry
           class InvalidError < StandardError; end
 
+          attr_writer :key
           attr_accessor :description
 
           def initialize(value)
@@ -40,10 +41,18 @@ module Gitlab
             allowed_nodes.none?
           end
 
+          def key
+            @key || self.class.name.demodulize.underscore
+          end
+
           def errors
             @errors + nodes.map(&:errors).flatten
           end
 
+          def add_error(message)
+            @errors << Error.new(message, self)
+          end
+
           def allowed_nodes
             {}
           end

lib/gitlab/ci/config/node/error.rb

+module Gitlab
+  module Ci
+    class Config
+      module Node
+        class Error
+          def initialize(message, parent)
+            @message = message
+            @parent = parent
+          end
+
+          def key
+            @parent.key
+          end
+
+          def to_s
+            "#{key}: #{@message}"
+          end
+
+          def ==(other)
+            other.to_s == to_s
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/ci/config/node/factory.rb

@@ -30,6 +30,7 @@ module Gitlab
 
             @entry_class.new(@attributes[:value]).tap do |entry|
               entry.description = @attributes[:description]
+              entry.key = @attributes[:key]
             end
           end
         end

lib/gitlab/ci/config/node/script.rb

@@ -19,7 +19,7 @@ module Gitlab
 
           def validate!
             unless validate_array_of_strings(@value)
-              @errors << 'before_script should be an array of strings'
+              add_error('should be an array of strings')
             end
           end
         end

spec/lib/ci/gitlab_ci_yaml_processor_spec.rb

@@ -820,7 +820,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 should be an array of strings")
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script: should be an array of strings")
       end
 
       it "returns errors if job before_script parameter is not an array of strings" do

spec/lib/gitlab/ci/config/node/error_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Error do
+  let(:error) { described_class.new(message, parent) }
+  let(:message) { 'some error' }
+  let(:parent) { spy('parent') }
+
+  before do
+    allow(parent).to receive(:key).and_return('parent_key')
+  end
+
+  describe '#key' do
+    it 'returns underscored class name' do
+      expect(error.key).to eq 'parent_key'
+    end
+  end
+
+  describe '#to_s' do
+    it 'returns valid error message' do
+      expect(error.to_s).to eq 'parent_key: some error'
+    end
+  end
+end

spec/lib/gitlab/ci/config/node/factory_spec.rb

@@ -25,6 +25,16 @@ describe Gitlab::Ci::Config::Node::Factory do
           expect(entry.description).to eq 'test description'
         end
       end
+
+      context 'when setting key' do
+        it 'creates entry with custom key' do
+          entry = factory
+            .with(value: ['ls', 'pwd'], key: 'test key')
+            .create!
+
+          expect(entry.key).to eq 'test key'
+        end
+      end
     end
 
     context 'when not setting value' do

spec/lib/gitlab/ci/config/node/global_spec.rb

@@ -13,6 +13,12 @@ describe Gitlab::Ci::Config::Node::Global do
     end
   end
 
+  describe '#key' do
+    it 'returns underscored class name' do
+      expect(global.key).to eq 'global'
+    end
+  end
+
   context 'when hash is valid' do
     let(:hash) do
       { before_script: ['ls', 'pwd'] }
@@ -79,7 +85,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 should be an array of strings'
+          .to include 'before_script: should be an array of strings'
       end
     end
 

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 /should be an array of strings/
+            .to include 'script: should be an array of strings'
         end
       end
 

spec/lib/gitlab/ci/config_spec.rb

@@ -67,6 +67,12 @@ describe Gitlab::Ci::Config do
             expect(config.errors).not_to be_empty
           end
         end
+
+        describe '#errors' do
+          it 'returns an array of strings' do
+            expect(config.errors).to all(be_an_instance_of(String))
+          end
+        end
       end
     end
   end