Merge branch 'csslab' into login-page-css-refactorlogin-page-css-refactor

5 files changed, 136 insertions, 62 deletions

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index c27313ed88b..6e202de0db9 100644
--- a/spec/models/ci/build_spec.rb
+++ b/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,39 +1843,6 @@ describe Ci::Build do
       it { is_expected.to include(ci_config_path) }
     end
 
-    context 'returns variables in valid order' 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' } }
-      let(:build_yaml_var) { { key: 'yaml', value: 'value' } }
-
-      before do
-        allow(build).to receive(:predefined_variables) { [build_pre_var] }
-        allow(build).to receive(:yaml_variables) { [build_yaml_var] }
-
-        allow_any_instance_of(Project)
-          .to receive(:predefined_variables) { [project_pre_var] }
-
-        allow_any_instance_of(Project)
-          .to receive(:secret_variables_for)
-          .with(ref: 'master', environment: nil) do
-          [create(:ci_variable, key: 'secret', value: 'value')]
-        end
-
-        allow_any_instance_of(Ci::Pipeline)
-          .to receive(:predefined_variables) { [pipeline_pre_var] }
-      end
-
-      it do
-        is_expected.to eq(
-          [build_pre_var,
-           project_pre_var,
-           pipeline_pre_var,
-           build_yaml_var,
-           { key: 'secret', value: 'value', public: false }])
-      end
-    end
-
     context 'when using auto devops' do
       context 'and is enabled' do
         before do
@@ -1890,6 +1866,81 @@ describe Ci::Build do
         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' } }
+        let(:project_pre_var) { { key: 'project', value: 'value' } }
+        let(:pipeline_pre_var) { { key: 'pipeline', value: 'value' } }
+        let(:build_yaml_var) { { key: 'yaml', value: 'value' } }
+
+        before do
+          allow(build).to receive(:predefined_variables) { [build_pre_var] }
+          allow(build).to receive(:yaml_variables) { [build_yaml_var] }
+
+          allow_any_instance_of(Project)
+            .to receive(:predefined_variables) { [project_pre_var] }
+
+          allow_any_instance_of(Project)
+            .to receive(:secret_variables_for)
+            .with(ref: 'master', environment: nil) do
+            [create(:ci_variable, key: 'secret', value: 'value')]
+          end
+
+          allow_any_instance_of(Ci::Pipeline)
+            .to receive(:predefined_variables) { [pipeline_pre_var] }
+        end
+
+        it 'returns variables in order depending on resource hierarchy' do
+          is_expected.to eq(
+            [build_pre_var,
+             project_pre_var,
+             pipeline_pre_var,
+             build_yaml_var,
+             { key: 'secret', value: 'value', public: false }])
+        end
+      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
+
+        before do
+          create(:environment, project: build.project,
+                               name: 'staging')
+
+          build.yaml_variables = [{ key: 'YAML_VARIABLE',
+                                    value: 'var',
+                                    public: true }]
+          build.environment = 'staging'
+        end
+
+        it 'matches explicit variables ordering' do
+          received_variables = subject.map { |variable| variable.fetch(:key) }
+
+          expect(received_variables).to eq expected_variables
+        end
+      end
+    end
   end
 
   describe 'state transition: any => [:pending]' do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 14d234f6aab..86bb2fefae1 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -172,10 +172,10 @@ describe Ci::Pipeline, :mailer do
 
     it { is_expected.to be_an(Array) }
 
-    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
 
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index e626efd054d..ee142718f7e 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -204,43 +204,67 @@ describe Namespace do
         expect(gitlab_shell.exists?(project.repository_storage_path, "#{namespace.path}/#{project.path}.git")).to be_truthy
       end
 
-      context 'with subgroups' do
+      context 'with subgroups', :nested_groups do
         let(:parent) { create(:group, name: 'parent', path: 'parent') }
+        let(:new_parent) { create(:group, name: 'new_parent', path: 'new_parent') }
         let(:child) { create(:group, name: 'child', path: 'child', parent: parent) }
         let!(:project) { create(:project_empty_repo, :legacy_storage, path: 'the-project', namespace: child, skip_disk_validation: true) }
         let(:uploads_dir) { FileUploader.root }
         let(:pages_dir) { File.join(TestEnv.pages_path) }
 
+        def expect_project_directories_at(namespace_path)
+          expected_repository_path = File.join(TestEnv.repos_path, namespace_path, 'the-project.git')
+          expected_upload_path = File.join(uploads_dir, namespace_path, 'the-project')
+          expected_pages_path = File.join(pages_dir, namespace_path, 'the-project')
+
+          expect(File.directory?(expected_repository_path)).to be_truthy
+          expect(File.directory?(expected_upload_path)).to be_truthy
+          expect(File.directory?(expected_pages_path)).to be_truthy
+        end
+
         before do
+          FileUtils.mkdir_p(File.join(TestEnv.repos_path, "#{project.full_path}.git"))
           FileUtils.mkdir_p(File.join(uploads_dir, project.full_path))
           FileUtils.mkdir_p(File.join(pages_dir, project.full_path))
         end
 
         context 'renaming child' do
           it 'correctly moves the repository, uploads and pages' do
-            expected_repository_path = File.join(TestEnv.repos_path, 'parent', 'renamed', 'the-project.git')
-            expected_upload_path = File.join(uploads_dir, 'parent', 'renamed', 'the-project')
-            expected_pages_path = File.join(pages_dir, 'parent', 'renamed', 'the-project')
+            child.update!(path: 'renamed')
 
-            child.update_attributes!(path: 'renamed')
-
-            expect(File.directory?(expected_repository_path)).to be(true)
-            expect(File.directory?(expected_upload_path)).to be(true)
-            expect(File.directory?(expected_pages_path)).to be(true)
+            expect_project_directories_at('parent/renamed')
           end
         end
 
         context 'renaming parent' do
           it 'correctly moves the repository, uploads and pages' do
-            expected_repository_path = File.join(TestEnv.repos_path, 'renamed', 'child', 'the-project.git')
-            expected_upload_path = File.join(uploads_dir, 'renamed', 'child', 'the-project')
-            expected_pages_path = File.join(pages_dir, 'renamed', 'child', 'the-project')
+            parent.update!(path: 'renamed')
+
+            expect_project_directories_at('renamed/child')
+          end
+        end
+
+        context 'moving from one parent to another' do
+          it 'correctly moves the repository, uploads and pages' do
+            child.update!(parent: new_parent)
 
-            parent.update_attributes!(path: 'renamed')
+            expect_project_directories_at('new_parent/child')
+          end
+        end
+
+        context 'moving from having a parent to root' do
+          it 'correctly moves the repository, uploads and pages' do
+            child.update!(parent: nil)
+
+            expect_project_directories_at('child')
+          end
+        end
+
+        context 'moving from root to having a parent' do
+          it 'correctly moves the repository, uploads and pages' do
+            parent.update!(parent: new_parent)
 
-            expect(File.directory?(expected_repository_path)).to be(true)
-            expect(File.directory?(expected_upload_path)).to be(true)
-            expect(File.directory?(expected_pages_path)).to be(true)
+            expect_project_directories_at('new_parent/parent/child')
           end
         end
       end
@@ -525,7 +549,6 @@ describe Namespace do
       end
     end
 
-    # Note: Group transfers are not yet implemented
     context 'when a group is transferred into a root group' do
       context 'when the root group "Share with group lock" is enabled' do
         let(:root_group) { create(:group, share_with_group_lock: true) }
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index e970cd7dfdb..4cf8d861595 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1378,7 +1378,7 @@ describe Project do
 
     context 'using a regular repository' do
       it 'creates the repository' do
-        expect(shell).to receive(:add_repository)
+        expect(shell).to receive(:create_repository)
           .with(project.repository_storage, project.disk_path)
           .and_return(true)
 
@@ -1388,7 +1388,7 @@ describe Project do
       end
 
       it 'adds an error if the repository could not be created' do
-        expect(shell).to receive(:add_repository)
+        expect(shell).to receive(:create_repository)
           .with(project.repository_storage, project.disk_path)
           .and_return(false)
 
@@ -1402,7 +1402,7 @@ describe Project do
     context 'using a forked repository' do
       it 'does nothing' do
         expect(project).to receive(:forked?).and_return(true)
-        expect(shell).not_to receive(:add_repository)
+        expect(shell).not_to receive(:create_repository)
 
         project.create_repository
       end
@@ -1421,7 +1421,7 @@ describe Project do
       allow(project).to receive(:repository_exists?)
         .and_return(false)
 
-      allow(shell).to receive(:add_repository)
+      allow(shell).to receive(:create_repository)
         .with(project.repository_storage_path, project.disk_path)
         .and_return(true)
 
@@ -1445,7 +1445,7 @@ describe Project do
       allow(project).to receive(:repository_exists?)
         .and_return(false)
 
-      expect(shell).to receive(:add_repository)
+      expect(shell).to receive(:create_repository)
         .with(project.repository_storage, project.disk_path)
         .and_return(true)
 
diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb
index 8b4b5873704..d87c1ca14f0 100644
--- a/spec/models/project_wiki_spec.rb
+++ b/spec/models/project_wiki_spec.rb
@@ -74,7 +74,7 @@ describe ProjectWiki do
       # Create a fresh project which will not have a wiki
       project_wiki = described_class.new(create(:project), user)
       gitlab_shell = double(:gitlab_shell)
-      allow(gitlab_shell).to receive(:add_repository)
+      allow(gitlab_shell).to receive(:create_repository)
       allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell)
 
       expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError)