From ce7eb4e49279ab56cac992b6743fe77cac578b48 Mon Sep 17 00:00:00 2001
From: Sean McGivern <sean@gitlab.com>
Date: Fri, 5 Aug 2016 12:15:06 +0100
Subject: [PATCH] Add more tests for conflicts

---
 .../projects/merge_requests_controller.rb     |  2 +-
 lib/gitlab/conflict/file_collection.rb        |  2 +-
 .../merge_requests_controller_spec.rb         |  8 +--
 .../features/merge_requests/conflicts_spec.rb | 68 +++++++++++++++++++
 .../gitlab/conflict/file_collection_spec.rb   |  4 +-
 spec/lib/gitlab/conflict/file_spec.rb         |  6 +-
 spec/models/merge_request_spec.rb             | 52 ++++++++++++++
 spec/support/test_env.rb                      | 46 +++++++------
 8 files changed, 156 insertions(+), 32 deletions(-)
 create mode 100644 spec/features/merge_requests/conflicts_spec.rb

diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index a3e02f84f88..2edd76de4c4 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -139,7 +139,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
           render json: @merge_request.conflicts
         else
           render json: {
-            message: 'Unable to resolve conflicts in the web interface for this merge request',
+            message: 'Unable to resolve conflicts in the web interface for this merge request.',
             type: 'error'
           }
         end
diff --git a/lib/gitlab/conflict/file_collection.rb b/lib/gitlab/conflict/file_collection.rb
index e8157fb9e91..bbd0427a2c8 100644
--- a/lib/gitlab/conflict/file_collection.rb
+++ b/lib/gitlab/conflict/file_collection.rb
@@ -46,7 +46,7 @@ module Gitlab
         end
 
         <<EOM.chomp
-Merge branch '#{merge_request.source_branch}' into '#{merge_request.target_branch}'
+Merge branch '#{merge_request.target_branch}' into '#{merge_request.source_branch}'
 
 # Conflicts:
 #{conflict_filenames.join("\n")}
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 88bab0c08e8..c64c2b075c5 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -5,7 +5,7 @@ describe Projects::MergeRequestsController do
   let(:user)    { create(:user) }
   let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
   let(:merge_request_with_conflicts) do
-    create(:merge_request, source_branch: 'conflict-a', target_branch: 'conflict-b', source_project: project) do |mr|
+    create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) do |mr|
       mr.mark_as_unmergeable
     end
   end
@@ -625,7 +625,7 @@ describe Projects::MergeRequestsController do
 
     context 'with valid params' do
       before do
-        resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_4_4' => 'head',
+        resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head',
                           '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_9_9' => 'head',
                           '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_21_21' => 'origin',
                           '6eb14e00385d2fb284765eb1cd8d420d33d63fc9_49_49' => 'origin')
@@ -636,14 +636,14 @@ describe Projects::MergeRequestsController do
         expect(merge_request_with_conflicts.source_branch_head.message).to include('Commit message')
       end
 
-      it 'returns an OK resposne' do
+      it 'returns an OK response' do
         expect(response).to have_http_status(:ok)
       end
     end
 
     context 'when sections are missing' do
       before do
-        resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_4_4' => 'head')
+        resolve_conflicts('2f6fcd96b88b36ce98c38da085c795a27d92a3dd_14_14' => 'head')
       end
 
       it 'returns a 400 error' do
diff --git a/spec/features/merge_requests/conflicts_spec.rb b/spec/features/merge_requests/conflicts_spec.rb
new file mode 100644
index 00000000000..0b39b4dbebb
--- /dev/null
+++ b/spec/features/merge_requests/conflicts_spec.rb
@@ -0,0 +1,68 @@
+require 'spec_helper'
+
+feature 'Merge request conflict resolution', js: true, feature: true do
+  include WaitForAjax
+
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+
+  def create_merge_request(source_branch)
+    create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start', source_project: project) do |mr|
+      mr.mark_as_unmergeable
+    end
+  end
+
+  context 'when a merge request can be resolved in the UI' do
+    let(:merge_request) { create_merge_request('conflict-resolvable') }
+
+    before do
+      project.team << [user, :developer]
+      login_as(user)
+
+      visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+    end
+
+    it 'shows a link to the conflict resolution page' do
+      expect(page).to have_link('conflicts', href: /\/conflicts\Z/)
+    end
+
+    context 'visiting the conflicts resolution page' do
+      before { click_link('conflicts', href: /\/conflicts\Z/) }
+
+      it 'shows the conflicts' do
+        expect(find('#conflicts')).to have_content('popen.rb')
+      end
+    end
+  end
+
+  UNRESOLVABLE_CONFLICTS = {
+    'conflict-too-large' => 'when the conflicts contain a large file',
+    'conflict-binary-file' => 'when the conflicts contain a binary file',
+    'conflict-contains-conflict-markers' => 'when the conflicts contain a file with ambiguous conflict markers',
+    'conflict-missing-side' => 'when the conflicts contain a file edited in one branch and deleted in another'
+  }
+
+  UNRESOLVABLE_CONFLICTS.each do |source_branch, description|
+    context description do
+      let(:merge_request) { create_merge_request(source_branch) }
+
+      before do
+        project.team << [user, :developer]
+        login_as(user)
+
+        visit namespace_project_merge_request_path(project.namespace, project, merge_request)
+      end
+
+      it 'does not show a link to the conflict resolution page' do
+        expect(page).not_to have_link('conflicts', href: /\/conflicts\Z/)
+      end
+
+      it 'shows an error if the conflicts page is visited directly' do
+        visit current_url + '/conflicts'
+        wait_for_ajax
+
+        expect(find('#conflicts')).to have_content('Unable to resolve conflicts')
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/conflict/file_collection_spec.rb b/spec/lib/gitlab/conflict/file_collection_spec.rb
index 2a09adb2321..39d892c18c0 100644
--- a/spec/lib/gitlab/conflict/file_collection_spec.rb
+++ b/spec/lib/gitlab/conflict/file_collection_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::Conflict::FileCollection, lib: true do
-  let(:merge_request) { create(:merge_request, source_branch: 'conflict-a', target_branch: 'conflict-b') }
+  let(:merge_request) { create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start') }
   let(:file_collection) { Gitlab::Conflict::FileCollection.new(merge_request) }
 
   describe '#files' do
@@ -13,7 +13,7 @@ describe Gitlab::Conflict::FileCollection, lib: true do
   describe '#default_commit_message' do
     it 'matches the format of the git CLI commit message' do
       expect(file_collection.default_commit_message).to eq(<<EOM.chomp)
-Merge branch 'conflict-a' into 'conflict-b'
+Merge branch 'conflict-start' into 'conflict-resolvable'
 
 # Conflicts:
 #   files/ruby/popen.rb
diff --git a/spec/lib/gitlab/conflict/file_spec.rb b/spec/lib/gitlab/conflict/file_spec.rb
index 405b97686f5..643c5b31791 100644
--- a/spec/lib/gitlab/conflict/file_spec.rb
+++ b/spec/lib/gitlab/conflict/file_spec.rb
@@ -4,9 +4,9 @@ describe Gitlab::Conflict::File, lib: true do
   let(:project) { create(:project) }
   let(:repository) { project.repository }
   let(:rugged) { repository.rugged }
-  let(:their_commit) { rugged.branches['conflict-a'].target }
-  let(:our_commit) { rugged.branches['conflict-b'].target }
-  let(:merge_request) { create(:merge_request, source_branch: 'conflict-b', target_branch: 'conflict-a', source_project: project) }
+  let(:their_commit) { rugged.branches['conflict-start'].target }
+  let(:our_commit) { rugged.branches['conflict-resolvable'].target }
+  let(:merge_request) { create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) }
   let(:index) { rugged.merge_commits(our_commit, their_commit) }
   let(:conflict) { index.conflicts.last }
   let(:merge_file_result) { index.merge_file('files/ruby/regex.rb') }
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 3270b877c1a..b4741bd3bcb 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -741,4 +741,56 @@ describe MergeRequest, models: true do
       end
     end
   end
+
+  describe '#can_resolve_conflicts_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
+
+    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(merge_request.can_resolve_conflicts_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(merge_request.can_resolve_conflicts_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(merge_request.can_resolve_conflicts_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(merge_request.can_resolve_conflicts_in_ui?).to be_falsey
+    end
+
+    it 'returns a falsey value when the conflicts contain a file with ambiguous conflict markers' do
+      merge_request = create_merge_request('conflict-contains-conflict-markers')
+
+      expect(merge_request.can_resolve_conflicts_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(merge_request.can_resolve_conflicts_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(merge_request.can_resolve_conflicts_in_ui?).to be_truthy
+    end
+  end
 end
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 976b1f36270..edbbfc3c9e5 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -5,27 +5,31 @@ module TestEnv
 
   # When developing the seed repository, comment out the branch you will modify.
   BRANCH_SHA = {
-    'empty-branch'          => '7efb185',
-    'ends-with.json'        => '98b0d8b3',
-    'flatten-dir'           => 'e56497b',
-    'feature'               => '0b4bc9a',
-    'feature_conflict'      => 'bb5206f',
-    'fix'                   => '48f0be4',
-    'improve/awesome'       => '5937ac0',
-    'markdown'              => '0ed8c6c',
-    'lfs'                   => 'be93687',
-    'master'                => '5937ac0',
-    "'test'"                => 'e56497b',
-    'orphaned-branch'       => '45127a9',
-    'binary-encoding'       => '7b1cf43',
-    'gitattributes'         => '5a62481',
-    'conflict-a'            => 'dfdd207',
-    'conflict-b'            => '4e75a62',
-    'expand-collapse-diffs' => '4842455',
-    'expand-collapse-files' => '025db92',
-    'expand-collapse-lines' => '238e82d',
-    'video'                 => '8879059',
-    'crlf-diff'             => '5938907'
+    'empty-branch'                       => '7efb185',
+    'ends-with.json'                     => '98b0d8b3',
+    'flatten-dir'                        => 'e56497b',
+    'feature'                            => '0b4bc9a',
+    'feature_conflict'                   => 'bb5206f',
+    'fix'                                => '48f0be4',
+    'improve/awesome'                    => '5937ac0',
+    'markdown'                           => '0ed8c6c',
+    'lfs'                                => 'be93687',
+    'master'                             => '5937ac0',
+    "'test'"                             => 'e56497b',
+    'orphaned-branch'                    => '45127a9',
+    'binary-encoding'                    => '7b1cf43',
+    'gitattributes'                      => '5a62481',
+    'expand-collapse-diffs'              => '4842455',
+    'expand-collapse-files'              => '025db92',
+    'expand-collapse-lines'              => '238e82d',
+    'video'                              => '8879059',
+    'crlf-diff'                          => '5938907',
+    'conflict-start'                     => '14fa46b',
+    'conflict-resolvable'                => '1450cd6',
+    'conflict-binary-file'               => '259a6fb',
+    'conflict-contains-conflict-markers' => '5e0964c',
+    'conflict-missing-side'              => 'eb227b3',
+    'conflict-too-large'                 => '39fa04f',
   }
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily