Merge branch 'master' into 4009-external-users

25 files changed, 570 insertions, 148 deletions

diff --git a/spec/controllers/namespaces_controller_spec.rb b/spec/controllers/namespaces_controller_spec.rb
index d4a380cc2ee..77436958711 100644
--- a/spec/controllers/namespaces_controller_spec.rb
+++ b/spec/controllers/namespaces_controller_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe NamespacesController do
-  let!(:user) { create(:user, :with_avatar) }
+  let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
 
   describe "GET show" do
     context "when the namespace belongs to a user" do
diff --git a/spec/controllers/profiles/avatars_controller_spec.rb b/spec/controllers/profiles/avatars_controller_spec.rb
index 85dff009bcf..ad5855df0a4 100644
--- a/spec/controllers/profiles/avatars_controller_spec.rb
+++ b/spec/controllers/profiles/avatars_controller_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Profiles::AvatarsController do
-  let(:user)    { create(:user, :with_avatar) }
+  let(:user)    { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png")) }
 
   before do
     sign_in(user)
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 0d9f4b299bc..af5d043cf02 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe UploadsController do
-  let!(:user) { create(:user, :with_avatar) }
+  let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
 
   describe "GET show" do
     context "when viewing a user avatar" do
diff --git a/spec/factories/labels.rb b/spec/factories/labels.rb
index 6e70af10af3..ea2be8928d5 100644
--- a/spec/factories/labels.rb
+++ b/spec/factories/labels.rb
@@ -13,7 +13,7 @@
 
 FactoryGirl.define do
   factory :label do
-    title "Bug"
+    sequence(:title) { |n| "label#{n}" }
     color "#990000"
     project
   end
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index ca1c636fce4..a9df5fa1d3a 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -56,6 +56,10 @@ FactoryGirl.define do
       target_branch "feature"
     end
 
+    trait :merged do
+      state :merged
+    end
+
     trait :closed do
       state :closed
     end
@@ -84,6 +88,7 @@ FactoryGirl.define do
       merge_user author
     end
 
+    factory :merged_merge_request, traits: [:merged]
     factory :closed_merge_request, traits: [:closed]
     factory :reopened_merge_request, traits: [:reopened]
     factory :merge_request_with_diffs, traits: [:with_diffs]
diff --git a/spec/factories/users.rb b/spec/factories/users.rb
index 785c2a3d811..a5c60c51c5b 100644
--- a/spec/factories/users.rb
+++ b/spec/factories/users.rb
@@ -23,13 +23,6 @@ FactoryGirl.define do
       end
     end
 
-    trait :with_avatar do
-      avatar { fixture_file_upload(Rails.root.join(*%w(spec fixtures dk.png)), 'image/png') }
-      avatar_crop_x 0
-      avatar_crop_y 0
-      avatar_crop_size 256
-    end
-
     factory :omniauth_user do
       transient do
         extern_uid '123456'
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index 8013b31524f..f6c1005d265 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -77,7 +77,7 @@ describe ApplicationHelper do
     let(:avatar_file_path) { File.join(Rails.root, 'spec', 'fixtures', 'banana_sample.gif') }
 
     it 'should return an url for the avatar' do
-      user = create(:user, :with_avatar, avatar: File.open(avatar_file_path))
+      user = create(:user, avatar: File.open(avatar_file_path))
 
       expect(helper.avatar_icon(user.email).to_s).
         to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
@@ -88,7 +88,7 @@ describe ApplicationHelper do
       # Must be stubbed after the stub above, and separately
       stub_config_setting(url: Settings.send(:build_gitlab_url))
 
-      user = create(:user, :with_avatar, avatar: File.open(avatar_file_path))
+      user = create(:user, avatar: File.open(avatar_file_path))
 
       expect(helper.avatar_icon(user.email).to_s).
         to match("/gitlab/uploads/user/avatar/#{user.id}/banana_sample.gif")
@@ -102,7 +102,7 @@ describe ApplicationHelper do
 
     describe 'using a User' do
       it 'should return an URL for the avatar' do
-        user = create(:user, :with_avatar, avatar: File.open(avatar_file_path))
+        user = create(:user, avatar: File.open(avatar_file_path))
 
         expect(helper.avatar_icon(user).to_s).
           to match("/uploads/user/avatar/#{user.id}/banana_sample.gif")
diff --git a/spec/lib/banzai/filter/sanitization_filter_spec.rb b/spec/lib/banzai/filter/sanitization_filter_spec.rb
index 4a7b00c7660..27ce312b11c 100644
--- a/spec/lib/banzai/filter/sanitization_filter_spec.rb
+++ b/spec/lib/banzai/filter/sanitization_filter_spec.rb
@@ -149,10 +149,20 @@ describe Banzai::Filter::SanitizationFilter, lib: true do
         output: '<a href="java"></a>'
       },
 
+      'protocol-based JS injection: invalid URL char' => {
+        input: '<img src=java\script:alert("XSS")>',
+        output: '<img>'
+      },
+
       'protocol-based JS injection: spaces and entities' => {
         input:  '<a href=" &#14;  javascript:alert(\'XSS\');">foo</a>',
         output: '<a href="">foo</a>'
       },
+
+      'protocol whitespace' => {
+        input: '<a href=" http://example.com/"></a>',
+        output: '<a href="http://example.com/"></a>'
+      }
     }
 
     protocols.each do |name, data|
@@ -177,6 +187,16 @@ describe Banzai::Filter::SanitizationFilter, lib: true do
       expect(output.to_html).to eq '<a>XSS</a>'
     end
 
+    it 'disallows invalid URIs' do
+      expect(Addressable::URI).to receive(:parse).with('foo://example.com').
+        and_raise(Addressable::URI::InvalidURIError)
+
+      input = '<a href="foo://example.com">Foo</a>'
+      output = filter(input)
+
+      expect(output.to_html).to eq '<a>Foo</a>'
+    end
+
     it 'allows non-standard anchor schemes' do
       exp = %q{<a href="irc://irc.freenode.net/git">IRC</a>}
       act = filter(exp)
diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
index 1e98280d045..fab6412d29f 100644
--- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
+++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb
@@ -397,7 +397,7 @@ module Ci
                              services:      ["mysql"],
                              before_script: ["pwd"],
                              rspec:         {
-                               artifacts: { paths: ["logs/", "binaries/"], untracked: true },
+                               artifacts: { paths: ["logs/", "binaries/"], untracked: true, name: "custom_name" },
                                script: "rspec"
                              }
                            })
@@ -417,6 +417,7 @@ module Ci
             image: "ruby:2.1",
             services: ["mysql"],
             artifacts: {
+              name: "custom_name",
               paths: ["logs/", "binaries/"],
               untracked: true
             }
@@ -427,6 +428,73 @@ module Ci
       end
     end
 
+    describe "Dependencies" do
+      let(:config) do
+        {
+          build1: { stage: 'build', script: 'test' },
+          build2: { stage: 'build', script: 'test' },
+          test1: { stage: 'test', script: 'test', dependencies: dependencies },
+          test2: { stage: 'test', script: 'test' },
+          deploy: { stage: 'test', script: 'test' }
+        }
+      end
+
+      subject { GitlabCiYamlProcessor.new(YAML.dump(config)) }
+
+      context 'no dependencies' do
+        let(:dependencies) { }
+
+        it { expect { subject }.to_not raise_error }
+      end
+
+      context 'dependencies to builds' do
+        let(:dependencies) { [:build1, :build2] }
+
+        it { expect { subject }.to_not raise_error }
+      end
+
+      context 'undefined dependency' do
+        let(:dependencies) { [:undefined] }
+
+        it { expect { subject }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'test1 job: undefined dependency: undefined') }
+      end
+
+      context 'dependencies to deploy' do
+        let(:dependencies) { [:deploy] }
+
+        it { expect { subject }.to raise_error(GitlabCiYamlProcessor::ValidationError, 'test1 job: dependency deploy is not defined in prior stages') }
+      end
+    end
+
+    describe "Hidden jobs" do
+      let(:config) do
+        YAML.dump({
+                    '.hidden_job' => { script: 'test' },
+                    'normal_job' => { script: 'test' }
+                  })
+      end
+
+      let(:config_processor) { GitlabCiYamlProcessor.new(config) }
+
+      subject { config_processor.builds_for_stage_and_ref("test", "master") }
+
+      it "doesn't create jobs that starts with dot" do
+        expect(subject.size).to eq(1)
+        expect(subject.first).to eq({
+          except: nil,
+          stage: "test",
+          stage_idx: 1,
+          name: :normal_job,
+          only: nil,
+          commands: "\ntest",
+          tag_list: [],
+          options: {},
+          when: "on_success",
+          allow_failure: false
+        })
+      end
+    end
+
     describe "YAML Alias/Anchor" do
       it "is correctly supported for jobs" do
         config = <<EOT
@@ -629,6 +697,13 @@ EOT
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: when parameter should be on_success, on_failure or always")
       end
 
+      it "returns errors if job artifacts:name is not an a string" do
+        config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { name: 1 } } })
+        expect do
+          GitlabCiYamlProcessor.new(config)
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: artifacts:name parameter should be a string")
+      end
+
       it "returns errors if job artifacts:untracked is not an array of strings" do
         config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", artifacts: { untracked: "string" } } })
         expect do
@@ -684,6 +759,13 @@ EOT
           GitlabCiYamlProcessor.new(config)
         end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: cache:paths parameter should be an array of strings")
       end
+
+      it "returns errors if job dependencies is not an array of strings" do
+        config = YAML.dump({ types: ["build", "test"], rspec: { script: "test", dependencies: "string" } })
+        expect do
+          GitlabCiYamlProcessor.new(config)
+        end.to raise_error(GitlabCiYamlProcessor::ValidationError, "rspec job: dependencies parameter should be an array of strings")
+      end
     end
   end
 end
diff --git a/spec/mailers/emails/profile_spec.rb b/spec/mailers/emails/profile_spec.rb
index 5b575da34f3..c6758ccad39 100644
--- a/spec/mailers/emails/profile_spec.rb
+++ b/spec/mailers/emails/profile_spec.rb
@@ -11,7 +11,7 @@ describe Notify do
       let(:example_site_path) { root_path }
       let(:new_user) { create(:user, email: new_user_address, created_by_id: 1) }
       let(:token) { 'kETLwRaayvigPq_x3SNM' }
-      
+
       subject { Notify.new_user_email(new_user.id, token) }
 
       it_behaves_like 'an email sent from GitLab'
@@ -77,6 +77,10 @@ describe Notify do
       it 'includes a link to ssh keys page' do
         is_expected.to have_body_text /#{profile_keys_path}/
       end
+
+      context 'with SSH key that does not exist' do
+        it { expect { Notify.new_ssh_key_email('foo') }.not_to raise_error }
+      end
     end
 
     describe 'user added email' do
diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb
index 232a11245a6..f910424d85b 100644
--- a/spec/mailers/notify_spec.rb
+++ b/spec/mailers/notify_spec.rb
@@ -100,6 +100,34 @@ describe Notify do
           end
         end
 
+        describe 'that have been relabeled' do
+          subject { Notify.relabeled_issue_email(recipient.id, issue.id, %w[foo bar baz], current_user.id) }
+
+          it_behaves_like 'a multiple recipients email'
+          it_behaves_like 'an answer to an existing thread', 'issue'
+          it_behaves_like 'it should show Gmail Actions View Issue link'
+          it_behaves_like 'a user cannot unsubscribe through footer link'
+          it_behaves_like 'an email with a labels subscriptions link in its footer'
+
+          it 'is sent as the author' do
+            sender = subject.header[:from].addrs[0]
+            expect(sender.display_name).to eq(current_user.name)
+            expect(sender.address).to eq(gitlab_sender)
+          end
+
+          it 'has the correct subject' do
+            is_expected.to have_subject /#{issue.title} \(##{issue.iid}\)/
+          end
+
+          it 'contains the names of the added labels' do
+            is_expected.to have_body_text /foo, bar, and baz/
+          end
+
+          it 'contains a link to the issue' do
+            is_expected.to have_body_text /#{namespace_project_issue_path project.namespace, project, issue}/
+          end
+        end
+
         describe 'status changed' do
           let(:status) { 'closed' }
           subject { Notify.issue_status_changed_email(recipient.id, issue.id, status, current_user.id) }
@@ -219,6 +247,34 @@ describe Notify do
           end
         end
 
+        describe 'that have been relabeled' do
+          subject { Notify.relabeled_merge_request_email(recipient.id, merge_request.id, %w[foo bar baz], current_user.id) }
+
+          it_behaves_like 'a multiple recipients email'
+          it_behaves_like 'an answer to an existing thread', 'merge_request'
+          it_behaves_like 'it should show Gmail Actions View Merge request link'
+          it_behaves_like 'a user cannot unsubscribe through footer link'
+          it_behaves_like 'an email with a labels subscriptions link in its footer'
+
+          it 'is sent as the author' do
+            sender = subject.header[:from].addrs[0]
+            expect(sender.display_name).to eq(current_user.name)
+            expect(sender.address).to eq(gitlab_sender)
+          end
+
+          it 'has the correct subject' do
+            is_expected.to have_subject /#{merge_request.title} \(##{merge_request.iid}\)/
+          end
+
+          it 'contains the names of the added labels' do
+            is_expected.to have_body_text /foo, bar, and baz/
+          end
+
+          it 'contains a link to the merge request' do
+            is_expected.to have_body_text /#{namespace_project_merge_request_path project.namespace, project, merge_request}/
+          end
+        end
+
         describe 'status changed' do
           let(:status) { 'reopened' }
           subject { Notify.merge_request_status_email(recipient.id, merge_request.id, status, current_user.id) }
diff --git a/spec/mailers/shared/notify.rb b/spec/mailers/shared/notify.rb
index 48c851ebbd6..6019af544d3 100644
--- a/spec/mailers/shared/notify.rb
+++ b/spec/mailers/shared/notify.rb
@@ -112,6 +112,10 @@ shared_examples 'an unsubscribeable thread' do
   it { is_expected.to have_body_text /unsubscribe/ }
 end
 
-shared_examples "a user cannot unsubscribe through footer link" do
+shared_examples 'a user cannot unsubscribe through footer link' do
   it { is_expected.not_to have_body_text /unsubscribe/ }
 end
+
+shared_examples 'an email with a labels subscriptions link in its footer' do
+  it { is_expected.to have_body_text /label subscriptions/ }
+end
diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb
index e3d3d453653..b7457808040 100644
--- a/spec/models/build_spec.rb
+++ b/spec/models/build_spec.rb
@@ -9,7 +9,7 @@ describe Ci::Build, models: true do
 
   it { is_expected.to respond_to :trace_html }
 
-  describe :first_pending do
+  describe '#first_pending' do
     let(:first) { FactoryGirl.create :ci_build, commit: commit, status: 'pending', created_at: Date.yesterday }
     let(:second) { FactoryGirl.create :ci_build, commit: commit, status: 'pending' }
     before { first; second }
@@ -19,7 +19,7 @@ describe Ci::Build, models: true do
     it('returns with the first pending build') { is_expected.to eq(first) }
   end
 
-  describe :create_from do
+  describe '#create_from' do
     before do
       build.status = 'success'
       build.save
@@ -33,7 +33,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :ignored? do
+  describe '#ignored?' do
     subject { build.ignored? }
 
     context 'if build is not allowed to fail' do
@@ -69,7 +69,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :trace do
+  describe '#trace' do
     subject { build.trace_html }
 
     it { is_expected.to be_empty }
@@ -101,7 +101,7 @@ describe Ci::Build, models: true do
   #   it { is_expected.to eq(commit.project.timeout) }
   # end
 
-  describe :options do
+  describe '#options' do
     let(:options) do
       {
         image: "ruby:2.1",
@@ -122,25 +122,25 @@ describe Ci::Build, models: true do
   #   it { is_expected.to eq(project.allow_git_fetch) }
   # end
 
-  describe :project do
+  describe '#project' do
     subject { build.project }
 
     it { is_expected.to eq(commit.project) }
   end
 
-  describe :project_id do
+  describe '#project_id' do
     subject { build.project_id }
 
     it { is_expected.to eq(commit.project_id) }
   end
 
-  describe :project_name do
+  describe '#project_name' do
     subject { build.project_name }
 
     it { is_expected.to eq(project.name) }
   end
 
-  describe :extract_coverage do
+  describe '#extract_coverage' do
     context 'valid content & regex' do
       subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', '\(\d+.\d+\%\) covered') }
 
@@ -172,7 +172,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :variables do
+  describe '#variables' do
     context 'returns variables' do
       subject { build.variables }
 
@@ -242,7 +242,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :can_be_served? do
+  describe '#can_be_served?' do
     let(:runner) { FactoryGirl.create :ci_runner }
 
     before { build.project.runners << runner }
@@ -277,7 +277,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :any_runners_online? do
+  describe '#any_runners_online?' do
     subject { build.any_runners_online? }
 
     context 'when no runners' do
@@ -312,8 +312,8 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :show_warning? do
-    subject { build.show_warning? }
+  describe '#stuck?' do
+    subject { build.stuck? }
 
     %w(pending).each do |state|
       context "if commit_status.status is #{state}" do
@@ -343,35 +343,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :artifacts_download_url do
-    subject { build.artifacts_download_url }
-
-    context 'artifacts file does not exist' do
-      before { build.update_attributes(artifacts_file: nil) }
-      it { is_expected.to be_nil }
-    end
-
-    context 'artifacts file exists' do
-      let(:build) { create(:ci_build, :artifacts) }
-      it { is_expected.to_not be_nil }
-    end
-  end
-
-  describe :artifacts_browse_url do
-    subject { build.artifacts_browse_url }
-
-    it "should be nil if artifacts browser is unsupported" do
-      allow(build).to receive(:artifacts_metadata?).and_return(false)
-      is_expected.to be_nil
-    end
-
-    it 'should not be nil if artifacts browser is supported' do
-      allow(build).to receive(:artifacts_metadata?).and_return(true)
-      is_expected.to_not be_nil
-    end
-  end
-
-  describe :artifacts? do
+  describe '#artifacts?' do
     subject { build.artifacts? }
 
     context 'artifacts archive does not exist' do
@@ -386,7 +358,7 @@ describe Ci::Build, models: true do
   end
 
 
-  describe :artifacts_metadata? do
+  describe '#artifacts_metadata?' do
     subject { build.artifacts_metadata? }
     context 'artifacts metadata does not exist' do
       it { is_expected.to be_falsy }
@@ -398,7 +370,7 @@ describe Ci::Build, models: true do
     end
   end
 
-  describe :repo_url do
+  describe '#repo_url' do
     let(:build) { FactoryGirl.create :ci_build }
     let(:project) { build.project }
 
@@ -412,7 +384,7 @@ describe Ci::Build, models: true do
     it { is_expected.to include(project.web_url[7..-1]) }
   end
 
-  describe :depends_on_builds do
+  describe '#depends_on_builds' do
     let!(:build) { FactoryGirl.create :ci_build, commit: commit, name: 'build', stage_idx: 0, stage: 'build' }
     let!(:rspec_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rspec', stage_idx: 1, stage: 'test' }
     let!(:rubocop_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rubocop', stage_idx: 1, stage: 'test' }
@@ -444,7 +416,7 @@ describe Ci::Build, models: true do
                        created_at: created_at)
   end
 
-  describe :merge_request do
+  describe '#merge_request' do
     context 'when a MR has a reference to the commit' do
       before do
         @merge_request = create_mr(build, commit, factory: :merge_request)
diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb
index 4dc309a4255..412842337ba 100644
--- a/spec/models/ci/commit_spec.rb
+++ b/spec/models/ci/commit_spec.rb
@@ -32,50 +32,6 @@ describe Ci::Commit, models: true do
   it { is_expected.to respond_to :git_author_email }
   it { is_expected.to respond_to :short_sha }
 
-  describe :ordered do
-    let(:project) { FactoryGirl.create :empty_project }
-
-    it 'returns ordered list of commits' do
-      commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project
-      commit2 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project
-      expect(project.ci_commits.ordered).to eq([commit2, commit1])
-    end
-
-    it 'returns commits ordered by committed_at and id, with nulls last' do
-      commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project
-      commit2 = FactoryGirl.create :ci_commit, committed_at: nil, project: project
-      commit3 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project
-      commit4 = FactoryGirl.create :ci_commit, committed_at: nil, project: project
-      expect(project.ci_commits.ordered).to eq([commit2, commit4, commit3, commit1])
-    end
-  end
-
-  describe :last_build do
-    subject { commit.last_build }
-    before do
-      @first = FactoryGirl.create :ci_build, commit: commit, created_at: Date.yesterday
-      @second = FactoryGirl.create :ci_build, commit: commit
-    end
-
-    it { is_expected.to be_a(Ci::Build) }
-    it('returns with the most recently created build') { is_expected.to eq(@second) }
-  end
-
-  describe :retry do
-    before do
-      @first = FactoryGirl.create :ci_build, commit: commit, created_at: Date.yesterday
-      @second = FactoryGirl.create :ci_build, commit: commit
-    end
-
-    it "creates only a new build" do
-      expect(commit.builds.count(:all)).to eq 2
-      expect(commit.statuses.count(:all)).to eq 2
-      commit.retry
-      expect(commit.builds.count(:all)).to eq 3
-      expect(commit.statuses.count(:all)).to eq 3
-    end
-  end
-
   describe :valid_commit_sha do
     context 'commit.sha can not start with 00000000' do
       before do
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index aff384c2949..be29b6d66ff 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -113,6 +113,48 @@ describe Issue, "Issuable" do
     end
   end
 
+  describe '#subscribed?' do
+    context 'user is not a participant in the issue' do
+      before { allow(issue).to receive(:participants).with(user).and_return([]) }
+
+      it 'returns false when no subcription exists' do
+        expect(issue.subscribed?(user)).to be_falsey
+      end
+
+      it 'returns true when a subcription exists and subscribed is true' do
+        issue.subscriptions.create(user: user, subscribed: true)
+
+        expect(issue.subscribed?(user)).to be_truthy
+      end
+
+      it 'returns false when a subcription exists and subscribed is false' do
+        issue.subscriptions.create(user: user, subscribed: false)
+
+        expect(issue.subscribed?(user)).to be_falsey
+      end
+    end
+
+    context 'user is a participant in the issue' do
+      before { allow(issue).to receive(:participants).with(user).and_return([user]) }
+
+      it 'returns false when no subcription exists' do
+        expect(issue.subscribed?(user)).to be_truthy
+      end
+
+      it 'returns true when a subcription exists and subscribed is true' do
+        issue.subscriptions.create(user: user, subscribed: true)
+
+        expect(issue.subscribed?(user)).to be_truthy
+      end
+
+      it 'returns false when a subcription exists and subscribed is false' do
+        issue.subscriptions.create(user: user, subscribed: false)
+
+        expect(issue.subscribed?(user)).to be_falsey
+      end
+    end
+  end
+
   describe "#to_hook_data" do
     let(:data) { issue.to_hook_data(user) }
     let(:project) { issue.project }
diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb
new file mode 100644
index 00000000000..e31fdb0bffb
--- /dev/null
+++ b/spec/models/concerns/subscribable_spec.rb
@@ -0,0 +1,57 @@
+require 'spec_helper'
+
+describe Subscribable, 'Subscribable' do
+  let(:resource) { create(:issue) }
+  let(:user) { create(:user) }
+
+  describe '#subscribed?' do
+    it 'returns false when no subcription exists' do
+      expect(resource.subscribed?(user)).to be_falsey
+    end
+
+    it 'returns true when a subcription exists and subscribed is true' do
+      resource.subscriptions.create(user: user, subscribed: true)
+
+      expect(resource.subscribed?(user)).to be_truthy
+    end
+
+    it 'returns false when a subcription exists and subscribed is false' do
+      resource.subscriptions.create(user: user, subscribed: false)
+
+      expect(resource.subscribed?(user)).to be_falsey
+    end
+  end
+  describe '#subscribers' do
+    it 'returns [] when no subcribers exists' do
+      expect(resource.subscribers).to be_empty
+    end
+
+    it 'returns the subscribed users' do
+      resource.subscriptions.create(user: user, subscribed: true)
+      resource.subscriptions.create(user: create(:user), subscribed: false)
+
+      expect(resource.subscribers).to eq [user]
+    end
+  end
+
+  describe '#toggle_subscription' do
+    it 'toggles the current subscription state for the given user' do
+      expect(resource.subscribed?(user)).to be_falsey
+
+      resource.toggle_subscription(user)
+
+      expect(resource.subscribed?(user)).to be_truthy
+    end
+  end
+
+  describe '#unsubscribe' do
+    it 'unsubscribes the given current user' do
+      resource.subscriptions.create(user: user, subscribed: true)
+      expect(resource.subscribed?(user)).to be_truthy
+
+      resource.unsubscribe(user)
+
+      expect(resource.subscribed?(user)).to be_falsey
+    end
+  end
+end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 59c5ffa6b9c..b8b9a455b83 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -634,6 +634,12 @@ describe Project, models: true do
     it 'returns projects with a matching namespace name regardless of the casing' do
       expect(described_class.search(project.namespace.name.upcase)).to eq([project])
     end
+
+    it 'returns projects when eager loading namespaces' do
+      relation = described_class.all.includes(:namespace)
+
+      expect(relation.search(project.namespace.name)).to eq([project])
+    end
   end
 
   describe '#rename_repo' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index ca0c84c8632..0ab7fd88ce6 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -174,32 +174,6 @@ describe User, models: true do
         end
       end
     end
-
-    describe 'avatar' do
-      it 'only validates when avatar is present and changed' do
-        user = build(:user, :with_avatar)
-
-        user.avatar_crop_x    = nil
-        user.avatar_crop_y    = nil
-        user.avatar_crop_size = nil
-
-        expect(user).not_to be_valid
-        expect(user.errors.keys).
-          to match_array %i(avatar_crop_x avatar_crop_y avatar_crop_size)
-      end
-
-      it 'does not validate when avatar has not changed' do
-        user = create(:user, :with_avatar)
-
-        expect { user.avatar_crop_x = nil }.not_to change(user, :valid?)
-      end
-
-      it 'does not validate when avatar is not present' do
-        user = create(:user)
-
-        expect { user.avatar_crop_y = nil }.not_to change(user, :valid?)
-      end
-    end
   end
 
   describe "Respond to" do
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index a7e2e1b1792..145bc937560 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -401,6 +401,45 @@ describe GitPushService, services: true do
     end
   end
 
+  describe "housekeeping" do
+    let(:housekeeping) { Projects::HousekeepingService.new(project) }
+
+    before do
+      allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping)
+    end
+
+    it 'does not perform housekeeping when not needed' do
+      expect(housekeeping).not_to receive(:execute)
+
+      execute_service(project, user, @oldrev, @newrev, @ref)
+    end
+
+    context 'when housekeeping is needed' do
+      before do
+        allow(housekeeping).to receive(:needed?).and_return(true)
+      end
+
+      it 'performs housekeeping' do
+        expect(housekeeping).to receive(:execute)
+
+        execute_service(project, user, @oldrev, @newrev, @ref)
+      end
+
+      it 'does not raise an exception' do
+        allow(housekeeping).to receive(:try_obtain_lease).and_return(false)
+
+        execute_service(project, user, @oldrev, @newrev, @ref)
+      end
+    end
+
+
+    it 'increments the push counter' do
+      expect(housekeeping).to receive(:increment!)
+
+      execute_service(project, user, @oldrev, @newrev, @ref)
+    end
+  end
+
   def execute_service(project, user, oldrev, newrev, ref)
     service = described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref )
     service.execute
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index e579e49dfa7..4ffe753fef5 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -6,6 +6,7 @@ describe Issues::UpdateService, services: true do
   let(:user3) { create(:user) }
   let(:issue) { create(:issue, title: 'Old title', assignee_id: user3.id) }
   let(:label) { create(:label) }
+  let(:label2) { create(:label) }
   let(:project) { issue.project }
 
   before do
@@ -48,7 +49,7 @@ describe Issues::UpdateService, services: true do
       it { expect(@issue.assignee).to eq(user2) }
       it { expect(@issue).to be_closed }
       it { expect(@issue.labels.count).to eq(1) }
-      it { expect(@issue.labels.first.title).to eq('Bug') }
+      it { expect(@issue.labels.first.title).to eq(label.name) }
 
       it 'should send email to user2 about assign of new issue and email to user3 about issue unassignment' do
         deliveries = ActionMailer::Base.deliveries
@@ -148,6 +149,48 @@ describe Issues::UpdateService, services: true do
       end
     end
 
+    context 'when the issue is relabeled' do
+      let!(:non_subscriber) { create(:user) }
+      let!(:subscriber) { create(:user).tap { |u| label.toggle_subscription(u) } }
+
+      it 'sends notifications for subscribers of newly added labels' do
+        opts = { label_ids: [label.id] }
+
+        perform_enqueued_jobs do
+          @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
+        end
+
+        should_email(subscriber)
+        should_not_email(non_subscriber)
+      end
+
+      context 'when issue has the `label` label' do
+        before { issue.labels << label }
+
+        it 'does not send notifications for existing labels' do
+          opts = { label_ids: [label.id, label2.id] }
+
+          perform_enqueued_jobs do
+            @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
+          end
+
+          should_not_email(subscriber)
+          should_not_email(non_subscriber)
+        end
+
+        it 'does not send notifications for removed labels' do
+          opts = { label_ids: [label2.id] }
+
+          perform_enqueued_jobs do
+            @issue = Issues::UpdateService.new(project, user, opts).execute(issue)
+          end
+
+          should_not_email(subscriber)
+          should_not_email(non_subscriber)
+        end
+      end
+    end
+
     context 'when Issue has tasks' do
       before { update_issue({ description: "- [ ] Task 1\n- [ ] Task 2" }) }
 
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 99703c7a8ec..cb8cff2fa8c 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -7,6 +7,7 @@ describe MergeRequests::UpdateService, services: true do
   let(:merge_request) { create(:merge_request, :simple, title: 'Old title', assignee_id: user3.id) }
   let(:project) { merge_request.project }
   let(:label) { create(:label) }
+  let(:label2) { create(:label) }
 
   before do
     project.team << [user, :master]
@@ -53,7 +54,7 @@ describe MergeRequests::UpdateService, services: true do
       it { expect(@merge_request.assignee).to eq(user2) }
       it { expect(@merge_request).to be_closed }
       it { expect(@merge_request.labels.count).to eq(1) }
-      it { expect(@merge_request.labels.first.title).to eq('Bug') }
+      it { expect(@merge_request.labels.first.title).to eq(label.name) }
       it { expect(@merge_request.target_branch).to eq('target') }
 
       it 'should execute hooks with update action' do
@@ -176,6 +177,48 @@ describe MergeRequests::UpdateService, services: true do
       end
     end
 
+    context 'when the issue is relabeled' do
+      let!(:non_subscriber) { create(:user) }
+      let!(:subscriber) { create(:user).tap { |u| label.toggle_subscription(u) } }
+
+      it 'sends notifications for subscribers of newly added labels' do
+        opts = { label_ids: [label.id] }
+
+        perform_enqueued_jobs do
+          @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
+        end
+
+        should_email(subscriber)
+        should_not_email(non_subscriber)
+      end
+
+      context 'when issue has the `label` label' do
+        before { merge_request.labels << label }
+
+        it 'does not send notifications for existing labels' do
+          opts = { label_ids: [label.id, label2.id] }
+
+          perform_enqueued_jobs do
+            @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
+          end
+
+          should_not_email(subscriber)
+          should_not_email(non_subscriber)
+        end
+
+        it 'does not send notifications for removed labels' do
+          opts = { label_ids: [label2.id] }
+
+          perform_enqueued_jobs do
+            @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request)
+          end
+
+          should_not_email(subscriber)
+          should_not_email(non_subscriber)
+        end
+      end
+    end
+
     context 'when MergeRequest has tasks' do
       before { update_merge_request({ description: "- [ ] Task 1\n- [ ] Task 2" }) }
 
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index 2d0b5df4224..b5407397c1d 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -224,6 +224,15 @@ describe NotificationService, services: true do
 
         should_not_email(issue.assignee)
       end
+
+      it "emails subscribers of the issue's labels" do
+        subscriber = create(:user)
+        label = create(:label, issues: [issue])
+        label.toggle_subscription(subscriber)
+        notification.new_issue(issue, @u_disabled)
+
+        should_email(subscriber)
+      end
     end
 
     describe :reassigned_issue do
@@ -296,6 +305,35 @@ describe NotificationService, services: true do
       end
     end
 
+    describe '#relabeled_issue' do
+      let(:label) { create(:label, issues: [issue]) }
+      let(:label2) { create(:label) }
+      let!(:subscriber_to_label) { create(:user).tap { |u| label.toggle_subscription(u) } }
+      let!(:subscriber_to_label2) { create(:user).tap { |u| label2.toggle_subscription(u) } }
+
+      it "emails subscribers of the issue's added labels only" do
+        notification.relabeled_issue(issue, [label2], @u_disabled)
+
+        should_not_email(subscriber_to_label)
+        should_email(subscriber_to_label2)
+      end
+
+      it "doesn't send email to anyone but subscribers of the given labels" do
+        notification.relabeled_issue(issue, [label2], @u_disabled)
+
+        should_not_email(issue.assignee)
+        should_not_email(issue.author)
+        should_not_email(@u_watcher)
+        should_not_email(@u_participant_mentioned)
+        should_not_email(@subscriber)
+        should_not_email(@watcher_and_subscriber)
+        should_not_email(@unsubscriber)
+        should_not_email(@u_participating)
+        should_not_email(subscriber_to_label)
+        should_email(subscriber_to_label2)
+      end
+    end
+
     describe :close_issue do
       it 'should sent email to issue assignee and issue author' do
         notification.close_issue(issue, @u_disabled)
@@ -349,6 +387,15 @@ describe NotificationService, services: true do
         should_not_email(@u_participating)
         should_not_email(@u_disabled)
       end
+
+      it "emails subscribers of the merge request's labels" do
+        subscriber = create(:user)
+        label = create(:label, merge_requests: [merge_request])
+        label.toggle_subscription(subscriber)
+        notification.new_merge_request(merge_request, @u_disabled)
+
+        should_email(subscriber)
+      end
     end
 
     describe :reassigned_merge_request do
@@ -366,6 +413,35 @@ describe NotificationService, services: true do
       end
     end
 
+    describe :relabel_merge_request do
+      let(:label) { create(:label, merge_requests: [merge_request]) }
+      let(:label2) { create(:label) }
+      let!(:subscriber_to_label) { create(:user).tap { |u| label.toggle_subscription(u) } }
+      let!(:subscriber_to_label2) { create(:user).tap { |u| label2.toggle_subscription(u) } }
+
+      it "emails subscribers of the merge request's added labels only" do
+        notification.relabeled_merge_request(merge_request, [label2], @u_disabled)
+
+        should_not_email(subscriber_to_label)
+        should_email(subscriber_to_label2)
+      end
+
+      it "doesn't send email to anyone but subscribers of the given labels" do
+        notification.relabeled_merge_request(merge_request, [label2], @u_disabled)
+
+        should_not_email(merge_request.assignee)
+        should_not_email(merge_request.author)
+        should_not_email(@u_watcher)
+        should_not_email(@u_participant_mentioned)
+        should_not_email(@subscriber)
+        should_not_email(@watcher_and_subscriber)
+        should_not_email(@unsubscriber)
+        should_not_email(@u_participating)
+        should_not_email(subscriber_to_label)
+        should_email(subscriber_to_label2)
+      end
+    end
+
     describe :closed_merge_request do
       it do
         notification.close_mr(merge_request, @u_disabled)
@@ -467,16 +543,4 @@ describe NotificationService, services: true do
     # Make the watcher a subscriber to detect dupes
     issuable.subscriptions.create(user: @watcher_and_subscriber, subscribed: true)
   end
-
-  def sent_to_user?(user)
-    ActionMailer::Base.deliveries.map(&:to).flatten.count(user.email) == 1
-  end
-
-  def should_email(user)
-    expect(sent_to_user?(user)).to be_truthy
-  end
-
-  def should_not_email(user)
-    expect(sent_to_user?(user)).to be_falsey
-  end
 end
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
new file mode 100644
index 00000000000..93bf1b81fbe
--- /dev/null
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -0,0 +1,48 @@
+require 'spec_helper'
+
+describe Projects::HousekeepingService do
+  subject { Projects::HousekeepingService.new(project) }
+  let(:project) { create :project }
+
+  describe 'execute' do
+    before do
+      project.pushes_since_gc = 3
+      project.save!
+    end
+
+    it 'enqueues a sidekiq job' do
+      expect(subject).to receive(:try_obtain_lease).and_return(true)
+      expect(GitlabShellWorker).to receive(:perform_async).with(:gc, project.path_with_namespace)
+
+      subject.execute
+      expect(project.pushes_since_gc).to eq(0)
+    end
+
+    it 'does not enqueue a job when no lease can be obtained' do
+      expect(subject).to receive(:try_obtain_lease).and_return(false)
+      expect(GitlabShellWorker).not_to receive(:perform_async)
+
+      expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
+      expect(project.pushes_since_gc).to eq(0)
+    end
+  end
+
+  describe 'needed?' do
+    it 'when the count is low enough' do
+      expect(subject.needed?).to eq(false)
+    end
+
+    it 'when the count is high enough' do
+      allow(project).to receive(:pushes_since_gc).and_return(10)
+      expect(subject.needed?).to eq(true)
+    end
+  end
+
+  describe 'increment!' do
+    it 'increments the pushes_since_gc counter' do
+      expect(project.pushes_since_gc).to eq(0)
+      subject.increment!
+      expect(project.pushes_since_gc).to eq(1)
+    end
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 7d939ca7509..596d607f2a1 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -32,6 +32,7 @@ RSpec.configure do |config|
   config.include LoginHelpers,        type: :feature
   config.include LoginHelpers,        type: :request
   config.include StubConfiguration
+  config.include EmailHelpers
   config.include RelativeUrl,         type: feature
   config.include TestEnv
   config.include ActiveJob::TestHelper
diff --git a/spec/support/email_helpers.rb b/spec/support/email_helpers.rb
new file mode 100644
index 00000000000..a85ab22ce36
--- /dev/null
+++ b/spec/support/email_helpers.rb
@@ -0,0 +1,13 @@
+module EmailHelpers
+  def sent_to_user?(user)
+    ActionMailer::Base.deliveries.map(&:to).flatten.count(user.email) == 1
+  end
+
+  def should_email(user)
+    expect(sent_to_user?(user)).to be_truthy
+  end
+
+  def should_not_email(user)
+    expect(sent_to_user?(user)).to be_falsey
+  end
+end