diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index c52d4d6338209d5c1cb05f6ad247b929649e7072..01ef13df57ab4fc33c814e98bc854f5c96beadf8 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 adfd097736e3e0347dd6dd889ed13b4aa38b658e..e6cc1529760d9664f5031f07ba4625894527000e 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 0000000000000000000000000000000000000000..84b03ee7832d4d5b205ca99e1aae3812d6015142 --- /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 0000000000000000000000000000000000000000..cdf8ba2e35d249b56156c5b00a2aad05d1f3a8dd --- /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 374ff71d0f5544e40bc0ac9eeb04a164d000da57..37936fc82421d21dd0818021c822aa48f3fcb1e2 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 f044ef965e9c1a19b100a2f9bd9d55caf96a724a..9e79e170a4fabf8cbc385763e71a0e9fc6133cdf 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 025ae40ef944ebb5a8ea4ccbe5d8a7949f11142a..5919a2832836593045328f9c9a84be5b9d95b3dc 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 044603423d51d5489b6a3e644b300b7c65683d54..f92e1eccbcf4f0d2b32a3c03133d8f429acafab8 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 0000000000000000000000000000000000000000..5d3c7c5eab059fdc8daa9eddb9535a2af9bdee41 --- /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 0000000000000000000000000000000000000000..f8b461ca098bbcb3677d8cdb7bdd8e826042e363 --- /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 4f590f6bec80425e5f22b82d362730571dfa0124..0000000000000000000000000000000000000000 --- 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 0000000000000000000000000000000000000000..3c6d3a529661102e37728647a3d15ac75da58499 --- /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 c044f5c5e717b38edb77c22b8fcee2f9978065c0..39328f0fade9cf656d1a605053fd1761b3d3b211 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 0000000000000000000000000000000000000000..481e2b66adce13e93c978aecd8d7417eed61bde2 --- /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 0000000000000000000000000000000000000000..b1fe45357ff266c3dfd1317aa29693338a8edb42 --- /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 0000000000000000000000000000000000000000..699605e1e3aef2ceffc324b5ba92463ca1a15135 --- /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 02edc9219c395330ee2fc6eac9bfbceb6373858c..758a6cf435672d50610df01516211ad45f73d198 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 dc9cdb9a2205038fc928b8a8f06662e55f649476..7b2f57990b5c468fb99b0051b4888853a79666f8 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 0000000000000000000000000000000000000000..5f813f81f556cda9367c8e4024ec1027654b5091 --- /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 ec658668c61b3687377272e06bdd6627b96d71de..bad439bc48937e990fa883eef7717647da5133de 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 0000000000000000000000000000000000000000..deafa8bf8a7b0fa5cd767edc2636058b0f7d8582 --- /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 0000000000000000000000000000000000000000..50f619ce26e6c1f96d79aa7f644828e93d4bd3ce --- /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 9bbda6e73967ba358f44808a5cf1bca7784210f5..c468ecf957b8fe277e0fb71b970cf750159ebc89 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 01a707a6bd49846bd865ada732d6b02c294cd33f..91ddef7bfbf05aba58db11eb034a68ebf86eee58 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 fddd53a2b57508c713f14072eb21611f555bc96e..c87c9e97bc82b49b711e714a101b5422d1be18e0 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 0000000000000000000000000000000000000000..d11bb39f328883fe9c97f11da31ba7298a69afa5 --- /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 0000000000000000000000000000000000000000..8cda43173fe5e156453f98b6200d69fd9c5ac94f --- /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 36101c624624fb873799513e93104b2665b00dfc..0000000000000000000000000000000000000000 --- 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 0000000000000000000000000000000000000000..6fd744b397577e5b8cb152cf1a04e9cd730449ef --- /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 6af6aa15eef2dbe618bc20bdda64b9cfb3531d1b..ee7395362a96dc3978e4f77af87b7274a6fcc5d4 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 0000000000000000000000000000000000000000..be0fe46befdc4fd11aa454f08ab7a38b80b5e5f7 --- /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 0000000000000000000000000000000000000000..1a3818d8997269b2bb659fa3f38c4d9e68668084 --- /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 0000000000000000000000000000000000000000..0c6608d906d6e2a59862757a5cf24e1d6cd3eb7f --- /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 ad875d553840bf6b0b0797e8d2c645fd8ae35edd..090fd63b84484de12fe99e947271ec66ec9f2ec4 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 0000000000000000000000000000000000000000..4b6d971ec71f9a7e0fd8ce6b7868b56219276cf2 --- /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 2a5d132db7bb6f218df734d21f3227e1d7e84afd..bc5a5e431033a5c373866bf537aa078df308ac74 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