summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2016-07-05 08:37:16 +0000
committerRémy Coutable <remy@rymai.me>2016-07-05 08:37:16 +0000
commit06c7d6f3a863a1ac8d9f47fed8423387d6e672a6 (patch)
treec3c182c203626e4fd67f13b2df25998f46d57f79
parentba9ef7f3935cfaa42fcdb2317567cc383c7e9c22 (diff)
parentbfad4c61f10f689868817cf0b94cddaa1de22240 (diff)
downloadgitlab-ce-06c7d6f3a863a1ac8d9f47fed8423387d6e672a6.tar.gz
Merge branch 'refactor/ci-config-move-global-entries' into 'master'
Move global ci entries handling from legacy to new config ## What does this MR do? This MR moves responsibility of handling global CI config entries (like `image`, `services`), from legacy `GitlabCiYamlProcessor` to new CI Config ## Why was this MR needed? This is the next iteration of CI configuration refactoring ## What are the relevant issue numbers? #15060 ## Does this MR meet the acceptance criteria? - Tests - [x] Added for this feature/bug - [x] All builds are passing - [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides) - [x] Branch has no merge conflicts with `master` (if you do - rebase it please) See merge request !4820
-rw-r--r--lib/ci/gitlab_ci_yaml_processor.rb93
-rw-r--r--lib/gitlab/ci/config.rb3
-rw-r--r--lib/gitlab/ci/config/node/boolean.rb18
-rw-r--r--lib/gitlab/ci/config/node/cache.rb27
-rw-r--r--lib/gitlab/ci/config/node/configurable.rb30
-rw-r--r--lib/gitlab/ci/config/node/entry.rb23
-rw-r--r--lib/gitlab/ci/config/node/factory.rb30
-rw-r--r--lib/gitlab/ci/config/node/global.rb30
-rw-r--r--lib/gitlab/ci/config/node/image.rb18
-rw-r--r--lib/gitlab/ci/config/node/key.rb18
-rw-r--r--lib/gitlab/ci/config/node/null.rb27
-rw-r--r--lib/gitlab/ci/config/node/paths.rb18
-rw-r--r--lib/gitlab/ci/config/node/script.rb9
-rw-r--r--lib/gitlab/ci/config/node/services.rb18
-rw-r--r--lib/gitlab/ci/config/node/stages.rb22
-rw-r--r--lib/gitlab/ci/config/node/undefined.rb30
-rw-r--r--lib/gitlab/ci/config/node/validator.rb18
-rw-r--r--lib/gitlab/ci/config/node/validators.rb49
-rw-r--r--lib/gitlab/ci/config/node/variables.rb22
-rw-r--r--spec/lib/ci/gitlab_ci_yaml_processor_spec.rb47
-rw-r--r--spec/lib/gitlab/ci/config/node/boolean_spec.rb34
-rw-r--r--spec/lib/gitlab/ci/config/node/cache_spec.rb60
-rw-r--r--spec/lib/gitlab/ci/config/node/configurable_spec.rb34
-rw-r--r--spec/lib/gitlab/ci/config/node/factory_spec.rb27
-rw-r--r--spec/lib/gitlab/ci/config/node/global_spec.rb186
-rw-r--r--spec/lib/gitlab/ci/config/node/image_spec.rb46
-rw-r--r--spec/lib/gitlab/ci/config/node/key_spec.rb34
-rw-r--r--spec/lib/gitlab/ci/config/node/null_spec.rb23
-rw-r--r--spec/lib/gitlab/ci/config/node/paths_spec.rb34
-rw-r--r--spec/lib/gitlab/ci/config/node/script_spec.rb6
-rw-r--r--spec/lib/gitlab/ci/config/node/services_spec.rb40
-rw-r--r--spec/lib/gitlab/ci/config/node/stages_spec.rb46
-rw-r--r--spec/lib/gitlab/ci/config/node/undefined_spec.rb40
-rw-r--r--spec/lib/gitlab/ci/config/node/validator_spec.rb42
-rw-r--r--spec/lib/gitlab/ci/config/node/variables_spec.rb48
-rw-r--r--spec/lib/gitlab/ci/config_spec.rb46
36 files changed, 1021 insertions, 275 deletions
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index c52d4d63382..01ef13df57a 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -4,7 +4,6 @@ module Ci
include Gitlab::Ci::Config::Node::LegacyValidationHelpers
- DEFAULT_STAGES = %w(build test deploy)
DEFAULT_STAGE = 'test'
ALLOWED_YAML_KEYS = [:before_script, :after_script, :image, :services, :types, :stages, :variables, :cache]
ALLOWED_JOB_KEYS = [:tags, :script, :only, :except, :type, :image, :services,
@@ -14,7 +13,7 @@ module Ci
ALLOWED_CACHE_KEYS = [:key, :untracked, :paths]
ALLOWED_ARTIFACTS_KEYS = [:name, :untracked, :paths, :when, :expire_in]
- attr_reader :after_script, :image, :services, :path, :cache
+ attr_reader :path, :cache, :stages
def initialize(config, path = nil)
@ci_config = Gitlab::Ci::Config.new(config)
@@ -22,8 +21,11 @@ module Ci
@path = path
- initial_parsing
+ unless @ci_config.valid?
+ raise ValidationError, @ci_config.errors.first
+ end
+ initial_parsing
validate!
rescue Gitlab::Ci::Config::Loader::FormatError => e
raise ValidationError, e.message
@@ -42,10 +44,6 @@ module Ci
end
end
- def stages
- @stages || DEFAULT_STAGES
- end
-
def global_variables
@variables
end
@@ -60,12 +58,14 @@ module Ci
private
def initial_parsing
- @after_script = @config[:after_script]
- @image = @config[:image]
- @services = @config[:services]
- @stages = @config[:stages] || @config[:types]
- @variables = @config[:variables] || {}
- @cache = @config[:cache]
+ @before_script = @ci_config.before_script
+ @image = @ci_config.image
+ @after_script = @ci_config.after_script
+ @services = @ci_config.services
+ @variables = @ci_config.variables
+ @stages = @ci_config.stages
+ @cache = @ci_config.cache
+
@jobs = {}
@config.except!(*ALLOWED_YAML_KEYS)
@@ -85,9 +85,14 @@ module Ci
def build_job(name, job)
{
- stage_idx: stages.index(job[:stage]),
+ stage_idx: @stages.index(job[:stage]),
stage: job[:stage],
- commands: [job[:before_script] || [@ci_config.before_script], job[:script]].flatten.compact.join("\n"),
+ ##
+ # 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] || @before_script, job[:script]].flatten.compact.join("\n"),
tag_list: job[:tags] || [],
name: name,
only: job[:only],
@@ -107,12 +112,6 @@ module Ci
end
def validate!
- unless @ci_config.valid?
- raise ValidationError, @ci_config.errors.first
- end
-
- validate_global!
-
@jobs.each do |name, job|
validate_job!(name, job)
end
@@ -120,50 +119,6 @@ module Ci
true
end
- def validate_global!
- unless @after_script.nil? || validate_array_of_strings(@after_script)
- raise ValidationError, "after_script should be an array of strings"
- end
-
- unless @image.nil? || @image.is_a?(String)
- raise ValidationError, "image should be a string"
- end
-
- unless @services.nil? || validate_array_of_strings(@services)
- raise ValidationError, "services should be an array of strings"
- end
-
- unless @stages.nil? || validate_array_of_strings(@stages)
- raise ValidationError, "stages should be an array of strings"
- end
-
- unless @variables.nil? || validate_variables(@variables)
- raise ValidationError, "variables should be a map of key-value strings"
- end
-
- validate_global_cache! if @cache
- end
-
- def validate_global_cache!
- @cache.keys.each do |key|
- unless ALLOWED_CACHE_KEYS.include? key
- raise ValidationError, "#{name} cache unknown parameter #{key}"
- end
- end
-
- if @cache[:key] && !validate_string(@cache[:key])
- raise ValidationError, "cache:key parameter should be a string"
- end
-
- if @cache[:untracked] && !validate_boolean(@cache[:untracked])
- raise ValidationError, "cache:untracked parameter should be an boolean"
- end
-
- if @cache[:paths] && !validate_array_of_strings(@cache[:paths])
- raise ValidationError, "cache:paths parameter should be an array of strings"
- end
- end
-
def validate_job!(name, job)
validate_job_name!(name)
validate_job_keys!(name, job)
@@ -240,8 +195,8 @@ module Ci
end
def validate_job_stage!(name, job)
- unless job[:stage].is_a?(String) && job[:stage].in?(stages)
- raise ValidationError, "#{name} job: stage parameter should be #{stages.join(", ")}"
+ unless job[:stage].is_a?(String) && job[:stage].in?(@stages)
+ raise ValidationError, "#{name} job: stage parameter should be #{@stages.join(", ")}"
end
end
@@ -305,12 +260,12 @@ module Ci
raise ValidationError, "#{name} job: dependencies parameter should be an array of strings"
end
- stage_index = stages.index(job[:stage])
+ stage_index = @stages.index(job[:stage])
job[:dependencies].each do |dependency|
raise ValidationError, "#{name} job: undefined dependency: #{dependency}" unless @jobs[dependency.to_sym]
- unless stages.index(@jobs[dependency.to_sym][:stage]) < stage_index
+ unless @stages.index(@jobs[dependency.to_sym][:stage]) < stage_index
raise ValidationError, "#{name} job: dependency #{dependency} is not defined in prior stages"
end
end
diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb
index adfd097736e..e6cc1529760 100644
--- a/lib/gitlab/ci/config.rb
+++ b/lib/gitlab/ci/config.rb
@@ -7,7 +7,8 @@ module Gitlab
##
# Temporary delegations that should be removed after refactoring
#
- delegate :before_script, to: :@global
+ delegate :before_script, :image, :services, :after_script, :variables,
+ :stages, :cache, to: :@global
def initialize(config)
@config = Loader.new(config).load!
diff --git a/lib/gitlab/ci/config/node/boolean.rb b/lib/gitlab/ci/config/node/boolean.rb
new file mode 100644
index 00000000000..84b03ee7832
--- /dev/null
+++ b/lib/gitlab/ci/config/node/boolean.rb
@@ -0,0 +1,18 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents a boolean value.
+ #
+ class Boolean < Entry
+ include Validatable
+
+ validations do
+ validates :config, boolean: true
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/cache.rb b/lib/gitlab/ci/config/node/cache.rb
new file mode 100644
index 00000000000..cdf8ba2e35d
--- /dev/null
+++ b/lib/gitlab/ci/config/node/cache.rb
@@ -0,0 +1,27 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents a cache configuration
+ #
+ class Cache < Entry
+ include Configurable
+
+ node :key, Node::Key,
+ description: 'Cache key used to define a cache affinity.'
+
+ node :untracked, Node::Boolean,
+ description: 'Cache all untracked files.'
+
+ node :paths, Node::Paths,
+ description: 'Specify which paths should be cached across builds.'
+
+ validations do
+ validates :config, allowed_keys: true
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/configurable.rb b/lib/gitlab/ci/config/node/configurable.rb
index 374ff71d0f5..37936fc8242 100644
--- a/lib/gitlab/ci/config/node/configurable.rb
+++ b/lib/gitlab/ci/config/node/configurable.rb
@@ -19,35 +19,45 @@ module Gitlab
included do
validations do
- validates :config, hash: true
+ validates :config, type: Hash
end
end
private
def create_node(key, factory)
- factory.with(value: @config[key], key: key)
- factory.nullify! unless @config.has_key?(key)
+ factory.with(value: @config[key], key: key, parent: self)
+
factory.create!
end
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])
- define_method(symbol) do
- raise Entry::InvalidError unless valid?
- @nodes[symbol].try(:value)
- end
+ (@nodes ||= {}).merge!(symbol.to_sym => factory)
+ end
- (@allowed_nodes ||= {}).merge!(symbol => factory)
+ def helpers(*nodes)
+ nodes.each do |symbol|
+ define_method("#{symbol}_defined?") do
+ @nodes[symbol].try(:defined?)
+ end
+
+ define_method("#{symbol}_value") do
+ raise Entry::InvalidError unless valid?
+ @nodes[symbol].try(:value)
+ end
+
+ alias_method symbol.to_sym, "#{symbol}_value".to_sym
+ end
end
end
end
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index f044ef965e9..9e79e170a4f 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -9,7 +9,7 @@ module Gitlab
class InvalidError < StandardError; end
attr_reader :config
- attr_accessor :key, :description
+ attr_accessor :key, :parent, :description
def initialize(config)
@config = config
@@ -34,8 +34,8 @@ module Gitlab
self.class.nodes.none?
end
- def key
- @key || self.class.name.demodulize.underscore
+ def ancestors
+ @parent ? @parent.ancestors + [@parent] : []
end
def valid?
@@ -43,12 +43,23 @@ module Gitlab
end
def errors
- @validator.full_errors +
- nodes.map(&:errors).flatten
+ @validator.messages + nodes.flat_map(&:errors)
end
def value
- raise NotImplementedError
+ if leaf?
+ @config
+ else
+ defined = @nodes.select { |_key, value| value.defined? }
+ Hash[defined.map { |key, node| [key, node.value] }]
+ end
+ end
+
+ def defined?
+ true
+ end
+
+ def self.default
end
def self.nodes
diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb
index 025ae40ef94..5919a283283 100644
--- a/lib/gitlab/ci/config/node/factory.rb
+++ b/lib/gitlab/ci/config/node/factory.rb
@@ -5,13 +5,11 @@ module Gitlab
##
# Factory class responsible for fabricating node entry objects.
#
- # It uses Fluent Interface pattern to set all necessary attributes.
- #
class Factory
class InvalidFactory < StandardError; end
- def initialize(entry_class)
- @entry_class = entry_class
+ def initialize(node)
+ @node = node
@attributes = {}
end
@@ -20,17 +18,27 @@ module Gitlab
self
end
- def nullify!
- @entry_class = Node::Null
- self
- end
-
def create!
raise InvalidFactory unless @attributes.has_key?(:value)
- @entry_class.new(@attributes[:value]).tap do |entry|
- entry.description = @attributes[:description]
+ fabricate.tap do |entry|
entry.key = @attributes[:key]
+ entry.parent = @attributes[:parent]
+ entry.description = @attributes[:description]
+ end
+ end
+
+ private
+
+ def fabricate
+ ##
+ # We assume that unspecified entry is undefined.
+ # See issue #18775.
+ #
+ if @attributes[:value].nil?
+ Node::Undefined.new(@node)
+ else
+ @node.new(@attributes[:value])
end
end
end
diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb
index 044603423d5..f92e1eccbcf 100644
--- a/lib/gitlab/ci/config/node/global.rb
+++ b/lib/gitlab/ci/config/node/global.rb
@@ -9,8 +9,36 @@ module Gitlab
class Global < Entry
include Configurable
- allow_node :before_script, Script,
+ node :before_script, Node::Script,
description: 'Script that will be executed before each job.'
+
+ node :image, Node::Image,
+ description: 'Docker image that will be used to execute jobs.'
+
+ node :services, Node::Services,
+ description: 'Docker images that will be linked to the container.'
+
+ node :after_script, Node::Script,
+ description: 'Script that will be executed after each job.'
+
+ node :variables, Node::Variables,
+ description: 'Environment variables that will be used.'
+
+ node :stages, Node::Stages,
+ description: 'Configuration of stages for this pipeline.'
+
+ node :types, Node::Stages,
+ description: 'Deprecated: stages for this pipeline.'
+
+ node :cache, Node::Cache,
+ description: 'Configure caching between build jobs.'
+
+ helpers :before_script, :image, :services, :after_script,
+ :variables, :stages, :types, :cache
+
+ def stages
+ stages_defined? ? stages_value : types_value
+ end
end
end
end
diff --git a/lib/gitlab/ci/config/node/image.rb b/lib/gitlab/ci/config/node/image.rb
new file mode 100644
index 00000000000..5d3c7c5eab0
--- /dev/null
+++ b/lib/gitlab/ci/config/node/image.rb
@@ -0,0 +1,18 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents a Docker image.
+ #
+ class Image < Entry
+ include Validatable
+
+ validations do
+ validates :config, type: String
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/key.rb b/lib/gitlab/ci/config/node/key.rb
new file mode 100644
index 00000000000..f8b461ca098
--- /dev/null
+++ b/lib/gitlab/ci/config/node/key.rb
@@ -0,0 +1,18 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents a key.
+ #
+ class Key < Entry
+ include Validatable
+
+ validations do
+ validates :config, key: true
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb
deleted file mode 100644
index 4f590f6bec8..00000000000
--- a/lib/gitlab/ci/config/node/null.rb
+++ /dev/null
@@ -1,27 +0,0 @@
-module Gitlab
- module Ci
- class Config
- 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
- end
-
- def validate!
- nil
- end
-
- def method_missing(*)
- nil
- end
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/ci/config/node/paths.rb b/lib/gitlab/ci/config/node/paths.rb
new file mode 100644
index 00000000000..3c6d3a52966
--- /dev/null
+++ b/lib/gitlab/ci/config/node/paths.rb
@@ -0,0 +1,18 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents an array of paths.
+ #
+ class Paths < Entry
+ include Validatable
+
+ validations do
+ validates :config, array_of_strings: true
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/script.rb b/lib/gitlab/ci/config/node/script.rb
index c044f5c5e71..39328f0fade 100644
--- a/lib/gitlab/ci/config/node/script.rb
+++ b/lib/gitlab/ci/config/node/script.rb
@@ -5,21 +5,12 @@ 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 these commands with
- # new line character as a separator, what is compatible with
- # implementation in Runner.
- #
class Script < Entry
include Validatable
validations do
validates :config, array_of_strings: true
end
-
- def value
- @config.join("\n")
- end
end
end
end
diff --git a/lib/gitlab/ci/config/node/services.rb b/lib/gitlab/ci/config/node/services.rb
new file mode 100644
index 00000000000..481e2b66adc
--- /dev/null
+++ b/lib/gitlab/ci/config/node/services.rb
@@ -0,0 +1,18 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents a configuration of Docker services.
+ #
+ class Services < Entry
+ include Validatable
+
+ validations do
+ validates :config, array_of_strings: true
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/stages.rb b/lib/gitlab/ci/config/node/stages.rb
new file mode 100644
index 00000000000..b1fe45357ff
--- /dev/null
+++ b/lib/gitlab/ci/config/node/stages.rb
@@ -0,0 +1,22 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents a configuration for pipeline stages.
+ #
+ class Stages < Entry
+ include Validatable
+
+ validations do
+ validates :config, array_of_strings: true
+ end
+
+ def self.default
+ %w[build test deploy]
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb
new file mode 100644
index 00000000000..699605e1e3a
--- /dev/null
+++ b/lib/gitlab/ci/config/node/undefined.rb
@@ -0,0 +1,30 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # This class represents an undefined entry node.
+ #
+ # It takes original entry class as configuration and returns default
+ # value of original entry as self value.
+ #
+ #
+ class Undefined < Entry
+ include Validatable
+
+ validations do
+ validates :config, type: Class
+ end
+
+ def value
+ @config.default
+ end
+
+ def defined?
+ false
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb
index 02edc9219c3..758a6cf4356 100644
--- a/lib/gitlab/ci/config/node/validator.rb
+++ b/lib/gitlab/ci/config/node/validator.rb
@@ -11,15 +11,29 @@ module Gitlab
@node = node
end
- def full_errors
+ def messages
errors.full_messages.map do |error|
- "#{@node.key} #{error}".humanize
+ "#{location} #{error}".downcase
end
end
def self.name
'Validator'
end
+
+ def unknown_keys
+ return [] unless config.is_a?(Hash)
+
+ config.keys - @node.class.nodes.keys
+ end
+
+ private
+
+ def location
+ predecessors = ancestors.map(&:key).compact
+ current = key || @node.class.name.demodulize.underscore
+ predecessors.append(current).join(':')
+ end
end
end
end
diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb
index dc9cdb9a220..7b2f57990b5 100644
--- a/lib/gitlab/ci/config/node/validators.rb
+++ b/lib/gitlab/ci/config/node/validators.rb
@@ -3,6 +3,16 @@ module Gitlab
class Config
module Node
module Validators
+ class AllowedKeysValidator < ActiveModel::EachValidator
+ def validate_each(record, attribute, value)
+ if record.unknown_keys.any?
+ unknown_list = record.unknown_keys.join(', ')
+ record.errors.add(:config,
+ "contains unknown keys: #{unknown_list}")
+ end
+ end
+ end
+
class ArrayOfStringsValidator < ActiveModel::EachValidator
include LegacyValidationHelpers
@@ -13,10 +23,43 @@ module Gitlab
end
end
- class HashValidator < ActiveModel::EachValidator
+ class BooleanValidator < ActiveModel::EachValidator
+ include LegacyValidationHelpers
+
+ def validate_each(record, attribute, value)
+ unless validate_boolean(value)
+ record.errors.add(attribute, 'should be a boolean value')
+ end
+ end
+ end
+
+ class KeyValidator < ActiveModel::EachValidator
+ include LegacyValidationHelpers
+
+ def validate_each(record, attribute, value)
+ unless validate_string(value)
+ record.errors.add(attribute, 'should be a string or symbol')
+ end
+ end
+ end
+
+ class TypeValidator < ActiveModel::EachValidator
+ def validate_each(record, attribute, value)
+ type = options[:with]
+ raise unless type.is_a?(Class)
+
+ unless value.is_a?(type)
+ record.errors.add(attribute, "should be a #{type.name}")
+ end
+ end
+ end
+
+ class VariablesValidator < ActiveModel::EachValidator
+ include LegacyValidationHelpers
+
def validate_each(record, attribute, value)
- unless value.is_a?(Hash)
- record.errors.add(attribute, 'should be a configuration entry hash')
+ unless validate_variables(value)
+ record.errors.add(attribute, 'should be a hash of key value pairs')
end
end
end
diff --git a/lib/gitlab/ci/config/node/variables.rb b/lib/gitlab/ci/config/node/variables.rb
new file mode 100644
index 00000000000..5f813f81f55
--- /dev/null
+++ b/lib/gitlab/ci/config/node/variables.rb
@@ -0,0 +1,22 @@
+module Gitlab
+ module Ci
+ class Config
+ module Node
+ ##
+ # Entry that represents environment variables.
+ #
+ class Variables < Entry
+ include Validatable
+
+ validations do
+ validates :config, variables: true
+ end
+
+ def self.default
+ {}
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index ec658668c61..bad439bc489 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -550,8 +550,8 @@ module Ci
config_processor = GitlabCiYamlProcessor.new(config, path)
##
- # TODO, in next version of CI configuration processor this
- # should be invalid configuration, see #18775 and #15060
+ # When variables config is empty, we assume this is a valid
+ # configuration, see issue #18775
#
expect(config_processor.job_variables(:rspec))
.to be_an_instance_of(Array).and be_empty
@@ -590,7 +590,20 @@ module Ci
end
end
- describe "Caches" do
+ describe 'cache' do
+ context 'when cache definition has unknown keys' do
+ it 'raises relevant validation error' do
+ config = YAML.dump(
+ { cache: { untracked: true, invalid: 'key' },
+ rspec: { script: 'rspec' } })
+
+ expect { GitlabCiYamlProcessor.new(config) }.to raise_error(
+ GitlabCiYamlProcessor::ValidationError,
+ 'cache config contains unknown keys: invalid'
+ )
+ end
+ end
+
it "returns cache when defined globally" do
config = YAML.dump({
cache: { paths: ["logs/", "binaries/"], untracked: true, key: 'key' },
@@ -950,7 +963,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 config should be an array of strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "before_script config should be an array of strings")
end
it "returns errors if job before_script parameter is not an array of strings" do
@@ -964,7 +977,7 @@ EOT
config = YAML.dump({ after_script: "bundle update", rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script should be an array of strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "after_script config should be an array of strings")
end
it "returns errors if job after_script parameter is not an array of strings" do
@@ -978,7 +991,7 @@ EOT
config = YAML.dump({ image: ["test"], rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image should be a string")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "image config should be a string")
end
it "returns errors if job name is blank" do
@@ -1006,14 +1019,14 @@ EOT
config = YAML.dump({ services: "test", rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
end
it "returns errors if services parameter is not an array of strings" do
config = YAML.dump({ services: [10, "test"], rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services should be an array of strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "services config should be an array of strings")
end
it "returns errors if job services parameter is not an array" do
@@ -1080,31 +1093,31 @@ EOT
end
it "returns errors if stages is not an array" do
- config = YAML.dump({ types: "test", rspec: { script: "test" } })
+ config = YAML.dump({ stages: "test", rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
end
it "returns errors if stages is not an array of strings" do
- config = YAML.dump({ types: [true, "test"], rspec: { script: "test" } })
+ config = YAML.dump({ stages: [true, "test"], rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages should be an array of strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "stages config should be an array of strings")
end
it "returns errors if variables is not a map" do
config = YAML.dump({ variables: "test", rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-value strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config should be a hash of key value pairs")
end
it "returns errors if variables is not a map of key-value strings" do
config = YAML.dump({ variables: { test: false }, rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config, path)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables should be a map of key-value strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "variables config should be a hash of key value pairs")
end
it "returns errors if job when is not on_success, on_failure or always" do
@@ -1160,21 +1173,21 @@ EOT
config = YAML.dump({ cache: { untracked: "string" }, rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked parameter should be an boolean")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:untracked config should be a boolean value")
end
it "returns errors if cache:paths is not an array of strings" do
config = YAML.dump({ cache: { paths: "string" }, rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths parameter should be an array of strings")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:paths config should be an array of strings")
end
it "returns errors if cache:key is not a string" do
config = YAML.dump({ cache: { key: 1 }, rspec: { script: "test" } })
expect do
GitlabCiYamlProcessor.new(config)
- end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key parameter should be a string")
+ end.to raise_error(GitlabCiYamlProcessor::ValidationError, "cache:key config should be a string or symbol")
end
it "returns errors if job cache:key is not an a string" do
diff --git a/spec/lib/gitlab/ci/config/node/boolean_spec.rb b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
new file mode 100644
index 00000000000..deafa8bf8a7
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/boolean_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Boolean do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validations' do
+ context 'when entry config value is valid' do
+ let(:config) { false }
+
+ describe '#value' do
+ it 'returns key value' do
+ expect(entry.value).to eq false
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not valid' do
+ let(:config) { ['incorrect'] }
+
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include 'boolean config should be a boolean value'
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/node/cache_spec.rb b/spec/lib/gitlab/ci/config/node/cache_spec.rb
new file mode 100644
index 00000000000..50f619ce26e
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/cache_spec.rb
@@ -0,0 +1,60 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Cache do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validations' do
+ before { entry.process! }
+
+ context 'when entry config value is correct' do
+ let(:config) do
+ { key: 'some key',
+ untracked: true,
+ paths: ['some/path/'] }
+ end
+
+ describe '#value' do
+ it 'returns hash value' do
+ expect(entry.value).to eq config
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not correct' do
+ describe '#errors' do
+ context 'when is not a hash' do
+ let(:config) { 'ls' }
+
+ it 'reports errors with config value' do
+ expect(entry.errors)
+ .to include 'cache config should be a hash'
+ end
+ end
+
+ context 'when descendants are invalid' do
+ let(:config) { { key: 1 } }
+
+ it 'reports error with descendants' do
+ expect(entry.errors)
+ .to include 'key config should be a string or symbol'
+ end
+ end
+
+ context 'when there is an unknown key present' do
+ let(:config) { { invalid: true } }
+
+ it 'reports error with descendants' do
+ expect(entry.errors)
+ .to include 'cache config contains unknown keys: invalid'
+ end
+ end
+ end
+ 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..c468ecf957b 100644
--- a/spec/lib/gitlab/ci/config/node/configurable_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/configurable_spec.rb
@@ -7,10 +7,42 @@ describe Gitlab::Ci::Config::Node::Configurable do
node.include(described_class)
end
+ describe 'validations' do
+ let(:validator) { node.validator.new(instance) }
+
+ before do
+ node.class_eval do
+ attr_reader :config
+
+ def initialize(config)
+ @config = config
+ end
+ end
+
+ validator.validate
+ end
+
+ context 'when node validator is invalid' do
+ let(:instance) { node.new('ls') }
+
+ it 'returns invalid validator' do
+ expect(validator).to be_invalid
+ end
+ end
+
+ context 'when node instance is valid' do
+ let(:instance) { node.new(key: 'value') }
+
+ it 'returns valid validator' do
+ expect(validator).to be_valid
+ end
+ end
+ end
+
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/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb
index 01a707a6bd4..91ddef7bfbf 100644
--- a/spec/lib/gitlab/ci/config/node/factory_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb
@@ -5,13 +5,13 @@ describe Gitlab::Ci::Config::Node::Factory do
let(:factory) { described_class.new(entry_class) }
let(:entry_class) { Gitlab::Ci::Config::Node::Script }
- context 'when value setting value' do
+ context 'when setting up a value' do
it 'creates entry with valid value' do
entry = factory
.with(value: ['ls', 'pwd'])
.create!
- expect(entry.value).to eq "ls\npwd"
+ expect(entry.value).to eq ['ls', 'pwd']
end
context 'when setting description' do
@@ -21,7 +21,7 @@ describe Gitlab::Ci::Config::Node::Factory do
.with(description: 'test description')
.create!
- expect(entry.value).to eq "ls\npwd"
+ expect(entry.value).to eq ['ls', 'pwd']
expect(entry.description).to eq 'test description'
end
end
@@ -35,9 +35,21 @@ describe Gitlab::Ci::Config::Node::Factory do
expect(entry.key).to eq 'test key'
end
end
+
+ context 'when setting a parent' do
+ let(:parent) { Object.new }
+
+ it 'creates entry with valid parent' do
+ entry = factory
+ .with(value: 'ls', parent: parent)
+ .create!
+
+ expect(entry.parent).to eq parent
+ end
+ end
end
- context 'when not setting value' do
+ context 'when not setting up a value' do
it 'raises error' do
expect { factory.create! }.to raise_error(
Gitlab::Ci::Config::Node::Factory::InvalidFactory
@@ -45,14 +57,13 @@ describe Gitlab::Ci::Config::Node::Factory do
end
end
- context 'when creating a null entry' do
- it 'creates a null entry' do
+ context 'when creating entry with nil value' do
+ it 'creates an undefined entry' do
entry = factory
.with(value: nil)
- .nullify!
.create!
- expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Null
+ expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
end
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 fddd53a2b57..c87c9e97bc8 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -13,57 +13,163 @@ 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'] }
- end
+ context 'when all entries defined' do
+ let(:hash) do
+ { before_script: ['ls', 'pwd'],
+ image: 'ruby:2.2',
+ services: ['postgres:9.1', 'mysql:5.5'],
+ variables: { VAR: 'value' },
+ after_script: ['make clean'],
+ stages: ['build', 'pages'],
+ cache: { key: 'k', untracked: true, paths: ['public/'] } }
+ end
- describe '#process!' do
- before { global.process! }
+ describe '#process!' do
+ before { global.process! }
- it 'creates nodes hash' do
- expect(global.nodes).to be_an Array
+ it 'creates nodes hash' do
+ expect(global.nodes).to be_an Array
+ end
+
+ it 'creates node object for each entry' do
+ expect(global.nodes.count).to eq 8
+ end
+
+ it 'creates node object using valid class' do
+ expect(global.nodes.first)
+ .to be_an_instance_of Gitlab::Ci::Config::Node::Script
+ expect(global.nodes.second)
+ .to be_an_instance_of Gitlab::Ci::Config::Node::Image
+ end
+
+ it 'sets correct description for nodes' do
+ expect(global.nodes.first.description)
+ .to eq 'Script that will be executed before each job.'
+ expect(global.nodes.second.description)
+ .to eq 'Docker image that will be used to execute jobs.'
+ end
end
- it 'creates node object for each entry' do
- expect(global.nodes.count).to eq 1
+ describe '#leaf?' do
+ it 'is not leaf' do
+ expect(global).not_to be_leaf
+ end
end
- it 'creates node object using valid class' do
- expect(global.nodes.first)
- .to be_an_instance_of Gitlab::Ci::Config::Node::Script
+ context 'when not processed' do
+ describe '#before_script' do
+ it 'returns nil' do
+ expect(global.before_script).to be nil
+ end
+ end
end
- it 'sets correct description for nodes' do
- expect(global.nodes.first.description)
- .to eq 'Script that will be executed before each job.'
+ context 'when processed' do
+ before { global.process! }
+
+ describe '#before_script' do
+ it 'returns correct script' do
+ expect(global.before_script).to eq ['ls', 'pwd']
+ end
+ end
+
+ describe '#image' do
+ it 'returns valid image' do
+ expect(global.image).to eq 'ruby:2.2'
+ end
+ end
+
+ describe '#services' do
+ it 'returns array of services' do
+ expect(global.services).to eq ['postgres:9.1', 'mysql:5.5']
+ end
+ end
+
+ describe '#after_script' do
+ it 'returns after script' do
+ expect(global.after_script).to eq ['make clean']
+ end
+ end
+
+ describe '#variables' do
+ it 'returns variables' do
+ expect(global.variables).to eq(VAR: 'value')
+ end
+ end
+
+ describe '#stages' do
+ context 'when stages key defined' do
+ it 'returns array of stages' do
+ expect(global.stages).to eq %w[build pages]
+ end
+ end
+
+ context 'when deprecated types key defined' do
+ let(:hash) { { types: ['test', 'deploy'] } }
+
+ it 'returns array of types as stages' do
+ expect(global.stages).to eq %w[test deploy]
+ end
+ end
+ end
+
+ describe '#cache' do
+ it 'returns cache configuration' do
+ expect(global.cache)
+ .to eq(key: 'k', untracked: true, paths: ['public/'])
+ end
+ end
end
end
- describe '#leaf?' do
- it 'is not leaf' do
- expect(global).not_to be_leaf
+ context 'when most of entires not defined' do
+ let(:hash) { { cache: { key: 'a' }, rspec: {} } }
+ before { global.process! }
+
+ describe '#nodes' do
+ it 'instantizes all nodes' do
+ expect(global.nodes.count).to eq 8
+ end
+
+ it 'contains undefined nodes' do
+ expect(global.nodes.first)
+ .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined
+ end
end
- end
- describe '#before_script' do
- context 'when processed' do
- before { global.process! }
+ describe '#variables' do
+ it 'returns default value for variables' do
+ expect(global.variables).to eq({})
+ end
+ end
- it 'returns correct script' do
- expect(global.before_script).to eq "ls\npwd"
+ describe '#stages' do
+ it 'returns an array of default stages' do
+ expect(global.stages).to eq %w[build test deploy]
end
end
- context 'when not processed' do
- it 'returns nil' do
- expect(global.before_script).to be nil
+ describe '#cache' do
+ it 'returns correct cache definition' do
+ expect(global.cache).to eq(key: 'a')
+ end
+ end
+ end
+
+ ##
+ # When nodes are specified but not defined, we assume that
+ # configuration is valid, and we asume that entry is simply undefined,
+ # despite the fact, that key is present. See issue #18775 for more
+ # details.
+ #
+ context 'when entires specified but not defined' do
+ let(:hash) { { variables: nil } }
+ before { global.process! }
+
+ describe '#variables' do
+ it 'undefined entry returns a default value' do
+ expect(global.variables).to eq({})
end
end
end
@@ -85,7 +191,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 config should be an array of strings'
+ .to include 'before_script config should be an array of strings'
end
end
@@ -106,5 +212,17 @@ describe Gitlab::Ci::Config::Node::Global do
expect(global).not_to be_valid
end
end
+
+ describe '#errors' do
+ it 'returns error about invalid type' do
+ expect(global.errors.first).to match /should be a hash/
+ end
+ end
+ end
+
+ describe '#defined?' do
+ it 'is concrete entry that is defined' do
+ expect(global.defined?).to be true
+ end
end
end
diff --git a/spec/lib/gitlab/ci/config/node/image_spec.rb b/spec/lib/gitlab/ci/config/node/image_spec.rb
new file mode 100644
index 00000000000..d11bb39f328
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/image_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Image do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validation' do
+ context 'when entry config value is correct' do
+ let(:config) { 'ruby:2.2' }
+
+ describe '#value' do
+ it 'returns image string' do
+ expect(entry.value).to eq 'ruby:2.2'
+ end
+ end
+
+ describe '#errors' do
+ it 'does not append errors' do
+ expect(entry.errors).to be_empty
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not correct' do
+ let(:config) { ['ruby:2.2'] }
+
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include 'image config should be a string'
+ end
+ end
+
+ describe '#valid?' do
+ it 'is not valid' do
+ expect(entry).not_to be_valid
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/node/key_spec.rb b/spec/lib/gitlab/ci/config/node/key_spec.rb
new file mode 100644
index 00000000000..8cda43173fe
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/key_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Key do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validations' do
+ context 'when entry config value is correct' do
+ let(:config) { 'test' }
+
+ describe '#value' do
+ it 'returns key value' do
+ expect(entry.value).to eq 'test'
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not correct' do
+ let(:config) { [ 'incorrect' ] }
+
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include 'key config should be a string or symbol'
+ end
+ end
+ end
+ 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 36101c62462..00000000000
--- a/spec/lib/gitlab/ci/config/node/null_spec.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Node::Null do
- let(:entry) { described_class.new(nil) }
-
- describe '#leaf?' do
- it 'is leaf node' do
- expect(entry).to be_leaf
- end
- end
-
- describe '#any_method' do
- it 'responds with nil' do
- expect(entry.any_method).to be nil
- end
- end
-
- describe '#value' do
- it 'returns nil' do
- expect(entry.value).to be nil
- end
- end
-end
diff --git a/spec/lib/gitlab/ci/config/node/paths_spec.rb b/spec/lib/gitlab/ci/config/node/paths_spec.rb
new file mode 100644
index 00000000000..6fd744b3975
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/paths_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Paths do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validations' do
+ context 'when entry config value is valid' do
+ let(:config) { ['some/file', 'some/path/'] }
+
+ describe '#value' do
+ it 'returns key value' do
+ expect(entry.value).to eq config
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not valid' do
+ let(:config) { [ 1 ] }
+
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include 'paths config should be an array of strings'
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/node/script_spec.rb b/spec/lib/gitlab/ci/config/node/script_spec.rb
index 6af6aa15eef..ee7395362a9 100644
--- a/spec/lib/gitlab/ci/config/node/script_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/script_spec.rb
@@ -10,8 +10,8 @@ describe Gitlab::Ci::Config::Node::Script do
let(:config) { ['ls', 'pwd'] }
describe '#value' do
- it 'returns concatenated command' do
- expect(entry.value).to eq "ls\npwd"
+ it 'returns array of strings' do
+ expect(entry.value).to eq config
end
end
@@ -34,7 +34,7 @@ describe Gitlab::Ci::Config::Node::Script do
describe '#errors' do
it 'saves errors' do
expect(entry.errors)
- .to include 'Script config should be an array of strings'
+ .to include 'script config should be an array of strings'
end
end
diff --git a/spec/lib/gitlab/ci/config/node/services_spec.rb b/spec/lib/gitlab/ci/config/node/services_spec.rb
new file mode 100644
index 00000000000..be0fe46befd
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/services_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Services do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validations' do
+ context 'when entry config value is correct' do
+ let(:config) { ['postgres:9.1', 'mysql:5.5'] }
+
+ describe '#value' do
+ it 'returns array of services as is' do
+ expect(entry.value).to eq config
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not correct' do
+ let(:config) { 'ls' }
+
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include 'services config should be an array of strings'
+ end
+ end
+
+ describe '#valid?' do
+ it 'is not valid' do
+ expect(entry).not_to be_valid
+ end
+ end
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/node/stages_spec.rb b/spec/lib/gitlab/ci/config/node/stages_spec.rb
new file mode 100644
index 00000000000..1a3818d8997
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/stages_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Stages do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validations' do
+ context 'when entry config value is correct' do
+ let(:config) { [:stage1, :stage2] }
+
+ describe '#value' do
+ it 'returns array of stages' do
+ expect(entry.value).to eq config
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not correct' do
+ let(:config) { { test: true } }
+
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include 'stages config should be an array of strings'
+ end
+ end
+
+ describe '#valid?' do
+ it 'is not valid' do
+ expect(entry).not_to be_valid
+ end
+ end
+ end
+ end
+
+ describe '.default' do
+ it 'returns default stages' do
+ expect(described_class.default).to eq %w[build test deploy]
+ 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
new file mode 100644
index 00000000000..0c6608d906d
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb
@@ -0,0 +1,40 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Undefined do
+ let(:undefined) { described_class.new(entry) }
+ let(:entry) { Class.new }
+
+ describe '#leaf?' do
+ it 'is leaf node' do
+ expect(undefined).to be_leaf
+ end
+ end
+
+ 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
+ before do
+ allow(entry).to receive(:default).and_return('some value')
+ end
+
+ it 'returns default value for entry' do
+ expect(undefined.value).to eq 'some value'
+ end
+ end
+
+ describe '#undefined?' do
+ it 'is not a defined entry' do
+ expect(undefined.defined?).to be false
+ end
+ end
+end
diff --git a/spec/lib/gitlab/ci/config/node/validator_spec.rb b/spec/lib/gitlab/ci/config/node/validator_spec.rb
index ad875d55384..090fd63b844 100644
--- a/spec/lib/gitlab/ci/config/node/validator_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/validator_spec.rb
@@ -5,7 +5,18 @@ describe Gitlab::Ci::Config::Node::Validator do
let(:validator_instance) { validator.new(node) }
let(:node) { spy('node') }
- shared_examples 'delegated validator' do
+ before do
+ allow(node).to receive(:key).and_return('node')
+ allow(node).to receive(:ancestors).and_return([])
+ end
+
+ describe 'delegated validator' do
+ before do
+ validator.class_eval do
+ validates :test_attribute, presence: true
+ end
+ end
+
context 'when node is valid' do
before do
allow(node).to receive(:test_attribute).and_return('valid value')
@@ -19,7 +30,7 @@ describe Gitlab::Ci::Config::Node::Validator do
it 'returns no errors' do
validator_instance.validate
- expect(validator_instance.full_errors).to be_empty
+ expect(validator_instance.messages).to be_empty
end
end
@@ -36,32 +47,9 @@ describe Gitlab::Ci::Config::Node::Validator do
it 'returns errors' do
validator_instance.validate
- expect(validator_instance.full_errors).not_to be_empty
+ expect(validator_instance.messages)
+ .to include "node test attribute can't be blank"
end
end
end
-
- describe 'attributes validations' do
- before do
- validator.class_eval do
- validates :test_attribute, presence: true
- end
- end
-
- it_behaves_like 'delegated validator'
- end
-
- describe 'interface validations' do
- before do
- validator.class_eval do
- validate do
- unless @node.test_attribute == 'valid value'
- errors.add(:test_attribute, 'invalid value')
- end
- end
- end
- end
-
- it_behaves_like 'delegated validator'
- end
end
diff --git a/spec/lib/gitlab/ci/config/node/variables_spec.rb b/spec/lib/gitlab/ci/config/node/variables_spec.rb
new file mode 100644
index 00000000000..4b6d971ec71
--- /dev/null
+++ b/spec/lib/gitlab/ci/config/node/variables_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Config::Node::Variables do
+ let(:entry) { described_class.new(config) }
+
+ describe 'validations' do
+ context 'when entry config value is correct' do
+ let(:config) do
+ { 'VARIABLE_1' => 'value 1', 'VARIABLE_2' => 'value 2' }
+ end
+
+ describe '#value' do
+ it 'returns hash with key value strings' do
+ expect(entry.value).to eq config
+ end
+ end
+
+ describe '#errors' do
+ it 'does not append errors' do
+ expect(entry.errors).to be_empty
+ end
+ end
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when entry value is not correct' do
+ let(:config) { [ :VAR, 'test' ] }
+
+ describe '#errors' do
+ it 'saves errors' do
+ expect(entry.errors)
+ .to include /should be a hash of key value pairs/
+ end
+ end
+
+ describe '#valid?' do
+ it 'is not valid' do
+ expect(entry).not_to be_valid
+ end
+ end
+ 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