diff options
author | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2018-03-23 15:44:12 +0300 |
---|---|---|
committer | Grzegorz Bizon <grzesiek.bizon@gmail.com> | 2018-03-23 15:44:12 +0300 |
commit | 8a2bc9b4457903dfdb0efc88c5ba816f4ba3141e (patch) | |
tree | b2070a8ecb32449ecdb4a1bf7dc9c40afacee912 | |
parent | 96d6193cabacc92f43bd441f48d63b226d9dad21 (diff) |
Integration variables collections with expressions
-rw-r--r-- | lib/gitlab/ci/build/policy/variables.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/expression/lexeme/variable.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/ci/pipeline/expression/statement.rb | 19 | ||||
-rw-r--r-- | lib/gitlab/ci/variables/collection.rb | 8 | ||||
-rw-r--r-- | lib/gitlab/ci/variables/collection/item.rb | 4 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/build/policy/variables_spec.rb | 34 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb | 16 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/variables/collection/item_spec.rb | 10 | ||||
-rw-r--r-- | spec/lib/gitlab/ci/variables/collection_spec.rb | 17 |
9 files changed, 71 insertions, 45 deletions
diff --git a/lib/gitlab/ci/build/policy/variables.rb b/lib/gitlab/ci/build/policy/variables.rb index ec6c64d9547..e5fec4469b3 100644 --- a/lib/gitlab/ci/build/policy/variables.rb +++ b/lib/gitlab/ci/build/policy/variables.rb @@ -8,9 +8,13 @@ module Gitlab end def satisfied_by?(pipeline, build) + variables = Gitlab::Ci::Variables::Collection + .new(build.simple_variables) + .to_hash + statements = @expressions.map do |statement| ::Gitlab::Ci::Pipeline::Expression::Statement - .new(statement, pipeline) + .new(statement, variables) end statements.any?(&:truthful?) diff --git a/lib/gitlab/ci/pipeline/expression/lexeme/variable.rb b/lib/gitlab/ci/pipeline/expression/lexeme/variable.rb index b781c15fd67..37643c8ef53 100644 --- a/lib/gitlab/ci/pipeline/expression/lexeme/variable.rb +++ b/lib/gitlab/ci/pipeline/expression/lexeme/variable.rb @@ -11,7 +11,7 @@ module Gitlab end def evaluate(variables = {}) - HashWithIndifferentAccess.new(variables).fetch(@name, nil) + variables.with_indifferent_access.fetch(@name, nil) end def self.build(string) diff --git a/lib/gitlab/ci/pipeline/expression/statement.rb b/lib/gitlab/ci/pipeline/expression/statement.rb index f0f3d760b70..b04576f7978 100644 --- a/lib/gitlab/ci/pipeline/expression/statement.rb +++ b/lib/gitlab/ci/pipeline/expression/statement.rb @@ -14,24 +14,9 @@ module Gitlab %w[variable] ].freeze - def initialize(statement, pipeline = nil) + def initialize(statement, variables = {}) @lexer = Expression::Lexer.new(statement) - - return if pipeline.nil? - - # REFACTORING, temporary refactoring stubs - # - @variables = pipeline.project.predefined_variables.map do |variable| - [variable[:key], variable[:value]] - end - - @variables += pipeline.variables.map do |variable| - [variable.key, variable.value] - end - - @variables += pipeline.predefined_variables.map do |variable| - [variable[:key], variable[:value]] - end + @variables = variables.with_indifferent_access end def parse_tree diff --git a/lib/gitlab/ci/variables/collection.rb b/lib/gitlab/ci/variables/collection.rb index 0deca55fe8f..ad30b3f427c 100644 --- a/lib/gitlab/ci/variables/collection.rb +++ b/lib/gitlab/ci/variables/collection.rb @@ -30,7 +30,13 @@ module Gitlab end def to_runner_variables - self.map(&:to_hash) + self.map(&:to_runner_variable) + end + + def to_hash + self.to_runner_variables + .map { |env| [env.fetch(:key), env.fetch(:value)] } + .to_h.with_indifferent_access end end end diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb index 939912981e6..23ed71db8b0 100644 --- a/lib/gitlab/ci/variables/collection/item.rb +++ b/lib/gitlab/ci/variables/collection/item.rb @@ -17,7 +17,7 @@ module Gitlab end def ==(other) - to_hash == self.class.fabricate(other).to_hash + to_runner_variable == self.class.fabricate(other).to_runner_variable end ## @@ -25,7 +25,7 @@ module Gitlab # don't expose `file` attribute at all (stems from what the runner # expects). # - def to_hash + def to_runner_variable @variable.reject do |hash_key, hash_value| hash_key == :file && hash_value == false end diff --git a/spec/lib/gitlab/ci/build/policy/variables_spec.rb b/spec/lib/gitlab/ci/build/policy/variables_spec.rb index ce229ed256a..ac5d05e0583 100644 --- a/spec/lib/gitlab/ci/build/policy/variables_spec.rb +++ b/spec/lib/gitlab/ci/build/policy/variables_spec.rb @@ -1,42 +1,58 @@ require 'spec_helper' describe Gitlab::Ci::Build::Policy::Variables do - let(:pipeline) { build_stubbed(:ci_empty_pipeline, ref: 'master') } - let(:build) { build_stubbed(:ci_build, pipeline: pipeline, ref: 'master') } + set(:project) { create(:project) } + + let(:ci_pipeline) do + build(:ci_empty_pipeline, project: project, ref: 'master') + end + + let(:ci_build) do + build(:ci_build, pipeline: ci_pipeline, project: project, ref: 'master') + end before do - pipeline.variables.build(key: 'CI_PROJECT_NAME', value: '') + ci_pipeline.variables.build(key: 'CI_PROJECT_NAME', value: '') end describe '#satisfied_by?' do it 'is satisfied by a defined and existing variable' do policy = described_class.new(['$CI_PROJECT_ID', '$UNDEFINED']) - expect(policy).to be_satisfied_by(pipeline, build) + expect(policy).to be_satisfied_by(ci_pipeline, ci_build) end it 'is not satisfied by an overriden empty variable' do policy = described_class.new(['$CI_PROJECT_NAME']) - expect(policy).not_to be_satisfied_by(pipeline, build) + expect(policy).not_to be_satisfied_by(ci_pipeline, ci_build) end it 'is satisfied by a truthy pipeline expression' do - policy = described_class.new([%($CI_PIPELINE_SOURCE == "#{pipeline.source}")]) + policy = described_class.new([%($CI_PIPELINE_SOURCE == "#{ci_pipeline.source}")]) - expect(policy).to be_satisfied_by(pipeline, build) + expect(policy).to be_satisfied_by(ci_pipeline, ci_build) end it 'is not satisfied by a falsy pipeline expression' do policy = described_class.new([%($CI_PIPELINE_SOURCE == "invalid source")]) - expect(policy).not_to be_satisfied_by(pipeline, build) + expect(policy).not_to be_satisfied_by(ci_pipeline, ci_build) end it 'is satisfied by a truthy expression using undefined variable' do policy = described_class.new(['$UNDEFINED', '$UNDEFINED == null']) - expect(policy).to be_satisfied_by(pipeline, build) + expect(policy).to be_satisfied_by(ci_pipeline, ci_build) end + + it 'does not persist neither pipeline nor build' do + described_class.new('$VAR').satisfied_by?(ci_pipeline, ci_build) + + expect(ci_pipeline).not_to be_persisted + expect(ci_build).not_to be_persisted + end + + pending 'test for secret variables' end end diff --git a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb index 475ac7afcb3..6685bf5385b 100644 --- a/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/expression/statement_spec.rb @@ -1,22 +1,18 @@ require 'spec_helper' describe Gitlab::Ci::Pipeline::Expression::Statement do - let(:pipeline) { build(:ci_pipeline) } - subject do - described_class.new(text, pipeline) + described_class.new(text, variables) end - before do - variables = [{ key: 'PRESENT_VARIABLE', value: 'my variable' }, - { key: 'EMPTY_VARIABLE', value: '' }] - - pipeline.variables.build(variables) + let(:variables) do + { 'PRESENT_VARIABLE' => 'my variable', + EMPTY_VARIABLE: '' } end describe '.new' do - context 'when pipeline is not provided' do - it 'allows to properly initialize the statement' do + context 'when variables are not provided' do + it 'allows to properly initializes the statement' do statement = described_class.new('$PRESENT_VARIABLE') expect(statement.evaluate).to be_nil diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb index cc1257484d2..bf9208f1ff4 100644 --- a/spec/lib/gitlab/ci/variables/collection/item_spec.rb +++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb @@ -46,9 +46,13 @@ describe Gitlab::Ci::Variables::Collection::Item do end end - describe '#to_hash' do - it 'returns a hash representation of a collection item' do - expect(described_class.new(**variable).to_hash).to eq variable + describe '#to_runner_variable' do + it 'returns a runner-compatible hash representation' do + runner_variable = described_class + .new(**variable) + .to_runner_variable + + expect(runner_variable).to eq variable end end end diff --git a/spec/lib/gitlab/ci/variables/collection_spec.rb b/spec/lib/gitlab/ci/variables/collection_spec.rb index 90b6e178242..cb2f7718c9c 100644 --- a/spec/lib/gitlab/ci/variables/collection_spec.rb +++ b/spec/lib/gitlab/ci/variables/collection_spec.rb @@ -7,7 +7,7 @@ describe Gitlab::Ci::Variables::Collection do collection = described_class.new([variable]) - expect(collection.first.to_hash).to eq variable + expect(collection.first.to_runner_variable).to eq variable end it 'can be initialized without an argument' do @@ -96,4 +96,19 @@ describe Gitlab::Ci::Variables::Collection do .to eq [{ key: 'TEST', value: 1, public: true }] end end + + describe '#to_hash' do + it 'returns regular hash in valid order without duplicates' do + collection = described_class.new + .append(key: 'TEST1', value: 'test-1') + .append(key: 'TEST2', value: 'test-2') + .append(key: 'TEST1', value: 'test-3') + + expect(collection.to_hash).to eq('TEST1' => 'test-3', + 'TEST2' => 'test-2') + + expect(collection.to_hash).to include(TEST1: 'test-3') + expect(collection.to_hash).not_to include(TEST1: 'test-1') + end + end end |