Add latest changes from gitlab-org/gitlab@master

4 files changed, 127 insertions, 106 deletions

diff --git a/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb
index 5fa414f5bd1..32c92724f62 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/build/associations_spec.rb
@@ -3,10 +3,16 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Pipeline::Chain::Build::Associations do
-  let(:project) { create(:project, :repository) }
-  let(:user) { create(:user, developer_projects: [project]) }
+  let_it_be_with_reload(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user, developer_projects: [project]) }
+
   let(:pipeline) { Ci::Pipeline.new }
-  let(:step) { described_class.new(pipeline, command) }
+  let(:bridge) { nil }
+
+  let(:variables_attributes) do
+    [{ key: 'first', secret_value: 'world' },
+     { key: 'second', secret_value: 'second_world' }]
+  end
 
   let(:command) do
     Gitlab::Ci::Pipeline::Chain::Command.new(
@@ -20,7 +26,26 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build::Associations do
       merge_request: nil,
       project: project,
       current_user: user,
-      bridge: bridge)
+      bridge: bridge,
+      variables_attributes: variables_attributes)
+  end
+
+  let(:step) { described_class.new(pipeline, command) }
+
+  shared_examples 'breaks the chain' do
+    it 'returns true' do
+      step.perform!
+
+      expect(step.break?).to be true
+    end
+  end
+
+  shared_examples 'does not break the chain' do
+    it 'returns false' do
+      step.perform!
+
+      expect(step.break?).to be false
+    end
   end
 
   context 'when a bridge is passed in to the pipeline creation' do
@@ -37,26 +62,83 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build::Associations do
       )
     end
 
-    it 'never breaks the chain' do
-      step.perform!
-
-      expect(step.break?).to eq(false)
-    end
+    it_behaves_like 'does not break the chain'
   end
 
   context 'when a bridge is not passed in to the pipeline creation' do
-    let(:bridge) { nil }
-
     it 'leaves the source pipeline empty' do
       step.perform!
 
       expect(pipeline.source_pipeline).to be_nil
     end
 
-    it 'never breaks the chain' do
+    it_behaves_like 'does not break the chain'
+  end
+
+  it 'sets pipeline variables' do
+    step.perform!
+
+    expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
+      .to eq variables_attributes.map(&:with_indifferent_access)
+  end
+
+  context 'when project setting restrict_user_defined_variables is enabled' do
+    before do
+      project.update!(restrict_user_defined_variables: true)
+    end
+
+    context 'when user is developer' do
+      it_behaves_like 'breaks the chain'
+
+      it 'returns an error on variables_attributes', :aggregate_failures do
+        step.perform!
+
+        expect(pipeline.errors.full_messages).to eq(['Insufficient permissions to set pipeline variables'])
+        expect(pipeline.variables).to be_empty
+      end
+
+      context 'when variables_attributes is not specified' do
+        let(:variables_attributes) { nil }
+
+        it_behaves_like 'does not break the chain'
+
+        it 'assigns empty variables' do
+          step.perform!
+
+          expect(pipeline.variables).to be_empty
+        end
+      end
+    end
+
+    context 'when user is maintainer' do
+      before do
+        project.add_maintainer(user)
+      end
+
+      it_behaves_like 'does not break the chain'
+
+      it 'assigns variables_attributes' do
+        step.perform!
+
+        expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
+          .to eq variables_attributes.map(&:with_indifferent_access)
+      end
+    end
+  end
+
+  context 'with duplicate pipeline variables' do
+    let(:variables_attributes) do
+      [{ key: 'first', secret_value: 'world' },
+       { key: 'first', secret_value: 'second_world' }]
+    end
+
+    it_behaves_like 'breaks the chain'
+
+    it 'returns an error for variables_attributes' do
       step.perform!
 
-      expect(step.break?).to eq(false)
+      expect(pipeline.errors.full_messages).to eq(['Duplicate variable name: first'])
+      expect(pipeline.variables).to be_empty
     end
   end
 end
diff --git a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb
index 7771289abe6..dca2204f544 100644
--- a/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb
+++ b/spec/lib/gitlab/ci/pipeline/chain/build_spec.rb
@@ -8,11 +8,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
 
   let(:pipeline) { Ci::Pipeline.new }
 
-  let(:variables_attributes) do
-    [{ key: 'first', secret_value: 'world' },
-     { key: 'second', secret_value: 'second_world' }]
-  end
-
   let(:command) do
     Gitlab::Ci::Pipeline::Chain::Command.new(
       source: :push,
@@ -24,100 +19,26 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Build do
       schedule: nil,
       merge_request: nil,
       project: project,
-      current_user: user,
-      variables_attributes: variables_attributes)
+      current_user: user)
   end
 
   let(:step) { described_class.new(pipeline, command) }
 
-  shared_examples 'builds pipeline' do
-    it 'builds a pipeline with the expected attributes' do
-      step.perform!
-
-      expect(pipeline.sha).not_to be_empty
-      expect(pipeline.sha).to eq project.commit.id
-      expect(pipeline.ref).to eq 'master'
-      expect(pipeline.tag).to be false
-      expect(pipeline.user).to eq user
-      expect(pipeline.project).to eq project
-    end
-  end
-
-  shared_examples 'breaks the chain' do
-    it 'returns true' do
-      step.perform!
-
-      expect(step.break?).to be true
-    end
-  end
-
-  shared_examples 'does not break the chain' do
-    it 'returns false' do
-      step.perform!
-
-      expect(step.break?).to be false
-    end
-  end
-
-  before do
-    stub_ci_pipeline_yaml_file(gitlab_ci_yaml)
-  end
-
-  it_behaves_like 'does not break the chain'
-  it_behaves_like 'builds pipeline'
-
-  it 'sets pipeline variables' do
+  it 'does not break the chain' do
     step.perform!
 
-    expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
-      .to eq variables_attributes.map(&:with_indifferent_access)
+    expect(step.break?).to be false
   end
 
-  context 'when project setting restrict_user_defined_variables is enabled' do
-    before do
-      project.update!(restrict_user_defined_variables: true)
-    end
-
-    context 'when user is developer' do
-      it_behaves_like 'breaks the chain'
-      it_behaves_like 'builds pipeline'
-
-      it 'returns an error on variables_attributes', :aggregate_failures do
-        step.perform!
-
-        expect(pipeline.errors.full_messages).to eq(['Insufficient permissions to set pipeline variables'])
-        expect(pipeline.variables).to be_empty
-      end
-
-      context 'when variables_attributes is not specified' do
-        let(:variables_attributes) { nil }
-
-        it_behaves_like 'does not break the chain'
-        it_behaves_like 'builds pipeline'
-
-        it 'assigns empty variables' do
-          step.perform!
-
-          expect(pipeline.variables).to be_empty
-        end
-      end
-    end
-
-    context 'when user is maintainer' do
-      before do
-        project.add_maintainer(user)
-      end
-
-      it_behaves_like 'does not break the chain'
-      it_behaves_like 'builds pipeline'
-
-      it 'assigns variables_attributes' do
-        step.perform!
+  it 'builds a pipeline with the expected attributes' do
+    step.perform!
 
-        expect(pipeline.variables.map { |var| var.slice(:key, :secret_value) })
-          .to eq variables_attributes.map(&:with_indifferent_access)
-      end
-    end
+    expect(pipeline.sha).not_to be_empty
+    expect(pipeline.sha).to eq project.commit.id
+    expect(pipeline.ref).to eq 'master'
+    expect(pipeline.tag).to be false
+    expect(pipeline.user).to eq user
+    expect(pipeline.project).to eq project
   end
 
   it 'returns a valid pipeline' do
diff --git a/spec/lib/gitlab/tracking/standard_context_spec.rb b/spec/lib/gitlab/tracking/standard_context_spec.rb
index a0fb6a270a5..ca7a6b6b1c3 100644
--- a/spec/lib/gitlab/tracking/standard_context_spec.rb
+++ b/spec/lib/gitlab/tracking/standard_context_spec.rb
@@ -87,8 +87,26 @@ RSpec.describe Gitlab::Tracking::StandardContext do
       end
     end
 
-    it 'does not contain any ids' do
-      expect(snowplow_context.to_json[:data].keys).not_to include(:user_id, :project_id, :namespace_id)
+    it 'does not contain user id' do
+      expect(snowplow_context.to_json[:data].keys).not_to include(:user_id)
+    end
+
+    it 'contains namespace and project ids' do
+      expect(snowplow_context.to_json[:data].keys).to include(:project_id, :namespace_id)
+    end
+
+    it 'accepts just project id as integer' do
+      expect { described_class.new(project: 1).to_context }.not_to raise_error
+    end
+
+    context 'without add_namespace_and_project_to_snowplow_tracking feature' do
+      before do
+        stub_feature_flags(add_namespace_and_project_to_snowplow_tracking: false)
+      end
+
+      it 'does not contain any ids' do
+        expect(snowplow_context.to_json[:data].keys).not_to include(:user_id, :project_id, :namespace_id)
+      end
     end
   end
 end
diff --git a/spec/lib/gitlab/tracking_spec.rb b/spec/lib/gitlab/tracking_spec.rb
index 994316f38ee..90109db6db2 100644
--- a/spec/lib/gitlab/tracking_spec.rb
+++ b/spec/lib/gitlab/tracking_spec.rb
@@ -47,7 +47,7 @@ RSpec.describe Gitlab::Tracking do
       it "delegates to #{klass} destination" do
         other_context = double(:context)
 
-        project = double(:project)
+        project = build_stubbed(:project)
         user = double(:user)
 
         expect(Gitlab::Tracking::StandardContext)