diff options
25 files changed, 823 insertions, 208 deletions
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index 01ef13df57a..0704e8f1683 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -17,16 +17,13 @@ module Ci def initialize(config, path = nil) @ci_config = Gitlab::Ci::Config.new(config) - @config = @ci_config.to_hash - - @path = path + @config, @path = @ci_config.to_hash, path 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 end @@ -58,6 +55,9 @@ module Ci private def initial_parsing + ## + # Global config + # @before_script = @ci_config.before_script @image = @ci_config.image @after_script = @ci_config.after_script @@ -66,35 +66,23 @@ module Ci @stages = @ci_config.stages @cache = @ci_config.cache - @jobs = {} - - @config.except!(*ALLOWED_YAML_KEYS) - @config.each { |name, param| add_job(name, param) } - - raise ValidationError, "Please define at least one job" if @jobs.none? - end - - def add_job(name, job) - return if name.to_s.start_with?('.') + ## + # Jobs + # + @jobs = @ci_config.jobs - raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script) - - stage = job[:stage] || job[:type] || DEFAULT_STAGE - @jobs[name] = { stage: stage }.merge(job) + @jobs.each do |name, job| + validate_job!(name, job) + end end def build_job(name, job) { stage_idx: @stages.index(job[:stage]), stage: job[:stage], - ## - # Refactoring note: - # - before script behaves differently than after script - # - after script returns an array of commands - # - before script should be a concatenated command commands: [job[:before_script] || @before_script, job[:script]].flatten.compact.join("\n"), tag_list: job[:tags] || [], - name: name, + name: job[:name], only: job[:only], except: job[:except], allow_failure: job[:allow_failure] || false, @@ -111,36 +99,21 @@ module Ci } end - def validate! - @jobs.each do |name, job| - validate_job!(name, job) - end - - true - end - def validate_job!(name, job) - validate_job_name!(name) + raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script) + validate_job_keys!(name, job) validate_job_types!(name, job) - validate_job_script!(name, job) validate_job_stage!(name, job) if job[:stage] validate_job_variables!(name, job) if job[:variables] - validate_job_cache!(name, job) if job[:cache] validate_job_artifacts!(name, job) if job[:artifacts] validate_job_dependencies!(name, job) if job[:dependencies] end - def validate_job_name!(name) - if name.blank? || !validate_string(name) - raise ValidationError, "job name should be non-empty string" - end - end - def validate_job_keys!(name, job) job.keys.each do |key| - unless ALLOWED_JOB_KEYS.include? key + unless (ALLOWED_JOB_KEYS + %i[name]).include? key raise ValidationError, "#{name} job: unknown parameter #{key}" end end @@ -180,20 +153,6 @@ module Ci end end - def validate_job_script!(name, job) - if !validate_string(job[:script]) && !validate_array_of_strings(job[:script]) - raise ValidationError, "#{name} job: script should be a string or an array of a strings" - end - - if job[:before_script] && !validate_array_of_strings(job[:before_script]) - raise ValidationError, "#{name} job: before_script should be an array of strings" - end - - if job[:after_script] && !validate_array_of_strings(job[:after_script]) - raise ValidationError, "#{name} job: after_script should be an array of strings" - end - 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(", ")}" @@ -207,26 +166,6 @@ module Ci end end - def validate_job_cache!(name, job) - job[:cache].keys.each do |key| - unless ALLOWED_CACHE_KEYS.include? key - raise ValidationError, "#{name} job: cache unknown parameter #{key}" - end - end - - if job[:cache][:key] && !validate_string(job[:cache][:key]) - raise ValidationError, "#{name} job: cache:key parameter should be a string" - end - - if job[:cache][:untracked] && !validate_boolean(job[:cache][:untracked]) - raise ValidationError, "#{name} job: cache:untracked parameter should be an boolean" - end - - if job[:cache][:paths] && !validate_array_of_strings(job[:cache][:paths]) - raise ValidationError, "#{name} job: cache:paths parameter should be an array of strings" - end - end - def validate_job_artifacts!(name, job) job[:artifacts].keys.each do |key| unless ALLOWED_ARTIFACTS_KEYS.include? key diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index e6cc1529760..ae82c0db3f1 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -8,7 +8,7 @@ module Gitlab # Temporary delegations that should be removed after refactoring # delegate :before_script, :image, :services, :after_script, :variables, - :stages, :cache, to: :@global + :stages, :cache, :jobs, to: :@global def initialize(config) @config = Loader.new(config).load! diff --git a/lib/gitlab/ci/config/node/commands.rb b/lib/gitlab/ci/config/node/commands.rb new file mode 100644 index 00000000000..f7e6950001e --- /dev/null +++ b/lib/gitlab/ci/config/node/commands.rb @@ -0,0 +1,35 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents a job script. + # + class Commands < Entry + include Validatable + + validations do + include LegacyValidationHelpers + + validate :string_or_array_of_strings + + def string_or_array_of_strings + unless config_valid? + errors.add(:config, + 'should be a string or an array of strings') + end + end + + def config_valid? + validate_string(config) || validate_array_of_strings(config) + end + end + + def value + [@config].flatten + 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 37936fc8242..93a9a253322 100644 --- a/lib/gitlab/ci/config/node/configurable.rb +++ b/lib/gitlab/ci/config/node/configurable.rb @@ -25,10 +25,14 @@ module Gitlab private - def create_node(key, factory) - factory.with(value: @config[key], key: key, parent: self) + def compose! + self.class.nodes.each do |key, factory| + factory + .value(@config[key]) + .with(key: key, parent: self) - factory.create! + @entries[key] = factory.create! + end end class_methods do @@ -38,22 +42,24 @@ module Gitlab private - def node(symbol, entry_class, metadata) - factory = Node::Factory.new(entry_class) + def node(key, node, metadata) + factory = Node::Factory.new(node) .with(description: metadata[:description]) - (@nodes ||= {}).merge!(symbol.to_sym => factory) + (@nodes ||= {}).merge!(key.to_sym => factory) end def helpers(*nodes) nodes.each do |symbol| define_method("#{symbol}_defined?") do - @nodes[symbol].try(:defined?) + @entries[symbol].specified? if @entries[symbol] end define_method("#{symbol}_value") do - raise Entry::InvalidError unless valid? - @nodes[symbol].try(:value) + if @entries[symbol] + return unless @entries[symbol].valid? + @entries[symbol].value + end end alias_method symbol.to_sym, "#{symbol}_value".to_sym diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb index 9e79e170a4f..813e394e51b 100644 --- a/lib/gitlab/ci/config/node/entry.rb +++ b/lib/gitlab/ci/config/node/entry.rb @@ -8,30 +8,31 @@ module Gitlab class Entry class InvalidError < StandardError; end - attr_reader :config + attr_reader :config, :metadata attr_accessor :key, :parent, :description - def initialize(config) + def initialize(config, **metadata) @config = config - @nodes = {} + @metadata = metadata + @entries = {} + @validator = self.class.validator.new(self) - @validator.validate + @validator.validate(:new) end def process! - return if leaf? return unless valid? compose! - process_nodes! + @entries.each_value(&:process!) end - def nodes - @nodes.values + def leaf? + @entries.none? end - def leaf? - self.class.nodes.none? + def descendants + @entries.values end def ancestors @@ -43,27 +44,30 @@ module Gitlab end def errors - @validator.messages + nodes.flat_map(&:errors) + @validator.messages + @entries.values.flat_map(&:errors) end def value if leaf? @config else - defined = @nodes.select { |_key, value| value.defined? } - Hash[defined.map { |key, node| [key, node.value] }] + meaningful = @entries.select do |_key, value| + value.specified? && value.relevant? + end + + Hash[meaningful.map { |key, entry| [key, entry.value] }] end end - def defined? + def specified? true end - def self.default + def relevant? + true end - def self.nodes - {} + def self.default end def self.validator @@ -73,17 +77,6 @@ module Gitlab private def compose! - self.class.nodes.each do |key, essence| - @nodes[key] = create_node(key, essence) - end - end - - def process_nodes! - nodes.each(&:process!) - end - - def create_node(key, essence) - raise NotImplementedError end end end diff --git a/lib/gitlab/ci/config/node/factory.rb b/lib/gitlab/ci/config/node/factory.rb index 5919a283283..707b052e6a8 100644 --- a/lib/gitlab/ci/config/node/factory.rb +++ b/lib/gitlab/ci/config/node/factory.rb @@ -10,35 +10,60 @@ module Gitlab def initialize(node) @node = node + @metadata = {} @attributes = {} end + def value(value) + @value = value + self + end + + def metadata(metadata) + @metadata.merge!(metadata) + self + end + def with(attributes) @attributes.merge!(attributes) self end def create! - raise InvalidFactory unless @attributes.has_key?(:value) + raise InvalidFactory unless defined?(@value) - fabricate.tap do |entry| - entry.key = @attributes[:key] - entry.parent = @attributes[:parent] - entry.description = @attributes[:description] + ## + # We assume that unspecified entry is undefined. + # See issue #18775. + # + if @value.nil? + Node::Undefined.new( + fabricate_undefined + ) + else + fabricate(@node, @value) end end private - def fabricate + def fabricate_undefined ## - # We assume that unspecified entry is undefined. - # See issue #18775. + # If node has a default value we fabricate concrete node + # with default value. # - if @attributes[:value].nil? - Node::Undefined.new(@node) + if @node.default.nil? + fabricate(Node::Null) else - @node.new(@attributes[:value]) + fabricate(@node, @node.default) + end + end + + def fabricate(node, value = nil) + node.new(value, @metadata).tap do |entry| + entry.key = @attributes[:key] + entry.parent = @attributes[:parent] + entry.description = @attributes[:description] end end end diff --git a/lib/gitlab/ci/config/node/global.rb b/lib/gitlab/ci/config/node/global.rb index f92e1eccbcf..b545b78a940 100644 --- a/lib/gitlab/ci/config/node/global.rb +++ b/lib/gitlab/ci/config/node/global.rb @@ -34,10 +34,36 @@ module Gitlab description: 'Configure caching between build jobs.' helpers :before_script, :image, :services, :after_script, - :variables, :stages, :types, :cache + :variables, :stages, :types, :cache, :jobs - def stages - stages_defined? ? stages_value : types_value + private + + def compose! + super + + compose_jobs! + compose_stages! + end + + def compose_jobs! + factory = Node::Factory.new(Node::Jobs) + .value(@config.except(*self.class.nodes.keys)) + .with(key: :jobs, parent: self, + description: 'Jobs definition for this pipeline') + + @entries[:jobs] = factory.create! + end + + def compose_stages! + ## + # Deprecated `:types` key workaround - if types are defined and + # stages are not defined we use types definition as stages. + # + if types_defined? && !stages_defined? + @entries[:stages] = @entries[:types] + end + + @entries.delete(:types) end end end diff --git a/lib/gitlab/ci/config/node/hidden_job.rb b/lib/gitlab/ci/config/node/hidden_job.rb new file mode 100644 index 00000000000..073044b66f8 --- /dev/null +++ b/lib/gitlab/ci/config/node/hidden_job.rb @@ -0,0 +1,23 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents a hidden CI/CD job. + # + class HiddenJob < Entry + include Validatable + + validations do + validates :config, type: Hash + validates :config, presence: true + end + + def relevant? + false + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/node/job.rb b/lib/gitlab/ci/config/node/job.rb new file mode 100644 index 00000000000..9280412a638 --- /dev/null +++ b/lib/gitlab/ci/config/node/job.rb @@ -0,0 +1,69 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents a concrete CI/CD job. + # + class Job < Entry + include Configurable + + validations do + validates :config, presence: true + validates :name, presence: true + validates :name, type: Symbol + end + + node :before_script, Script, + description: 'Global before script overridden in this job.' + + node :script, Commands, + description: 'Commands that will be executed in this job.' + + node :stage, Stage, + description: 'Pipeline stage this job will be executed into.' + + node :type, Stage, + description: 'Deprecated: stage this job will be executed into.' + + node :after_script, Script, + description: 'Commands that will be executed when finishing job.' + + node :cache, Cache, + description: 'Cache definition for this job.' + + helpers :before_script, :script, :stage, :type, :after_script, :cache + + def name + @metadata[:name] + end + + def value + @config.merge(to_hash.compact) + end + + private + + def to_hash + { name: name, + before_script: before_script, + script: script, + stage: stage, + cache: cache, + after_script: after_script } + end + + def compose! + super + + if type_defined? && !stage_defined? + @entries[:stage] = @entries[:type] + end + + @entries.delete(:type) + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb new file mode 100644 index 00000000000..908c8f4f120 --- /dev/null +++ b/lib/gitlab/ci/config/node/jobs.rb @@ -0,0 +1,48 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents a set of jobs. + # + class Jobs < Entry + include Validatable + + validations do + validates :config, type: Hash + + validate do + unless has_visible_job? + errors.add(:config, 'should contain at least one visible job') + end + end + + def has_visible_job? + config.any? { |key, _| !key.to_s.start_with?('.') } + end + end + + private + + def compose! + @config.each do |name, config| + node = hidden?(name) ? Node::HiddenJob : Node::Job + + factory = Node::Factory.new(node) + .value(config || {}) + .metadata(name: name) + .with(key: name, parent: self, + description: "#{name} job definition.") + + @entries[name] = factory.create! + end + end + + def hidden?(name) + name.to_s.start_with?('.') + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/node/null.rb b/lib/gitlab/ci/config/node/null.rb new file mode 100644 index 00000000000..880d29f663d --- /dev/null +++ b/lib/gitlab/ci/config/node/null.rb @@ -0,0 +1,34 @@ +module Gitlab + module Ci + class Config + module Node + ## + # This class represents an undefined and unspecified node. + # + # Implements the Null Object pattern. + # + class Null < Entry + def value + nil + end + + def valid? + true + end + + def errors + [] + end + + def specified? + false + end + + def relevant? + false + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/node/stage.rb b/lib/gitlab/ci/config/node/stage.rb new file mode 100644 index 00000000000..cbc97641f5a --- /dev/null +++ b/lib/gitlab/ci/config/node/stage.rb @@ -0,0 +1,22 @@ +module Gitlab + module Ci + class Config + module Node + ## + # Entry that represents a stage for a job. + # + class Stage < Entry + include Validatable + + validations do + validates :config, type: String + end + + def self.default + 'test' + end + end + end + end + end +end diff --git a/lib/gitlab/ci/config/node/undefined.rb b/lib/gitlab/ci/config/node/undefined.rb index 699605e1e3a..384774c9b69 100644 --- a/lib/gitlab/ci/config/node/undefined.rb +++ b/lib/gitlab/ci/config/node/undefined.rb @@ -3,24 +3,17 @@ module Gitlab class Config module Node ## - # This class represents an undefined entry node. + # This class represents an undefined and unspecified entry node. # - # It takes original entry class as configuration and returns default - # value of original entry as self value. + # It decorates original entry adding method that idicates it is + # unspecified. # - # - class Undefined < Entry - include Validatable - - validations do - validates :config, type: Class - end - - def value - @config.default + class Undefined < SimpleDelegator + def initialize(entry) + super end - def defined? + def specified? false end end diff --git a/lib/gitlab/ci/config/node/validator.rb b/lib/gitlab/ci/config/node/validator.rb index 758a6cf4356..ca000f245aa 100644 --- a/lib/gitlab/ci/config/node/validator.rb +++ b/lib/gitlab/ci/config/node/validator.rb @@ -31,8 +31,15 @@ module Gitlab def location predecessors = ancestors.map(&:key).compact - current = key || @node.class.name.demodulize.underscore - predecessors.append(current).join(':') + predecessors.append(key_name).join(':') + end + + def key_name + if key.blank? || key.nil? + @node.class.name.demodulize.underscore.humanize + else + key + end end end end diff --git a/lib/gitlab/ci/config/node/validators.rb b/lib/gitlab/ci/config/node/validators.rb index 7b2f57990b5..d33b407af68 100644 --- a/lib/gitlab/ci/config/node/validators.rb +++ b/lib/gitlab/ci/config/node/validators.rb @@ -33,6 +33,15 @@ module Gitlab end end + class RequiredValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + if value.nil? + raise Entry::InvalidError, + "Entry needs #{attribute} attribute set internally." + end + end + end + class KeyValidator < ActiveModel::EachValidator include LegacyValidationHelpers diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index bcbf409c8b0..d629bd252e2 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -970,7 +970,7 @@ EOT config = YAML.dump({ rspec: { script: "test", before_script: [10, "test"] } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: before_script should be an array of strings") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:before_script config should be an array of strings") end it "returns errors if after_script parameter is invalid" do @@ -984,7 +984,7 @@ EOT config = YAML.dump({ rspec: { script: "test", after_script: [10, "test"] } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: after_script should be an array of strings") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:after_script config should be an array of strings") end it "returns errors if image parameter is invalid" do @@ -998,14 +998,14 @@ EOT config = YAML.dump({ '' => { script: "test" } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:job name can't be blank") end it "returns errors if job name is non-string" do config = YAML.dump({ 10 => { script: "test" } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "job name should be non-empty string") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:10 name should be a symbol") end it "returns errors if job image parameter is invalid" do @@ -1043,11 +1043,11 @@ EOT end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: services should be an array of strings") end - it "returns errors if there are unknown parameters" do + it "returns error if job configuration is invalid" do config = YAML.dump({ extra: "bundle update" }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Unknown parameter: extra") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:extra config should be a hash") end it "returns errors if there are unknown parameters that are hashes, but doesn't have a script" do @@ -1061,7 +1061,14 @@ EOT config = YAML.dump({ before_script: ["bundle update"] }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "Please define at least one job") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs config should contain at least one visible job") + end + + it "returns errors if there are no visible jobs defined" do + config = YAML.dump({ before_script: ["bundle update"], '.hidden'.to_sym => { script: 'ls' } }) + expect do + GitlabCiYamlProcessor.new(config, path) + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs config should contain at least one visible job") end it "returns errors if job allow_failure parameter is not an boolean" do @@ -1075,7 +1082,7 @@ EOT config = YAML.dump({ rspec: { script: "test", type: 1 } }) expect do GitlabCiYamlProcessor.new(config, path) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: stage parameter should be build, test, deploy") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:type config should be a string") end it "returns errors if job stage is not a pre-defined stage" do @@ -1194,21 +1201,21 @@ EOT config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", cache: { key: 1 } } }) expect do GitlabCiYamlProcessor.new(config) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: cache:key parameter should be a string") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:cache:key config should be a string or symbol") end it "returns errors if job cache:untracked is not an array of strings" do config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", cache: { untracked: "string" } } }) expect do GitlabCiYamlProcessor.new(config) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: cache:untracked parameter should be an boolean") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:cache:untracked config should be a boolean value") end it "returns errors if job cache:paths is not an array of strings" do config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", cache: { paths: "string" } } }) expect do GitlabCiYamlProcessor.new(config) - end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: cache:paths parameter should be an array of strings") + end.to raise_error(GitlabCiYamlProcessor::ValidationError, "jobs:rspec:cache:paths config should be an array of strings") end it "returns errors if job dependencies is not an array of strings" do diff --git a/spec/lib/gitlab/ci/config/node/commands_spec.rb b/spec/lib/gitlab/ci/config/node/commands_spec.rb new file mode 100644 index 00000000000..e373c40706f --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/commands_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Commands do + let(:entry) { described_class.new(config) } + + context 'when entry config value is an array' do + let(:config) { ['ls', 'pwd'] } + + describe '#value' do + it 'returns array of 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 + end + + context 'when entry config value is a string' do + let(:config) { 'ls' } + + describe '#value' do + it 'returns array with single element' do + expect(entry.value).to eq ['ls'] + 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 'commands config should be a ' \ + 'string or an array of strings' + end + end + end +end diff --git a/spec/lib/gitlab/ci/config/node/factory_spec.rb b/spec/lib/gitlab/ci/config/node/factory_spec.rb index 91ddef7bfbf..d26185ba585 100644 --- a/spec/lib/gitlab/ci/config/node/factory_spec.rb +++ b/spec/lib/gitlab/ci/config/node/factory_spec.rb @@ -2,13 +2,13 @@ require 'spec_helper' describe Gitlab::Ci::Config::Node::Factory do describe '#create!' do - let(:factory) { described_class.new(entry_class) } - let(:entry_class) { Gitlab::Ci::Config::Node::Script } + let(:factory) { described_class.new(node) } + let(:node) { Gitlab::Ci::Config::Node::Script } - context 'when setting up a value' do + context 'when setting a concrete value' do it 'creates entry with valid value' do entry = factory - .with(value: ['ls', 'pwd']) + .value(['ls', 'pwd']) .create! expect(entry.value).to eq ['ls', 'pwd'] @@ -17,7 +17,7 @@ describe Gitlab::Ci::Config::Node::Factory do context 'when setting description' do it 'creates entry with description' do entry = factory - .with(value: ['ls', 'pwd']) + .value(['ls', 'pwd']) .with(description: 'test description') .create! @@ -29,7 +29,8 @@ describe Gitlab::Ci::Config::Node::Factory do context 'when setting key' do it 'creates entry with custom key' do entry = factory - .with(value: ['ls', 'pwd'], key: 'test key') + .value(['ls', 'pwd']) + .with(key: 'test key') .create! expect(entry.key).to eq 'test key' @@ -37,19 +38,20 @@ describe Gitlab::Ci::Config::Node::Factory do end context 'when setting a parent' do - let(:parent) { Object.new } + let(:object) { Object.new } it 'creates entry with valid parent' do entry = factory - .with(value: 'ls', parent: parent) + .value('ls') + .with(parent: object) .create! - expect(entry.parent).to eq parent + expect(entry.parent).to eq object end end end - context 'when not setting up a value' do + context 'when not setting a value' do it 'raises error' do expect { factory.create! }.to raise_error( Gitlab::Ci::Config::Node::Factory::InvalidFactory @@ -60,11 +62,25 @@ describe Gitlab::Ci::Config::Node::Factory do context 'when creating entry with nil value' do it 'creates an undefined entry' do entry = factory - .with(value: nil) + .value(nil) .create! expect(entry).to be_an_instance_of Gitlab::Ci::Config::Node::Undefined end end + + context 'when passing metadata' do + let(:node) { spy('node') } + + it 'passes metadata as a parameter' do + factory + .value('some value') + .metadata(some: 'hash') + .create! + + expect(node).to have_received(:new) + .with('some value', { some: 'hash' }) + end + 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 c87c9e97bc8..2f87d270b36 100644 --- a/spec/lib/gitlab/ci/config/node/global_spec.rb +++ b/spec/lib/gitlab/ci/config/node/global_spec.rb @@ -22,38 +22,40 @@ describe Gitlab::Ci::Config::Node::Global do variables: { VAR: 'value' }, after_script: ['make clean'], stages: ['build', 'pages'], - cache: { key: 'k', untracked: true, paths: ['public/'] } } + cache: { key: 'k', untracked: true, paths: ['public/'] }, + rspec: { script: %w[rspec ls] }, + spinach: { script: 'spinach' } } end describe '#process!' do before { global.process! } it 'creates nodes hash' do - expect(global.nodes).to be_an Array + expect(global.descendants).to be_an Array end it 'creates node object for each entry' do - expect(global.nodes.count).to eq 8 + expect(global.descendants.count).to eq 8 end it 'creates node object using valid class' do - expect(global.nodes.first) + expect(global.descendants.first) .to be_an_instance_of Gitlab::Ci::Config::Node::Script - expect(global.nodes.second) + expect(global.descendants.second) .to be_an_instance_of Gitlab::Ci::Config::Node::Image end it 'sets correct description for nodes' do - expect(global.nodes.first.description) + expect(global.descendants.first.description) .to eq 'Script that will be executed before each job.' - expect(global.nodes.second.description) + expect(global.descendants.second.description) .to eq 'Docker image that will be used to execute jobs.' end - end - describe '#leaf?' do - it 'is not leaf' do - expect(global).not_to be_leaf + describe '#leaf?' do + it 'is not leaf' do + expect(global).not_to be_leaf + end end end @@ -63,6 +65,12 @@ describe Gitlab::Ci::Config::Node::Global do expect(global.before_script).to be nil end end + + describe '#leaf?' do + it 'is leaf' do + expect(global).to be_leaf + end + end end context 'when processed' do @@ -106,7 +114,10 @@ describe Gitlab::Ci::Config::Node::Global do end context 'when deprecated types key defined' do - let(:hash) { { types: ['test', 'deploy'] } } + let(:hash) do + { types: ['test', 'deploy'], + rspec: { script: 'rspec' } } + end it 'returns array of types as stages' do expect(global.stages).to eq %w[test deploy] @@ -120,20 +131,33 @@ describe Gitlab::Ci::Config::Node::Global do .to eq(key: 'k', untracked: true, paths: ['public/']) end end + + describe '#jobs' do + it 'returns jobs configuration' do + expect(global.jobs).to eq( + rspec: { name: :rspec, + script: %w[rspec ls], + stage: 'test' }, + spinach: { name: :spinach, + script: %w[spinach], + stage: 'test' } + ) + end + end end end context 'when most of entires not defined' do - let(:hash) { { cache: { key: 'a' }, rspec: {} } } + let(:hash) { { cache: { key: 'a' }, rspec: { script: %w[ls] } } } before { global.process! } describe '#nodes' do it 'instantizes all nodes' do - expect(global.nodes.count).to eq 8 + expect(global.descendants.count).to eq 8 end it 'contains undefined nodes' do - expect(global.nodes.first) + expect(global.descendants.first) .to be_an_instance_of Gitlab::Ci::Config::Node::Undefined end end @@ -164,7 +188,7 @@ describe Gitlab::Ci::Config::Node::Global do # details. # context 'when entires specified but not defined' do - let(:hash) { { variables: nil } } + let(:hash) { { variables: nil, rspec: { script: 'rspec' } } } before { global.process! } describe '#variables' do @@ -196,10 +220,8 @@ describe Gitlab::Ci::Config::Node::Global do end describe '#before_script' do - it 'raises error' do - expect { global.before_script }.to raise_error( - Gitlab::Ci::Config::Node::Entry::InvalidError - ) + it 'returns nil' do + expect(global.before_script).to be_nil end end end @@ -220,9 +242,9 @@ describe Gitlab::Ci::Config::Node::Global do end end - describe '#defined?' do + describe '#specified?' do it 'is concrete entry that is defined' do - expect(global.defined?).to be true + expect(global.specified?).to be true end end end diff --git a/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb new file mode 100644 index 00000000000..cc44e2cc054 --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/hidden_job_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::HiddenJob do + let(:entry) { described_class.new(config) } + + describe 'validations' do + context 'when entry config value is correct' do + let(:config) { { image: 'ruby:2.2' } } + + describe '#value' do + it 'returns key value' do + expect(entry.value).to eq(image: 'ruby:2.2') + 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 + context 'incorrect config value type' do + let(:config) { ['incorrect'] } + + describe '#errors' do + it 'saves errors' do + expect(entry.errors) + .to include 'hidden job config should be a hash' + end + end + end + + context 'when config is empty' do + let(:config) { {} } + + describe '#valid' do + it 'is invalid' do + expect(entry).not_to be_valid + end + end + end + end + end + + describe '#leaf?' do + it 'is a leaf' do + expect(entry).to be_leaf + end + end + + describe '#relevant?' do + it 'is not a relevant entry' do + expect(entry).not_to be_relevant + end + end +end diff --git a/spec/lib/gitlab/ci/config/node/job_spec.rb b/spec/lib/gitlab/ci/config/node/job_spec.rb new file mode 100644 index 00000000000..2721908c5d7 --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/job_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Job do + let(:entry) { described_class.new(config, name: :rspec) } + + before { entry.process! } + + describe 'validations' do + context 'when entry config value is correct' do + let(:config) { { script: 'rspec' } } + + describe '#valid?' do + it 'is valid' do + expect(entry).to be_valid + end + end + + context 'when job name is empty' do + let(:entry) { described_class.new(config, name: ''.to_sym) } + + it 'reports error' do + expect(entry.errors) + .to include "job name can't be blank" + end + end + end + + context 'when entry value is not correct' do + context 'incorrect config value type' do + let(:config) { ['incorrect'] } + + describe '#errors' do + it 'reports error about a config type' do + expect(entry.errors) + .to include 'job config should be a hash' + end + end + end + + context 'when config is empty' do + let(:config) { {} } + + describe '#valid' do + it 'is invalid' do + expect(entry).not_to be_valid + end + end + end + end + end + + describe '#value' do + context 'when entry is correct' do + let(:config) do + { before_script: %w[ls pwd], + script: 'rspec', + after_script: %w[cleanup] } + end + + it 'returns correct value' do + expect(entry.value) + .to eq(name: :rspec, + before_script: %w[ls pwd], + script: %w[rspec], + stage: 'test', + after_script: %w[cleanup]) + end + end + end + + describe '#relevant?' do + it 'is a relevant entry' do + expect(entry).to be_relevant + end + end +end diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb new file mode 100644 index 00000000000..b8d9c70479c --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb @@ -0,0 +1,87 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Jobs do + let(:entry) { described_class.new(config) } + + describe 'validations' do + before { entry.process! } + + context 'when entry config value is correct' do + let(:config) { { rspec: { script: 'rspec' } } } + + 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 'incorrect config value type' do + let(:config) { ['incorrect'] } + + it 'returns error about incorrect type' do + expect(entry.errors) + .to include 'jobs config should be a hash' + end + end + + context 'when job is unspecified' do + let(:config) { { rspec: nil } } + + it 'reports error' do + expect(entry.errors).to include "rspec config can't be blank" + end + end + + context 'when no visible jobs present' do + let(:config) { { '.hidden'.to_sym => { script: [] } } } + + it 'returns error about no visible jobs defined' do + expect(entry.errors) + .to include 'jobs config should contain at least one visible job' + end + end + end + end + end + + context 'when valid job entries processed' do + before { entry.process! } + + let(:config) do + { rspec: { script: 'rspec' }, + spinach: { script: 'spinach' }, + '.hidden'.to_sym => {} } + end + + describe '#value' do + it 'returns key value' do + expect(entry.value).to eq( + rspec: { name: :rspec, + script: %w[rspec], + stage: 'test' }, + spinach: { name: :spinach, + script: %w[spinach], + stage: 'test' }) + end + end + + describe '#descendants' do + it 'creates valid descendant nodes' do + expect(entry.descendants.count).to eq 3 + expect(entry.descendants.first(2)) + .to all(be_an_instance_of(Gitlab::Ci::Config::Node::Job)) + expect(entry.descendants.last) + .to be_an_instance_of(Gitlab::Ci::Config::Node::HiddenJob) + end + end + + describe '#value' do + it 'returns value of visible jobs only' do + expect(entry.value.keys).to eq [:rspec, :spinach] + 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 new file mode 100644 index 00000000000..1ab5478dcfa --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/null_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Null do + let(:null) { described_class.new(nil) } + + describe '#leaf?' do + it 'is leaf node' do + expect(null).to be_leaf + end + end + + describe '#valid?' do + it 'is always valid' do + expect(null).to be_valid + end + end + + describe '#errors' do + it 'is does not contain errors' do + expect(null.errors).to be_empty + end + end + + describe '#value' do + it 'returns nil' do + expect(null.value).to eq nil + end + end + + describe '#relevant?' do + it 'is not relevant' do + expect(null.relevant?).to eq false + end + end + + describe '#specified?' do + it 'is not defined' do + expect(null.specified?).to eq false + end + end +end diff --git a/spec/lib/gitlab/ci/config/node/stage_spec.rb b/spec/lib/gitlab/ci/config/node/stage_spec.rb new file mode 100644 index 00000000000..fb9ec70762a --- /dev/null +++ b/spec/lib/gitlab/ci/config/node/stage_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe Gitlab::Ci::Config::Node::Stage do + let(:stage) { described_class.new(config) } + + describe 'validations' do + context 'when stage config value is correct' do + let(:config) { 'build' } + + describe '#value' do + it 'returns a stage key' do + expect(stage.value).to eq config + end + end + + describe '#valid?' do + it 'is valid' do + expect(stage).to be_valid + end + end + end + + context 'when value has a wrong type' do + let(:config) { { test: true } } + + it 'reports errors about wrong type' do + expect(stage.errors) + .to include 'stage config should be a string' + end + end + end + + describe '.default' do + it 'returns default stage' do + expect(described_class.default).to eq 'test' + end + end +end diff --git a/spec/lib/gitlab/ci/config/node/undefined_spec.rb b/spec/lib/gitlab/ci/config/node/undefined_spec.rb index 0c6608d906d..2d43e1c1a9d 100644 --- a/spec/lib/gitlab/ci/config/node/undefined_spec.rb +++ b/spec/lib/gitlab/ci/config/node/undefined_spec.rb @@ -2,39 +2,31 @@ 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 + let(:entry) { spy('Entry') } describe '#valid?' do - it 'is always valid' do - expect(undefined).to be_valid + it 'delegates method to entry' do + expect(undefined.valid).to eq entry end end describe '#errors' do - it 'is does not contain errors' do - expect(undefined.errors).to be_empty + it 'delegates method to entry' do + expect(undefined.errors).to eq entry 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' + it 'delegates method to entry' do + expect(undefined.value).to eq entry end end - describe '#undefined?' do - it 'is not a defined entry' do - expect(undefined.defined?).to be false + describe '#specified?' do + it 'is always false' do + allow(entry).to receive(:specified?).and_return(true) + + expect(undefined.specified?).to be false end end end |