From 12080ba150328963987674d282f435fc0e88b9d6 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 10 Jun 2016 11:20:46 +0200 Subject: 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 -- cgit v1.2.1