Merge branch 'master' of gitlab.com:gitlab-org/gitlab-ce into orderable-issues

11 files changed, 153 insertions, 161 deletions

diff --git a/spec/services/ci/image_for_build_service_spec.rb b/spec/services/ci/image_for_build_service_spec.rb
deleted file mode 100644
index b3e0a7b9b58..00000000000
--- a/spec/services/ci/image_for_build_service_spec.rb
+++ /dev/null
@@ -1,50 +0,0 @@
-require 'spec_helper'
-
-module Ci
-  describe ImageForBuildService, services: true do
-    let(:service) { ImageForBuildService.new }
-    let(:project) { FactoryGirl.create(:empty_project) }
-    let(:commit_sha) { '01234567890123456789' }
-    let(:pipeline) { project.ensure_pipeline('master', commit_sha) }
-    let(:build) { FactoryGirl.create(:ci_build, pipeline: pipeline) }
-
-    describe '#execute' do
-      before { build }
-
-      context 'branch name' do
-        before { allow(project).to receive(:commit).and_return(OpenStruct.new(sha: commit_sha)) }
-        before { build.run! }
-        let(:image) { service.execute(project, ref: 'master') }
-
-        it { expect(image).to be_kind_of(OpenStruct) }
-        it { expect(image.path.to_s).to include('public/ci/build-running.svg') }
-        it { expect(image.name).to eq('build-running.svg') }
-      end
-
-      context 'unknown branch name' do
-        let(:image) { service.execute(project, ref: 'feature') }
-
-        it { expect(image).to be_kind_of(OpenStruct) }
-        it { expect(image.path.to_s).to include('public/ci/build-unknown.svg') }
-        it { expect(image.name).to eq('build-unknown.svg') }
-      end
-
-      context 'commit sha' do
-        before { build.run! }
-        let(:image) { service.execute(project, sha: build.sha) }
-
-        it { expect(image).to be_kind_of(OpenStruct) }
-        it { expect(image.path.to_s).to include('public/ci/build-running.svg') }
-        it { expect(image.name).to eq('build-running.svg') }
-      end
-
-      context 'unknown commit sha' do
-        let(:image) { service.execute(project, sha: '0000000') }
-
-        it { expect(image).to be_kind_of(OpenStruct) }
-        it { expect(image.path.to_s).to include('public/ci/build-unknown.svg') }
-        it { expect(image.name).to eq('build-unknown.svg') }
-      end
-    end
-  end
-end
diff --git a/spec/services/ci/process_pipeline_service_spec.rb b/spec/services/ci/process_pipeline_service_spec.rb
index b818dfdd50c..de68fb64726 100644
--- a/spec/services/ci/process_pipeline_service_spec.rb
+++ b/spec/services/ci/process_pipeline_service_spec.rb
@@ -341,7 +341,7 @@ describe Ci::ProcessPipelineService, :services do
         expect(builds.pending.count).to eq(1)
         expect(all_builds.count).to eq(4)
 
-        # When pending build succeeds in stage test, we enqueue deploy stage.
+        # When pending merge_when_pipeline_succeeds in stage test, we enqueue deploy stage.
         #
         succeed_pending
         process_pipeline
diff --git a/spec/services/ci/register_build_service_spec.rb b/spec/services/ci/register_build_service_spec.rb
index d9f774a1095..cd7dd53025c 100644
--- a/spec/services/ci/register_build_service_spec.rb
+++ b/spec/services/ci/register_build_service_spec.rb
@@ -170,6 +170,51 @@ module Ci
         end
       end
 
+      context 'when first build is stalled' do
+        before do
+          pending_build.lock_version = 10
+        end
+
+        subject { described_class.new(specific_runner).execute }
+
+        context 'with multiple builds are in queue' do
+          let!(:other_build) { create :ci_build, pipeline: pipeline }
+
+          before do
+            allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner)
+              .and_return([pending_build, other_build])
+          end
+
+          it "receives second build from the queue" do
+            expect(subject).to be_valid
+            expect(subject.build).to eq(other_build)
+          end
+        end
+
+        context 'when single build is in queue' do
+          before do
+            allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner)
+              .and_return([pending_build])
+          end
+
+          it "does not receive any valid result" do
+            expect(subject).not_to be_valid
+          end
+        end
+
+        context 'when there is no build in queue' do
+          before do
+            allow_any_instance_of(Ci::RegisterBuildService).to receive(:builds_for_specific_runner)
+              .and_return([])
+          end
+
+          it "does not receive builds but result is valid" do
+            expect(subject).to be_valid
+            expect(subject.build).to be_nil
+          end
+        end
+      end
+
       def execute(runner)
         described_class.new(runner).execute.build
       end
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index d03f7505eac..65af4e13118 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -10,22 +10,39 @@ describe Ci::RetryBuildService, :services do
     described_class.new(project, user)
   end
 
+  CLONE_ACCESSORS = described_class::CLONE_ACCESSORS
+
+  REJECT_ACCESSORS =
+    %i[id status user token coverage trace runner artifacts_expire_at
+       artifacts_file artifacts_metadata artifacts_size created_at
+       updated_at started_at finished_at queued_at erased_by
+       erased_at].freeze
+
+  IGNORE_ACCESSORS =
+    %i[type lock_version target_url gl_project_id deploy job_id base_tags
+       commit_id deployments erased_by_id last_deployment project_id
+       runner_id tag_taggings taggings tags trigger_request_id
+       user_id].freeze
+
   shared_examples 'build duplication' do
     let(:build) do
-      create(:ci_build, :failed, :artifacts_expired, :erased, :trace,
-             :queued, :coverage, pipeline: pipeline)
+      create(:ci_build, :failed, :artifacts_expired, :erased,
+             :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
+             :teardown_environment, :triggered, :trace,
+             description: 'some build', pipeline: pipeline)
     end
 
-    describe 'clone attributes' do
-      described_class::CLONE_ATTRIBUTES.each do |attribute|
+    describe 'clone accessors' do
+      CLONE_ACCESSORS.each do |attribute|
         it "clones #{attribute} build attribute" do
+          expect(new_build.send(attribute)).to be_present
           expect(new_build.send(attribute)).to eq build.send(attribute)
         end
       end
     end
 
-    describe 'reject attributes' do
-      described_class::REJECT_ATTRIBUTES.each do |attribute|
+    describe 'reject acessors' do
+      REJECT_ACCESSORS.each do |attribute|
         it "does not clone #{attribute} build attribute" do
           expect(new_build.send(attribute)).not_to eq build.send(attribute)
         end
@@ -33,12 +50,20 @@ describe Ci::RetryBuildService, :services do
     end
 
     it 'has correct number of known attributes' do
-      attributes =
-        described_class::CLONE_ATTRIBUTES +
-        described_class::IGNORE_ATTRIBUTES +
-        described_class::REJECT_ATTRIBUTES
+      known_accessors = CLONE_ACCESSORS + REJECT_ACCESSORS + IGNORE_ACCESSORS
+
+      # :tag_list is a special case, this accessor does not exist
+      # in reflected associations, comes from `act_as_taggable` and
+      # we use it to copy tags, instead of reusing tags.
+      #
+      current_accessors =
+        Ci::Build.attribute_names.map(&:to_sym) +
+        Ci::Build.reflect_on_all_associations.map(&:name) +
+        [:tag_list]
+
+      current_accessors.uniq!
 
-      expect(build.attributes.size).to eq(attributes.size)
+      expect(known_accessors).to contain_exactly(*current_accessors)
     end
   end
 
diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
index 0ff6e8fda16..c2f205c389d 100644
--- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
+++ b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb
@@ -5,7 +5,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
   let(:project) { create(:project) }
 
   let(:mr_merge_if_green_enabled) do
-    create(:merge_request, merge_when_build_succeeds: true, merge_user: user,
+    create(:merge_request, merge_when_pipeline_succeeds: true, merge_user: user,
                            source_branch: "master", target_branch: 'feature',
                            source_project: project, target_project: project, state: "opened")
   end
@@ -36,7 +36,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
 
       it 'sets the params, merge_user, and flag' do
         expect(merge_request).to be_valid
-        expect(merge_request.merge_when_build_succeeds).to be_truthy
+        expect(merge_request.merge_when_pipeline_succeeds).to be_truthy
         expect(merge_request.merge_params).to eq commit_message: 'Awesome message'
         expect(merge_request.merge_user).to be user
       end
@@ -62,7 +62,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
       end
 
       it 'updates the merge params' do
-        expect(SystemNoteService).not_to receive(:merge_when_build_succeeds)
+        expect(SystemNoteService).not_to receive(:merge_when_pipeline_succeeds)
 
         service.execute(mr_merge_if_green_enabled)
         expect(mr_merge_if_green_enabled.merge_params).to have_key(:new_key)
@@ -82,7 +82,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
                              sha: merge_request_head, status: 'success')
       end
 
-      it "merges all merge requests with merge when build succeeds enabled" do
+      it "merges all merge requests with merge when the pipeline succeeds enabled" do
         expect(MergeWorker).to receive(:perform_async)
         service.trigger(triggering_pipeline)
       end
@@ -114,7 +114,7 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
 
     context 'when the merge request is not mergeable' do
       let(:mr_conflict) do
-        create(:merge_request, merge_when_build_succeeds: true, merge_user: user,
+        create(:merge_request, merge_when_pipeline_succeeds: true, merge_user: user,
                                source_branch: 'master', target_branch: 'feature-conflict',
                                source_project: project, target_project: project)
       end
@@ -143,8 +143,8 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
       service.cancel(mr_merge_if_green_enabled)
     end
 
-    it "resets all the merge_when_build_succeeds params" do
-      expect(mr_merge_if_green_enabled.merge_when_build_succeeds).to be_falsey
+    it "resets all the pipeline succeeds params" do
+      expect(mr_merge_if_green_enabled.merge_when_pipeline_succeeds).to be_falsey
       expect(mr_merge_if_green_enabled.merge_params).to eq({})
       expect(mr_merge_if_green_enabled.merge_user).to be nil
     end
diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb
index 983dac6efdb..ff367f54d2a 100644
--- a/spec/services/merge_requests/refresh_service_spec.rb
+++ b/spec/services/merge_requests/refresh_service_spec.rb
@@ -18,7 +18,7 @@ describe MergeRequests::RefreshService, services: true do
                               source_branch: 'master',
                               target_branch: 'feature',
                               target_project: @project,
-                              merge_when_build_succeeds: true,
+                              merge_when_pipeline_succeeds: true,
                               merge_user: @user)
 
       @fork_merge_request = create(:merge_request,
@@ -62,7 +62,7 @@ describe MergeRequests::RefreshService, services: true do
 
       it { expect(@merge_request.notes).not_to be_empty }
       it { expect(@merge_request).to be_open }
-      it { expect(@merge_request.merge_when_build_succeeds).to be_falsey }
+      it { expect(@merge_request.merge_when_pipeline_succeeds).to be_falsey }
       it { expect(@merge_request.diff_head_sha).to eq(@newrev) }
       it { expect(@fork_merge_request).to be_open }
       it { expect(@fork_merge_request.notes).to be_empty }
diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb
index 9c92a5080c6..152c6d20daa 100644
--- a/spec/services/notes/create_service_spec.rb
+++ b/spec/services/notes/create_service_spec.rb
@@ -102,47 +102,19 @@ describe Notes::CreateService, services: true do
         expect(subject.note).to eq(params[:note])
       end
     end
-  end
-
-  describe "award emoji" do
-    before do
-      project.team << [user, :master]
-    end
-
-    it "creates an award emoji" do
-      opts = {
-        note: ':smile: ',
-        noteable_type: 'Issue',
-        noteable_id: issue.id
-      }
-      note = described_class.new(project, user, opts).execute
-
-      expect(note).to be_valid
-      expect(note.name).to eq('smile')
-    end
 
-    it "creates regular note if emoji name is invalid" do
-      opts = {
-        note: ':smile: moretext:',
-        noteable_type: 'Issue',
-        noteable_id: issue.id
-      }
-      note = described_class.new(project, user, opts).execute
-
-      expect(note).to be_valid
-      expect(note.note).to eq(opts[:note])
-    end
-
-    it "normalizes the emoji name" do
-      opts = {
-        note: ':+1:',
-        noteable_type: 'Issue',
-        noteable_id: issue.id
-      }
-
-      expect_any_instance_of(TodoService).to receive(:new_award_emoji).with(issue, user)
+    describe 'note with emoji only' do
+      it 'creates regular note' do
+        opts = {
+          note: ':smile: ',
+          noteable_type: 'Issue',
+          noteable_id: issue.id
+        }
+        note = described_class.new(project, user, opts).execute
 
-      described_class.new(project, user, opts).execute
+        expect(note).to be_valid
+        expect(note.note).to eq(':smile:')
+      end
     end
   end
 end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 839250b7d84..ebbaea4e59a 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -1050,22 +1050,22 @@ describe NotificationService, services: true do
         should_not_email(@u_lazy_participant)
       end
 
-      it "notifies the merger when merge_when_build_succeeds is true" do
-        merge_request.merge_when_build_succeeds = true
+      it "notifies the merger when the pipeline succeeds is true" do
+        merge_request.merge_when_pipeline_succeeds = true
         notification.merge_mr(merge_request, @u_watcher)
 
         should_email(@u_watcher)
       end
 
-      it "does not notify the merger when merge_when_build_succeeds is false" do
-        merge_request.merge_when_build_succeeds = false
+      it "does not notify the merger when the pipeline succeeds is false" do
+        merge_request.merge_when_pipeline_succeeds = false
         notification.merge_mr(merge_request, @u_watcher)
 
         should_not_email(@u_watcher)
       end
 
-      it "notifies the merger when merge_when_build_succeeds is false but they've opted into notifications about their activity" do
-        merge_request.merge_when_build_succeeds = false
+      it "notifies the merger when the pipeline succeeds is false but they've opted into notifications about their activity" do
+        merge_request.merge_when_pipeline_succeeds = false
         @u_watcher.notified_of_own_activity = true
         notification.merge_mr(merge_request, @u_watcher)
 
diff --git a/spec/services/slash_commands/interpret_service_spec.rb b/spec/services/slash_commands/interpret_service_spec.rb
index 0b0925983eb..52e8678cb9d 100644
--- a/spec/services/slash_commands/interpret_service_spec.rb
+++ b/spec/services/slash_commands/interpret_service_spec.rb
@@ -267,6 +267,14 @@ describe SlashCommands::InterpretService, services: true do
       end
     end
 
+    shared_examples 'award command' do
+      it 'toggle award 100 emoji if content containts /award :100:' do
+        _, updates = service.execute(content, issuable)
+
+        expect(updates).to eq(emoji_award: "100")
+      end
+    end
+
     it_behaves_like 'reopen command' do
       let(:content) { '/reopen' }
       let(:issuable) { issue }
@@ -654,6 +662,37 @@ describe SlashCommands::InterpretService, services: true do
       end
     end
 
+    context '/award command' do
+      it_behaves_like 'award command' do
+        let(:content) { '/award :100:' }
+        let(:issuable) { issue }
+      end
+
+      it_behaves_like 'award command' do
+        let(:content) { '/award :100:' }
+        let(:issuable) { merge_request }
+      end
+
+      context 'ignores command with no argument' do
+        it_behaves_like 'empty command' do
+          let(:content) { '/award' }
+          let(:issuable) { issue }
+        end
+      end
+
+      context 'ignores non-existing / invalid  emojis' do
+        it_behaves_like 'empty command' do
+          let(:content) { '/award noop' }
+          let(:issuable) { issue }
+        end
+
+        it_behaves_like 'empty command' do
+          let(:content) { '/award :lorem_ipsum:' }
+          let(:issuable) { issue }
+        end
+      end
+    end
+
     context '/target_branch command' do
       let(:non_empty_project) { create(:project) }
       let(:another_merge_request) { create(:merge_request, author: developer, source_project: non_empty_project) }
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index eca5a418f2a..36a17a3bf2e 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -215,13 +215,13 @@ describe SystemNoteService, services: true do
     end
   end
 
-  describe '.merge_when_build_succeeds' do
+  describe '.merge_when_pipeline_succeeds' do
     let(:pipeline) { build(:ci_pipeline_without_jobs )}
     let(:noteable) do
       create(:merge_request, source_project: project, target_project: project)
     end
 
-    subject { described_class.merge_when_build_succeeds(noteable, project, author, noteable.diff_head_commit) }
+    subject { described_class.merge_when_pipeline_succeeds(noteable, project, author, noteable.diff_head_commit) }
 
     it_behaves_like 'a system note'
 
@@ -230,12 +230,12 @@ describe SystemNoteService, services: true do
     end
   end
 
-  describe '.cancel_merge_when_build_succeeds' do
+  describe '.cancel_merge_when_pipeline_succeeds' do
     let(:noteable) do
       create(:merge_request, source_project: project, target_project: project)
     end
 
-    subject { described_class.cancel_merge_when_build_succeeds(noteable, project, author) }
+    subject { described_class.cancel_merge_when_pipeline_succeeds(noteable, project, author) }
 
     it_behaves_like 'a system note'
 
@@ -418,45 +418,6 @@ describe SystemNoteService, services: true do
           to be_truthy
       end
     end
-
-    context 'when noteable is an Issue' do
-      let(:issue) { create(:issue, project: project) }
-
-      it 'is truthy when issue is closed' do
-        issue.close
-
-        expect(described_class.cross_reference_disallowed?(issue, project.commit)).
-          to be_truthy
-      end
-
-      it 'is falsey when issue is open' do
-        expect(described_class.cross_reference_disallowed?(issue, project.commit)).
-          to be_falsy
-      end
-    end
-
-    context 'when noteable is a Merge Request' do
-      let(:merge_request) { create(:merge_request, :simple, source_project: project) }
-
-      it 'is truthy when merge request is closed' do
-        allow(merge_request).to receive(:closed?).and_return(:true)
-
-        expect(described_class.cross_reference_disallowed?(merge_request, project.commit)).
-          to be_truthy
-      end
-
-      it 'is truthy when merge request is merged' do
-        allow(merge_request).to receive(:closed?).and_return(:true)
-
-        expect(described_class.cross_reference_disallowed?(merge_request, project.commit)).
-          to be_truthy
-      end
-
-      it 'is falsey when merge request is open' do
-        expect(described_class.cross_reference_disallowed?(merge_request, project.commit)).
-          to be_falsy
-      end
-    end
   end
 
   describe '.cross_reference_exists?' do
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 9f24cc0f3f2..fb9a8462f84 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -680,7 +680,7 @@ describe TodoService, services: true do
       end
 
       it 'creates a pending todo for merge_user' do
-        mr_unassigned.update(merge_when_build_succeeds: true, merge_user: admin)
+        mr_unassigned.update(merge_when_pipeline_succeeds: true, merge_user: admin)
         service.merge_request_build_failed(mr_unassigned)
 
         should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::BUILD_FAILED)
@@ -700,7 +700,7 @@ describe TodoService, services: true do
 
     describe '#merge_request_became_unmergeable' do
       it 'creates a pending todo for a merge_user' do
-        mr_unassigned.update(merge_when_build_succeeds: true, merge_user: admin)
+        mr_unassigned.update(merge_when_pipeline_succeeds: true, merge_user: admin)
         service.merge_request_became_unmergeable(mr_unassigned)
 
         should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::UNMERGEABLE)