summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKamil TrzciƄski <ayufan@ayufan.eu>2018-12-13 10:39:55 +0000
committerGrzegorz Bizon <grzegorz@gitlab.com>2018-12-13 10:39:55 +0000
commit0165cfaa078a50f52c9f24e1c468b77b509c89f3 (patch)
tree0d1ab70049703c76440281aba201f3a6d2d81d52
parent6b68d82fbf2da3f73d411e7a6fbda16cd3b94604 (diff)
downloadgitlab-ce-0165cfaa078a50f52c9f24e1c468b77b509c89f3.tar.gz
Re-define default only except policy
-rw-r--r--changelogs/unreleased/define-default-value-for-only-except-keys.yml2
-rw-r--r--lib/gitlab/ci/config/entry/except_policy.rb17
-rw-r--r--lib/gitlab/ci/config/entry/job.rb15
-rw-r--r--lib/gitlab/ci/config/entry/only_policy.rb18
-rw-r--r--lib/gitlab/ci/config/entry/policy.rb15
-rw-r--r--spec/lib/gitlab/ci/config/entry/except_policy_spec.rb15
-rw-r--r--spec/lib/gitlab/ci/config/entry/global_spec.rb6
-rw-r--r--spec/lib/gitlab/ci/config/entry/job_spec.rb3
-rw-r--r--spec/lib/gitlab/ci/config/entry/jobs_spec.rb6
-rw-r--r--spec/lib/gitlab/ci/config/entry/only_policy_spec.rb15
-rw-r--r--spec/lib/gitlab/ci/config/entry/policy_spec.rb167
-rw-r--r--spec/services/ci/create_pipeline_service_spec.rb31
-rw-r--r--spec/support/shared_examples/only_except_policy_examples.rb167
13 files changed, 223 insertions, 254 deletions
diff --git a/changelogs/unreleased/define-default-value-for-only-except-keys.yml b/changelogs/unreleased/define-default-value-for-only-except-keys.yml
index 3e5ecdcf51e..ed0e982f0fc 100644
--- a/changelogs/unreleased/define-default-value-for-only-except-keys.yml
+++ b/changelogs/unreleased/define-default-value-for-only-except-keys.yml
@@ -1,5 +1,5 @@
---
title: Define the default value for only/except policies
-merge_request: 23531
+merge_request: 23765
author:
type: changed
diff --git a/lib/gitlab/ci/config/entry/except_policy.rb b/lib/gitlab/ci/config/entry/except_policy.rb
deleted file mode 100644
index 46ded35325d..00000000000
--- a/lib/gitlab/ci/config/entry/except_policy.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module Ci
- class Config
- module Entry
- ##
- # Entry that represents an only/except trigger policy for the job.
- #
- class ExceptPolicy < Policy
- def self.default
- end
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/ci/config/entry/job.rb b/lib/gitlab/ci/config/entry/job.rb
index 085be5da08d..50942fbdb40 100644
--- a/lib/gitlab/ci/config/entry/job.rb
+++ b/lib/gitlab/ci/config/entry/job.rb
@@ -16,6 +16,13 @@ module Gitlab
dependencies before_script after_script variables
environment coverage retry parallel extends].freeze
+ DEFAULT_ONLY_POLICY = {
+ refs: %w(branches tags)
+ }.freeze
+
+ DEFAULT_EXCEPT_POLICY = {
+ }.freeze
+
validations do
validates :config, allowed_keys: ALLOWED_KEYS
validates :config, presence: true
@@ -65,10 +72,10 @@ module Gitlab
entry :services, Entry::Services,
description: 'Services that will be used to execute this job.'
- entry :only, Entry::OnlyPolicy,
+ entry :only, Entry::Policy,
description: 'Refs policy this job will be executed for.'
- entry :except, Entry::ExceptPolicy,
+ entry :except, Entry::Policy,
description: 'Refs policy this job will be executed for.'
entry :variables, Entry::Variables,
@@ -154,8 +161,8 @@ module Gitlab
services: services_value,
stage: stage_value,
cache: cache_value,
- only: only_value,
- except: except_value,
+ only: DEFAULT_ONLY_POLICY.deep_merge(only_value.to_h),
+ except: DEFAULT_EXCEPT_POLICY.deep_merge(except_value.to_h),
variables: variables_defined? ? variables_value : nil,
environment: environment_defined? ? environment_value : nil,
environment_name: environment_defined? ? environment_value[:name] : nil,
diff --git a/lib/gitlab/ci/config/entry/only_policy.rb b/lib/gitlab/ci/config/entry/only_policy.rb
deleted file mode 100644
index 9a581b8e97e..00000000000
--- a/lib/gitlab/ci/config/entry/only_policy.rb
+++ /dev/null
@@ -1,18 +0,0 @@
-# frozen_string_literal: true
-
-module Gitlab
- module Ci
- class Config
- module Entry
- ##
- # Entry that represents an only/except trigger policy for the job.
- #
- class OnlyPolicy < Policy
- def self.default
- { refs: %w[branches tags] }
- end
- end
- end
- end
- end
-end
diff --git a/lib/gitlab/ci/config/entry/policy.rb b/lib/gitlab/ci/config/entry/policy.rb
index 81e74a639fc..998da1f6837 100644
--- a/lib/gitlab/ci/config/entry/policy.rb
+++ b/lib/gitlab/ci/config/entry/policy.rb
@@ -5,9 +5,12 @@ module Gitlab
class Config
module Entry
##
- # Base class for OnlyPolicy and ExceptPolicy
+ # Entry that represents an only/except trigger policy for the job.
#
class Policy < ::Gitlab::Config::Entry::Simplifiable
+ strategy :RefsPolicy, if: -> (config) { config.is_a?(Array) }
+ strategy :ComplexPolicy, if: -> (config) { config.is_a?(Hash) }
+
class RefsPolicy < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
@@ -63,16 +66,6 @@ module Gitlab
def self.default
end
-
- ##
- # Class-level execution won't be inherited by subclasses by default.
- # Therefore, we need to explicitly execute that for OnlyPolicy and ExceptPolicy
- def self.inherited(klass)
- super
-
- klass.strategy :RefsPolicy, if: -> (config) { config.is_a?(Array) }
- klass.strategy :ComplexPolicy, if: -> (config) { config.is_a?(Hash) }
- end
end
end
end
diff --git a/spec/lib/gitlab/ci/config/entry/except_policy_spec.rb b/spec/lib/gitlab/ci/config/entry/except_policy_spec.rb
deleted file mode 100644
index d036bf2f4d1..00000000000
--- a/spec/lib/gitlab/ci/config/entry/except_policy_spec.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Entry::ExceptPolicy do
- let(:entry) { described_class.new(config) }
-
- it_behaves_like 'correct only except policy'
-
- describe '.default' do
- it 'does not have a default value' do
- expect(described_class.default).to be_nil
- end
- end
-end
diff --git a/spec/lib/gitlab/ci/config/entry/global_spec.rb b/spec/lib/gitlab/ci/config/entry/global_spec.rb
index 12f4b9dc624..61d78f86b51 100644
--- a/spec/lib/gitlab/ci/config/entry/global_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/global_spec.rb
@@ -161,7 +161,8 @@ describe Gitlab::Ci::Config::Entry::Global do
variables: { 'VAR' => 'value' },
ignore: false,
after_script: ['make clean'],
- only: { refs: %w[branches tags] } },
+ only: { refs: %w[branches tags] },
+ except: {} },
spinach: { name: :spinach,
before_script: [],
script: %w[spinach],
@@ -173,7 +174,8 @@ describe Gitlab::Ci::Config::Entry::Global do
variables: {},
ignore: false,
after_script: ['make clean'],
- only: { refs: %w[branches tags] } }
+ only: { refs: %w[branches tags] },
+ except: {} }
)
end
end
diff --git a/spec/lib/gitlab/ci/config/entry/job_spec.rb b/spec/lib/gitlab/ci/config/entry/job_spec.rb
index c1f4a060063..8e32cede3b5 100644
--- a/spec/lib/gitlab/ci/config/entry/job_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/job_spec.rb
@@ -259,7 +259,8 @@ describe Gitlab::Ci::Config::Entry::Job do
stage: 'test',
ignore: false,
after_script: %w[cleanup],
- only: { refs: %w[branches tags] })
+ only: { refs: %w[branches tags] },
+ except: {})
end
end
end
diff --git a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb
index 2a753408f54..1a2c30d3571 100644
--- a/spec/lib/gitlab/ci/config/entry/jobs_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/jobs_spec.rb
@@ -68,13 +68,15 @@ describe Gitlab::Ci::Config::Entry::Jobs do
commands: 'rspec',
ignore: false,
stage: 'test',
- only: { refs: %w[branches tags] } },
+ only: { refs: %w[branches tags] },
+ except: {} },
spinach: { name: :spinach,
script: %w[spinach],
commands: 'spinach',
ignore: false,
stage: 'test',
- only: { refs: %w[branches tags] } })
+ only: { refs: %w[branches tags] },
+ except: {} })
end
end
diff --git a/spec/lib/gitlab/ci/config/entry/only_policy_spec.rb b/spec/lib/gitlab/ci/config/entry/only_policy_spec.rb
deleted file mode 100644
index 5518b68e51a..00000000000
--- a/spec/lib/gitlab/ci/config/entry/only_policy_spec.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Gitlab::Ci::Config::Entry::OnlyPolicy do
- let(:entry) { described_class.new(config) }
-
- it_behaves_like 'correct only except policy'
-
- describe '.default' do
- it 'haa a default value' do
- expect(described_class.default).to eq( { refs: %w[branches tags] } )
- end
- end
-end
diff --git a/spec/lib/gitlab/ci/config/entry/policy_spec.rb b/spec/lib/gitlab/ci/config/entry/policy_spec.rb
index cf40a22af2e..83001b7fdd8 100644
--- a/spec/lib/gitlab/ci/config/entry/policy_spec.rb
+++ b/spec/lib/gitlab/ci/config/entry/policy_spec.rb
@@ -1,8 +1,173 @@
-require 'spec_helper'
+require 'fast_spec_helper'
+require_dependency 'active_model'
describe Gitlab::Ci::Config::Entry::Policy do
let(:entry) { described_class.new(config) }
+ context 'when using simplified policy' do
+ describe 'validations' do
+ context 'when entry config value is valid' do
+ context 'when config is a branch or tag name' do
+ let(:config) { %w[master feature/branch] }
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+
+ describe '#value' do
+ it 'returns refs hash' do
+ expect(entry.value).to eq(refs: config)
+ end
+ end
+ end
+
+ context 'when config is a regexp' do
+ let(:config) { ['/^issue-.*$/'] }
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ end
+ end
+
+ context 'when config is a special keyword' do
+ let(:config) { %w[tags triggers branches] }
+
+ describe '#valid?' do
+ it 'is valid' do
+ expect(entry).to be_valid
+ end
+ 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 /policy config should be an array of strings or regexps/
+ end
+ end
+ end
+ end
+ end
+
+ context 'when using complex policy' do
+ context 'when specifying refs policy' do
+ let(:config) { { refs: ['master'] } }
+
+ it 'is a correct configuraton' do
+ expect(entry).to be_valid
+ expect(entry.value).to eq(refs: %w[master])
+ end
+ end
+
+ context 'when specifying kubernetes policy' do
+ let(:config) { { kubernetes: 'active' } }
+
+ it 'is a correct configuraton' do
+ expect(entry).to be_valid
+ expect(entry.value).to eq(kubernetes: 'active')
+ end
+ end
+
+ context 'when specifying invalid kubernetes policy' do
+ let(:config) { { kubernetes: 'something' } }
+
+ it 'reports an error about invalid policy' do
+ expect(entry.errors).to include /unknown value: something/
+ end
+ end
+
+ context 'when specifying valid variables expressions policy' do
+ let(:config) { { variables: ['$VAR == null'] } }
+
+ it 'is a correct configuraton' do
+ expect(entry).to be_valid
+ expect(entry.value).to eq(config)
+ end
+ end
+
+ context 'when specifying variables expressions in invalid format' do
+ let(:config) { { variables: '$MY_VAR' } }
+
+ it 'reports an error about invalid format' do
+ expect(entry.errors).to include /should be an array of strings/
+ end
+ end
+
+ context 'when specifying invalid variables expressions statement' do
+ let(:config) { { variables: ['$MY_VAR =='] } }
+
+ it 'reports an error about invalid statement' do
+ expect(entry.errors).to include /invalid expression syntax/
+ end
+ end
+
+ context 'when specifying invalid variables expressions token' do
+ let(:config) { { variables: ['$MY_VAR == 123'] } }
+
+ it 'reports an error about invalid expression' do
+ expect(entry.errors).to include /invalid expression syntax/
+ end
+ end
+
+ context 'when using invalid variables expressions regexp' do
+ let(:config) { { variables: ['$MY_VAR =~ /some ( thing/'] } }
+
+ it 'reports an error about invalid expression' do
+ expect(entry.errors).to include /invalid expression syntax/
+ end
+ end
+
+ context 'when specifying a valid changes policy' do
+ let(:config) { { changes: %w[some/* paths/**/*.rb] } }
+
+ it 'is a correct configuraton' do
+ expect(entry).to be_valid
+ expect(entry.value).to eq(config)
+ end
+ end
+
+ context 'when changes policy is invalid' do
+ let(:config) { { changes: [1, 2] } }
+
+ it 'returns errors' do
+ expect(entry.errors).to include /changes should be an array of strings/
+ end
+ end
+
+ context 'when specifying unknown policy' do
+ let(:config) { { refs: ['master'], invalid: :something } }
+
+ it 'returns error about invalid key' do
+ expect(entry.errors).to include /unknown keys: invalid/
+ end
+ end
+
+ context 'when policy is empty' do
+ let(:config) { {} }
+
+ it 'is not a valid configuration' do
+ expect(entry.errors).to include /can't be blank/
+ end
+ end
+ end
+
+ context 'when policy strategy does not match' do
+ let(:config) { 'string strategy' }
+
+ it 'returns information about errors' do
+ expect(entry.errors)
+ .to include /has to be either an array of conditions or a hash/
+ end
+ end
+
describe '.default' do
it 'does not have a default value' do
expect(described_class.default).to be_nil
diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb
index 8b8021ecbc8..ffa47d527f7 100644
--- a/spec/services/ci/create_pipeline_service_spec.rb
+++ b/spec/services/ci/create_pipeline_service_spec.rb
@@ -840,6 +840,37 @@ describe Ci::CreatePipelineService do
end
end
+ context "when config uses variables for only keyword" do
+ let(:config) do
+ {
+ build: {
+ stage: 'build',
+ script: 'echo',
+ only: {
+ variables: %w($CI)
+ }
+ }
+ }
+ end
+
+ context 'when merge request is specified' do
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: project,
+ source_branch: ref_name,
+ target_project: project,
+ target_branch: 'master')
+ end
+
+ it 'does not create a merge request pipeline' do
+ expect(pipeline).not_to be_persisted
+
+ expect(pipeline.errors[:base])
+ .to eq(['No stages / jobs for this pipeline.'])
+ end
+ end
+ end
+
context "when config has 'except: [tags]'" do
let(:config) do
{
diff --git a/spec/support/shared_examples/only_except_policy_examples.rb b/spec/support/shared_examples/only_except_policy_examples.rb
deleted file mode 100644
index 35240af1d74..00000000000
--- a/spec/support/shared_examples/only_except_policy_examples.rb
+++ /dev/null
@@ -1,167 +0,0 @@
-# frozen_string_literal: true
-
-shared_examples 'correct only except policy' do
- context 'when using simplified policy' do
- describe 'validations' do
- context 'when entry config value is valid' do
- context 'when config is a branch or tag name' do
- let(:config) { %w[master feature/branch] }
-
- describe '#valid?' do
- it 'is valid' do
- expect(entry).to be_valid
- end
- end
-
- describe '#value' do
- it 'returns refs hash' do
- expect(entry.value).to eq(refs: config)
- end
- end
- end
-
- context 'when config is a regexp' do
- let(:config) { ['/^issue-.*$/'] }
-
- describe '#valid?' do
- it 'is valid' do
- expect(entry).to be_valid
- end
- end
- end
-
- context 'when config is a special keyword' do
- let(:config) { %w[tags triggers branches] }
-
- describe '#valid?' do
- it 'is valid' do
- expect(entry).to be_valid
- end
- 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 /policy config should be an array of strings or regexps/
- end
- end
- end
- end
- end
-
- context 'when using complex policy' do
- context 'when specifying refs policy' do
- let(:config) { { refs: ['master'] } }
-
- it 'is a correct configuraton' do
- expect(entry).to be_valid
- expect(entry.value).to eq(refs: %w[master])
- end
- end
-
- context 'when specifying kubernetes policy' do
- let(:config) { { kubernetes: 'active' } }
-
- it 'is a correct configuraton' do
- expect(entry).to be_valid
- expect(entry.value).to eq(kubernetes: 'active')
- end
- end
-
- context 'when specifying invalid kubernetes policy' do
- let(:config) { { kubernetes: 'something' } }
-
- it 'reports an error about invalid policy' do
- expect(entry.errors).to include /unknown value: something/
- end
- end
-
- context 'when specifying valid variables expressions policy' do
- let(:config) { { variables: ['$VAR == null'] } }
-
- it 'is a correct configuraton' do
- expect(entry).to be_valid
- expect(entry.value).to eq(config)
- end
- end
-
- context 'when specifying variables expressions in invalid format' do
- let(:config) { { variables: '$MY_VAR' } }
-
- it 'reports an error about invalid format' do
- expect(entry.errors).to include /should be an array of strings/
- end
- end
-
- context 'when specifying invalid variables expressions statement' do
- let(:config) { { variables: ['$MY_VAR =='] } }
-
- it 'reports an error about invalid statement' do
- expect(entry.errors).to include /invalid expression syntax/
- end
- end
-
- context 'when specifying invalid variables expressions token' do
- let(:config) { { variables: ['$MY_VAR == 123'] } }
-
- it 'reports an error about invalid expression' do
- expect(entry.errors).to include /invalid expression syntax/
- end
- end
-
- context 'when using invalid variables expressions regexp' do
- let(:config) { { variables: ['$MY_VAR =~ /some ( thing/'] } }
-
- it 'reports an error about invalid expression' do
- expect(entry.errors).to include /invalid expression syntax/
- end
- end
-
- context 'when specifying a valid changes policy' do
- let(:config) { { changes: %w[some/* paths/**/*.rb] } }
-
- it 'is a correct configuraton' do
- expect(entry).to be_valid
- expect(entry.value).to eq(config)
- end
- end
-
- context 'when changes policy is invalid' do
- let(:config) { { changes: [1, 2] } }
-
- it 'returns errors' do
- expect(entry.errors).to include /changes should be an array of strings/
- end
- end
-
- context 'when specifying unknown policy' do
- let(:config) { { refs: ['master'], invalid: :something } }
-
- it 'returns error about invalid key' do
- expect(entry.errors).to include /unknown keys: invalid/
- end
- end
-
- context 'when policy is empty' do
- let(:config) { {} }
-
- it 'is not a valid configuration' do
- expect(entry.errors).to include /can't be blank/
- end
- end
- end
-
- context 'when policy strategy does not match' do
- let(:config) { 'string strategy' }
-
- it 'returns information about errors' do
- expect(entry.errors)
- .to include /has to be either an array of conditions or a hash/
- end
- end
-end