From bc2348f2e4365099e2a99df3d8e2a55fe7d138f4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 22 Jun 2016 14:26:33 +0200
Subject: [PATCH] Return default config value when entry is undefined
---
lib/gitlab/ci/config/node/configurable.rb | 6 +--
lib/gitlab/ci/config/node/entry.rb | 3 ++
lib/gitlab/ci/config/node/factory.rb | 9 ++--
lib/gitlab/ci/config/node/global.rb | 10 ++--
lib/gitlab/ci/config/node/undefined.rb | 18 +++++---
lib/gitlab/ci/config/node/variables.rb | 6 ++-
.../ci/config/node/configurable_spec.rb | 2 +-
.../gitlab/ci/config/node/undefined_spec.rb | 25 +++++++---
spec/lib/gitlab/ci/config_spec.rb | 46 +++++++++----------
9 files changed, 74 insertions(+), 51 deletions(-)
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index f26924fab26..25b5c2c9e21 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -33,12 +33,12 @@ module Gitlab
class_methods do
def nodes
- Hash[@allowed_nodes.map { |key, factory| [key, factory.dup] }]
+ Hash[@nodes.map { |key, factory| [key, factory.dup] }]
end
private
- def allow_node(symbol, entry_class, metadata)
+ def node(symbol, entry_class, metadata)
factory = Node::Factory.new(entry_class)
.with(description: metadata[:description])
@@ -47,7 +47,7 @@ module Gitlab
@nodes[symbol].try(:value)
end
- (@allowed_nodes ||= {}).merge!(symbol => factory)
+ (@nodes ||= {}).merge!(symbol => factory)
end
end
end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 91f3fd0e236..444a276d5c9 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -52,6 +52,9 @@ module Gitlab
@config
end
+ def self.default
+ end
+
def self.nodes
{}
end
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index dbf027b6d8e..2271c386df6 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -8,8 +8,8 @@ module Gitlab
class Factory
class InvalidFactory < StandardError; end
- def initialize(entry_class)
- @entry_class = entry_class
+ def initialize(node)
+ @node = node
@attributes = {}
end
@@ -19,14 +19,15 @@ module Gitlab
end
def undefine!
- @entry_class = Node::Undefined
+ @attributes[:value] = @node.dup
+ @node = Node::Undefined
self
end
def create!
raise InvalidFactory unless @attributes.has_key?(:value)
- @entry_class.new(@attributes[:value]).tap do |entry|
+ @node.new(@attributes[:value]).tap do |entry|
entry.description = @attributes[:description]
entry.key = @attributes[:key]
end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 3bb6f98ed0f..b5d177c5285 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -9,19 +9,19 @@ module Gitlab
class Global < Entry
include Configurable
- allow_node :before_script, Script,
+ node :before_script, Script,
description: 'Script that will be executed before each job.'
- allow_node :image, Image,
+ node :image, Image,
description: 'Docker image that will be used to execute jobs.'
- allow_node :services, Services,
+ node :services, Services,
description: 'Docker images that will be linked to the container.'
- allow_node :after_script, Script,
+ node :after_script, Script,
description: 'Script that will be executed after each job.'
- allow_node :variables, Variables,
+ node :variables, Variables,
description: 'Environment variables that will be used.'
end
end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index a812d803f3e..e8a69b810e0 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -3,17 +3,21 @@ module Gitlab
class Config
module Node
##
- # This class represents a configuration entry that is not defined
- # in configuration file.
+ # This class represents an undefined entry node.
#
- # This implements a Null Object pattern.
+ # It takes original entry class as configuration and returns default
+ # value of original entry as self value.
#
- # It can be initialized using a default value of entry that is not
- # present in configuration.
#
class Undefined < Entry
- def method_missing(*)
- nil
+ include Validatable
+
+ validations do
+ validates :config, type: Class
+ end
+
+ def value
+ @config.default
end
end
end
diff --git a/lib/gitlab/ci/config/node/variables.rb b/lib/gitlab/ci/config/node/variables.rb
index 5decd777bdb..fd3ce8715a9 100644
--- a/lib/gitlab/ci/config/node/variables.rb
+++ b/lib/gitlab/ci/config/node/variables.rb
@@ -13,7 +13,11 @@ module Gitlab
end
def value
- @config || {}
+ @config || self.class.default
+ end
+
+ def self.default
+ {}
end
end
end
diff --git a/spec/lib/gitlab/ci/config/node/configurable_spec.rb b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
index 9bbda6e7396..2ac436cb4b2 100644
--- a/spec/lib/gitlab/ci/config/node/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -10,7 +10,7 @@ describe Gitlab::Ci::Config::Node::Configurable do
describe 'configured nodes' do
before do
node.class_eval do
- allow_node :object, Object, description: 'test object'
+ node :object, Object, description: 'test object'
end
end
diff --git a/spec/lib/gitlab/ci/config/node/undefined_spec.rb b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
index 2d01a8a6ec8..5ded0504a3f 100644
--- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -1,23 +1,34 @@
require 'spec_helper'
describe Gitlab::Ci::Config::Node::Undefined do
- let(:entry) { described_class.new('some value') }
+ let(:undefined) { described_class.new(entry) }
+ let(:entry) { Class.new }
describe '#leaf?' do
it 'is leaf node' do
- expect(entry).to be_leaf
+ expect(undefined).to be_leaf
end
end
- describe '#any_method' do
- it 'responds with nil' do
- expect(entry.any_method).to be nil
+ describe '#valid?' do
+ it 'is always valid' do
+ expect(undefined).to be_valid
+ end
+ end
+
+ describe '#errors' do
+ it 'is does not contain errors' do
+ expect(undefined.errors).to be_empty
end
end
describe '#value' do
- it 'returns configured value' do
- expect(entry.value).to eq 'some value'
+ before do
+ allow(entry).to receive(:default).and_return('some value')
+ end
+
+ it 'returns default value for entry that is undefined' do
+ expect(undefined.value).to eq 'some value'
end
end
end
diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb
index 2a5d132db7b..bc5a5e43103 100644
--- a/spec/lib/gitlab/ci/config_spec.rb
+++ b/spec/lib/gitlab/ci/config_spec.rb
@@ -40,38 +40,38 @@ describe Gitlab::Ci::Config do
end
end
end
+ end
- context 'when config is invalid' do
- context 'when yml is incorrect' do
- let(:yml) { '// invalid' }
+ context 'when config is invalid' do
+ context 'when yml is incorrect' do
+ let(:yml) { '// invalid' }
- describe '.new' do
- it 'raises error' do
- expect { config }.to raise_error(
- Gitlab::Ci::Config::Loader::FormatError,
- /Invalid configuration format/
- )
- end
+ describe '.new' do
+ it 'raises error' do
+ expect { config }.to raise_error(
+ Gitlab::Ci::Config::Loader::FormatError,
+ /Invalid configuration format/
+ )
end
end
+ end
- context 'when config logic is incorrect' do
- let(:yml) { 'before_script: "ls"' }
+ context 'when config logic is incorrect' do
+ let(:yml) { 'before_script: "ls"' }
- describe '#valid?' do
- it 'is not valid' do
- expect(config).not_to be_valid
- end
+ describe '#valid?' do
+ it 'is not valid' do
+ expect(config).not_to be_valid
+ end
- it 'has errors' do
- expect(config.errors).not_to be_empty
- end
+ it 'has errors' 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
+ describe '#errors' do
+ it 'returns an array of strings' do
+ expect(config.errors).to all(be_an_instance_of(String))
end
end
end
--
GitLab