7 files changed, 137 insertions, 45 deletions

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 4080092405d..2dfca8bcfce 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1,6 +1,7 @@
 require 'spec_helper'
 
 describe Ci::Build, :models do
+  let(:user) { create(:user) }
   let(:project) { create(:project, :repository) }
   let(:build) { create(:ci_build, pipeline: pipeline) }
   let(:test_trace) { 'This is a test' }
@@ -207,14 +208,16 @@ describe Ci::Build, :models do
     end
 
     it 'expects to have retried builds instead the original ones' do
-      retried_rspec = Ci::Build.retry(rspec_test)
-      expect(staging.depends_on_builds.map(&:id)).to contain_exactly(build.id, retried_rspec.id, rubocop_test.id)
+      project.add_developer(user)
+
+      retried_rspec = Ci::Build.retry(rspec_test, user)
+
+      expect(staging.depends_on_builds.map(&:id))
+        .to contain_exactly(build.id, retried_rspec.id, rubocop_test.id)
     end
   end
 
   describe '#detailed_status' do
-    let(:user) { create(:user) }
-
     it 'returns a detailed status' do
       expect(build.detailed_status(user))
         .to be_a Gitlab::Ci::Status::Build::Cancelable
@@ -813,12 +816,16 @@ describe Ci::Build, :models do
 
     subject { build.other_actions }
 
+    before do
+      project.add_developer(user)
+    end
+
     it 'returns other actions' do
       is_expected.to contain_exactly(other_build)
     end
 
     context 'when build is retried' do
-      let!(:new_build) { Ci::Build.retry(build) }
+      let!(:new_build) { Ci::Build.retry(build, user) }
 
       it 'does not return any of them' do
         is_expected.not_to include(build, new_build)
@@ -826,7 +833,7 @@ describe Ci::Build, :models do
     end
 
     context 'when other build is retried' do
-      let!(:retried_build) { Ci::Build.retry(other_build) }
+      let!(:retried_build) { Ci::Build.retry(other_build, user) }
 
       it 'returns a retried build' do
         is_expected.to contain_exactly(retried_build)
@@ -857,21 +864,29 @@ describe Ci::Build, :models do
   describe '#play' do
     let(:build) { create(:ci_build, :manual, pipeline: pipeline) }
 
-    subject { build.play }
+    before do
+      project.add_developer(user)
+    end
+
+    context 'when build is manual' do
+      it 'enqueues a build' do
+        new_build = build.play(user)
 
-    it 'enqueues a build' do
-      is_expected.to be_pending
-      is_expected.to eq(build)
+        expect(new_build).to be_pending
+        expect(new_build).to eq(build)
+      end
     end
 
-    context 'for successful build' do
+    context 'when build is passed' do
       before do
         build.update(status: 'success')
       end
 
       it 'creates a new build' do
-        is_expected.to be_pending
-        is_expected.not_to eq(build)
+        new_build = build.play(user)
+
+        expect(new_build).to be_pending
+        expect(new_build).not_to eq(build)
       end
     end
   end
@@ -1246,12 +1261,9 @@ describe Ci::Build, :models do
     end
 
     context 'when build has user' do
-      let(:user) { create(:user, username: 'starter') }
       let(:user_variables) do
-        [
-          { key: 'GITLAB_USER_ID',    value: user.id.to_s, public: true },
-          { key: 'GITLAB_USER_EMAIL', value: user.email,   public: true }
-        ]
+        [ { key: 'GITLAB_USER_ID',    value: user.id.to_s, public: true },
+          { key: 'GITLAB_USER_EMAIL', value: user.email,   public: true } ]
       end
 
       before do
diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb
index 426be74cd02..10c2bfbb400 100644
--- a/spec/models/ci/pipeline_spec.rb
+++ b/spec/models/ci/pipeline_spec.rb
@@ -3,8 +3,12 @@ require 'spec_helper'
 describe Ci::Pipeline, models: true do
   include EmailHelpers
 
-  let(:project) { FactoryGirl.create :empty_project }
-  let(:pipeline) { FactoryGirl.create :ci_empty_pipeline, status: 'created', project: project }
+  let(:user) { create(:user) }
+  let(:project) { create(:empty_project) }
+
+  let(:pipeline) do
+    create(:ci_empty_pipeline, status: :created, project: project)
+  end
 
   it { is_expected.to belong_to(:project) }
   it { is_expected.to belong_to(:user) }
@@ -503,7 +507,9 @@ describe Ci::Pipeline, models: true do
   end
 
   describe '#status' do
-    let!(:build) { create(:ci_build, :created, pipeline: pipeline, name: 'test') }
+    let(:build) do
+      create(:ci_build, :created, pipeline: pipeline, name: 'test')
+    end
 
     subject { pipeline.reload.status }
 
@@ -545,13 +551,21 @@ describe Ci::Pipeline, models: true do
         build.cancel
       end
 
-      it { is_expected.to eq('canceled') }
+      context 'when build is pending' do
+        let(:build) do
+          create(:ci_build, :pending, pipeline: pipeline)
+        end
+
+        it { is_expected.to eq('canceled') }
+      end
     end
 
     context 'on failure and build retry' do
       before do
         build.drop
-        Ci::Build.retry(build)
+        project.add_developer(user)
+
+        Ci::Build.retry(build, user)
       end
 
       # We are changing a state: created > failed > running
@@ -563,8 +577,6 @@ describe Ci::Pipeline, models: true do
   end
 
   describe '#detailed_status' do
-    let(:user) { create(:user) }
-
     subject { pipeline.detailed_status(user) }
 
     context 'when pipeline is created' do
@@ -720,7 +732,7 @@ describe Ci::Pipeline, models: true do
   describe '#cancel_running' do
     let(:latest_status) { pipeline.statuses.pluck(:status) }
 
-    context 'when there is a running external job and created build' do
+    context 'when there is a running external job and a regular job' do
       before do
         create(:ci_build, :running, pipeline: pipeline)
         create(:generic_commit_status, :running, pipeline: pipeline)
@@ -733,7 +745,7 @@ describe Ci::Pipeline, models: true do
       end
     end
 
-    context 'when builds are in different stages' do
+    context 'when jobs are in different stages' do
       before do
         create(:ci_build, :running, stage_idx: 0, pipeline: pipeline)
         create(:ci_build, :running, stage_idx: 1, pipeline: pipeline)
@@ -745,17 +757,34 @@ describe Ci::Pipeline, models: true do
         expect(latest_status).to contain_exactly('canceled', 'canceled')
       end
     end
+
+    context 'when there are created builds present in the pipeline' do
+      before do
+        create(:ci_build, :running, stage_idx: 0, pipeline: pipeline)
+        create(:ci_build, :created, stage_idx: 1, pipeline: pipeline)
+
+        pipeline.cancel_running
+      end
+
+      it 'cancels created builds' do
+        expect(latest_status).to eq ['canceled', 'canceled']
+      end
+    end
   end
 
   describe '#retry_failed' do
     let(:latest_status) { pipeline.statuses.latest.pluck(:status) }
 
+    before do
+      project.add_developer(user)
+    end
+
     context 'when there is a failed build and failed external status' do
       before do
         create(:ci_build, :failed, name: 'build', pipeline: pipeline)
         create(:generic_commit_status, :failed, name: 'jenkins', pipeline: pipeline)
 
-        pipeline.retry_failed(create(:user))
+        pipeline.retry_failed(user)
       end
 
       it 'retries only build' do
@@ -768,11 +797,11 @@ describe Ci::Pipeline, models: true do
         create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline)
         create(:ci_build, :failed, name: 'jenkins', stage_idx: 1, pipeline: pipeline)
 
-        pipeline.retry_failed(create(:user))
+        pipeline.retry_failed(user)
       end
 
       it 'retries both builds' do
-        expect(latest_status).to contain_exactly('pending', 'pending')
+        expect(latest_status).to contain_exactly('pending', 'created')
       end
     end
 
@@ -781,11 +810,11 @@ describe Ci::Pipeline, models: true do
         create(:ci_build, :failed, name: 'build', stage_idx: 0, pipeline: pipeline)
         create(:ci_build, :canceled, name: 'jenkins', stage_idx: 1, pipeline: pipeline)
 
-        pipeline.retry_failed(create(:user))
+        pipeline.retry_failed(user)
       end
 
       it 'retries both builds' do
-        expect(latest_status).to contain_exactly('pending', 'pending')
+        expect(latest_status).to contain_exactly('pending', 'created')
       end
     end
   end
diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb
index 3f32248e52b..f8513ac8b1c 100644
--- a/spec/models/ci/runner_spec.rb
+++ b/spec/models/ci/runner_spec.rb
@@ -290,7 +290,7 @@ describe Ci::Runner, models: true do
       let!(:last_update) { runner.ensure_runner_queue_value }
 
       before do
-        runner.update(description: 'new runner')
+        Ci::UpdateRunnerService.new(runner).update(description: 'new runner')
       end
 
       it 'sets a new last_update value' do
@@ -318,6 +318,25 @@ describe Ci::Runner, models: true do
     end
   end
 
+  describe '#destroy' do
+    let(:runner) { create(:ci_runner) }
+
+    context 'when there is a tick in the queue' do
+      let!(:queue_key) { runner.send(:runner_queue_key) }
+
+      before do
+        runner.tick_runner_queue
+        runner.destroy
+      end
+
+      it 'cleans up the queue' do
+        Gitlab::Redis.with do |redis|
+          expect(redis.get(queue_key)).to be_nil
+        end
+      end
+    end
+  end
+
   describe '.assignable_for' do
     let(:runner) { create(:ci_runner) }
     let(:project) { create(:empty_project) }
diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb
index bf4394f7d5b..36533bdd11e 100644
--- a/spec/models/commit_status_spec.rb
+++ b/spec/models/commit_status_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-describe CommitStatus, models: true do
+describe CommitStatus, :models do
   let(:project) { create(:project, :repository) }
 
   let(:pipeline) do
@@ -127,7 +127,7 @@ describe CommitStatus, models: true do
   end
 
   describe '.latest' do
-    subject { CommitStatus.latest.order(:id) }
+    subject { described_class.latest.order(:id) }
 
     let(:statuses) do
       [create_status(name: 'aa', ref: 'bb', status: 'running'),
@@ -143,7 +143,7 @@ describe CommitStatus, models: true do
   end
 
   describe '.running_or_pending' do
-    subject { CommitStatus.running_or_pending.order(:id) }
+    subject { described_class.running_or_pending.order(:id) }
 
     let(:statuses) do
       [create_status(name: 'aa', ref: 'bb', status: 'running'),
@@ -159,7 +159,21 @@ describe CommitStatus, models: true do
   end
 
   describe '.exclude_ignored' do
-    subject { CommitStatus.exclude_ignored.order(:id) }
+    subject { described_class.after_stage(0) }
+
+    let(:statuses) do
+      [create_status(name: 'aa', stage_idx: 0),
+       create_status(name: 'cc', stage_idx: 1),
+       create_status(name: 'aa', stage_idx: 2)]
+    end
+
+    it 'returns statuses from second and third stage' do
+      is_expected.to eq(statuses.values_at(1, 2))
+    end
+  end
+
+  describe '.exclude_ignored' do
+    subject { described_class.exclude_ignored.order(:id) }
 
     let(:statuses) do
       [create_status(when: 'manual', status: 'skipped'),
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 960f29f3805..f0ed0c679d5 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -155,7 +155,7 @@ describe Environment, models: true do
   end
 
   describe '#stop_with_action!' do
-    let(:user) { create(:user) }
+    let(:user) { create(:admin) }
 
     subject { environment.stop_with_action!(user) }
 
diff --git a/spec/models/project_services/chat_message/build_message_spec.rb b/spec/models/project_services/chat_message/build_message_spec.rb
index 50ad5013df9..3bd7ec18ae0 100644
--- a/spec/models/project_services/chat_message/build_message_spec.rb
+++ b/spec/models/project_services/chat_message/build_message_spec.rb
@@ -11,21 +11,28 @@ describe ChatMessage::BuildMessage do
 
       project_name: 'project_name',
       project_url: 'http://example.gitlab.com',
+      build_id: 1,
+      build_name: build_name,
+      build_stage: stage,
 
       commit: {
         status: status,
         author_name: 'hacker',
+        author_url: 'http://example.gitlab.com/hacker',
         duration: duration,
       },
     }
   end
 
   let(:message) { build_message }
+  let(:stage) { 'test' }
+  let(:status) { 'success' }
+  let(:build_name) { 'rspec' }
+  let(:duration) { 10 }
 
   context 'build succeeded' do
     let(:status) { 'success' }
     let(:color) { 'good' }
-    let(:duration) { 10 }
     let(:message) { build_message('passed') }
 
     it 'returns a message with information about succeeded build' do
@@ -38,7 +45,6 @@ describe ChatMessage::BuildMessage do
   context 'build failed' do
     let(:status) { 'failed' }
     let(:color) { 'danger' }
-    let(:duration) { 10 }
 
     it 'returns a message with information about failed build' do
       expect(subject.pretext).to be_empty
@@ -47,11 +53,25 @@ describe ChatMessage::BuildMessage do
     end
   end
 
-  def build_message(status_text = status)
+  it 'returns a message with information on build' do
+    expect(subject.fallback).to include("on build <http://example.gitlab.com/builds/1|#{build_name}>")
+  end
+
+  it 'returns a message with stage name' do
+    expect(subject.fallback).to include("of stage #{stage}")
+  end
+
+  it 'returns a message with link to author' do
+    expect(subject.fallback).to include("by <http://example.gitlab.com/hacker|hacker>")
+  end
+
+  def build_message(status_text = status, stage_text = stage, build_text = build_name)
     "<http://example.gitlab.com|project_name>:" \
     " Commit <http://example.gitlab.com/commit/" \
     "97de212e80737a608d939f648d959671fb0a0142/builds|97de212e>" \
     " of <http://example.gitlab.com/commits/develop|develop> branch" \
-    " by hacker #{status_text} in #{duration} #{'second'.pluralize(duration)}"
+    " by <http://example.gitlab.com/hacker|hacker> #{status_text}" \
+    " on build <http://example.gitlab.com/builds/1|#{build_text}>" \
+    " of stage #{stage_text} in #{duration} #{'second'.pluralize(duration)}"
   end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 89cef7ab978..584a4facd94 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -582,18 +582,16 @@ describe User, models: true do
       it "applies defaults to user" do
         expect(user.projects_limit).to eq(Gitlab.config.gitlab.default_projects_limit)
         expect(user.can_create_group).to eq(Gitlab.config.gitlab.default_can_create_group)
-        expect(user.theme_id).to eq(Gitlab.config.gitlab.default_theme)
         expect(user.external).to be_falsey
       end
     end
 
     describe 'with default overrides' do
-      let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true, theme_id: 1) }
+      let(:user) { User.new(projects_limit: 123, can_create_group: false, can_create_team: true) }
 
       it "applies defaults to user" do
         expect(user.projects_limit).to eq(123)
         expect(user.can_create_group).to be_falsey
-        expect(user.theme_id).to eq(1)
       end
     end