Merge branch 'blackst0ne-squash-and-merge-in-gitlab-core-ce' into 'master'

Resolve "Squash and merge in GitLab Core (CE)"

Closes #34591

See merge request gitlab-org/gitlab-ce!18956

2 files changed, 246 insertions, 13 deletions

diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index e8568bf8bb3..dc30a9bccc1 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -249,24 +249,58 @@ describe MergeRequests::MergeService do
         expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
       end
 
-      context "when fast-forward merge is not allowed" do
+      context 'when squashing' do
         before do
-          allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil)
+          merge_request.update!(source_branch: 'master', target_branch: 'feature')
         end
 
-        %w(semi-linear ff).each do |merge_method|
-          it "logs and saves error if merge is #{merge_method} only" do
-            merge_method = 'rebase_merge' if merge_method == 'semi-linear'
-            merge_request.project.update(merge_method: merge_method)
-            error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch'
-            allow(service).to receive(:execute_hooks)
+        it 'logs and saves error if there is an error when squashing' do
+          error_message = 'Failed to squash. Should be done manually'
 
-            service.execute(merge_request)
+          allow_any_instance_of(MergeRequests::SquashService).to receive(:squash).and_return(nil)
+          merge_request.update(squash: true)
+
+          service.execute(merge_request)
+
+          expect(merge_request).to be_open
+          expect(merge_request.merge_commit_sha).to be_nil
+          expect(merge_request.merge_error).to include(error_message)
+          expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
+        end
+
+        it 'logs and saves error if there is a squash in progress' do
+          error_message = 'another squash is already in progress'
+
+          allow_any_instance_of(MergeRequest).to receive(:squash_in_progress?).and_return(true)
+          merge_request.update(squash: true)
+
+          service.execute(merge_request)
 
-            expect(merge_request).to be_open
-            expect(merge_request.merge_commit_sha).to be_nil
-            expect(merge_request.merge_error).to include(error_message)
-            expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
+          expect(merge_request).to be_open
+          expect(merge_request.merge_commit_sha).to be_nil
+          expect(merge_request.merge_error).to include(error_message)
+          expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
+        end
+
+        context "when fast-forward merge is not allowed" do
+          before do
+            allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil)
+          end
+
+          %w(semi-linear ff).each do |merge_method|
+            it "logs and saves error if merge is #{merge_method} only" do
+              merge_method = 'rebase_merge' if merge_method == 'semi-linear'
+              merge_request.project.update(merge_method: merge_method)
+              error_message = 'Only fast-forward merge is allowed for your project. Please update your source branch'
+              allow(service).to receive(:execute_hooks)
+
+              service.execute(merge_request)
+
+              expect(merge_request).to be_open
+              expect(merge_request.merge_commit_sha).to be_nil
+              expect(merge_request.merge_error).to include(error_message)
+              expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
+            end
           end
         end
       end
diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb
new file mode 100644
index 00000000000..bd884787425
--- /dev/null
+++ b/spec/services/merge_requests/squash_service_spec.rb
@@ -0,0 +1,199 @@
+require 'spec_helper'
+
+describe MergeRequests::SquashService do
+  let(:service) { described_class.new(project, user, {}) }
+  let(:user) { project.owner }
+  let(:project) { create(:project, :repository) }
+  let(:repository) { project.repository.raw }
+  let(:log_error) { "Failed to squash merge request #{merge_request.to_reference(full: true)}:" }
+  let(:squash_dir_path) do
+    File.join(Gitlab.config.shared.path, 'tmp/squash', repository.gl_repository, merge_request.id.to_s)
+  end
+  let(:merge_request_with_one_commit) do
+    create(:merge_request,
+           source_branch: 'feature', source_project: project,
+           target_branch: 'master', target_project: project)
+  end
+
+  let(:merge_request_with_only_new_files) do
+    create(:merge_request,
+           source_branch: 'video', source_project: project,
+           target_branch: 'master', target_project: project)
+  end
+
+  let(:merge_request_with_large_files) do
+    create(:merge_request,
+           source_branch: 'squash-large-files', source_project: project,
+           target_branch: 'master', target_project: project)
+  end
+
+  shared_examples 'the squash succeeds' do
+    it 'returns the squashed commit SHA' do
+      result = service.execute(merge_request)
+
+      expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/))
+      expect(result[:squash_sha]).not_to eq(merge_request.diff_head_sha)
+    end
+
+    it 'cleans up the temporary directory' do
+      service.execute(merge_request)
+
+      expect(File.exist?(squash_dir_path)).to be(false)
+    end
+
+    it 'does not keep the branch push event' do
+      expect { service.execute(merge_request) }.not_to change { Event.count }
+    end
+
+    context 'the squashed commit' do
+      let(:squash_sha) { service.execute(merge_request)[:squash_sha] }
+      let(:squash_commit) { project.repository.commit(squash_sha) }
+
+      it 'copies the author info and message from the merge request' do
+        expect(squash_commit.author_name).to eq(merge_request.author.name)
+        expect(squash_commit.author_email).to eq(merge_request.author.email)
+
+        # Commit messages have a trailing newline, but titles don't.
+        expect(squash_commit.message.chomp).to eq(merge_request.title)
+      end
+
+      it 'sets the current user as the committer' do
+        expect(squash_commit.committer_name).to eq(user.name.chomp('.'))
+        expect(squash_commit.committer_email).to eq(user.email)
+      end
+
+      it 'has the same diff as the merge request, but a different SHA' do
+        rugged = project.repository.rugged
+        mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha)
+        squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha)
+
+        expect(squash_diff.patch.length).to eq(mr_diff.patch.length)
+        expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha)
+      end
+    end
+  end
+
+  describe '#execute' do
+    context 'when there is only one commit in the merge request' do
+      it 'returns that commit SHA' do
+        result = service.execute(merge_request_with_one_commit)
+
+        expect(result).to match(status: :success, squash_sha: merge_request_with_one_commit.diff_head_sha)
+      end
+
+      it 'does not perform any git actions' do
+        expect(repository).not_to receive(:popen)
+
+        service.execute(merge_request_with_one_commit)
+      end
+    end
+
+    context 'when squashing only new files' do
+      let(:merge_request) { merge_request_with_only_new_files }
+
+      include_examples 'the squash succeeds'
+    end
+
+    context 'when squashing with files too large to display' do
+      let(:merge_request) { merge_request_with_large_files }
+
+      include_examples 'the squash succeeds'
+    end
+
+    context 'git errors' do
+      let(:merge_request) { merge_request_with_only_new_files }
+      let(:error) { 'A test error' }
+
+      context 'with gitaly enabled' do
+        before do
+          allow(repository.gitaly_operation_client).to receive(:user_squash)
+            .and_raise(Gitlab::Git::Repository::GitError, error)
+        end
+
+        it 'logs the stage and output' do
+          expect(service).to receive(:log_error).with(log_error)
+          expect(service).to receive(:log_error).with(error)
+
+          service.execute(merge_request)
+        end
+
+        it 'returns an error' do
+          expect(service.execute(merge_request)).to match(status: :error,
+                                                          message: a_string_including('squash'))
+        end
+      end
+
+      context 'with Gitaly disabled', :skip_gitaly_mock do
+        stages = {
+          'add worktree for squash' => 'worktree',
+          'configure sparse checkout' => 'config',
+          'get files in diff' => 'diff --name-only',
+          'check out target branch' => 'checkout',
+          'apply patch' => 'diff --binary',
+          'commit squashed changes' => 'commit',
+          'get SHA of squashed commit' => 'rev-parse'
+        }
+
+        stages.each do |stage, command|
+          context "when the #{stage} stage fails" do
+            before do
+              git_command = a_collection_containing_exactly(
+                a_string_starting_with("#{Gitlab.config.git.bin_path} #{command}")
+              ).or(
+                a_collection_starting_with([Gitlab.config.git.bin_path] + command.split)
+              )
+
+              allow(repository).to receive(:popen).and_return(['', 0])
+              allow(repository).to receive(:popen).with(git_command, anything, anything, anything).and_return([error, 1])
+            end
+
+            it 'logs the stage and output' do
+              expect(service).to receive(:log_error).with(log_error)
+              expect(service).to receive(:log_error).with(error)
+
+              service.execute(merge_request)
+            end
+
+            it 'returns an error' do
+              expect(service.execute(merge_request)).to match(status: :error,
+                                                              message: a_string_including('squash'))
+            end
+
+            it 'cleans up the temporary directory' do
+              expect(File.exist?(squash_dir_path)).to be(false)
+
+              service.execute(merge_request)
+            end
+          end
+        end
+      end
+    end
+
+    context 'when any other exception is thrown' do
+      let(:merge_request) { merge_request_with_only_new_files }
+      let(:error) { 'A test error' }
+
+      before do
+        allow(merge_request).to receive(:commits_count).and_raise(error)
+      end
+
+      it 'logs the MR reference and exception' do
+        expect(service).to receive(:log_error).with(a_string_including("#{project.full_path}#{merge_request.to_reference}"))
+        expect(service).to receive(:log_error).with(error)
+
+        service.execute(merge_request)
+      end
+
+      it 'returns an error' do
+        expect(service.execute(merge_request)).to match(status: :error,
+                                                        message: a_string_including('squash'))
+      end
+
+      it 'cleans up the temporary directory' do
+        service.execute(merge_request)
+
+        expect(File.exist?(squash_dir_path)).to be(false)
+      end
+    end
+  end
+end