Commit 56ae9f6b authored by Grzegorz Bizon's avatar Grzegorz Bizon

Improve CI job entry validations in new config

parent 036e297c
......@@ -102,7 +102,6 @@ module Ci
def validate_job!(name, job)
raise ValidationError, "Unknown parameter: #{name}" unless job.is_a?(Hash) && job.has_key?(:script)
validate_job_name!(name)
validate_job_keys!(name, job)
validate_job_types!(name, job)
......@@ -112,12 +111,6 @@ module Ci
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)
##
# TODO, remove refactoring keys
......
......@@ -55,8 +55,10 @@ module Gitlab
end
define_method("#{symbol}_value") do
return unless valid?
@entries[symbol].value if @entries[symbol]
if @entries[symbol]
return unless @entries[symbol].valid?
@entries[symbol].value
end
end
alias_method symbol.to_sym, "#{symbol}_value".to_sym
......
......@@ -10,7 +10,12 @@ module Gitlab
validations do
validates :config, presence: true
validates :global, required: true, on: :processed
with_options on: :processed do
validates :global, required: true
validates :name, presence: true
validates :name, type: Symbol
end
end
node :before_script, Script,
......@@ -30,11 +35,11 @@ module Gitlab
helpers :before_script, :script, :stage, :type, :after_script
def name
@key
end
def value
##
# TODO, refactoring step: do not expose internal configuration,
# return only hash value without merging it to internal config.
#
@config.merge(to_hash.compact)
end
......@@ -53,10 +58,9 @@ module Gitlab
private
def to_hash
{ before_script: before_script,
script: script,
commands: commands,
{ script: script,
stage: stage,
commands: commands,
after_script: after_script }
end
......
......@@ -10,20 +10,21 @@ module Gitlab
validations do
validates :config, type: Hash
validate :jobs_presence, on: :processed
def jobs_presence
unless relevant?
with_options on: :processed do
validate do
unless has_visible_job?
errors.add(:config, 'should contain at least one visible job')
end
end
end
end
def nodes
@config
end
def relevant?
def has_visible_job?
@entries.values.any?(&:relevant?)
end
......
......@@ -31,8 +31,15 @@ module Gitlab
def location
predecessors = ancestors.map(&:key).compact
current = key || @node.class.name.demodulize.underscore.humanize
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
......
......@@ -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
......
......@@ -129,14 +129,12 @@ describe Gitlab::Ci::Config::Node::Global do
describe '#jobs' do
it 'returns jobs configuration' do
expect(global.jobs)
.to eq(rspec: { before_script: %w[ls pwd],
script: %w[rspec ls],
commands: "ls\npwd\nrspec\nls",
stage: 'test' },
spinach: { before_script: %w[ls pwd],
script: %w[spinach],
commands: "ls\npwd\nspinach",
stage: 'test' })
.to eq(rspec: { script: %w[rspec ls],
stage: 'test',
commands: "ls\npwd\nrspec\nls" },
spinach: { script: %w[spinach],
stage: 'test',
commands: "ls\npwd\nspinach" })
end
end
end
......
require 'spec_helper'
describe Gitlab::Ci::Config::Node::Job do
let(:entry) { described_class.new(config, global: global) }
let(:global) { spy('Global') }
let(:entry) { described_class.new(config, attributes) }
let(:attributes) { { key: :rspec, global: global } }
let(:global) { double('global', stages: %w[test]) }
before do
entry.process!
......@@ -18,6 +19,15 @@ describe Gitlab::Ci::Config::Node::Job do
expect(entry).to be_valid
end
end
context 'when job name is empty' do
let(:attributes) { { key: '', global: global } }
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
......@@ -27,7 +37,7 @@ describe Gitlab::Ci::Config::Node::Job do
describe '#errors' do
it 'reports error about a config type' do
expect(entry.errors)
.to include 'job config should be a hash'
.to include 'rspec config should be a hash'
end
end
end
......
......@@ -34,8 +34,8 @@ describe Gitlab::Ci::Config::Node::Jobs do
context 'when job is unspecified' do
let(:config) { { rspec: nil } }
it 'is not valid' do
expect(entry).not_to be_valid
it 'reports error' do
expect(entry.errors).to include "rspec config can't be blank"
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment