Merge remote-tracking branch 'upstream/master' into pipeline-notifications

* upstream/master: (70 commits)
  Fix routing spec for group controller
  Add small improvements to constrainers and specs
  Faster search
  Fix broken commits search
  Changed helper method to check for none on params Moved if statements around in view
  API: Return 400 when creating a systemhook fails
  Update non-exist group spinach test to match routing
  Bump omniauth-gitlab to 1.0.2 to fix incompatibility with omniauth-oauth2
  Replace trigger with the new ID of the docs project
  Refactor method name
  17492 Update link color for more accessible contrast
  Fixed todos empty state when filtering
  Refactor namespace regex
  implements reset incoming email token on issues modal and account page, reactivates all tests and writes more tests for it
  Use separate email-friendly token for incoming email and let incoming email token be reset
  Use the Gitlab Workhorse HTTP header in the admin dashboard
  Refactor project routing
  Fix 404 when visit /projects page
  Rewritten spinach git_blame tests to rspec feature tests
  Add tests for project#index routing
  ...

3 files changed, 75 insertions, 18 deletions

diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 45bc44ba172..cea7e6429f9 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -302,6 +302,9 @@ describe GitPushService, services: true do
         author_email: commit_author.email
       )
 
+      allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+        and_return(commit)
+
       allow(project.repository).to receive(:commits_between).and_return([commit])
     end
 
@@ -357,6 +360,9 @@ describe GitPushService, services: true do
         committed_date: commit_time
       )
 
+      allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+        and_return(commit)
+
       allow(project.repository).to receive(:commits_between).and_return([commit])
     end
 
@@ -393,6 +399,9 @@ describe GitPushService, services: true do
       allow(project.repository).to receive(:commits_between).
         and_return([closing_commit])
 
+      allow_any_instance_of(ProcessCommitWorker).to receive(:find_commit).
+        and_return(closing_commit)
+
       project.team << [commit_author, :master]
     end
 
@@ -538,9 +547,16 @@ describe GitPushService, services: true do
     let(:housekeeping) { Projects::HousekeepingService.new(project) }
 
     before do
+      # Flush any raw Redis data stored by the housekeeping code.
+      Gitlab::Redis.with { |conn| conn.flushall }
+
       allow(Projects::HousekeepingService).to receive(:new).and_return(housekeeping)
     end
 
+    after do
+      Gitlab::Redis.with { |conn| conn.flushall }
+    end
+
     it 'does not perform housekeeping when not needed' do
       expect(housekeeping).not_to receive(:execute)
 
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 5dfb33f4b28..4465f22a001 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -15,10 +15,39 @@ describe Issues::CloseService, services: true do
   end
 
   describe '#execute' do
+    let(:service) { described_class.new(project, user) }
+
+    it 'checks if the user is authorized to update the issue' do
+      expect(service).to receive(:can?).with(user, :update_issue, issue).
+        and_call_original
+
+      service.execute(issue)
+    end
+
+    it 'does not close the issue when the user is not authorized to do so' do
+      allow(service).to receive(:can?).with(user, :update_issue, issue).
+        and_return(false)
+
+      expect(service).not_to receive(:close_issue)
+      expect(service.execute(issue)).to eq(issue)
+    end
+
+    it 'closes the issue when the user is authorized to do so' do
+      allow(service).to receive(:can?).with(user, :update_issue, issue).
+        and_return(true)
+
+      expect(service).to receive(:close_issue).
+        with(issue, commit: nil, notifications: true, system_note: true)
+
+      service.execute(issue)
+    end
+  end
+
+  describe '#close_issue' do
     context "valid params" do
       before do
         perform_enqueued_jobs do
-          described_class.new(project, user).execute(issue)
+          described_class.new(project, user).close_issue(issue)
         end
       end
 
@@ -41,24 +70,12 @@ describe Issues::CloseService, services: true do
       end
     end
 
-    context 'current user is not authorized to close issue' do
-      before do
-        perform_enqueued_jobs do
-          described_class.new(project, guest).execute(issue)
-        end
-      end
-
-      it 'does not close the issue' do
-        expect(issue).to be_open
-      end
-    end
-
     context 'when issue is not confidential' do
       it 'executes issue hooks' do
         expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks)
         expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks)
 
-        described_class.new(project, user).execute(issue)
+        described_class.new(project, user).close_issue(issue)
       end
     end
 
@@ -69,14 +86,14 @@ describe Issues::CloseService, services: true do
         expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks)
         expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks)
 
-        described_class.new(project, user).execute(issue)
+        described_class.new(project, user).close_issue(issue)
       end
     end
 
     context 'external issue tracker' do
       before do
         allow(project).to receive(:default_issues_tracker?).and_return(false)
-        described_class.new(project, user).execute(issue)
+        described_class.new(project, user).close_issue(issue)
       end
 
       it { expect(issue).to be_valid }
diff --git a/spec/services/projects/housekeeping_service_spec.rb b/spec/services/projects/housekeeping_service_spec.rb
index cf90b33dfb4..57a5aa5cedc 100644
--- a/spec/services/projects/housekeeping_service_spec.rb
+++ b/spec/services/projects/housekeeping_service_spec.rb
@@ -14,8 +14,10 @@ describe Projects::HousekeepingService do
 
   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)
+      expect(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
+      expect(subject).to receive(:lease_key).and_return(:the_lease_key)
+      expect(subject).to receive(:task).and_return(:the_task)
+      expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :the_task, :the_lease_key, :the_uuid)
 
       subject.execute
       expect(project.reload.pushes_since_gc).to eq(0)
@@ -58,4 +60,26 @@ describe Projects::HousekeepingService do
       end.to change { project.pushes_since_gc }.from(0).to(1)
     end
   end
+
+  it 'uses all three kinds of housekeeping we offer' do
+    allow(subject).to receive(:try_obtain_lease).and_return(:the_uuid)
+    allow(subject).to receive(:lease_key).and_return(:the_lease_key)
+
+    # At push 200
+    expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :gc, :the_lease_key, :the_uuid).
+      exactly(1).times
+    # At push 50, 100, 150
+    expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :full_repack, :the_lease_key, :the_uuid).
+      exactly(3).times
+    # At push 10, 20, ... (except those above)
+    expect(GitGarbageCollectWorker).to receive(:perform_async).with(project.id, :incremental_repack, :the_lease_key, :the_uuid).
+      exactly(16).times
+
+    201.times do
+      subject.increment!
+      subject.execute if subject.needed?
+    end
+
+    expect(project.pushes_since_gc).to eq(1)
+  end
 end