Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSean McGivern <sean@gitlab.com>2017-05-11 18:23:02 +0300
committerSean McGivern <sean@gitlab.com>2017-05-12 22:47:51 +0300
commitad2bfeb85756db8c4cea9290be743665efd1c918 (patch)
treecf51b926c348db7a7f5df26117a692f55416fe25 /spec/services/merge_requests/conflicts
parentaec53bab05afd0a20b15bd9311643f8bf5efd140 (diff)
Fix conflict resolution from corrupted upstream
I don't know why this happens exactly, but given an upstream and fork repository from a customer, both of which required GC, resolving conflicts would corrupt the fork so badly that it couldn't be cloned. This isn't a perfect fix for that case, because the MR may still need to be merged manually, but it does ensure that the repository is at least usable. My best guess is that when we generate the index for the conflict resolution (which we previously did in the target project), we obtain a reference to an OID that doesn't exist in the source, even though we already fetch the refs from the target into the source. Explicitly setting the source project as the place to get the merge index from seems to prevent repository corruption in this way.
Diffstat (limited to 'spec/services/merge_requests/conflicts')
-rw-r--r--spec/services/merge_requests/conflicts/list_service_spec.rb73
-rw-r--r--spec/services/merge_requests/conflicts/resolve_service_spec.rb222
2 files changed, 295 insertions, 0 deletions
diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb
new file mode 100644
index 00000000000..e8a305d6130
--- /dev/null
+++ b/spec/services/merge_requests/conflicts/list_service_spec.rb
@@ -0,0 +1,73 @@
+require 'spec_helper'
+
+describe MergeRequests::Conflicts::ListService do
+ describe '#can_be_resolved_in_ui?' do
+ def create_merge_request(source_branch)
+ create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start') do |mr|
+ mr.mark_as_unmergeable
+ end
+ end
+
+ def conflicts_service(merge_request)
+ described_class.new(merge_request)
+ end
+
+ it 'returns a falsey value when the MR can be merged without conflicts' do
+ merge_request = create_merge_request('master')
+ merge_request.mark_as_mergeable
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
+ end
+
+ it 'returns a falsey value when the MR is marked as having conflicts, but has none' do
+ merge_request = create_merge_request('master')
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
+ end
+
+ it 'returns a falsey value when the MR has a missing ref after a force push' do
+ merge_request = create_merge_request('conflict-resolvable')
+ service = conflicts_service(merge_request)
+ allow(service.conflicts).to receive(:merge_index).and_raise(Rugged::OdbError)
+
+ expect(service.can_be_resolved_in_ui?).to be_falsey
+ end
+
+ it 'returns a falsey value when the MR does not support new diff notes' do
+ merge_request = create_merge_request('conflict-resolvable')
+ merge_request.merge_request_diff.update_attributes(start_commit_sha: nil)
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
+ end
+
+ it 'returns a falsey value when the conflicts contain a large file' do
+ merge_request = create_merge_request('conflict-too-large')
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
+ end
+
+ it 'returns a falsey value when the conflicts contain a binary file' do
+ merge_request = create_merge_request('conflict-binary-file')
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
+ end
+
+ it 'returns a falsey value when the conflicts contain a file edited in one branch and deleted in another' do
+ merge_request = create_merge_request('conflict-missing-side')
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
+ end
+
+ it 'returns a truthy value when the conflicts are resolvable in the UI' do
+ merge_request = create_merge_request('conflict-resolvable')
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_truthy
+ end
+
+ it 'returns a truthy value when the conflicts have to be resolved in an editor' do
+ merge_request = create_merge_request('conflict-contains-conflict-markers')
+
+ expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_truthy
+ end
+ end
+end
diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb
new file mode 100644
index 00000000000..19e8d5cc5f1
--- /dev/null
+++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb
@@ -0,0 +1,222 @@
+require 'spec_helper'
+
+describe MergeRequests::Conflicts::ResolveService do
+ let(:user) { create(:user) }
+ let(:project) { create(:project, :repository) }
+
+ let(:fork_project) do
+ create(:forked_project_with_submodules) do |fork_project|
+ fork_project.build_forked_project_link(forked_to_project_id: fork_project.id, forked_from_project_id: project.id)
+ fork_project.save
+ end
+ end
+
+ let(:merge_request) do
+ create(:merge_request,
+ source_branch: 'conflict-resolvable', source_project: project,
+ target_branch: 'conflict-start')
+ end
+
+ let(:merge_request_from_fork) do
+ create(:merge_request,
+ source_branch: 'conflict-resolvable-fork', source_project: fork_project,
+ target_branch: 'conflict-start', target_project: project)
+ end
+
+ describe '#execute' do
+ let(:service) { described_class.new(merge_request) }
+
+ context 'with section params' do
+ let(:params) do
+ {
+ files: [
+ {
+ old_path: 'files/ruby/popen.rb',
+ new_path: 'files/ruby/popen.rb',
+ sections: {
+ '2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head'
+ }
+ }, {
+ old_path: 'files/ruby/regex.rb',
+ new_path: 'files/ruby/regex.rb',
+ sections: {
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+ }
+ }
+ ],
+ commit_message: 'This is a commit message!'
+ }
+ end
+
+ context 'when the source and target project are the same' do
+ before do
+ service.execute(user, params)
+ end
+
+ it 'creates a commit with the message' do
+ expect(merge_request.source_branch_head.message).to eq(params[:commit_message])
+ end
+
+ it 'creates a commit with the correct parents' do
+ expect(merge_request.source_branch_head.parents.map(&:id)).
+ to eq(%w(1450cd639e0bc6721eb02800169e464f212cde06
+ 824be604a34828eb682305f0d963056cfac87b2d))
+ end
+ end
+
+ context 'when the source project is a fork and does not contain the HEAD of the target branch' do
+ let!(:target_head) do
+ project.repository.create_file(
+ user,
+ 'new-file-in-target',
+ '',
+ message: 'Add new file in target',
+ branch_name: 'conflict-start')
+ end
+
+ def resolve_conflicts
+ described_class.new(merge_request_from_fork).execute(user, params)
+ end
+
+ it 'gets conflicts from the source project' do
+ expect(fork_project.repository.rugged).to receive(:merge_commits).and_call_original
+ expect(project.repository.rugged).not_to receive(:merge_commits)
+
+ resolve_conflicts
+ end
+
+ it 'creates a commit with the message' do
+ resolve_conflicts
+
+ expect(merge_request_from_fork.source_branch_head.message).to eq(params[:commit_message])
+ end
+
+ it 'creates a commit with the correct parents' do
+ resolve_conflicts
+
+ expect(merge_request_from_fork.source_branch_head.parents.map(&:id)).
+ to eq(['404fa3fc7c2c9b5dacff102f353bdf55b1be2813',
+ target_head])
+ end
+ end
+ end
+
+ context 'with content and sections params' do
+ let(:popen_content) { "class Popen\nend" }
+
+ let(:params) do
+ {
+ files: [
+ {
+ old_path: 'files/ruby/popen.rb',
+ new_path: 'files/ruby/popen.rb',
+ content: popen_content
+ }, {
+ old_path: 'files/ruby/regex.rb',
+ new_path: 'files/ruby/regex.rb',
+ sections: {
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
+ '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin'
+ }
+ }
+ ],
+ commit_message: 'This is a commit message!'
+ }
+ end
+
+ before do
+ service.execute(user, params)
+ end
+
+ it 'creates a commit with the message' do
+ expect(merge_request.source_branch_head.message).to eq(params[:commit_message])
+ end
+
+ it 'creates a commit with the correct parents' do
+ expect(merge_request.source_branch_head.parents.map(&:id)).
+ to eq(%w(1450cd639e0bc6721eb02800169e464f212cde06
+ 824be604a34828eb682305f0d963056cfac87b2d))
+ end
+
+ it 'sets the content to the content given' do
+ blob = merge_request.source_project.repository.blob_at(merge_request.source_branch_head.sha,
+ 'files/ruby/popen.rb')
+
+ expect(blob.data).to eq(popen_content)
+ end
+ end
+
+ context 'when a resolution section is missing' do
+ let(:invalid_params) do
+ {
+ files: [
+ {
+ old_path: 'files/ruby/popen.rb',
+ new_path: 'files/ruby/popen.rb',
+ content: ''
+ }, {
+ old_path: 'files/ruby/regex.rb',
+ new_path: 'files/ruby/regex.rb',
+ sections: { '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head' }
+ }
+ ],
+ commit_message: 'This is a commit message!'
+ }
+ end
+
+ it 'raises a MissingResolution error' do
+ expect { service.execute(user, invalid_params) }.
+ to raise_error(Gitlab::Conflict::File::MissingResolution)
+ end
+ end
+
+ context 'when the content of a file is unchanged' do
+ let(:list_service) { MergeRequests::Conflicts::ListService.new(merge_request) }
+
+ let(:invalid_params) do
+ {
+ files: [
+ {
+ old_path: 'files/ruby/popen.rb',
+ new_path: 'files/ruby/popen.rb',
+ content: ''
+ }, {
+ old_path: 'files/ruby/regex.rb',
+ new_path: 'files/ruby/regex.rb',
+ content: list_service.conflicts.file_for_path('files/ruby/regex.rb', 'files/ruby/regex.rb').content
+ }
+ ],
+ commit_message: 'This is a commit message!'
+ }
+ end
+
+ it 'raises a MissingResolution error' do
+ expect { service.execute(user, invalid_params) }.
+ to raise_error(Gitlab::Conflict::File::MissingResolution)
+ end
+ end
+
+ context 'when a file is missing' do
+ let(:invalid_params) do
+ {
+ files: [
+ {
+ old_path: 'files/ruby/popen.rb',
+ new_path: 'files/ruby/popen.rb',
+ content: ''
+ }
+ ],
+ commit_message: 'This is a commit message!'
+ }
+ end
+
+ it 'raises a MissingFiles error' do
+ expect { service.execute(user, invalid_params) }.
+ to raise_error(described_class::MissingFiles)
+ end
+ end
+ end
+end