Merge branch 'master' into jej-pages-picked-from-ee

9 files changed, 243 insertions, 138 deletions

diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 47cd5075a7d..4080092405d 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -221,6 +221,47 @@ describe Ci::Build, :models do
     end
   end
 
+  describe '#coverage_regex' do
+    subject { build.coverage_regex }
+
+    context 'when project has build_coverage_regex set' do
+      let(:project_regex) { '\(\d+\.\d+\) covered' }
+
+      before do
+        project.build_coverage_regex = project_regex
+      end
+
+      context 'and coverage_regex attribute is not set' do
+        it { is_expected.to eq(project_regex) }
+      end
+
+      context 'but coverage_regex attribute is also set' do
+        let(:build_regex) { 'Code coverage: \d+\.\d+' }
+
+        before do
+          build.coverage_regex = build_regex
+        end
+
+        it { is_expected.to eq(build_regex) }
+      end
+    end
+
+    context 'when neither project nor build has coverage regex set' do
+      it { is_expected.to be_nil }
+    end
+  end
+
+  describe '#update_coverage' do
+    context "regarding coverage_regex's value," do
+      it "saves the correct extracted coverage value" do
+        build.coverage_regex = '\(\d+.\d+\%\) covered'
+        allow(build).to receive(:trace) { 'Coverage 1033 / 1051 LOC (98.29%) covered' }
+        expect(build).to receive(:update_attributes).with(coverage: 98.29) { true }
+        expect(build.update_coverage).to be true
+      end
+    end
+  end
+
   describe 'deployment' do
     describe '#last_deployment' do
       subject { build.last_deployment }
@@ -443,11 +484,11 @@ describe Ci::Build, :models do
         let!(:build) { create(:ci_build, :trace, :success, :artifacts) }
         subject { build.erased? }
 
-        context 'build has not been erased' do
+        context 'job has not been erased' do
           it { is_expected.to be_falsey }
         end
 
-        context 'build has been erased' do
+        context 'job has been erased' do
           before do
             build.erase
           end
diff --git a/spec/models/cycle_analytics/code_spec.rb b/spec/models/cycle_analytics/code_spec.rb
index 3b7cc7d9e2e..9053485939e 100644
--- a/spec/models/cycle_analytics/code_spec.rb
+++ b/spec/models/cycle_analytics/code_spec.rb
@@ -27,15 +27,13 @@ describe 'CycleAnalytics#code', feature: true do
 
     context "when a regular merge request (that doesn't close the issue) is created" do
       it "returns nil" do
-        5.times do
-          issue = create(:issue, project: project)
+        issue = create(:issue, project: project)
 
-          create_commit_referencing_issue(issue)
-          create_merge_request_closing_issue(issue, message: "Closes nothing")
+        create_commit_referencing_issue(issue)
+        create_merge_request_closing_issue(issue, message: "Closes nothing")
 
-          merge_merge_requests_closing_issue(issue)
-          deploy_master
-        end
+        merge_merge_requests_closing_issue(issue)
+        deploy_master
 
         expect(subject[:code].median).to be_nil
       end
@@ -60,14 +58,12 @@ describe 'CycleAnalytics#code', feature: true do
 
     context "when a regular merge request (that doesn't close the issue) is created" do
       it "returns nil" do
-        5.times do
-          issue = create(:issue, project: project)
+        issue = create(:issue, project: project)
 
-          create_commit_referencing_issue(issue)
-          create_merge_request_closing_issue(issue, message: "Closes nothing")
+        create_commit_referencing_issue(issue)
+        create_merge_request_closing_issue(issue, message: "Closes nothing")
 
-          merge_merge_requests_closing_issue(issue)
-        end
+        merge_merge_requests_closing_issue(issue)
 
         expect(subject[:code].median).to be_nil
       end
diff --git a/spec/models/cycle_analytics/issue_spec.rb b/spec/models/cycle_analytics/issue_spec.rb
index 5c73edbbc53..fc7d18bd40e 100644
--- a/spec/models/cycle_analytics/issue_spec.rb
+++ b/spec/models/cycle_analytics/issue_spec.rb
@@ -33,14 +33,12 @@ describe 'CycleAnalytics#issue', models: true do
 
   context "when a regular label (instead of a list label) is added to the issue" do
     it "returns nil" do
-      5.times do
-        regular_label = create(:label)
-        issue = create(:issue, project: project)
-        issue.update(label_ids: [regular_label.id])
+      regular_label = create(:label)
+      issue = create(:issue, project: project)
+      issue.update(label_ids: [regular_label.id])
 
-        create_merge_request_closing_issue(issue)
-        merge_merge_requests_closing_issue(issue)
-      end
+      create_merge_request_closing_issue(issue)
+      merge_merge_requests_closing_issue(issue)
 
       expect(subject[:issue].median).to be_nil
     end
diff --git a/spec/models/cycle_analytics/production_spec.rb b/spec/models/cycle_analytics/production_spec.rb
index 591bbdddf55..b9fe492fe2c 100644
--- a/spec/models/cycle_analytics/production_spec.rb
+++ b/spec/models/cycle_analytics/production_spec.rb
@@ -21,7 +21,13 @@ describe 'CycleAnalytics#production', feature: true do
        ["production deploy happens after merge request is merged (along with other changes)",
         lambda do |context, data|
           # Make other changes on master
-          sha = context.project.repository.commit_file(context.user, context.random_git_name, "content", "commit message", 'master', false)
+          sha = context.project.repository.commit_file(
+            context.user,
+            context.random_git_name,
+            'content',
+            message: 'commit message',
+            branch_name: 'master',
+            update: false)
           context.project.repository.commit(sha)
 
           context.deploy_master
@@ -29,11 +35,9 @@ describe 'CycleAnalytics#production', feature: true do
 
   context "when a regular merge request (that doesn't close the issue) is merged and deployed" do
     it "returns nil" do
-      5.times do
-        merge_request = create(:merge_request)
-        MergeRequests::MergeService.new(project, user).execute(merge_request)
-        deploy_master
-      end
+      merge_request = create(:merge_request)
+      MergeRequests::MergeService.new(project, user).execute(merge_request)
+      deploy_master
 
       expect(subject[:production].median).to be_nil
     end
@@ -41,12 +45,10 @@ describe 'CycleAnalytics#production', feature: true do
 
   context "when the deployment happens to a non-production environment" do
     it "returns nil" do
-      5.times do
-        issue = create(:issue, project: project)
-        merge_request = create_merge_request_closing_issue(issue)
-        MergeRequests::MergeService.new(project, user).execute(merge_request)
-        deploy_master(environment: 'staging')
-      end
+      issue = create(:issue, project: project)
+      merge_request = create_merge_request_closing_issue(issue)
+      MergeRequests::MergeService.new(project, user).execute(merge_request)
+      deploy_master(environment: 'staging')
 
       expect(subject[:production].median).to be_nil
     end
diff --git a/spec/models/cycle_analytics/review_spec.rb b/spec/models/cycle_analytics/review_spec.rb
index 33d2c0a7416..febb18c9884 100644
--- a/spec/models/cycle_analytics/review_spec.rb
+++ b/spec/models/cycle_analytics/review_spec.rb
@@ -23,9 +23,7 @@ describe 'CycleAnalytics#review', feature: true do
 
   context "when a regular merge request (that doesn't close the issue) is created and merged" do
     it "returns nil" do
-      5.times do
-        MergeRequests::MergeService.new(project, user).execute(create(:merge_request))
-      end
+      MergeRequests::MergeService.new(project, user).execute(create(:merge_request))
 
       expect(subject[:review].median).to be_nil
     end
diff --git a/spec/models/cycle_analytics/staging_spec.rb b/spec/models/cycle_analytics/staging_spec.rb
index 00693d67475..9a024d533a1 100644
--- a/spec/models/cycle_analytics/staging_spec.rb
+++ b/spec/models/cycle_analytics/staging_spec.rb
@@ -29,10 +29,10 @@ describe 'CycleAnalytics#staging', feature: true do
                                sha = context.project.repository.commit_file(
                                  context.user,
                                  context.random_git_name,
-                                 "content",
-                                 "commit message",
-                                 'master',
-                                 false)
+                                 'content',
+                                 message: 'commit message',
+                                 branch_name: 'master',
+                                 update: false)
                                context.project.repository.commit(sha)
 
                                context.deploy_master
@@ -40,11 +40,9 @@ describe 'CycleAnalytics#staging', feature: true do
 
   context "when a regular merge request (that doesn't close the issue) is merged and deployed" do
     it "returns nil" do
-      5.times do
-        merge_request = create(:merge_request)
-        MergeRequests::MergeService.new(project, user).execute(merge_request)
-        deploy_master
-      end
+      merge_request = create(:merge_request)
+      MergeRequests::MergeService.new(project, user).execute(merge_request)
+      deploy_master
 
       expect(subject[:staging].median).to be_nil
     end
@@ -52,12 +50,10 @@ describe 'CycleAnalytics#staging', feature: true do
 
   context "when the deployment happens to a non-production environment" do
     it "returns nil" do
-      5.times do
-        issue = create(:issue, project: project)
-        merge_request = create_merge_request_closing_issue(issue)
-        MergeRequests::MergeService.new(project, user).execute(merge_request)
-        deploy_master(environment: 'staging')
-      end
+      issue = create(:issue, project: project)
+      merge_request = create_merge_request_closing_issue(issue)
+      MergeRequests::MergeService.new(project, user).execute(merge_request)
+      deploy_master(environment: 'staging')
 
       expect(subject[:staging].median).to be_nil
     end
diff --git a/spec/models/cycle_analytics/test_spec.rb b/spec/models/cycle_analytics/test_spec.rb
index f857ea6cbec..c2ba012a0e6 100644
--- a/spec/models/cycle_analytics/test_spec.rb
+++ b/spec/models/cycle_analytics/test_spec.rb
@@ -24,16 +24,14 @@ describe 'CycleAnalytics#test', feature: true do
 
   context "when the pipeline is for a regular merge request (that doesn't close an issue)" do
     it "returns nil" do
-      5.times do
-        issue = create(:issue, project: project)
-        merge_request = create_merge_request_closing_issue(issue)
-        pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha)
+      issue = create(:issue, project: project)
+      merge_request = create_merge_request_closing_issue(issue)
+      pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha)
 
-        pipeline.run!
-        pipeline.succeed!
+      pipeline.run!
+      pipeline.succeed!
 
-        merge_merge_requests_closing_issue(issue)
-      end
+      merge_merge_requests_closing_issue(issue)
 
       expect(subject[:test].median).to be_nil
     end
@@ -41,12 +39,10 @@ describe 'CycleAnalytics#test', feature: true do
 
   context "when the pipeline is not for a merge request" do
     it "returns nil" do
-      5.times do
-        pipeline = create(:ci_pipeline, ref: "refs/heads/master", sha: project.repository.commit('master').sha)
+      pipeline = create(:ci_pipeline, ref: "refs/heads/master", sha: project.repository.commit('master').sha)
 
-        pipeline.run!
-        pipeline.succeed!
-      end
+      pipeline.run!
+      pipeline.succeed!
 
       expect(subject[:test].median).to be_nil
     end
@@ -54,16 +50,14 @@ describe 'CycleAnalytics#test', feature: true do
 
   context "when the pipeline is dropped (failed)" do
     it "returns nil" do
-      5.times do
-        issue = create(:issue, project: project)
-        merge_request = create_merge_request_closing_issue(issue)
-        pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha)
+      issue = create(:issue, project: project)
+      merge_request = create_merge_request_closing_issue(issue)
+      pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha)
 
-        pipeline.run!
-        pipeline.drop!
+      pipeline.run!
+      pipeline.drop!
 
-        merge_merge_requests_closing_issue(issue)
-      end
+      merge_merge_requests_closing_issue(issue)
 
       expect(subject[:test].median).to be_nil
     end
@@ -71,16 +65,14 @@ describe 'CycleAnalytics#test', feature: true do
 
   context "when the pipeline is cancelled" do
     it "returns nil" do
-      5.times do
-        issue = create(:issue, project: project)
-        merge_request = create_merge_request_closing_issue(issue)
-        pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha)
+      issue = create(:issue, project: project)
+      merge_request = create_merge_request_closing_issue(issue)
+      pipeline = create(:ci_pipeline, ref: "refs/heads/#{merge_request.source_branch}", sha: merge_request.diff_head_sha)
 
-        pipeline.run!
-        pipeline.cancel!
+      pipeline.run!
+      pipeline.cancel!
 
-        merge_merge_requests_closing_issue(issue)
-      end
+      merge_merge_requests_closing_issue(issue)
 
       expect(subject[:test].median).to be_nil
     end
diff --git a/spec/models/members/project_member_spec.rb b/spec/models/members/project_member_spec.rb
index 90d14c2c0b9..e4be0aba7a6 100644
--- a/spec/models/members/project_member_spec.rb
+++ b/spec/models/members/project_member_spec.rb
@@ -117,7 +117,7 @@ describe ProjectMember, models: true do
       users = create_list(:user, 2)
 
       described_class.add_users_to_projects(
-        [projects.first.id, projects.second],
+        [projects.first.id, projects.second.id],
         [users.first.id, users.second],
         described_class::MASTER)
 
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 829b69093c9..53b98ba05f8 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -15,7 +15,12 @@ describe Repository, models: true do
 
   let(:merge_commit) do
     merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project)
-    merge_commit_id = repository.merge(user, merge_request, commit_options)
+
+    merge_commit_id = repository.merge(user,
+                                       merge_request.diff_head_sha,
+                                       merge_request,
+                                       commit_options)
+
     repository.commit(merge_commit_id)
   end
 
@@ -289,17 +294,39 @@ describe Repository, models: true do
   describe "#commit_dir" do
     it "commits a change that creates a new directory" do
       expect do
-        repository.commit_dir(user, 'newdir', 'Create newdir', 'master')
+        repository.commit_dir(user, 'newdir',
+          message: 'Create newdir', branch_name: 'master')
       end.to change { repository.commits('master').count }.by(1)
 
       newdir = repository.tree('master', 'newdir')
       expect(newdir.path).to eq('newdir')
     end
 
+    context "when committing to another project" do
+      let(:forked_project) { create(:project) }
+
+      it "creates a fork and commit to the forked project" do
+        expect do
+          repository.commit_dir(user, 'newdir',
+            message: 'Create newdir', branch_name: 'patch',
+            start_branch_name: 'master', start_project: forked_project)
+        end.to change { repository.commits('master').count }.by(0)
+
+        expect(repository.branch_exists?('patch')).to be_truthy
+        expect(forked_project.repository.branch_exists?('patch')).to be_falsy
+
+        newdir = repository.tree('patch', 'newdir')
+        expect(newdir.path).to eq('newdir')
+      end
+    end
+
     context "when an author is specified" do
       it "uses the given email/name to set the commit's author" do
         expect do
-          repository.commit_dir(user, "newdir", "Add newdir", 'master', author_email: author_email, author_name: author_name)
+          repository.commit_dir(user, 'newdir',
+            message: 'Add newdir',
+            branch_name: 'master',
+            author_email: author_email, author_name: author_name)
         end.to change { repository.commits('master').count }.by(1)
 
         last_commit = repository.commit
@@ -314,8 +341,9 @@ describe Repository, models: true do
     it 'commits change to a file successfully' do
       expect do
         repository.commit_file(user, 'CHANGELOG', 'Changelog!',
-                              'Updates file content',
-                              'master', true)
+                               message: 'Updates file content',
+                               branch_name: 'master',
+                               update: true)
       end.to change { repository.commits('master').count }.by(1)
 
       blob = repository.blob_at('master', 'CHANGELOG')
@@ -326,8 +354,12 @@ describe Repository, models: true do
     context "when an author is specified" do
       it "uses the given email/name to set the commit's author" do
         expect do
-          repository.commit_file(user, "README", 'README!', 'Add README',
-                                'master', true, author_email: author_email, author_name: author_name)
+          repository.commit_file(user, 'README', 'README!',
+                                 message: 'Add README',
+                                 branch_name: 'master',
+                                 update: true,
+                                 author_email: author_email,
+                                 author_name: author_name)
         end.to change { repository.commits('master').count }.by(1)
 
         last_commit = repository.commit
@@ -342,7 +374,7 @@ describe Repository, models: true do
     it 'updates filename successfully' do
       expect do
         repository.update_file(user, 'NEWLICENSE', 'Copyright!',
-                                     branch: 'master',
+                                     branch_name: 'master',
                                      previous_path: 'LICENSE',
                                      message: 'Changes filename')
       end.to change { repository.commits('master').count }.by(1)
@@ -355,15 +387,16 @@ describe Repository, models: true do
 
     context "when an author is specified" do
       it "uses the given email/name to set the commit's author" do
-        repository.commit_file(user, "README", 'README!', 'Add README', 'master', true)
+        repository.commit_file(user, 'README', 'README!',
+          message: 'Add README', branch_name: 'master', update: true)
 
         expect do
-          repository.update_file(user, 'README', "Updated README!",
-                                branch: 'master',
-                                previous_path: 'README',
-                                message: 'Update README',
-                                author_email: author_email,
-                                author_name: author_name)
+          repository.update_file(user, 'README', 'Updated README!',
+                                 branch_name: 'master',
+                                 previous_path: 'README',
+                                 message: 'Update README',
+                                 author_email: author_email,
+                                 author_name: author_name)
         end.to change { repository.commits('master').count }.by(1)
 
         last_commit = repository.commit
@@ -376,10 +409,12 @@ describe Repository, models: true do
 
   describe "#remove_file" do
     it 'removes file successfully' do
-      repository.commit_file(user, "README", 'README!', 'Add README', 'master', true)
+      repository.commit_file(user, 'README', 'README!',
+        message: 'Add README', branch_name: 'master', update: true)
 
       expect do
-        repository.remove_file(user, "README", "Remove README", 'master')
+        repository.remove_file(user, 'README',
+          message: 'Remove README', branch_name: 'master')
       end.to change { repository.commits('master').count }.by(1)
 
       expect(repository.blob_at('master', 'README')).to be_nil
@@ -387,10 +422,13 @@ describe Repository, models: true do
 
     context "when an author is specified" do
       it "uses the given email/name to set the commit's author" do
-        repository.commit_file(user, "README", 'README!', 'Add README', 'master', true)
+        repository.commit_file(user, 'README', 'README!',
+          message: 'Add README', branch_name: 'master', update: true)
 
         expect do
-          repository.remove_file(user, "README", "Remove README", 'master', author_email: author_email, author_name: author_name)
+          repository.remove_file(user, 'README',
+            message: 'Remove README', branch_name: 'master',
+            author_email: author_email, author_name: author_name)
         end.to change { repository.commits('master').count }.by(1)
 
         last_commit = repository.commit
@@ -538,11 +576,14 @@ describe Repository, models: true do
 
   describe "#license_blob", caching: true do
     before do
-      repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master')
+      repository.remove_file(
+        user, 'LICENSE', message: 'Remove LICENSE', branch_name: 'master')
     end
 
     it 'handles when HEAD points to non-existent ref' do
-      repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false)
+      repository.commit_file(
+        user, 'LICENSE', 'Copyright!',
+        message: 'Add LICENSE', branch_name: 'master', update: false)
 
       allow(repository).to receive(:file_on_head).
         and_raise(Rugged::ReferenceError)
@@ -551,21 +592,27 @@ describe Repository, models: true do
     end
 
     it 'looks in the root_ref only' do
-      repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'markdown')
-      repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'markdown', false)
+      repository.remove_file(user, 'LICENSE',
+        message: 'Remove LICENSE', branch_name: 'markdown')
+      repository.commit_file(user, 'LICENSE',
+        Licensee::License.new('mit').content,
+        message: 'Add LICENSE', branch_name: 'markdown', update: false)
 
       expect(repository.license_blob).to be_nil
     end
 
     it 'detects license file with no recognizable open-source license content' do
-      repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false)
+      repository.commit_file(user, 'LICENSE', 'Copyright!',
+        message: 'Add LICENSE', branch_name: 'master', update: false)
 
       expect(repository.license_blob.name).to eq('LICENSE')
     end
 
     %w[LICENSE LICENCE LiCensE LICENSE.md LICENSE.foo COPYING COPYING.md].each do |filename|
       it "detects '#{filename}'" do
-        repository.commit_file(user, filename, Licensee::License.new('mit').content, "Add #{filename}", 'master', false)
+        repository.commit_file(user, filename,
+          Licensee::License.new('mit').content,
+          message: "Add #{filename}", branch_name: 'master', update: false)
 
         expect(repository.license_blob.name).to eq(filename)
       end
@@ -574,7 +621,8 @@ describe Repository, models: true do
 
   describe '#license_key', caching: true do
     before do
-      repository.remove_file(user, 'LICENSE', 'Remove LICENSE', 'master')
+      repository.remove_file(user, 'LICENSE',
+        message: 'Remove LICENSE', branch_name: 'master')
     end
 
     it 'returns nil when no license is detected' do
@@ -588,13 +636,16 @@ describe Repository, models: true do
     end
 
     it 'detects license file with no recognizable open-source license content' do
-      repository.commit_file(user, 'LICENSE', 'Copyright!', 'Add LICENSE', 'master', false)
+      repository.commit_file(user, 'LICENSE', 'Copyright!',
+        message: 'Add LICENSE', branch_name: 'master', update: false)
 
       expect(repository.license_key).to be_nil
     end
 
     it 'returns the license key' do
-      repository.commit_file(user, 'LICENSE', Licensee::License.new('mit').content, 'Add LICENSE', 'master', false)
+      repository.commit_file(user, 'LICENSE',
+        Licensee::License.new('mit').content,
+        message: 'Add LICENSE', branch_name: 'master', update: false)
 
       expect(repository.license_key).to eq('mit')
     end
@@ -707,7 +758,7 @@ describe Repository, models: true do
         allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
 
         expect do
-          repository.rm_branch(user, 'new_feature')
+          repository.rm_branch(user, 'feature')
         end.to raise_error(GitHooksService::PreReceiveError)
       end
 
@@ -728,36 +779,51 @@ describe Repository, models: true do
 
     context 'when pre hooks were successful' do
       before do
-        expect_any_instance_of(GitHooksService).to receive(:execute).
-          with(user, repository.path_to_repo, old_rev, new_rev, 'refs/heads/feature').
-          and_yield.and_return(true)
+        service = GitHooksService.new
+        expect(GitHooksService).to receive(:new).and_return(service)
+        expect(service).to receive(:execute).
+          with(
+            user,
+            repository.path_to_repo,
+            old_rev,
+            new_rev,
+            'refs/heads/feature').
+          and_yield(service).and_return(true)
       end
 
       it 'runs without errors' do
         expect do
-          repository.update_branch_with_hooks(user, 'feature') { new_rev }
+          GitOperationService.new(user, repository).with_branch('feature') do
+            new_rev
+          end
         end.not_to raise_error
       end
 
       it 'ensures the autocrlf Git option is set to :input' do
-        expect(repository).to receive(:update_autocrlf_option)
+        service = GitOperationService.new(user, repository)
 
-        repository.update_branch_with_hooks(user, 'feature') { new_rev }
+        expect(service).to receive(:update_autocrlf_option)
+
+        service.with_branch('feature') { new_rev }
       end
 
       context "when the branch wasn't empty" do
         it 'updates the head' do
           expect(repository.find_branch('feature').dereferenced_target.id).to eq(old_rev)
-          repository.update_branch_with_hooks(user, 'feature') { new_rev }
+
+          GitOperationService.new(user, repository).with_branch('feature') do
+            new_rev
+          end
+
           expect(repository.find_branch('feature').dereferenced_target.id).to eq(new_rev)
         end
       end
     end
 
     context 'when the update adds more than one commit' do
-      it 'runs without errors' do
-        old_rev = '33f3729a45c02fc67d00adb1b8bca394b0e761d9'
+      let(:old_rev) { '33f3729a45c02fc67d00adb1b8bca394b0e761d9' }
 
+      it 'runs without errors' do
         # old_rev is an ancestor of new_rev
         expect(repository.rugged.merge_base(old_rev, new_rev)).to eq(old_rev)
 
@@ -767,22 +833,28 @@ describe Repository, models: true do
         branch = 'feature-ff-target'
         repository.add_branch(user, branch, old_rev)
 
-        expect { repository.update_branch_with_hooks(user, branch) { new_rev } }.not_to raise_error
+        expect do
+          GitOperationService.new(user, repository).with_branch(branch) do
+            new_rev
+          end
+        end.not_to raise_error
       end
     end
 
     context 'when the update would remove commits from the target branch' do
-      it 'raises an exception' do
-        branch = 'master'
-        old_rev = repository.find_branch(branch).dereferenced_target.sha
+      let(:branch) { 'master' }
+      let(:old_rev) { repository.find_branch(branch).dereferenced_target.sha }
 
+      it 'raises an exception' do
         # The 'master' branch is NOT an ancestor of new_rev.
         expect(repository.rugged.merge_base(old_rev, new_rev)).not_to eq(old_rev)
 
         # Updating 'master' to new_rev would lose the commits on 'master' that
         # are not contained in new_rev. This should not be allowed.
         expect do
-          repository.update_branch_with_hooks(user, branch) { new_rev }
+          GitOperationService.new(user, repository).with_branch(branch) do
+            new_rev
+          end
         end.to raise_error(Repository::CommitError)
       end
     end
@@ -792,7 +864,9 @@ describe Repository, models: true do
         allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
 
         expect do
-          repository.update_branch_with_hooks(user, 'feature') { new_rev }
+          GitOperationService.new(user, repository).with_branch('feature') do
+            new_rev
+          end
         end.to raise_error(GitHooksService::PreReceiveError)
       end
     end
@@ -800,7 +874,6 @@ describe Repository, models: true do
     context 'when target branch is different from source branch' do
       before do
         allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, ''])
-        allow(repository).to receive(:update_ref!)
       end
 
       it 'expires branch cache' do
@@ -809,7 +882,10 @@ describe Repository, models: true do
         expect(repository).not_to receive(:expire_emptiness_caches)
         expect(repository).to     receive(:expire_branches_cache)
 
-        repository.update_branch_with_hooks(user, 'new-feature') { new_rev }
+        GitOperationService.new(user, repository).
+          with_branch('new-feature') do
+            new_rev
+          end
       end
     end
 
@@ -827,7 +903,9 @@ describe Repository, models: true do
         expect(empty_repository).to receive(:expire_branches_cache)
 
         empty_repository.commit_file(user, 'CHANGELOG', 'Changelog!',
-                                     'Updates file content', 'master', false)
+                                     message: 'Updates file content',
+                                     branch_name: 'master',
+                                     update: false)
       end
     end
   end
@@ -877,7 +955,7 @@ describe Repository, models: true do
       end
 
       it 'sets autocrlf to :input' do
-        repository.update_autocrlf_option
+        GitOperationService.new(nil, repository).send(:update_autocrlf_option)
 
         expect(repository.raw_repository.autocrlf).to eq(:input)
       end
@@ -892,7 +970,7 @@ describe Repository, models: true do
         expect(repository.raw_repository).not_to receive(:autocrlf=).
           with(:input)
 
-        repository.update_autocrlf_option
+        GitOperationService.new(nil, repository).send(:update_autocrlf_option)
       end
     end
   end
@@ -1009,8 +1087,11 @@ describe Repository, models: true do
 
     it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do
       merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'master', source_project: project)
-      merge_commit_id = repository.merge(user, merge_request, commit_options)
-      repository.commit(merge_commit_id)
+
+      merge_commit_id = repository.merge(user,
+                                         merge_request.diff_head_sha,
+                                         merge_request,
+                                         commit_options)
 
       expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id)
     end
@@ -1388,9 +1469,10 @@ describe Repository, models: true do
   describe '#rm_tag' do
     it 'removes a tag' do
       expect(repository).to receive(:before_remove_tag)
-      expect(repository.rugged.tags).to receive(:delete).with('v1.1.0')
 
-      repository.rm_tag('v1.1.0')
+      repository.rm_tag(create(:user), 'v1.1.0')
+
+      expect(repository.find_tag('v1.1.0')).to be_nil
     end
   end
 
@@ -1458,16 +1540,16 @@ describe Repository, models: true do
     end
   end
 
-  describe '#update_ref!' do
+  describe '#update_ref' do
     it 'can create a ref' do
-      repository.update_ref!('refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
+      GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/foobar', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
 
       expect(repository.find_branch('foobar')).not_to be_nil
     end
 
     it 'raises CommitError when the ref update fails' do
       expect do
-        repository.update_ref!('refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
+        GitOperationService.new(nil, repository).send(:update_ref, 'refs/heads/master', 'refs/heads/master', Gitlab::Git::BLANK_SHA)
       end.to raise_error(Repository::CommitError)
     end
   end