From 12080ba150328963987674d282f435fc0e88b9d6 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 10 Jun 2016 11:20:46 +0200
Subject: [PATCH] Simplify new ci config entry class interface
---
lib/gitlab/ci/config/node/configurable.rb | 6 +-
lib/gitlab/ci/config/node/entry.rb | 15 +----
lib/gitlab/ci/config/node/null.rb | 12 ++--
lib/gitlab/ci/config/node/script.rb | 4 +-
spec/lib/gitlab/ci/config/node/global_spec.rb | 12 ----
spec/lib/gitlab/ci/config/node/null_spec.rb | 2 +-
spec/lib/gitlab/ci/config/node/script_spec.rb | 55 ++++++++++---------
7 files changed, 44 insertions(+), 62 deletions(-)
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index c8c917f229f..120457690d8 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -19,7 +19,7 @@ module Gitlab
def initialize(*)
super
- unless leaf? || has_config?
+ unless @value.is_a?(Hash)
@errors << 'should be a configuration entry with hash value'
end
end
@@ -39,9 +39,9 @@ module Gitlab
def create_entry(key, entry_class)
if @value.has_key?(key)
- entry_class.new(@value[key], @root, self)
+ entry_class.new(@value[key])
else
- Node::Null.new(nil, @root, self)
+ Node::Null.new(nil)
end
end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 19fc997297a..ed1cdd6f15d 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -10,16 +10,15 @@ module Gitlab
attr_accessor :description
- def initialize(value, root = nil, parent = nil)
+ def initialize(value)
@value = value
- @root = root
- @parent = parent
@nodes = {}
@errors = []
end
def process!
- return if leaf? || invalid?
+ return if leaf?
+ return unless valid?
compose!
@@ -41,18 +40,10 @@ module Gitlab
errors.none?
end
- def invalid?
- !valid?
- end
-
def leaf?
allowed_nodes.none?
end
- def has_config?
- @value.is_a?(Hash)
- end
-
def errors
@errors + nodes.map(&:errors).flatten
end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
index ab7b0abaf23..4f590f6bec8 100644
--- a/lib/gitlab/ci/config/node/null.rb
+++ b/lib/gitlab/ci/config/node/null.rb
@@ -1,13 +1,13 @@
module Gitlab
module Ci
class Config
- ##
- # This class represents a configuration entry that is not being used
- # in configuration file.
- #
- # This implements Null Object pattern.
- #
module Node
+ ##
+ # This class represents a configuration entry that is not being used
+ # in configuration file.
+ #
+ # This implements Null Object pattern.
+ #
class Null < Entry
def value
nil
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index 84f9ec0eb04..5072bf0db7d 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -6,8 +6,8 @@ module Gitlab
# Entry that represents a script.
#
# Each element in the value array is a command that will be executed
- # by GitLab Runner. Currently we concatenate this commands with
- # new line character as a separator what is compatbile with
+ # by GitLab Runner. Currently we concatenate these commands with
+ # new line character as a separator, what is compatible with
# implementation in Runner.
#
class Script < Entry
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 1a51528336b..2227fcec638 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -49,12 +49,6 @@ describe Gitlab::Ci::Config::Node::Global do
end
end
- describe '#has_config?' do
- it 'has config' do
- expect(global).to have_config
- end
- end
-
describe '#leaf?' do
it 'is not leaf' do
expect(global).not_to be_leaf
@@ -91,12 +85,6 @@ describe Gitlab::Ci::Config::Node::Global do
end
end
- describe '#invalid?' do
- it 'is not valid' do
- expect(global).to be_invalid
- end
- end
-
describe '#errors' do
it 'reports errors from child nodes' do
expect(global.errors)
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
index fa75bdcaa6f..fb6c3b5cbc0 100644
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/null_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe Gitlab::Ci::Config::Node::Null do
- let(:entry) { described_class.new(double, double) }
+ let(:entry) { described_class.new(nil) }
describe '#leaf?' do
it 'is leaf node' do
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index 0af97bab164..e4d6481f8a5 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -1,44 +1,47 @@
require 'spec_helper'
describe Gitlab::Ci::Config::Node::Script do
- let(:entry) { described_class.new(value, double)}
- before { entry.validate! }
+ let(:entry) { described_class.new(value) }
- context 'when entry value is correct' do
- let(:value) { ['ls', 'pwd'] }
+ describe '#validate!' do
+ before { entry.validate! }
- describe '#value' do
- it 'returns concatenated command' do
- expect(entry.value).to eq "ls\npwd"
+ context 'when entry value is correct' do
+ let(:value) { ['ls', 'pwd'] }
+
+ describe '#value' do
+ it 'returns concatenated command' do
+ expect(entry.value).to eq "ls\npwd"
+ end
end
- end
- describe '#errors' do
- it 'does not append errors' do
- expect(entry.errors).to be_empty
+ describe '#errors' do
+ it 'does not append errors' do
+ expect(entry.errors).to be_empty
+ end
end
- end
- describe '#has_config?' do
- it 'does not have config' do
- expect(entry).not_to have_config
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
end
end
- end
- context 'when entry value is not correct' do
- let(:value) { 'ls' }
+ context 'when entry value is not correct' do
+ let(:value) { 'ls' }
- describe '#errors' do
- it 'saves errors' do
- expect(entry.errors)
- .to include /should be an array of strings/
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include /should be an array of strings/
+ end
end
- end
- describe '#invalid?' do
- it 'is not valid' do
- expect(entry).to be_invalid
+ describe '#valid?' do
+ it 'is not valid' do
+ expect(entry).not_to be_valid
+ end
end
end
end
--
GitLab