Merge branch 'store-variables-and-when-in-builds-table' into 'master'

Store when and yaml variables in builds table

## What does this MR do?
Stores `when` and `yaml_variables` in `ci_builds` table.

## Why was this MR needed?
This is done in order to simplify the code responsible for creating a pipeline builds.

## What are the relevant issue numbers?
This made as a pre-step for https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5295.

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5296

3 files changed, 89 insertions, 83 deletions

diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb
index fe05a0cfc00..5fb671df570 100644
--- a/spec/factories/ci/builds.rb
+++ b/spec/factories/ci/builds.rb
@@ -15,6 +15,11 @@ FactoryGirl.define do
         services: ["postgres"]
       }
     end
+    yaml_variables do
+      [
+        { key: :DB_NAME, value: 'postgres', public: true }
+      ]
+    end
 
     pipeline factory: :ci_pipeline
 
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index bcbf409c8b0..ad6587b4c25 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -19,15 +19,14 @@ module Ci
         expect(config_processor.builds_for_stage_and_ref(type, "master").first).to eq({
           stage: "test",
           stage_idx: 1,
-          except: nil,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {},
           allow_failure: false,
           when: "on_success",
           environment: nil,
+          yaml_variables: []
         })
       end
 
@@ -432,11 +431,9 @@ module Ci
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
-          except: nil,
           stage: "test",
           stage_idx: 1,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -446,6 +443,7 @@ module Ci
           allow_failure: false,
           when: "on_success",
           environment: nil,
+          yaml_variables: []
         })
       end
 
@@ -461,11 +459,9 @@ module Ci
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
-          except: nil,
           stage: "test",
           stage_idx: 1,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -475,101 +471,126 @@ module Ci
           allow_failure: false,
           when: "on_success",
           environment: nil,
+          yaml_variables: []
         })
       end
     end
 
     describe 'Variables' do
-      context 'when global variables are defined' do
-        it 'returns global variables' do
-          variables = {
-            VAR1: 'value1',
-            VAR2: 'value2',
-          }
+      let(:config_processor) { GitlabCiYamlProcessor.new(YAML.dump(config), path) }
 
-          config = YAML.dump({
+      subject { config_processor.builds.first[:yaml_variables] }
+
+      context 'when global variables are defined' do
+        let(:variables) do
+          { VAR1: 'value1', VAR2: 'value2' }
+        end
+        let(:config) do
+          {
             variables: variables,
             before_script: ['pwd'],
             rspec: { script: 'rspec' }
-          })
+          }
+        end
 
-          config_processor = GitlabCiYamlProcessor.new(config, path)
+        it 'returns global variables' do
+          expect(subject).to contain_exactly(
+            { key: :VAR1, value: 'value1', public: true },
+            { key: :VAR2, value: 'value2', public: true }
+          )
+        end
+      end
+
+      context 'when job and global variables are defined' do
+        let(:global_variables) do
+          { VAR1: 'global1', VAR3: 'global3' }
+        end
+        let(:job_variables) do
+          { VAR1: 'value1', VAR2: 'value2' }
+        end
+        let(:config) do
+          {
+            before_script: ['pwd'],
+            variables: global_variables,
+            rspec: { script: 'rspec', variables: job_variables }
+          }
+        end
 
-          expect(config_processor.global_variables).to eq(variables)
+        it 'returns all unique variables' do
+          expect(subject).to contain_exactly(
+            { key: :VAR3, value: 'global3', public: true },
+            { key: :VAR1, value: 'value1', public: true },
+            { key: :VAR2, value: 'value2', public: true }
+          )
         end
       end
 
       context 'when job variables are defined' do
-        context 'when syntax is correct' do
-          it 'returns job variables' do
-            variables = {
-              KEY1: 'value1',
-              SOME_KEY_2: 'value2'
-            }
+        let(:config) do
+          {
+            before_script: ['pwd'],
+            rspec: { script: 'rspec', variables: variables }
+          }
+        end
+
+        context 'when also global variables are defined' do
 
-            config = YAML.dump(
-              { before_script: ['pwd'],
-                rspec: {
-                  variables: variables,
-                  script: 'rspec' }
-              })
+        end
 
-            config_processor = GitlabCiYamlProcessor.new(config, path)
+        context 'when syntax is correct' do
+          let(:variables) do
+            { VAR1: 'value1', VAR2: 'value2' }
+          end
 
-            expect(config_processor.job_variables(:rspec)).to eq variables
+          it 'returns job variables' do
+            expect(subject).to contain_exactly(
+              { key: :VAR1, value: 'value1', public: true },
+              { key: :VAR2, value: 'value2', public: true }
+            )
           end
         end
 
         context 'when syntax is incorrect' do
           context 'when variables defined but invalid' do
-            it 'raises error' do
-              variables = [:KEY1, 'value1', :KEY2, 'value2']
-
-              config =  YAML.dump(
-                { before_script: ['pwd'],
-                  rspec: {
-                    variables: variables,
-                    script: 'rspec' }
-                })
+            let(:variables) do
+              [ :VAR1, 'value1', :VAR2, 'value2' ]
+            end
 
-              expect { GitlabCiYamlProcessor.new(config, path) }
+            it 'raises error' do
+              expect { subject }
                 .to raise_error(GitlabCiYamlProcessor::ValidationError,
-                                 /job: variables should be a map/)
+                                /job: variables should be a map/)
             end
           end
 
           context 'when variables key defined but value not specified' do
-            it 'returns empty array' do
-              config =  YAML.dump(
-                { before_script: ['pwd'],
-                  rspec: {
-                    variables: nil,
-                    script: 'rspec' }
-                })
-
-              config_processor = GitlabCiYamlProcessor.new(config, path)
+            let(:variables) do
+              nil
+            end
 
+            it 'returns empty array' do
               ##
               # When variables config is empty, we assume this is a valid
               # configuration, see issue #18775
               #
-              expect(config_processor.job_variables(:rspec))
-                .to be_an_instance_of(Array).and be_empty
+              expect(subject).to be_an_instance_of(Array)
+              expect(subject).to be_empty
             end
           end
         end
       end
 
       context 'when job variables are not defined' do
-        it 'returns empty array' do
-          config = YAML.dump({
+        let(:config) do
+          {
             before_script: ['pwd'],
             rspec: { script: 'rspec' }
-          })
-
-          config_processor = GitlabCiYamlProcessor.new(config, path)
+          }
+        end
 
-          expect(config_processor.job_variables(:rspec)).to eq []
+        it 'returns empty array' do
+          expect(subject).to be_an_instance_of(Array)
+          expect(subject).to be_empty
         end
       end
     end
@@ -681,11 +702,9 @@ module Ci
 
         expect(config_processor.builds_for_stage_and_ref("test", "master").size).to eq(1)
         expect(config_processor.builds_for_stage_and_ref("test", "master").first).to eq({
-          except: nil,
           stage: "test",
           stage_idx: 1,
           name: :rspec,
-          only: nil,
           commands: "pwd\nrspec",
           tag_list: [],
           options: {
@@ -701,6 +720,7 @@ module Ci
           when: "on_success",
           allow_failure: false,
           environment: nil,
+          yaml_variables: []
         })
       end
 
@@ -819,17 +839,16 @@ module Ci
         it "doesn't create jobs that start with dot" do
           expect(subject.size).to eq(1)
           expect(subject.first).to eq({
-            except: nil,
             stage: "test",
             stage_idx: 1,
             name: :normal_job,
-            only: nil,
             commands: "test",
             tag_list: [],
             options: {},
             when: "on_success",
             allow_failure: false,
             environment: nil,
+            yaml_variables: []
           })
         end
       end
@@ -865,30 +884,28 @@ module Ci
         it "is correctly supported for jobs" do
           expect(subject.size).to eq(2)
           expect(subject.first).to eq({
-            except: nil,
             stage: "build",
             stage_idx: 0,
             name: :job1,
-            only: nil,
             commands: "execute-script-for-job",
             tag_list: [],
             options: {},
             when: "on_success",
             allow_failure: false,
             environment: nil,
+            yaml_variables: []
           })
           expect(subject.second).to eq({
-            except: nil,
             stage: "build",
             stage_idx: 0,
             name: :job2,
-            only: nil,
             commands: "execute-script-for-job",
             tag_list: [],
             options: {},
             when: "on_success",
             allow_failure: false,
             environment: nil,
+            yaml_variables: []
           })
         end
       end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index e8171788872..481416319dd 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -208,7 +208,7 @@ describe Ci::Build, models: true do
       end
 
       before do
-        build.update_attributes(stage: 'stage')
+        build.update_attributes(stage: 'stage', yaml_variables: yaml_variables)
       end
 
       it { is_expected.to eq(predefined_variables + yaml_variables) }
@@ -260,22 +260,6 @@ describe Ci::Build, models: true do
 
           it { is_expected.to eq(predefined_variables + predefined_trigger_variable + yaml_variables + secure_variables + trigger_variables) }
         end
-
-        context 'when job variables are defined' do
-          ##
-          # Job-level variables are defined in gitlab_ci.yml fixture
-          #
-          context 'when job variables are unique' do
-            let(:build) { create(:ci_build, name: 'staging') }
-
-            it 'includes job variables' do
-              expect(subject).to include(
-                { key: :KEY1, value: 'value1', public: true },
-                { key: :KEY2, value: 'value2', public: true }
-              )
-            end
-          end
-        end
       end
     end
   end