Merge branch 'fix-conflicts-exception-for-submodules' into 'master'

Make sure ConflictsService does not raise for conflicting submodules

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

lib/gitlab/gitaly_client/conflicts_service.rb

@@ -25,10 +25,12 @@ module Gitlab
 
       def conflicts?
         list_conflict_files.any?
-      rescue GRPC::FailedPrecondition
-        # The server raises this exception when it encounters ConflictSideMissing, which
-        # means a conflict exists but its `theirs` or `ours` data is nil due to a non-existent
-        # file in one of the trees.
+      rescue GRPC::FailedPrecondition, GRPC::Unknown
+        # The server raises FailedPrecondition when it encounters
+        # ConflictSideMissing, which means a conflict exists but its `theirs` or
+        # `ours` data is nil due to a non-existent file in one of the trees.
+        #
+        # GRPC::Unknown comes from Rugged::ReferenceError and Rugged::OdbError.
         true
       end
 

spec/models/repository_spec.rb

@@ -431,6 +431,18 @@ describe Repository do
 
       it { is_expected.to be_falsey }
     end
+
+    context 'non merged branch' do
+      subject { repository.merged_to_root_ref?('fix') }
+
+      it { is_expected.to be_falsey }
+    end
+
+    context 'non existent branch' do
+      subject { repository.merged_to_root_ref?('non_existent_branch') }
+
+      it { is_expected.to be_nil }
+    end
   end
 
   describe '#can_be_merged?' do
@@ -452,17 +464,11 @@ describe Repository do
       it { is_expected.to be_falsey }
     end
 
-    context 'non merged branch' do
-      subject { repository.merged_to_root_ref?('fix') }
+    context 'submodule changes that confuse rugged' do
+      subject { repository.can_be_merged?('update-gitlab-shell-v-6-0-1', 'update-gitlab-shell-v-6-0-3') }
 
       it { is_expected.to be_falsey }
     end
-
-    context 'non existent branch' do
-      subject { repository.merged_to_root_ref?('non_existent_branch') }
-
-      it { is_expected.to be_nil }
-    end
   end
 
   describe '#commit' do

spec/services/merge_requests/conflicts/list_service_spec.rb

@@ -2,8 +2,8 @@ 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', merge_status: :unchecked) do |mr|
+    def create_merge_request(source_branch, target_branch = 'conflict-start')
+      create(:merge_request, source_branch: source_branch, target_branch: target_branch, merge_status: :unchecked) do |mr|
         mr.mark_as_unmergeable
       end
     end
@@ -84,5 +84,11 @@ describe MergeRequests::Conflicts::ListService do
 
       expect(service.can_be_resolved_in_ui?).to be_falsey
     end
+
+    it 'returns a falsey value when the conflict is in a submodule revision' do
+      merge_request = create_merge_request('update-gitlab-shell-v-6-0-3', 'update-gitlab-shell-v-6-0-1')
+
+      expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey
+    end
   end
 end

spec/support/helpers/test_env.rb

@@ -49,7 +49,9 @@ module TestEnv
     'add-pdf-file'                       => 'e774ebd',
     'squash-large-files'                 => '54cec52',
     'add-pdf-text-binary'                => '79faa7b',
-    'add_images_and_changes'             => '010d106'
+    'add_images_and_changes'             => '010d106',
+    'update-gitlab-shell-v-6-0-1'        => '2f61d70',
+    'update-gitlab-shell-v-6-0-3'        => 'de78448'
   }.freeze
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily