From 4491bf28e10da258701b316f397c5802f5f9974e Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Jul 2016 14:08:19 +0200
Subject: [PATCH] Move CI job config validations to new classes
---
lib/ci/gitlab_ci_yaml_processor.rb | 2 -
lib/gitlab/ci/config/node/entry.rb | 1 +
lib/gitlab/ci/config/node/jobs.rb | 15 +++++++
spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 9 +++-
spec/lib/gitlab/ci/config/node/global_spec.rb | 7 +++-
spec/lib/gitlab/ci/config/node/jobs_spec.rb | 42 +++++++++++++------
6 files changed, 59 insertions(+), 17 deletions(-)
diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb
index d226ebc3229..ab77d4df841 100644
--- a/lib/ci/gitlab_ci_yaml_processor.rb
+++ b/lib/ci/gitlab_ci_yaml_processor.rb
@@ -71,8 +71,6 @@ module Ci
@ci_config.jobs.each do |name, param|
add_job(name, param)
end
-
- raise ValidationError, "Please define at least one job" if @jobs.none?
end
def add_job(name, job)
diff --git a/lib/gitlab/ci/config/node/entry.rb b/lib/gitlab/ci/config/node/entry.rb
index fa6569e8bb2..033f9f0e3d1 100644
--- a/lib/gitlab/ci/config/node/entry.rb
+++ b/lib/gitlab/ci/config/node/entry.rb
@@ -24,6 +24,7 @@ module Gitlab
compose!
process_nodes!
+ @validator.validate(:processed)
end
def leaf?
diff --git a/lib/gitlab/ci/config/node/jobs.rb b/lib/gitlab/ci/config/node/jobs.rb
index a76b7a260c4..e8e78e4088d 100644
--- a/lib/gitlab/ci/config/node/jobs.rb
+++ b/lib/gitlab/ci/config/node/jobs.rb
@@ -10,12 +10,27 @@ module Gitlab
validations do
validates :config, type: Hash
+ validate :jobs_presence, on: :processed
+
+ def jobs_presence
+ unless relevant?
+ errors.add(:config, 'should contain at least one visible job')
+ end
+ end
end
def nodes
@config
end
+ def relevant?
+ @nodes.values.any?(&:relevant?)
+ end
+
+ def leaf?
+ false
+ end
+
private
def create_node(key, essence)
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 49a786191b8..ac058ba1595 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -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 => {} })
+ 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
diff --git a/spec/lib/gitlab/ci/config/node/global_spec.rb b/spec/lib/gitlab/ci/config/node/global_spec.rb
index 254cb28190c..a98de73c06c 100644
--- a/spec/lib/gitlab/ci/config/node/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/global_spec.rb
@@ -108,7 +108,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]
@@ -174,7 +177,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
diff --git a/spec/lib/gitlab/ci/config/node/jobs_spec.rb b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
index b2d2a92d9e8..7ec28b642b4 100644
--- a/spec/lib/gitlab/ci/config/node/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/node/jobs_spec.rb
@@ -4,17 +4,9 @@ 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 '#value' do
- it 'returns key value' do
- expect(entry.value).to eq(rspec: { script: 'rspec' })
- end
- end
-
describe '#valid?' do
it 'is valid' do
expect(entry).to be_valid
@@ -23,15 +15,34 @@ describe Gitlab::Ci::Config::Node::Jobs do
end
context 'when entry value is not correct' do
- context 'incorrect config value type' do
- let(:config) { ['incorrect'] }
+ describe '#errors' do
+ context 'incorrect config value type' do
+ let(:config) { ['incorrect'] }
- describe '#errors' do
- it 'saves errors' do
+ it 'returns error about incorrect type' do
expect(entry.errors)
.to include 'jobs config should be a hash'
end
end
+
+ context 'when no visible jobs present' do
+ let(:config) { { '.hidden'.to_sym => {} } }
+
+ context 'when not processed' do
+ it 'is valid' do
+ expect(entry.errors).to be_empty
+ end
+ end
+
+ context 'when processed' do
+ before { entry.process! }
+
+ 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
end
@@ -45,6 +56,13 @@ describe Gitlab::Ci::Config::Node::Jobs do
'.hidden'.to_sym => {} }
end
+ describe '#value' do
+ it 'returns key value' do
+ expect(entry.value).to eq(rspec: { script: 'rspec' },
+ spinach: { script: 'spinach' })
+ end
+ end
+
describe '#descendants' do
it 'creates valid descendant nodes' do
expect(entry.descendants.count).to eq 3
--
GitLab