Improve tests for predefined variables for a build

app/models/ci/build.rb

@@ -268,7 +268,7 @@ module Ci
       variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule
       variables += persisted_environment_variables if environment
 
-      variables
+      variables.reverse.uniq { |variable| variable.fetch(:key) }.reverse
     end
 
     def features

spec/models/ci/build_spec.rb

@@ -1424,6 +1424,17 @@ describe Ci::Build do
         { key: 'CI_COMMIT_SHA', value: build.sha, public: true },
         { key: 'CI_COMMIT_REF_NAME', value: build.ref, public: true },
         { key: 'CI_COMMIT_REF_SLUG', value: build.ref_slug, public: true },
+        { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true },
+        { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false },
+        { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false },
+        { key: 'CI_BUILD_ID', value: build.id.to_s, public: true },
+        { key: 'CI_BUILD_TOKEN', value: build.token, public: false },
+        { key: 'CI_BUILD_REF', value: build.sha, public: true },
+        { key: 'CI_BUILD_BEFORE_SHA', value: build.before_sha, public: true },
+        { key: 'CI_BUILD_REF_NAME', value: build.ref, public: true },
+        { key: 'CI_BUILD_REF_SLUG', value: build.ref_slug, public: true },
+        { key: 'CI_BUILD_NAME', value: 'test', public: true },
+        { key: 'CI_BUILD_STAGE', value: 'test', public: true },
         { key: 'CI_PROJECT_ID', value: project.id.to_s, public: true },
         { key: 'CI_PROJECT_NAME', value: project.path, public: true },
         { key: 'CI_PROJECT_PATH', value: project.full_path, public: true },
@@ -1433,9 +1444,7 @@ describe Ci::Build do
         { key: 'CI_PROJECT_VISIBILITY', value: 'private', public: true },
         { key: 'CI_PIPELINE_ID', value: pipeline.id.to_s, public: true },
         { key: 'CI_CONFIG_PATH', value: pipeline.ci_yaml_file_path, public: true },
-        { key: 'CI_REGISTRY_USER', value: 'gitlab-ci-token', public: true },
-        { key: 'CI_REGISTRY_PASSWORD', value: build.token, public: false },
-        { key: 'CI_REPOSITORY_URL', value: build.repo_url, public: false }
+        { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true }
       ]
     end
 
@@ -1834,7 +1843,49 @@ describe Ci::Build do
       it { is_expected.to include(ci_config_path) }
     end
 
-    context 'returns variables in valid order' do
+    context 'when using auto devops' do
+      context 'and is enabled' do
+        before do
+          project.create_auto_devops!(enabled: true, domain: 'example.com')
+        end
+
+        it "includes AUTO_DEVOPS_DOMAIN" do
+          is_expected.to include(
+            { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true })
+        end
+      end
+
+      context 'and is disabled' do
+        before do
+          project.create_auto_devops!(enabled: false, domain: 'example.com')
+        end
+
+        it "includes AUTO_DEVOPS_DOMAIN" do
+          is_expected.not_to include(
+            { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true })
+        end
+      end
+    end
+
+    context 'when pipeline variable overrides build variable' do
+      before do
+        build.yaml_variables = [{ key: 'MYVAR', value: 'myvar', public: true }]
+        pipeline.variables.build(key: 'MYVAR', value: 'pipeline value')
+      end
+
+      it 'removes duplicates and leaves the latest occurence' do
+        expect(subject.count { |variable| variable.fetch(:key) == 'MYVAR' })
+          .to be 1
+      end
+
+      it 'overrides YAML variable using a pipeline variable' do
+        is_expected.not_to include(key: 'MYVAR', value: 'myvar', public: true)
+        is_expected.to include(key: 'MYVAR', value: 'pipeline value', public: false)
+      end
+    end
+
+    describe 'variables ordering' do
+      context 'when variables hierarchy is stubbed' do
         let(:build_pre_var) { { key: 'build', value: 'value' } }
         let(:project_pre_var) { { key: 'project', value: 'value' } }
         let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } }
@@ -1857,7 +1908,7 @@ describe Ci::Build do
             .to receive(:predefined_variables) { [pipeline_pre_var] }
         end
 
-      it do
+        it 'returns variables in order depending on resource hierarchy' do
           is_expected.to eq(
             [build_pre_var,
              project_pre_var,
@@ -1867,26 +1918,27 @@ describe Ci::Build do
         end
       end
 
-    context 'when using auto devops' do
-      context 'and is enabled' do
-        before do
-          project.create_auto_devops!(enabled: true, domain: 'example.com')
-        end
-
-        it "includes AUTO_DEVOPS_DOMAIN" do
-          is_expected.to include(
-            { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true })
-        end
+      context 'when build has environment and user-provided variables' do
+        let(:expected_variables) do
+          predefined_variables.map { |variable| variable.fetch(:key) } +
+           %w[YAML_VARIABLE CI_ENVIRONMENT_NAME CI_ENVIRONMENT_SLUG
+              CI_ENVIRONMENT_URL]
         end
 
-      context 'and is disabled' do
         before do
-          project.create_auto_devops!(enabled: false, domain: 'example.com')
+          create(:environment, project: build.project,
+                               name: 'staging')
+
+          build.yaml_variables = [{ key: 'YAML_VARIABLE',
+                                    value: 'var',
+                                    public: true }]
+          build.environment = 'staging'
         end
 
-        it "includes AUTO_DEVOPS_DOMAIN" do
-          is_expected.not_to include(
-            { key: 'AUTO_DEVOPS_DOMAIN', value: 'example.com', public: true })
+        it 'matches explicit variables ordering' do
+          received_variables = subject.map { |variable| variable.fetch(:key) }
+
+          expect(received_variables).to eq expected_variables
         end
       end
     end