From 6c704fd99e1fd5a86dd2eccc8b192a009546cc96 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 6 Aug 2016 16:45:02 +0200
Subject: [PATCH 1/9] Inject dependencies into each CI config entry node
---
lib/gitlab/ci/config/node/configurable.rb | 2 +-
lib/gitlab/ci/config/node/entry.rb | 11 +++++++----
lib/gitlab/ci/config/node/global.rb | 6 +++++-
lib/gitlab/ci/config/node/job.rb | 2 +-
lib/gitlab/ci/config/node/jobs.rb | 2 +-
5 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 2de82d40c9d..fb3604edac2 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -25,7 +25,7 @@ module Gitlab
private
- def compose!
+ def compose!(_deps)
self.class.nodes.each do |key, factory|
factory
.value(@config[key])
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 0c782c422b5..40ffa85d6da 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,11 +20,14 @@ module Gitlab
@validator.validate(:new)
end
- def process!
+ def process!(deps = nil)
return unless valid?
- compose!
- descendants.each(&:process!)
+ compose!(deps)
+
+ descendants.each do |entry|
+ entry.process!(deps)
+ end
end
def leaf?
@@ -76,7 +79,7 @@ module Gitlab
private
- def compose!
+ def compose!(_deps)
end
end
end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index ccd539fb003..4d65309cf44 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -36,9 +36,13 @@ module Gitlab
helpers :before_script, :image, :services, :after_script,
:variables, :stages, :types, :cache, :jobs
+ def process!(_deps = nil)
+ super(self)
+ end
+
private
- def compose!
+ def compose!(_deps)
super
compose_jobs!
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index e84737acbb9..55f34b446e5 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -107,7 +107,7 @@ module Gitlab
after_script: after_script }
end
- def compose!
+ def compose!(_deps)
super
if type_defined? && !stage_defined?
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index 51683c82ceb..a2a8554ed19 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -28,7 +28,7 @@ module Gitlab
private
- def compose!
+ def compose!(_deps)
@config.each do |name, config|
node = hidden?(name) ? Node::HiddenJob : Node::Job
--
GitLab
From 700078e8e43626a9be4fd752e35cd27ac526b410 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 6 Aug 2016 17:12:59 +0200
Subject: [PATCH 2/9] Rename unspecified and undefined CI config nodes
---
lib/gitlab/ci/config/node/factory.rb | 8 ++--
lib/gitlab/ci/config/node/null.rb | 34 ---------------
lib/gitlab/ci/config/node/undefined.rb | 27 ++++++++++--
lib/gitlab/ci/config/node/unspecified.rb | 19 +++++++++
.../lib/gitlab/ci/config/node/factory_spec.rb | 3 +-
spec/lib/gitlab/ci/config/node/global_spec.rb | 4 +-
spec/lib/gitlab/ci/config/node/null_spec.rb | 41 -------------------
.../gitlab/ci/config/node/undefined_spec.rb | 33 +++++++++------
.../gitlab/ci/config/node/unspecified_spec.rb | 32 +++++++++++++++
9 files changed, 103 insertions(+), 98 deletions(-)
delete mode 100644 lib/gitlab/ci/config/node/null.rb
create mode 100644 lib/gitlab/ci/config/node/unspecified.rb
delete mode 100644 spec/lib/gitlab/ci/config/node/null_spec.rb
create mode 100644 spec/lib/gitlab/ci/config/node/unspecified_spec.rb
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 707b052e6a8..5387f29ad59 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -37,8 +37,8 @@ module Gitlab
# See issue #18775.
#
if @value.nil?
- Node::Undefined.new(
- fabricate_undefined
+ Node::Unspecified.new(
+ fabricate_unspecified
)
else
fabricate(@node, @value)
@@ -47,13 +47,13 @@ module Gitlab
private
- def fabricate_undefined
+ def fabricate_unspecified
##
# If node has a default value we fabricate concrete node
# with default value.
#
if @node.default.nil?
- fabricate(Node::Null)
+ fabricate(Node::Undefined)
else
fabricate(@node, @node.default)
end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
deleted file mode 100644
index 88a5f53f13c..00000000000
--- a/lib/gitlab/ci/config/node/null.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-module Gitlab
- module Ci
- class Config
- module Node
- ##
- # This class represents an undefined node.
- #
- # Implements the Null Object pattern.
- #
- class Null < Entry
- def value
- nil
- end
-
- def valid?
- true
- end
-
- def errors
- []
- end
-
- def specified?
- false
- end
-
- def relevant?
- false
- end
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
index 45fef8c3ae5..33e78023539 100644
--- a/lib/gitlab/ci/config/node/undefined.rb
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -3,15 +3,34 @@ module Gitlab
class Config
module Node
##
- # This class represents an unspecified entry node.
+ # This class represents an undefined node.
#
- # It decorates original entry adding method that indicates it is
- # unspecified.
+ # Implements the Null Object pattern.
#
- class Undefined < SimpleDelegator
+ class Undefined < Entry
+ def initialize(*)
+ super(nil)
+ end
+
+ def value
+ nil
+ end
+
+ def valid?
+ true
+ end
+
+ def errors
+ []
+ end
+
def specified?
false
end
+
+ def relevant?
+ false
+ end
end
end
end
diff --git a/lib/gitlab/ci/config/node/unspecified.rb b/lib/gitlab/ci/config/node/unspecified.rb
new file mode 100644
index 00000000000..a7d1f6131b8
--- /dev/null
+++ b/lib/gitlab/ci/config/node/unspecified.rb
@@ -0,0 +1,19 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # This class represents an unspecified entry node.
+ #
+ # It decorates original entry adding method that indicates it is
+ # unspecified.
+ #
+ class Unspecified < SimpleDelegator
+ def specified?
+ false
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index d26185ba585..a699089c563 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -65,7 +65,8 @@ describe Gitlab::Ci::Config::Node::Factory do
.value(nil)
.create!
- expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
+ expect(entry)
+ .to be_an_instance_of Gitlab::Ci::Config::Node::Unspecified
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 2f87d270b36..dfe75bd09e5 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -156,9 +156,9 @@ describe Gitlab::Ci::Config::Node::Global do
expect(global.descendants.count).to eq 8
end
- it 'contains undefined nodes' do
+ it 'contains unspecified nodes' do
expect(global.descendants.first)
- .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
+ .to be_an_instance_of Gitlab::Ci::Config::Node::Unspecified
end
end
diff --git a/spec/lib/gitlab/ci/config/node/null_spec.rb b/spec/lib/gitlab/ci/config/node/null_spec.rb
deleted file mode 100644
index 1ab5478dcfa..00000000000
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ /dev/null
@@ -1,41 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Node::Null do
- let(:null) { described_class.new(nil) }
-
- describe '#leaf?' do
- it 'is leaf node' do
- expect(null).to be_leaf
- end
- end
-
- describe '#valid?' do
- it 'is always valid' do
- expect(null).to be_valid
- end
- end
-
- describe '#errors' do
- it 'is does not contain errors' do
- expect(null.errors).to be_empty
- end
- end
-
- describe '#value' do
- it 'returns nil' do
- expect(null.value).to eq nil
- end
- end
-
- describe '#relevant?' do
- it 'is not relevant' do
- expect(null.relevant?).to eq false
- end
- end
-
- describe '#specified?' do
- it 'is not defined' do
- expect(null.specified?).to eq false
- end
- 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 2d43e1c1a9d..6bde8602963 100644
--- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -1,32 +1,41 @@
require 'spec_helper'
describe Gitlab::Ci::Config::Node::Undefined do
- let(:undefined) { described_class.new(entry) }
- let(:entry) { spy('Entry') }
+ let(:entry) { described_class.new }
+
+ describe '#leaf?' do
+ it 'is leaf node' do
+ expect(entry).to be_leaf
+ end
+ end
describe '#valid?' do
- it 'delegates method to entry' do
- expect(undefined.valid).to eq entry
+ it 'is always valid' do
+ expect(entry).to be_valid
end
end
describe '#errors' do
- it 'delegates method to entry' do
- expect(undefined.errors).to eq entry
+ it 'is does not contain errors' do
+ expect(entry.errors).to be_empty
end
end
describe '#value' do
- it 'delegates method to entry' do
- expect(undefined.value).to eq entry
+ it 'returns nil' do
+ expect(entry.value).to eq nil
end
end
- describe '#specified?' do
- it 'is always false' do
- allow(entry).to receive(:specified?).and_return(true)
+ describe '#relevant?' do
+ it 'is not relevant' do
+ expect(entry.relevant?).to eq false
+ end
+ end
- expect(undefined.specified?).to be false
+ describe '#specified?' do
+ it 'is not defined' do
+ expect(entry.specified?).to eq false
end
end
end
diff --git a/spec/lib/gitlab/ci/config/node/unspecified_spec.rb b/spec/lib/gitlab/ci/config/node/unspecified_spec.rb
new file mode 100644
index 00000000000..ba3ceef24ce
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/unspecified_spec.rb
@@ -0,0 +1,32 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Unspecified do
+ let(:unspecified) { described_class.new(entry) }
+ let(:entry) { spy('Entry') }
+
+ describe '#valid?' do
+ it 'delegates method to entry' do
+ expect(unspecified.valid?).to eq entry
+ end
+ end
+
+ describe '#errors' do
+ it 'delegates method to entry' do
+ expect(unspecified.errors).to eq entry
+ end
+ end
+
+ describe '#value' do
+ it 'delegates method to entry' do
+ expect(unspecified.value).to eq entry
+ end
+ end
+
+ describe '#specified?' do
+ it 'is always false' do
+ allow(entry).to receive(:specified?).and_return(true)
+
+ expect(unspecified.specified?).to be false
+ end
+ end
+end
--
GitLab
From eaf211c2e393a2eeb5999b9ffec17112cdd52a0c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Aug 2016 13:49:15 +0200
Subject: [PATCH 3/9] Expose compose method in the ci config entry nodes
---
lib/gitlab/ci/config.rb | 2 +-
lib/gitlab/ci/config/node/configurable.rb | 10 ++++++--
lib/gitlab/ci/config/node/entry.rb | 17 ++++++--------
lib/gitlab/ci/config/node/global.rb | 14 ++++--------
lib/gitlab/ci/config/node/job.rb | 20 ++++++++--------
lib/gitlab/ci/config/node/jobs.rb | 28 +++++++++++++----------
6 files changed, 47 insertions(+), 44 deletions(-)
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index ae82c0db3f1..bbfa6cf7d05 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -14,7 +14,7 @@ module Gitlab
@config = Loader.new(config).load!
@global = Node::Global.new(@config)
- @global.process!
+ @global.compose!
end
def valid?
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index fb3604edac2..530f95a6251 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -23,9 +23,9 @@ module Gitlab
end
end
- private
+ def compose!(deps)
+ return unless valid?
- def compose!(_deps)
self.class.nodes.each do |key, factory|
factory
.value(@config[key])
@@ -33,6 +33,12 @@ module Gitlab
@entries[key] = factory.create!
end
+
+ yield if block_given?
+
+ @entries.each_value do |entry|
+ entry.compose!(deps)
+ end
end
class_methods do
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 40ffa85d6da..e02f4bf0a05 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,14 +20,16 @@ module Gitlab
@validator.validate(:new)
end
+ # Temporary method
+ #
def process!(deps = nil)
- return unless valid?
-
compose!(deps)
+ end
- descendants.each do |entry|
- entry.process!(deps)
- end
+ def compose!(deps = nil)
+ return unless valid?
+
+ yield if block_given?
end
def leaf?
@@ -76,11 +78,6 @@ module Gitlab
def self.validator
Validator
end
-
- private
-
- def compose!(_deps)
- end
end
end
end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 4d65309cf44..2a2943c9288 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -36,19 +36,15 @@ module Gitlab
helpers :before_script, :image, :services, :after_script,
:variables, :stages, :types, :cache, :jobs
- def process!(_deps = nil)
- super(self)
+ def compose!(_deps = nil)
+ super(self) do
+ compose_jobs!
+ compose_deprecated_entries!
+ end
end
private
- def compose!(_deps)
- super
-
- compose_jobs!
- compose_deprecated_entries!
- end
-
def compose_jobs!
factory = Node::Factory.new(Node::Jobs)
.value(@config.except(*self.class.nodes.keys))
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 55f34b446e5..9317bb40f50 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -82,6 +82,16 @@ module Gitlab
:cache, :image, :services, :only, :except, :variables,
:artifacts
+ def compose!(deps)
+ super do
+ if type_defined? && !stage_defined?
+ @entries[:stage] = @entries[:type]
+ end
+
+ @entries.delete(:type)
+ end
+ end
+
def name
@metadata[:name]
end
@@ -106,16 +116,6 @@ module Gitlab
artifacts: artifacts,
after_script: after_script }
end
-
- def compose!(_deps)
- super
-
- if type_defined? && !stage_defined?
- @entries[:stage] = @entries[:type]
- end
-
- @entries.delete(:type)
- end
end
end
end
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index a2a8554ed19..45865825e46 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -26,19 +26,23 @@ module Gitlab
name.to_s.start_with?('.')
end
- private
-
- def compose!(_deps)
- @config.each do |name, config|
- node = hidden?(name) ? Node::HiddenJob : Node::Job
-
- factory = Node::Factory.new(node)
- .value(config || {})
- .metadata(name: name)
- .with(key: name, parent: self,
- description: "#{name} job definition.")
+ def compose!(deps = nil)
+ super do
+ @config.each do |name, config|
+ node = hidden?(name) ? Node::HiddenJob : Node::Job
+
+ factory = Node::Factory.new(node)
+ .value(config || {})
+ .metadata(name: name)
+ .with(key: name, parent: self,
+ description: "#{name} job definition.")
+
+ @entries[name] = factory.create!
+ end
- @entries[name] = factory.create!
+ @entries.each_value do |entry|
+ entry.compose!(deps)
+ end
end
end
end
--
GitLab
From 4f837f6690970bde315b8c9d76190ed45f15e49e Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Aug 2016 13:58:46 +0200
Subject: [PATCH 4/9] Remove temporary stub method from ci config nodes
---
lib/gitlab/ci/config/node/configurable.rb | 2 +-
lib/gitlab/ci/config/node/entry.rb | 6 -----
lib/gitlab/ci/config/node/job.rb | 2 +-
spec/lib/gitlab/ci/config/node/cache_spec.rb | 2 +-
spec/lib/gitlab/ci/config/node/global_spec.rb | 26 ++++++++++++-------
spec/lib/gitlab/ci/config/node/job_spec.rb | 2 +-
spec/lib/gitlab/ci/config/node/jobs_spec.rb | 6 ++---
spec/lib/gitlab/ci/config/node/script_spec.rb | 4 +--
8 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 530f95a6251..6b7ab2fdaf2 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -23,7 +23,7 @@ module Gitlab
end
end
- def compose!(deps)
+ def compose!(deps = nil)
return unless valid?
self.class.nodes.each do |key, factory|
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index e02f4bf0a05..907d25a8c49 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,12 +20,6 @@ module Gitlab
@validator.validate(:new)
end
- # Temporary method
- #
- def process!(deps = nil)
- compose!(deps)
- end
-
def compose!(deps = nil)
return unless valid?
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 9317bb40f50..3e6666d751f 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -82,7 +82,7 @@ module Gitlab
:cache, :image, :services, :only, :except, :variables,
:artifacts
- def compose!(deps)
+ def compose!(deps = nil)
super do
if type_defined? && !stage_defined?
@entries[:stage] = @entries[:type]
diff --git a/spec/lib/gitlab/ci/config/node/cache_spec.rb b/spec/lib/gitlab/ci/config/node/cache_spec.rb
index 50f619ce26e..e251210949c 100644
--- a/spec/lib/gitlab/ci/config/node/cache_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/cache_spec.rb
@@ -4,7 +4,7 @@ describe Gitlab::Ci::Config::Node::Cache do
let(:entry) { described_class.new(config) }
describe 'validations' do
- before { entry.process! }
+ before { entry.compose! }
context 'when entry config value is correct' do
let(:config) do
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index dfe75bd09e5..e033b423fdd 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -27,8 +27,8 @@ describe Gitlab::Ci::Config::Node::Global do
spinach: { script: 'spinach' } }
end
- describe '#process!' do
- before { global.process! }
+ describe '#compose!' do
+ before { global.compose! }
it 'creates nodes hash' do
expect(global.descendants).to be_an Array
@@ -59,7 +59,7 @@ describe Gitlab::Ci::Config::Node::Global do
end
end
- context 'when not processed' do
+ context 'when not composed' do
describe '#before_script' do
it 'returns nil' do
expect(global.before_script).to be nil
@@ -73,8 +73,8 @@ describe Gitlab::Ci::Config::Node::Global do
end
end
- context 'when processed' do
- before { global.process! }
+ context 'when composed' do
+ before { global.compose! }
describe '#before_script' do
it 'returns correct script' do
@@ -148,8 +148,11 @@ describe Gitlab::Ci::Config::Node::Global do
end
context 'when most of entires not defined' do
- let(:hash) { { cache: { key: 'a' }, rspec: { script: %w[ls] } } }
- before { global.process! }
+ before { global.compose! }
+
+ let(:hash) do
+ { cache: { key: 'a' }, rspec: { script: %w[ls] } }
+ end
describe '#nodes' do
it 'instantizes all nodes' do
@@ -188,8 +191,11 @@ describe Gitlab::Ci::Config::Node::Global do
# details.
#
context 'when entires specified but not defined' do
- let(:hash) { { variables: nil, rspec: { script: 'rspec' } } }
- before { global.process! }
+ before { global.compose! }
+
+ let(:hash) do
+ { variables: nil, rspec: { script: 'rspec' } }
+ end
describe '#variables' do
it 'undefined entry returns a default value' do
@@ -200,7 +206,7 @@ describe Gitlab::Ci::Config::Node::Global do
end
context 'when hash is not valid' do
- before { global.process! }
+ before { global.compose! }
let(:hash) do
{ before_script: 'ls' }
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 1484fb60dd8..74aa616720e 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -3,7 +3,7 @@ require 'spec_helper'
describe Gitlab::Ci::Config::Node::Job do
let(:entry) { described_class.new(config, name: :rspec) }
- before { entry.process! }
+ before { entry.compose! }
describe 'validations' do
context 'when entry config value is correct' do
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index b8d9c70479c..4bf01835c0a 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -4,7 +4,7 @@ describe Gitlab::Ci::Config::Node::Jobs do
let(:entry) { described_class.new(config) }
describe 'validations' do
- before { entry.process! }
+ before { entry.compose! }
context 'when entry config value is correct' do
let(:config) { { rspec: { script: 'rspec' } } }
@@ -47,8 +47,8 @@ describe Gitlab::Ci::Config::Node::Jobs do
end
end
- context 'when valid job entries processed' do
- before { entry.process! }
+ context 'when valid job entries composed' do
+ before { entry.compose! }
let(:config) do
{ rspec: { script: 'rspec' },
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index ee7395362a9..219a7e981d3 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -3,9 +3,7 @@ require 'spec_helper'
describe Gitlab::Ci::Config::Node::Script do
let(:entry) { described_class.new(config) }
- describe '#process!' do
- before { entry.process! }
-
+ describe 'validations' do
context 'when entry config value is correct' do
let(:config) { ['ls', 'pwd'] }
--
GitLab
From 30f58cf3924610564ca95b0ac17b69d272e74f5f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Aug 2016 14:31:06 +0200
Subject: [PATCH 5/9] Add [] method for accessing ci entry dependencies
---
lib/gitlab/ci/config/node/entry.rb | 4 ++++
spec/lib/gitlab/ci/config/node/global_spec.rb | 23 +++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index 907d25a8c49..8717eabf81e 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -20,6 +20,10 @@ module Gitlab
@validator.validate(:new)
end
+ def [](key)
+ @entries[key] || Node::Undefined.new
+ end
+
def compose!(deps = nil)
return unless valid?
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index e033b423fdd..4ff2320f1bf 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -253,4 +253,27 @@ describe Gitlab::Ci::Config::Node::Global do
expect(global.specified?).to be true
end
end
+
+ describe '#[]' do
+ before { global.compose! }
+
+ let(:hash) do
+ { cache: { key: 'a' }, rspec: { script: 'ls' } }
+ end
+
+ context 'when node exists' do
+ it 'returns correct entry' do
+ expect(global[:cache])
+ .to be_an_instance_of Gitlab::Ci::Config::Node::Cache
+ expect(global[:jobs][:rspec][:script].value).to eq ['ls']
+ end
+ end
+
+ context 'when node does not exist' do
+ it 'always return unspecified node' do
+ expect(global[:some][:unknown][:node])
+ .not_to be_specified
+ end
+ end
+ end
end
--
GitLab
From 62f704c5ad421a538257917d75f68b3c698b9be0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 29 Aug 2016 12:44:14 +0200
Subject: [PATCH 6/9] Make it possible to inherit global ci config nodes
---
lib/gitlab/ci/config/node/job.rb | 15 +++++++
spec/lib/gitlab/ci/config/node/global_spec.rb | 30 ++++++++++---
spec/lib/gitlab/ci/config/node/job_spec.rb | 43 ++++++++++++++++++-
3 files changed, 80 insertions(+), 8 deletions(-)
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 3e6666d751f..745f03ae4d5 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -90,6 +90,8 @@ module Gitlab
@entries.delete(:type)
end
+
+ inherit!(deps)
end
def name
@@ -102,6 +104,19 @@ module Gitlab
private
+ def inherit!(deps)
+ return unless deps
+
+ self.class.nodes.each_key do |key|
+ global_entry = deps[key]
+ job_entry = @entries[key]
+
+ if global_entry.specified? && !job_entry.specified?
+ @entries[key] = global_entry
+ end
+ end
+ end
+
def to_hash
{ name: name,
before_script: before_script,
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 4ff2320f1bf..951f5f956d8 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -14,7 +14,7 @@ describe Gitlab::Ci::Config::Node::Global do
end
context 'when hash is valid' do
- context 'when all entries defined' do
+ context 'when some entries defined' do
let(:hash) do
{ before_script: ['ls', 'pwd'],
image: 'ruby:2.2',
@@ -24,7 +24,7 @@ describe Gitlab::Ci::Config::Node::Global do
stages: ['build', 'pages'],
cache: { key: 'k', untracked: true, paths: ['public/'] },
rspec: { script: %w[rspec ls] },
- spinach: { script: 'spinach' } }
+ spinach: { before_script: [], variables: {}, script: 'spinach' } }
end
describe '#compose!' do
@@ -76,6 +76,12 @@ describe Gitlab::Ci::Config::Node::Global do
context 'when composed' do
before { global.compose! }
+ describe '#errors' do
+ it 'has no errors' do
+ expect(global.errors).to be_empty
+ end
+ end
+
describe '#before_script' do
it 'returns correct script' do
expect(global.before_script).to eq ['ls', 'pwd']
@@ -135,12 +141,24 @@ describe Gitlab::Ci::Config::Node::Global do
describe '#jobs' do
it 'returns jobs configuration' do
expect(global.jobs).to eq(
- rspec: { name: :rspec,
- script: %w[rspec ls],
- stage: 'test' },
+ rspec: { script: %w[rspec ls],
+ name: :rspec,
+ before_script: ['ls', 'pwd'],
+ image: 'ruby:2.2',
+ services: ['postgres:9.1', 'mysql:5.5'],
+ stage: 'test',
+ cache: { key: 'k', untracked: true, paths: ['public/'] },
+ variables: { VAR: 'value' },
+ after_script: ['make clean'] },
spinach: { name: :spinach,
script: %w[spinach],
- stage: 'test' }
+ before_script: [],
+ image: 'ruby:2.2',
+ services: ['postgres:9.1', 'mysql:5.5'],
+ stage: 'test',
+ cache: { key: 'k', untracked: true, paths: ['public/'] },
+ variables: {},
+ after_script: ['make clean'] },
)
end
end
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index 74aa616720e..f34a3786a0d 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -3,9 +3,9 @@ require 'spec_helper'
describe Gitlab::Ci::Config::Node::Job do
let(:entry) { described_class.new(config, name: :rspec) }
- before { entry.compose! }
-
describe 'validations' do
+ before { entry.compose! }
+
context 'when entry config value is correct' do
let(:config) { { script: 'rspec' } }
@@ -60,6 +60,8 @@ describe Gitlab::Ci::Config::Node::Job do
end
describe '#value' do
+ before { entry.compose! }
+
context 'when entry is correct' do
let(:config) do
{ before_script: %w[ls pwd],
@@ -83,4 +85,41 @@ describe Gitlab::Ci::Config::Node::Job do
expect(entry).to be_relevant
end
end
+
+ describe '#compose!' do
+ let(:unspecified) { double('unspecified', 'specified?' => false) }
+
+ let(:specified) do
+ double('specified', 'specified?' => true, value: 'specified')
+ end
+
+ let(:deps) { spy('deps', '[]' => unspecified) }
+
+ context 'when job config overrides global config' do
+ before { entry.compose!(deps) }
+
+ let(:config) do
+ { image: 'some_image', cache: { key: 'test' } }
+ end
+
+ it 'overrides global config' do
+ expect(entry[:image].value).to eq 'some_image'
+ expect(entry[:cache].value).to eq(key: 'test')
+ end
+ end
+
+ context 'when job config does not override global config' do
+ before do
+ allow(deps).to receive('[]').with(:image).and_return(specified)
+ entry.compose!(deps)
+ end
+
+ let(:config) { { script: 'ls', cache: { key: 'test' } } }
+
+ it 'uses config from global entry' do
+ expect(entry[:image].value).to eq 'specified'
+ expect(entry[:cache].value).to eq(key: 'test')
+ end
+ end
+ end
end
--
GitLab
From bd807503d14f2592e2c89c361c6967866580e977 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 29 Aug 2016 13:10:57 +0200
Subject: [PATCH 7/9] Do not override job nodes in legacy ci config code
---
lib/ci/gitlab_ci_yaml_processor.rb | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 47efd5bd9f2..5f1ba97214e 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -60,7 +60,7 @@ module Ci
# - before script behaves differently than after script
# - after script returns an array of commands
# - before script should be a concatenated command
- commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"),
+ commands: [job[:before_script], job[:script]].flatten.compact.join("\n"),
tag_list: job[:tags] || [],
name: job[:name].to_s,
allow_failure: job[:allow_failure] || false,
@@ -68,12 +68,12 @@ module Ci
environment: job[:environment],
yaml_variables: yaml_variables(name),
options: {
- image: job[:image] || @image,
- services: job[:services] || @services,
+ image: job[:image],
+ services: job[:services],
artifacts: job[:artifacts],
- cache: job[:cache] || @cache,
+ cache: job[:cache],
dependencies: job[:dependencies],
- after_script: job[:after_script] || @after_script,
+ after_script: job[:after_script],
}.compact
}
end
--
GitLab
From 1255205d896d070a6d8bcefa3774116263db84be Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 29 Aug 2016 13:11:35 +0200
Subject: [PATCH 8/9] Add method that returns commands for ci job entry
---
lib/ci/gitlab_ci_yaml_processor.rb | 7 +--
lib/gitlab/ci/config/node/job.rb | 7 ++-
spec/lib/gitlab/ci/config/node/global_spec.rb | 8 ++-
spec/lib/gitlab/ci/config/node/job_spec.rb | 57 ++++++++++++-------
spec/lib/gitlab/ci/config/node/jobs_spec.rb | 2 +
5 files changed, 50 insertions(+), 31 deletions(-)
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index 5f1ba97214e..c00c2020b4f 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -55,12 +55,7 @@ module Ci
{
stage_idx: @stages.index(job[:stage]),
stage: job[:stage],
- ##
- # Refactoring note:
- # - before script behaves differently than after script
- # - after script returns an array of commands
- # - before script should be a concatenated command
- commands: [job[:before_script], job[:script]].flatten.compact.join("\n"),
+ commands: job[:commands],
tag_list: job[:tags] || [],
name: job[:name].to_s,
allow_failure: job[:allow_failure] || false,
diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb
index 745f03ae4d5..0cbdf7619c0 100644
--- a/lib/gitlab/ci/config/node/job.rb
+++ b/lib/gitlab/ci/config/node/job.rb
@@ -80,7 +80,7 @@ module Gitlab
helpers :before_script, :script, :stage, :type, :after_script,
:cache, :image, :services, :only, :except, :variables,
- :artifacts
+ :artifacts, :commands
def compose!(deps = nil)
super do
@@ -102,6 +102,10 @@ module Gitlab
@config.merge(to_hash.compact)
end
+ def commands
+ (before_script_value.to_a + script_value.to_a).join("\n")
+ end
+
private
def inherit!(deps)
@@ -121,6 +125,7 @@ module Gitlab
{ name: name,
before_script: before_script,
script: script,
+ commands: commands,
image: image,
services: services,
stage: stage,
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 951f5f956d8..12232ff7e2f 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -141,9 +141,10 @@ describe Gitlab::Ci::Config::Node::Global do
describe '#jobs' do
it 'returns jobs configuration' do
expect(global.jobs).to eq(
- rspec: { script: %w[rspec ls],
- name: :rspec,
+ rspec: { name: :rspec,
+ script: %w[rspec ls],
before_script: ['ls', 'pwd'],
+ commands: "ls\npwd\nrspec\nls",
image: 'ruby:2.2',
services: ['postgres:9.1', 'mysql:5.5'],
stage: 'test',
@@ -151,8 +152,9 @@ describe Gitlab::Ci::Config::Node::Global do
variables: { VAR: 'value' },
after_script: ['make clean'] },
spinach: { name: :spinach,
- script: %w[spinach],
before_script: [],
+ script: %w[spinach],
+ commands: 'spinach',
image: 'ruby:2.2',
services: ['postgres:9.1', 'mysql:5.5'],
stage: 'test',
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index f34a3786a0d..c660ce39c46 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -59,27 +59,6 @@ describe Gitlab::Ci::Config::Node::Job do
end
end
- describe '#value' do
- before { entry.compose! }
-
- context 'when entry is correct' do
- let(:config) do
- { before_script: %w[ls pwd],
- script: 'rspec',
- after_script: %w[cleanup] }
- end
-
- it 'returns correct value' do
- expect(entry.value)
- .to eq(name: :rspec,
- before_script: %w[ls pwd],
- script: %w[rspec],
- stage: 'test',
- after_script: %w[cleanup])
- end
- end
- end
-
describe '#relevant?' do
it 'is a relevant entry' do
expect(entry).to be_relevant
@@ -122,4 +101,40 @@ describe Gitlab::Ci::Config::Node::Job do
end
end
end
+
+ context 'when composed' do
+ before { entry.compose! }
+
+ describe '#value' do
+ before { entry.compose! }
+
+ context 'when entry is correct' do
+ let(:config) do
+ { before_script: %w[ls pwd],
+ script: 'rspec',
+ after_script: %w[cleanup] }
+ end
+
+ it 'returns correct value' do
+ expect(entry.value)
+ .to eq(name: :rspec,
+ before_script: %w[ls pwd],
+ script: %w[rspec],
+ commands: "ls\npwd\nrspec",
+ stage: 'test',
+ after_script: %w[cleanup])
+ end
+ end
+ end
+
+ describe '#commands' do
+ let(:config) do
+ { before_script: %w[ls pwd], script: 'rspec' }
+ end
+
+ it 'returns a string of commands concatenated with new line character' do
+ expect(entry.commands).to eq "ls\npwd\nrspec"
+ end
+ end
+ end
end
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index 4bf01835c0a..420d137270a 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -61,9 +61,11 @@ describe Gitlab::Ci::Config::Node::Jobs do
expect(entry.value).to eq(
rspec: { name: :rspec,
script: %w[rspec],
+ commands: 'rspec',
stage: 'test' },
spinach: { name: :spinach,
script: %w[spinach],
+ commands: 'spinach',
stage: 'test' })
end
end
--
GitLab
From b3cd41b4014fa96780218f0f086de239731ec91a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Sat, 3 Sep 2016 09:31:02 +0200
Subject: [PATCH 9/9] Use double instead of spy in job config node specs
---
spec/lib/gitlab/ci/config/node/job_spec.rb | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb
index c660ce39c46..91f676dae03 100644
--- a/spec/lib/gitlab/ci/config/node/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/job_spec.rb
@@ -72,7 +72,7 @@ describe Gitlab::Ci::Config::Node::Job do
double('specified', 'specified?' => true, value: 'specified')
end
- let(:deps) { spy('deps', '[]' => unspecified) }
+ let(:deps) { double('deps', '[]' => unspecified) }
context 'when job config overrides global config' do
before { entry.compose!(deps) }
--
GitLab