Merge branch 'master' into 8-12-stable

CHANGELOG

@@ -14,6 +14,7 @@ v 8.12.0 (unreleased)
   - Add two-factor recovery endpoint to internal API !5510
   - Pass the "Remember me" value to the U2F authentication form
   - Remove vendor prefixes for linear-gradient CSS (ClemMakesApps)
+  - Move pushes_since_gc from the database to Redis
   - Add font color contrast to external label in admin area (ClemMakesApps)
   - Change logo animation to CSS (ClemMakesApps)
   - Instructions for enabling Git packfile bitmaps !6104
@@ -131,6 +132,8 @@ v 8.12.0 (unreleased)
   - Remove duplication between project builds and admin builds view !5680 (Katarzyna Kobierska Ula Budziszewska)
   - Deleting source project with existing fork link will close all related merge requests !6177 (Katarzyna Kobierska Ula Budziszeska)
   - Return 204 instead of 404 for /ci/api/v1/builds/register.json if no builds are scheduled for a runner !6225
+  - Fix Gitlab::Popen.popen thread-safety issue
+  - Add specs to removing project (Katarzyna Kobierska Ula Budziszewska)
 
 v 8.11.6 (unreleased)
   - Fix an error where we were unable to create a CommitStatus for running state

GITLAB_SHELL_VERSION

-3.4.0
+3.5.0

Gemfile

@@ -26,7 +26,7 @@ gem 'omniauth-auth0',         '~> 1.4.1'
 gem 'omniauth-azure-oauth2',  '~> 0.0.6'
 gem 'omniauth-bitbucket',     '~> 0.0.2'
 gem 'omniauth-cas3',          '~> 1.1.2'
-gem 'omniauth-facebook',      '~> 3.0.0'
+gem 'omniauth-facebook',      '~> 4.0.0'
 gem 'omniauth-github',        '~> 1.1.1'
 gem 'omniauth-gitlab',        '~> 1.0.0'
 gem 'omniauth-google-oauth2', '~> 0.4.1'
@@ -53,7 +53,7 @@ gem 'browser', '~> 2.2'
 
 # Extracting information from a git repository
 # Provide access to Gitlab::Git library
-gem 'gitlab_git', '~> 10.6.3'
+gem 'gitlab_git', '~> 10.6.6'
 
 # LDAP Auth
 # GitLab fork with several improvements to original library. For full list of changes

Gemfile.lock

@@ -279,7 +279,7 @@ GEM
       diff-lcs (~> 1.1)
       mime-types (>= 1.16, < 3)
       posix-spawn (~> 0.3)
-    gitlab_git (10.6.3)
+    gitlab_git (10.6.6)
       activesupport (~> 4.0)
       charlock_holmes (~> 0.7.3)
       github-linguist (~> 4.7.0)
@@ -333,7 +333,7 @@ GEM
       temple (~> 0.7.6)
       thor
       tilt
-    hashie (3.4.3)
+    hashie (3.4.4)
     health_check (2.1.0)
       rails (>= 4.0)
     hipchat (1.5.2)
@@ -401,7 +401,7 @@ GEM
       mime-types (>= 1.16, < 4)
     mail_room (0.8.0)
     method_source (0.8.2)
-    mime-types (2.99.2)
+    mime-types (2.99.3)
     mimemagic (0.3.0)
     mini_portile2 (2.1.0)
     minitest (5.7.0)
@@ -444,7 +444,7 @@ GEM
       addressable (~> 2.3)
       nokogiri (~> 1.6.6)
       omniauth (~> 1.2)
-    omniauth-facebook (3.0.0)
+    omniauth-facebook (4.0.0)
       omniauth-oauth2 (~> 1.2)
     omniauth-github (1.1.2)
       omniauth (~> 1.0)
@@ -866,7 +866,7 @@ DEPENDENCIES
   github-linguist (~> 4.7.0)
   github-markup (~> 1.4)
   gitlab-flowdock-git-hook (~> 1.0.1)
-  gitlab_git (~> 10.6.3)
+  gitlab_git (~> 10.6.6)
   gitlab_meta (= 7.0)
   gitlab_omniauth-ldap (~> 1.2.1)
   gollum-lib (~> 4.2)
@@ -909,7 +909,7 @@ DEPENDENCIES
   omniauth-azure-oauth2 (~> 0.0.6)
   omniauth-bitbucket (~> 0.0.2)
   omniauth-cas3 (~> 1.1.2)
-  omniauth-facebook (~> 3.0.0)
+  omniauth-facebook (~> 4.0.0)
   omniauth-github (~> 1.1.1)
   omniauth-gitlab (~> 1.0.0)
   omniauth-google-oauth2 (~> 0.4.1)

app/helpers/search_helper.rb

@@ -30,6 +30,37 @@ module SearchHelper
     "Showing #{from} - #{to} of #{count} #{scope.humanize(capitalize: false)} for \"#{term}\""
   end
 
+  def parse_search_result(result)
+    ref = nil
+    filename = nil
+    basename = nil
+    startline = 0
+
+    result.each_line.each_with_index do |line, index|
+      if line =~ /^.*:.*:\d+:/
+        ref, filename, startline = line.split(':')
+        startline = startline.to_i - index
+        extname = Regexp.escape(File.extname(filename))
+        basename = filename.sub(/#{extname}$/, '')
+        break
+      end
+    end
+
+    data = ""
+
+    result.each_line do |line|
+      data << line.sub(ref, '').sub(filename, '').sub(/^:-\d+-/, '').sub(/^::\d+:/, '')
+    end
+
+    OpenStruct.new(
+      filename: filename,
+      basename: basename,
+      ref: ref,
+      startline: startline,
+      data: data
+    )
+  end
+
   private
 
   # Autocomplete results for various settings pages

app/models/blob.rb

@@ -22,6 +22,18 @@ class Blob < SimpleDelegator
     new(blob)
   end
 
+  # Returns the data of the blob.
+  #
+  # If the blob is a text based blob the content is converted to UTF-8 and any
+  # invalid byte sequences are replaced.
+  def data
+    if binary?
+      super
+    else
+      @data ||= super.encode(Encoding::UTF_8, invalid: :replace, undef: :replace)
+    end
+  end
+
   def no_highlighting?
     size && size > 1.megabyte
   end

app/models/project.rb

@@ -1288,8 +1288,24 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def pushes_since_gc
+    Gitlab::Redis.with { |redis| redis.get(pushes_since_gc_redis_key).to_i }
+  end
+
+  def increment_pushes_since_gc
+    Gitlab::Redis.with { |redis| redis.incr(pushes_since_gc_redis_key) }
+  end
+
+  def reset_pushes_since_gc
+    Gitlab::Redis.with { |redis| redis.del(pushes_since_gc_redis_key) }
+  end
+
   private
 
+  def pushes_since_gc_redis_key
+    "projects/#{id}/pushes_since_gc"
+  end
+
   # Prevents the creation of project_feature record for every project
   def setup_project_feature
     build_project_feature unless project_feature

app/models/repository.rb

@@ -990,37 +990,6 @@ class Repository
     Gitlab::Popen.popen(args, path_to_repo).first.scrub.split(/^--$/)
   end
 
-  def parse_search_result(result)
-    ref = nil
-    filename = nil
-    basename = nil
-    startline = 0
-
-    result.each_line.each_with_index do |line, index|
-      if line =~ /^.*:.*:\d+:/
-        ref, filename, startline = line.split(':')
-        startline = startline.to_i - index
-        extname = Regexp.escape(File.extname(filename))
-        basename = filename.sub(/#{extname}$/, '')
-        break
-      end
-    end
-
-    data = ""
-
-    result.each_line do |line|
-      data << line.sub(ref, '').sub(filename, '').sub(/^:-\d+-/, '').sub(/^::\d+:/, '')
-    end
-
-    OpenStruct.new(
-      filename: filename,
-      basename: basename,
-      ref: ref,
-      startline: startline,
-      data: data
-    )
-  end
-
   def fetch_ref(source_path, source_ref, target_ref)
     args = %W(#{Gitlab.config.git.bin_path} fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
     Gitlab::Popen.popen(args, path_to_repo)

app/services/projects/housekeeping_service.rb

@@ -30,10 +30,8 @@ module Projects
     end
 
     def increment!
-      if Gitlab::ExclusiveLease.new("project_housekeeping:increment!:#{@project.id}", timeout: 60).try_obtain
       Gitlab::Metrics.measure(:increment_pushes_since_gc) do
-          update_pushes_since_gc(@project.pushes_since_gc + 1)
-        end
+        @project.increment_pushes_since_gc
       end
     end
 
@@ -43,12 +41,8 @@ module Projects
       GitGarbageCollectWorker.perform_async(@project.id)
     ensure
       Gitlab::Metrics.measure(:reset_pushes_since_gc) do
-        update_pushes_since_gc(0)
-      end
+        @project.reset_pushes_since_gc
       end
-
-    def update_pushes_since_gc(new_value)
-      @project.update_column(:pushes_since_gc, new_value)
     end
 
     def try_obtain_lease

app/views/search/results/_blob.html.haml

-- blob = @project.repository.parse_search_result(blob)
+- blob = parse_search_result(blob)
 .blob-result
   .file-holder
     .file-title

app/views/search/results/_wiki_blob.html.haml

-- wiki_blob = @project.repository.parse_search_result(wiki_blob)
+- wiki_blob = parse_search_result(wiki_blob)
 .blob-result
   .file-holder
     .file-title

db/migrate/20160725104020_merge_request_diff_remove_uniq.rb

@@ -8,14 +8,28 @@ class MergeRequestDiffRemoveUniq < ActiveRecord::Migration
   DOWNTIME = false
 
   def up
+    constraint_name = 'merge_request_diffs_merge_request_id_key'
+
+    transaction do
       if index_exists?(:merge_request_diffs, :merge_request_id)
-      remove_index :merge_request_diffs, :merge_request_id
+        remove_index(:merge_request_diffs, :merge_request_id)
+      end
+
+      # In some bizarre cases PostgreSQL might have a separate unique constraint
+      # that we'll need to drop.
+      if constraint_exists?(constraint_name) && Gitlab::Database.postgresql?
+        execute("ALTER TABLE merge_request_diffs DROP CONSTRAINT IF EXISTS #{constraint_name};")
+      end
     end
   end
 
   def down
     unless index_exists?(:merge_request_diffs, :merge_request_id)
-      add_concurrent_index :merge_request_diffs, :merge_request_id, unique: true
+      add_concurrent_index(:merge_request_diffs, :merge_request_id, unique: true)
+    end
   end
+
+  def constraint_exists?(name)
+    indexes(:merge_request_diffs).map(&:name).include?(name)
   end
 end

db/migrate/20160913162434_remove_projects_pushes_since_gc.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveProjectsPushesSinceGc < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = true
+  DOWNTIME_REASON = 'This migration removes an existing column'
+
+  disable_ddl_transaction!
+
+  def up
+    remove_column :projects, :pushes_since_gc
+  end
+
+  def down
+    add_column_with_default! :projects, :pushes_since_gc, :integer, default: 0
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160902122721) do
+ActiveRecord::Schema.define(version: 20160913162434) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -824,7 +824,6 @@ ActiveRecord::Schema.define(version: 20160902122721) do
     t.integer  "build_timeout",                      default: 3600,      null: false
     t.boolean  "pending_delete",                     default: false
     t.boolean  "public_builds",                      default: true,      null: false
-    t.integer  "pushes_since_gc",                    default: 0
     t.boolean  "last_repository_check_failed"
     t.datetime "last_repository_check_at"
     t.boolean  "container_registry_enabled"

doc/update/8.11-to-8.12.md

@@ -70,7 +70,7 @@ sudo -u git -H git checkout 8-12-stable-ee
 ```bash
 cd /home/git/gitlab-shell
 sudo -u git -H git fetch --all --tags
-sudo -u git -H git checkout v3.4.0
+sudo -u git -H git checkout v3.5.0
 ```
 
 ### 6. Update gitlab-workhorse

doc/user/project/merge_requests.md

@@ -93,6 +93,9 @@ A merge request contains all the history from a repository, plus the additional
 commits added to the branch associated with the merge request. Here's a few
 tricks to checkout a merge request locally.
 
+Please note that you can checkout a merge request locally even if the source
+project is a fork (even a private fork) of the target project.
+
 #### Checkout locally by adding a git alias
 
 Add the following alias to your `~/.gitconfig`:

lib/gitlab/popen.rb

@@ -18,18 +18,18 @@ module Gitlab
         FileUtils.mkdir_p(path)
       end
 
-      @cmd_output = ""
-      @cmd_status = 0
+      cmd_output = ""
+      cmd_status = 0
       Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr|
         yield(stdin) if block_given?
         stdin.close
 
-        @cmd_output << stdout.read
-        @cmd_output << stderr.read
-        @cmd_status = wait_thr.value.exitstatus
+        cmd_output << stdout.read
+        cmd_output << stderr.read
+        cmd_status = wait_thr.value.exitstatus
       end
 
-      [@cmd_output, @cmd_status]
+      [cmd_output, cmd_status]
     end
   end
 end

lib/gitlab/workhorse.rb

@@ -102,7 +102,7 @@ module Gitlab
 
       def secret
         @secret ||= begin
-          bytes = Base64.strict_decode64(File.read(secret_path))
+          bytes = Base64.strict_decode64(File.read(secret_path).chomp)
           raise "#{secret_path} does not contain #{SECRET_LENGTH} bytes" if bytes.length != SECRET_LENGTH
           bytes
         end

spec/features/projects_spec.rb

@@ -57,7 +57,7 @@ feature 'Project', feature: true do
 
   describe 'removal', js: true do
     let(:user)    { create(:user) }
-    let(:project) { create(:project, namespace: user.namespace) }
+    let(:project) { create(:project, namespace: user.namespace, name: 'project1') }
 
     before do
       login_with(user)
@@ -65,8 +65,12 @@ feature 'Project', feature: true do
       visit edit_namespace_project_path(project.namespace, project)
     end
 
-    it 'removes project' do
+    it 'removes a project' do
       expect { remove_with_confirm('Remove project', project.path) }.to change {Project.count}.by(-1)
+      expect(page).to have_content "Project 'project1' will be deleted."
+      expect(Project.all.count).to be_zero
+      expect(project.issues).to be_empty
+      expect(project.merge_requests).to be_empty
     end
   end
 

spec/helpers/search_helper_spec.rb

@@ -6,6 +6,38 @@ describe SearchHelper do
     str
   end
 
+  describe 'parsing result' do
+    let(:project) { create(:project) }
+    let(:repository) { project.repository }
+    let(:results) { repository.search_files('feature', 'master') }
+    let(:search_result) { results.first }
+
+    subject { helper.parse_search_result(search_result) }
+
+    it "returns a valid OpenStruct object" do
+      is_expected.to be_an OpenStruct
+      expect(subject.filename).to eq('CHANGELOG')
+      expect(subject.basename).to eq('CHANGELOG')
+      expect(subject.ref).to eq('master')
+      expect(subject.startline).to eq(186)
+      expect(subject.data.lines[2]).to eq("  - Feature: Replace teams with group membership\n")
+    end
+
+    context "when filename has extension" do
+      let(:search_result) { "master:CONTRIBUTE.md:5:- [Contribute to GitLab](#contribute-to-gitlab)\n" }
+
+      it { expect(subject.filename).to eq('CONTRIBUTE.md') }
+      it { expect(subject.basename).to eq('CONTRIBUTE') }
+    end
+
+    context "when file under directory" do
+      let(:search_result) { "master:a/b/c.md:5:a b c\n" }
+
+      it { expect(subject.filename).to eq('a/b/c.md') }
+      it { expect(subject.basename).to eq('a/b/c') }
+    end
+  end
+
   describe 'search_autocomplete_source' do
     context "with no current user" do
       before do

spec/lib/gitlab/workhorse_spec.rb

@@ -30,6 +30,11 @@ describe Gitlab::Workhorse, lib: true do
       expect(subject.encoding).to eq(Encoding::ASCII_8BIT)
     end
 
+    it 'accepts a trailing newline' do
+      open(described_class.secret_path, 'a') { |f| f.write "\n" }
+      expect(subject.length).to eq(32)
+    end
+
     it 'raises an exception if the secret file cannot be read' do
       File.delete(described_class.secret_path)
       expect { subject }.to raise_exception(Errno::ENOENT)

spec/models/blob_spec.rb

+# encoding: utf-8
 require 'rails_helper'
 
 describe Blob do
@@ -7,6 +8,25 @@ describe Blob do
     end
   end
 
+  describe '#data' do
+    context 'using a binary blob' do
+      it 'returns the data as-is' do
+        data = "\n\xFF\xB9\xC3"
+        blob = described_class.new(double(binary?: true, data: data))
+
+        expect(blob.data).to eq(data)
+      end
+    end
+
+    context 'using a text blob' do
+      it 'converts the data to UTF-8' do
+        blob = described_class.new(double(binary?: false, data: "\n\xFF\xB9\xC3"))
+
+        expect(blob.data).to eq("\n���")
+      end
+    end
+  end
+
   describe '#svg?' do
     it 'is falsey when not text' do
       git_blob = double(text?: false)

spec/models/project_spec.rb

@@ -6,6 +6,7 @@ describe Project, models: true do
     it { is_expected.to belong_to(:namespace) }
     it { is_expected.to belong_to(:creator).class_name('User') }
     it { is_expected.to have_many(:users) }
+    it { is_expected.to have_many(:services) }
     it { is_expected.to have_many(:events).dependent(:destroy) }
     it { is_expected.to have_many(:merge_requests).dependent(:destroy) }
     it { is_expected.to have_many(:issues).dependent(:destroy) }
@@ -24,6 +25,30 @@ describe Project, models: true do
     it { is_expected.to have_one(:pushover_service).dependent(:destroy) }
     it { is_expected.to have_one(:asana_service).dependent(:destroy) }
     it { is_expected.to have_one(:board).dependent(:destroy) }
+    it { is_expected.to have_one(:campfire_service).dependent(:destroy) }
+    it { is_expected.to have_one(:drone_ci_service).dependent(:destroy) }
+    it { is_expected.to have_one(:emails_on_push_service).dependent(:destroy) }
+    it { is_expected.to have_one(:builds_email_service).dependent(:destroy) }
+    it { is_expected.to have_one(:emails_on_push_service).dependent(:destroy) }
+    it { is_expected.to have_one(:irker_service).dependent(:destroy) }
+    it { is_expected.to have_one(:pivotaltracker_service).dependent(:destroy) }
+    it { is_expected.to have_one(:hipchat_service).dependent(:destroy) }
+    it { is_expected.to have_one(:flowdock_service).dependent(:destroy) }
+    it { is_expected.to have_one(:assembla_service).dependent(:destroy) }
+    it { is_expected.to have_one(:gemnasium_service).dependent(:destroy) }
+    it { is_expected.to have_one(:buildkite_service).dependent(:destroy) }
+    it { is_expected.to have_one(:bamboo_service).dependent(:destroy) }
+    it { is_expected.to have_one(:teamcity_service).dependent(:destroy) }
+    it { is_expected.to have_one(:jira_service).dependent(:destroy) }
+    it { is_expected.to have_one(:redmine_service).dependent(:destroy) }
+    it { is_expected.to have_one(:custom_issue_tracker_service).dependent(:destroy) }
+    it { is_expected.to have_one(:bugzilla_service).dependent(:destroy) }
+    it { is_expected.to have_one(:gitlab_issue_tracker_service).dependent(:destroy) }
+    it { is_expected.to have_one(:external_wiki_service).dependent(:destroy) }
+    it { is_expected.to have_one(:project_feature).dependent(:destroy) }
+    it { is_expected.to have_one(:import_data).class_name('ProjectImportData').dependent(:destroy) }
+    it { is_expected.to have_one(:last_event).class_name('Event') }
+    it { is_expected.to have_one(:forked_from_project).through(:forked_project_link) }
     it { is_expected.to have_many(:commit_statuses) }
     it { is_expected.to have_many(:pipelines) }
     it { is_expected.to have_many(:builds) }
@@ -31,9 +56,16 @@ describe Project, models: true do
     it { is_expected.to have_many(:runners) }
     it { is_expected.to have_many(:variables) }
     it { is_expected.to have_many(:triggers) }
+    it { is_expected.to have_many(:labels).dependent(:destroy) }
+    it { is_expected.to have_many(:users_star_projects).dependent(:destroy) }
     it { is_expected.to have_many(:environments).dependent(:destroy) }
     it { is_expected.to have_many(:deployments).dependent(:destroy) }
     it { is_expected.to have_many(:todos).dependent(:destroy) }
+    it { is_expected.to have_many(:releases).dependent(:destroy) }
+    it { is_expected.to have_many(:lfs_objects_projects).dependent(:destroy) }
+    it { is_expected.to have_many(:project_group_links).dependent(:destroy) }
+    it { is_expected.to have_many(:notification_settings).dependent(:destroy) }
+    it { is_expected.to have_many(:forks).through(:forked_project_links) }
 
     describe '#members & #requesters' do
       let(:project) { create(:project) }
@@ -178,7 +210,7 @@ describe Project, models: true do
       expect(project.runners_token).not_to eq('')
     end
 
-    it 'does not set an random toke if one provided' do
+    it 'does not set an random token if one provided' do
       project = FactoryGirl.create :empty_project, runners_token: 'my-token'
       expect(project.runners_token).to eq('my-token')
     end
@@ -1497,4 +1529,56 @@ describe Project, models: true do
       project.change_head(project.default_branch)
     end
   end
+
+  describe '#pushes_since_gc' do
+    let(:project) { create(:project) }
+
+    after do
+      project.reset_pushes_since_gc
+    end
+
+    context 'without any pushes' do
+      it 'returns 0' do
+        expect(project.pushes_since_gc).to eq(0)
+      end
+    end
+
+    context 'with a number of pushes' do
+      it 'returns the number of pushes' do
+        3.times { project.increment_pushes_since_gc }
+
+        expect(project.pushes_since_gc).to eq(3)
+      end
+    end
+  end
+
+  describe '#increment_pushes_since_gc' do
+    let(:project) { create(:project) }
+
+    after do
+      project.reset_pushes_since_gc
+    end
+
+    it 'increments the number of pushes since the last GC' do
+      3.times { project.increment_pushes_since_gc }
+
+      expect(project.pushes_since_gc).to eq(3)
+    end
+  end
+
+  describe '#reset_pushes_since_gc' do
+    let(:project) { create(:project) }
+
+    after do
+      project.reset_pushes_since_gc
+    end
+
+    it 'resets the number of pushes since the last GC' do
+      3.times { project.increment_pushes_since_gc }
+
+      project.reset_pushes_since_gc
+
+      expect(project.pushes_since_gc).to eq(0)
+    end
+  end
 end

spec/models/repository_spec.rb

@@ -186,32 +186,6 @@ describe Repository, models: true do
       it { is_expected.to be_an String }
       it { expect(subject.lines[2]).to eq("master:CHANGELOG:188:  - Feature: Replace teams with group membership\n") }
     end
-
-    describe 'parsing result' do
-      subject { repository.parse_search_result(search_result) }
-      let(:search_result) { results.first }
-
-      it { is_expected.to be_an OpenStruct }
-      it { expect(subject.filename).to eq('CHANGELOG') }
-      it { expect(subject.basename).to eq('CHANGELOG') }
-      it { expect(subject.ref).to eq('master') }
-      it { expect(subject.startline).to eq(186) }
-      it { expect(subject.data.lines[2]).to eq("  - Feature: Replace teams with group membership\n") }
-
-      context "when filename has extension" do
-        let(:search_result) { "master:CONTRIBUTE.md:5:- [Contribute to GitLab](#contribute-to-gitlab)\n" }
-
-        it { expect(subject.filename).to eq('CONTRIBUTE.md') }
-        it { expect(subject.basename).to eq('CONTRIBUTE') }
-      end
-
-      context "when file under directory" do
-        let(:search_result) { "master:a/b/c.md:5:a b c\n" }
-
-        it { expect(subject.filename).to eq('a/b/c.md') }
-        it { expect(subject.basename).to eq('a/b/c') }
-      end
-    end
   end
 
   describe "#changelog" do

spec/services/projects/housekeeping_service_spec.rb

@@ -4,12 +4,11 @@ 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!
+  after do
+    project.reset_pushes_since_gc
   end
 
+  describe '#execute' do
     it 'enqueues a sidekiq job' do
       expect(subject).to receive(:try_obtain_lease).and_return(true)
       expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id)
@@ -32,12 +31,12 @@ describe Projects::HousekeepingService do
       it 'does not reset pushes_since_gc' do
         expect do
           expect { subject.execute }.to raise_error(Projects::HousekeepingService::LeaseTaken)
-        end.not_to change { project.pushes_since_gc }.from(3)
+        end.not_to change { project.pushes_since_gc }
       end
     end
   end
 
-  describe 'needed?' do
+  describe '#needed?' do
     it 'when the count is low enough' do
       expect(subject.needed?).to eq(false)
     end
@@ -48,25 +47,11 @@ describe Projects::HousekeepingService do
     end
   end
 
-  describe 'increment!' do
-    let(:lease_key) { "project_housekeeping:increment!:#{project.id}" }
-
+  describe '#increment!' do
     it 'increments the pushes_since_gc counter' do
-      lease = double(:lease, try_obtain: true)
-      expect(Gitlab::ExclusiveLease).to receive(:new).with(lease_key, anything).and_return(lease)
-
       expect do
         subject.increment!
       end.to change { project.pushes_since_gc }.from(0).to(1)
     end
-
-    it 'does not increment when no lease can be obtained' do
-      lease = double(:lease, try_obtain: false)
-      expect(Gitlab::ExclusiveLease).to receive(:new).with(lease_key, anything).and_return(lease)
-
-      expect do
-        subject.increment!
-      end.not_to change { project.pushes_since_gc }
-    end
   end
 end