From fe08bdf3961c6bc7db038b3c4e7c3e7d65d2344d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 17 Aug 2018 11:42:57 +0200 Subject: [PATCH] Add specs for extendable CI/CD hash entry class --- lib/gitlab/ci/config/extendable/entry.rb | 44 ++-- .../ci/config/extendable/collection_spec.rb | 37 +++- .../gitlab/ci/config/extendable/entry_spec.rb | 188 ++++++++++++++++++ 3 files changed, 249 insertions(+), 20 deletions(-) create mode 100644 spec/lib/gitlab/ci/config/extendable/entry_spec.rb diff --git a/lib/gitlab/ci/config/extendable/entry.rb b/lib/gitlab/ci/config/extendable/entry.rb index d5eef647b3e..5b26faaa806 100644 --- a/lib/gitlab/ci/config/extendable/entry.rb +++ b/lib/gitlab/ci/config/extendable/entry.rb @@ -9,28 +9,26 @@ module Gitlab @key = key @context = context @parent = parent + + raise StandardError, 'Invalid entry key!' unless @context.key?(@key) end - def valid? - true + def extensible? + value.is_a?(Hash) && value.key?(:extends) end def value @value ||= @context.fetch(@key) end - def base_hash - Extendable::Entry + def base_hash! + @base ||= Extendable::Entry .new(extends_key, @context, self) .extend! end - def extensible? - value.key?(:extends) - end - def extends_key - value.fetch(:extends).to_s.to_sym + value.fetch(:extends).to_s.to_sym if extensible? end def path @@ -38,19 +36,23 @@ module Gitlab end def extend! + return value unless extensible? + + if unknown_extension? + raise Extendable::Collection::InvalidExtensionError, + 'Unknown extension!' + end + + if invalid_base? + raise Extendable::Collection::InvalidExtensionError, + 'Invalid base hash!' + end + if circular_dependency? raise Extendable::Collection::CircularDependencyError end - if invalid_extends_key? - raise Extendable::Collection::InvalidExtensionError - end - - if extensible? - @context[key] = base_hash.deep_merge(value) - else - value - end + @context[key] = base_hash!.deep_merge(value) end private @@ -59,9 +61,13 @@ module Gitlab path.count(key) > 1 end - def invalid_extends_key? + def unknown_extension? !@context.key?(key) end + + def invalid_base? + !@context[extends_key].is_a?(Hash) + end end end end diff --git a/spec/lib/gitlab/ci/config/extendable/collection_spec.rb b/spec/lib/gitlab/ci/config/extendable/collection_spec.rb index 796683fea63..20483800315 100644 --- a/spec/lib/gitlab/ci/config/extendable/collection_spec.rb +++ b/spec/lib/gitlab/ci/config/extendable/collection_spec.rb @@ -20,7 +20,23 @@ describe Gitlab::Ci::Config::Extendable::Collection do end describe '#to_hash' do - context 'when a hash has a single simple extension' do + context 'when hash does not contain extensions' do + let(:hash) do + { + test: { script: 'test' }, + production: { + script: 'deploy', + only: { variables: %w[$SOMETHING] } + } + } + end + + it 'does not modify the hash' do + expect(subject.to_hash).to eq hash + end + end + + context 'when hash has a single simple extension' do let(:hash) do { something: { @@ -162,5 +178,24 @@ describe Gitlab::Ci::Config::Extendable::Collection do .to raise_error(described_class::InvalidExtensionError) end end + + context 'when extensible entry has non-hash inheritace defined' do + let(:hash) do + { + test: { + extends: 'something', + script: 'ls', + only: { refs: %w[master] } + }, + + something: 'some text' + } + end + + it 'raises an error about invalid base' do + expect { subject.to_hash } + .to raise_error(described_class::InvalidExtensionError) + end + end end end diff --git a/spec/lib/gitlab/ci/config/extendable/entry_spec.rb b/spec/lib/gitlab/ci/config/extendable/entry_spec.rb new file mode 100644 index 00000000000..7cc6e3f01f1 --- /dev/null +++ b/spec/lib/gitlab/ci/config/extendable/entry_spec.rb @@ -0,0 +1,188 @@ +require 'fast_spec_helper' + +describe Gitlab::Ci::Config::Extendable::Entry do + describe '.new' do + context 'when entry key is not included in the context hash' do + it 'raises error' do + expect { described_class.new(:test, something: 'something') } + .to raise_error StandardError, 'Invalid entry key!' + end + end + end + + describe '#value' do + it 'reads a hash value from the context' do + entry = described_class.new(:test, test: 'something') + + expect(entry.value).to eq 'something' + end + end + + describe '#extensible?' do + context 'when entry has inheritance defined' do + it 'is extensible' do + entry = described_class.new(:test, test: { extends: 'something' }) + + expect(entry).to be_extensible + end + end + + context 'when entry does not have inheritance specified' do + it 'is not extensible' do + entry = described_class.new(:test, test: { script: 'something' }) + + expect(entry).not_to be_extensible + end + end + + context 'when entry value is not a hash' do + it 'is not extensible' do + entry = described_class.new(:test, test: 'something') + + expect(entry).not_to be_extensible + end + end + end + + describe '#extends_key' do + context 'when entry is extensible' do + it 'returns symbolized extends key value' do + entry = described_class.new(:test, test: { extends: 'something' }) + + expect(entry.extends_key).to eq :something + end + end + + context 'when entry is not extensible' do + it 'returns nil' do + entry = described_class.new(:test, test: 'something') + + expect(entry.extends_key).to be_nil + end + end + end + + describe '#path' do + it 'returns inheritance chain path' do + parent = described_class.new(:test, test: { extends: 'something' }) + child = described_class.new(:job, { job: { script: 'something' } }, parent) + + expect(child.path).to eq [:test, :job] + end + end + + describe '#base_hash!' do + subject { described_class.new(:test, hash) } + + context 'when base hash is not extensible' do + let(:hash) do + { + template: { script: 'rspec' }, + test: { extends: 'template' } + } + end + + it 'returns unchanged base hash' do + expect(subject.base_hash!).to eq(script: 'rspec') + end + end + + context 'when base hash is extensible too' do + let(:hash) do + { + first: { script: 'rspec' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + it 'extends the base hash first' do + expect(subject.base_hash!).to eq(extends: 'first', script: 'rspec') + end + + it 'mutates original context' do + subject.base_hash! + + expect(hash.fetch(:second)).to eq(extends: 'first', script: 'rspec') + end + end + end + + describe '#extend!' do + subject { described_class.new(:test, hash) } + + context 'when extending a non-hash value' do + let(:hash) do + { + first: 'my value', + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + it 'raises an error' do + expect { subject.extend! }.to raise_error(StandardError) + end + end + + context 'when extending unknown key' do + let(:hash) do + { test: { extends: 'something' } } + end + + it 'raises an error' do + expect { subject.extend! }.to raise_error(StandardError) + end + end + + context 'when extending a hash correctly' do + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { extends: 'second' } + } + end + + let(:result) do + { + first: { script: 'my value' }, + second: { extends: 'first', script: 'my value' }, + test: { extends: 'second', script: 'my value' } + } + end + + it 'returns extended part of the hash' do + expect(subject.extend!).to eq result[:test] + end + + it 'mutates original context' do + subject.extend! + + expect(hash).to eq result + end + end + + context 'when hash is not extensible' do + let(:hash) do + { + first: { script: 'my value' }, + second: { extends: 'first' }, + test: { value: 'something' } + } + end + + it 'returns original key value' do + expect(subject.extend!).to eq(value: 'something') + end + + it 'does not mutate orignal context' do + original = hash.dup + + subject.extend! + + expect(hash).to eq original + end + end + end +end