Require stage:
to be set with needs:
Since we are unsure what would be the behavior of `stage:` when we work on DAG. Let's make `stage:` to be required today with `needs:`.
This commit is contained in:
parent
1438119df4
commit
583544d089
5 changed files with 57 additions and 4 deletions
|
@ -16,8 +16,11 @@ module Gitlab
|
|||
dependencies needs before_script after_script variables
|
||||
environment coverage retry parallel extends].freeze
|
||||
|
||||
REQUIRED_BY_NEEDS = %i[stage].freeze
|
||||
|
||||
validations do
|
||||
validates :config, allowed_keys: ALLOWED_KEYS
|
||||
validates :config, required_keys: REQUIRED_BY_NEEDS, if: :has_needs?
|
||||
validates :config, presence: true
|
||||
validates :script, presence: true
|
||||
validates :name, presence: true
|
||||
|
|
|
@ -18,6 +18,10 @@ module Gitlab
|
|||
|
||||
config[attribute]
|
||||
end
|
||||
|
||||
define_method("has_#{attribute}?") do
|
||||
config.is_a?(Hash) && config.key?(attribute)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -26,6 +26,17 @@ module Gitlab
|
|||
end
|
||||
end
|
||||
|
||||
class RequiredKeysValidator < ActiveModel::EachValidator
|
||||
def validate_each(record, attribute, value)
|
||||
present_keys = options[:in] - value.try(:keys).to_a
|
||||
|
||||
if present_keys.any?
|
||||
record.errors.add(attribute, "missing required keys: " +
|
||||
present_keys.join(', '))
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
class AllowedValuesValidator < ActiveModel::EachValidator
|
||||
def validate_each(record, attribute, value)
|
||||
unless options[:in].include?(value.to_s)
|
||||
|
|
|
@ -89,14 +89,23 @@ describe Gitlab::Ci::Config::Entry::Job do
|
|||
|
||||
context 'when has needs' do
|
||||
let(:config) do
|
||||
{ script: 'echo', needs: ['another-job'] }
|
||||
{
|
||||
stage: 'test',
|
||||
script: 'echo',
|
||||
needs: ['another-job']
|
||||
}
|
||||
end
|
||||
|
||||
it { expect(entry).to be_valid }
|
||||
|
||||
context 'when has dependencies' do
|
||||
let(:config) do
|
||||
{ script: 'echo', dependencies: ['another-job'], needs: ['another-job'] }
|
||||
{
|
||||
stage: 'test',
|
||||
script: 'echo',
|
||||
dependencies: ['another-job'],
|
||||
needs: ['another-job']
|
||||
}
|
||||
end
|
||||
|
||||
it { expect(entry).to be_valid }
|
||||
|
@ -256,7 +265,11 @@ describe Gitlab::Ci::Config::Entry::Job do
|
|||
context 'when has needs' do
|
||||
context 'that are not a array of strings' do
|
||||
let(:config) do
|
||||
{ script: 'echo', needs: 'build-job' }
|
||||
{
|
||||
stage: 'test',
|
||||
script: 'echo',
|
||||
needs: 'build-job'
|
||||
}
|
||||
end
|
||||
|
||||
it 'returns error about invalid type' do
|
||||
|
@ -267,7 +280,12 @@ describe Gitlab::Ci::Config::Entry::Job do
|
|||
|
||||
context 'when have dependencies that are not subset of needs' do
|
||||
let(:config) do
|
||||
{ script: 'echo', dependencies: ['another-job'], needs: ['build-job'] }
|
||||
{
|
||||
stage: 'test',
|
||||
script: 'echo',
|
||||
dependencies: ['another-job'],
|
||||
needs: ['build-job']
|
||||
}
|
||||
end
|
||||
|
||||
it 'returns error about invalid data' do
|
||||
|
@ -275,6 +293,20 @@ describe Gitlab::Ci::Config::Entry::Job do
|
|||
expect(entry.errors).to include 'job dependencies the another-job should be part of needs'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when stage: is missing' do
|
||||
let(:config) do
|
||||
{
|
||||
script: 'echo',
|
||||
needs: ['build-job']
|
||||
}
|
||||
end
|
||||
|
||||
it 'returns error about invalid data' do
|
||||
expect(entry).not_to be_valid
|
||||
expect(entry.errors).to include 'job config missing required keys: stage'
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -25,7 +25,9 @@ describe Gitlab::Config::Entry::Attributable do
|
|||
end
|
||||
|
||||
it 'returns the value of config' do
|
||||
expect(instance).to have_name
|
||||
expect(instance.name).to eq 'some name'
|
||||
expect(instance).to have_test
|
||||
expect(instance.test).to eq 'some test'
|
||||
end
|
||||
|
||||
|
@ -42,6 +44,7 @@ describe Gitlab::Config::Entry::Attributable do
|
|||
end
|
||||
|
||||
it 'returns nil' do
|
||||
expect(instance).not_to have_test
|
||||
expect(instance.test).to be_nil
|
||||
end
|
||||
end
|
||||
|
|
Loading…
Reference in a new issue