Merge branch 'master' into backstage/gb/refactor-ci-cd-variables-collections-ee

* master:
  Add changelog entry
  Use `parsePikadayDate` to parse start and end dates
  Fix pipeline build specs in EE
  Remove legacy code from EE version of pipeline class
  Remove difference between CE and EE in build test
  Improve pipeline / build variables specs / EE

Conflicts:
	spec/models/ci/build_spec.rb
	spec/models/ci/pipeline_spec.rb

ee/app/assets/javascripts/roadmap/store/roadmap_store.js

 import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
+import { parsePikadayDate } from '~/lib/utils/datefix';
 
 export default class RoadmapStore {
   constructor(groupId, timeframe) {
@@ -43,7 +44,7 @@ export default class RoadmapStore {
 
     if (rawEpic.start_date) {
       // If startDate is present
-      const startDate = new Date(rawEpic.start_date);
+      const startDate = parsePikadayDate(rawEpic.start_date);
 
       if (startDate <= firstTimeframeItem) {
         // If startDate is less than first timeframe item
@@ -69,7 +70,7 @@ export default class RoadmapStore {
     // This entire chunk can be moved into generic method
     // but we're keeping it here for the sake of simplicity.
     if (rawEpic.end_date) {
-      const endDate = new Date(rawEpic.end_date);
+      const endDate = parsePikadayDate(rawEpic.end_date);
       if (endDate >= lastTimeframeItem) {
         epicItem.endDateOutOfRange = true;
         epicItem.originalEndDate = endDate;

ee/app/models/ee/ci/pipeline.rb

@@ -12,13 +12,6 @@ module EE
         has_one :chat_data, class_name: 'Ci::PipelineChatData'
       end
 
-      def predefined_variables
-        result = super
-        result << { key: 'CI_PIPELINE_SOURCE', value: source.to_s, public: true }
-
-        result
-      end
-
       def codeclimate_artifact
         artifacts.codequality.find(&:has_codeclimate_json?)
       end

ee/changelogs/unreleased/5192-fix-epic-date-parsing.yml

+---
+title: Make Epic start and finish dates on Roadmap to be timezone neutral
+merge_request: 4964
+author:
+type: fixed

spec/models/ci/build_spec.rb

@@ -1444,6 +1444,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 },
@@ -1453,9 +1464,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
 
@@ -1854,7 +1863,48 @@ 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 'overrides YAML variable using a pipeline variable' do
+        variables = subject.reverse.uniq { |variable| variable[:key] }.reverse
+
+        expect(variables)
+          .not_to include(key: 'MYVAR', value: 'myvar', public: true)
+        expect(variables)
+          .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', public: true } }
         let(:project_pre_var) { { key: 'project', value: 'value', public: true } }
         let(:pipeline_pre_var) { { key: 'pipeline', value: 'value', public: true } }
@@ -1873,11 +1923,11 @@ describe Ci::Build do
             [create(:ci_variable, key: 'secret', value: 'value')]
           end
 
-        allow_any_instance_of(EE::Ci::Pipeline)
+          allow_any_instance_of(Ci::Pipeline)
             .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,
@@ -1887,26 +1937,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

spec/models/ci/pipeline_spec.rb

@@ -174,10 +174,10 @@ describe Ci::Pipeline, :mailer do
   describe '#predefined_variables' do
     subject { pipeline.predefined_variables }
 
-    it 'includes the defined keys' do
-      keys = subject.map { |v| v[:key] }
+    it 'includes all predefined variables in a valid order' do
+      keys = subject.map { |variable| variable.fetch(:key) }
 
-      expect(keys).to include('CI_PIPELINE_ID', 'CI_CONFIG_PATH', 'CI_PIPELINE_SOURCE')
+      expect(keys).to eq %w[CI_PIPELINE_ID CI_CONFIG_PATH CI_PIPELINE_SOURCE]
     end
   end