Add specs for extendable CI/CD hash entry class

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!
-            if circular_dependency?
-              raise Extendable::Collection::CircularDependencyError
+            return value unless extensible?
+
+            if unknown_extension?
+              raise Extendable::Collection::InvalidExtensionError,
+                    'Unknown extension!'
             end
 
-            if invalid_extends_key?
-              raise Extendable::Collection::InvalidExtensionError
+            if invalid_base?
+              raise Extendable::Collection::InvalidExtensionError,
+                    'Invalid base hash!'
             end
 
-            if extensible?
-              @context[key] = base_hash.deep_merge(value)
-            else
-              value
+            if circular_dependency?
+              raise Extendable::Collection::CircularDependencyError
             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

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

spec/lib/gitlab/ci/config/extendable/entry_spec.rb

+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