Stop mocking replicate repository to return a bool

It was assumed that replicate repository would return a bool, but the
response is actually empty

app/services/projects/update_repository_storage_service.rb

@@ -23,7 +23,7 @@ module Projects
 
       success
 
-    rescue Error => e
+    rescue Error, ArgumentError, Gitlab::Git::BaseError => e
       project.update(repository_read_only: false)
 
       Gitlab::ErrorTracking.track_exception(e, project_path: project.full_path)
@@ -58,10 +58,7 @@ module Projects
                                                    raw_repository.gl_repository,
                                                    full_path)
 
-      unless new_repository.replicate(raw_repository)
-        raise Error, s_('UpdateRepositoryStorage|Failed to fetch %{type} repository as mirror') % { type: type.name }
-      end
-
+      new_repository.replicate(raw_repository)
       new_checksum = new_repository.checksum
 
       if checksum != new_checksum

locale/gitlab.pot

@@ -21370,9 +21370,6 @@ msgstr ""
 msgid "UpdateRepositoryStorage|Error moving repository storage for %{project_full_path} - %{message}"
 msgstr ""
 
-msgid "UpdateRepositoryStorage|Failed to fetch %{type} repository as mirror"
-msgstr ""
-
 msgid "UpdateRepositoryStorage|Failed to verify %{type} repository checksum from %{old} to %{new}"
 msgstr ""
 

spec/lib/gitlab/gitaly_client/repository_service_spec.rb

@@ -304,7 +304,6 @@ describe Gitlab::GitalyClient::RepositoryService do
       expect_any_instance_of(Gitaly::RepositoryService::Stub)
         .to receive(:replicate_repository)
         .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
-        .and_return(double(value: true))
 
       client.replicate(source_repository)
     end

spec/services/projects/fork_service_spec.rb

@@ -312,8 +312,9 @@ describe Projects::ForkService do
 
       # Stub everything required to move a project to a Gitaly shard that does not exist
       stub_storage_settings('test_second_storage' => { 'path' => 'tmp/tests/second_storage' })
-      allow_any_instance_of(Gitlab::Git::Repository).to receive(:replicate).and_return(true)
-      allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum).and_return(::Gitlab::Git::BLANK_SHA)
+      allow_any_instance_of(Gitlab::Git::Repository).to receive(:replicate)
+      allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum)
+        .and_return(::Gitlab::Git::BLANK_SHA)
 
       Projects::UpdateRepositoryStorageService.new(project).execute('test_second_storage')
       fork_after_move = fork_project(project)

spec/services/projects/update_repository_storage_service_spec.rb

@@ -33,7 +33,7 @@ describe Projects::UpdateRepositoryStorageService do
           end
 
           expect(project_repository_double).to receive(:replicate)
-            .with(project.repository.raw).and_return(true)
+            .with(project.repository.raw)
           expect(project_repository_double).to receive(:checksum)
             .and_return(checksum)
 
@@ -49,16 +49,18 @@ describe Projects::UpdateRepositoryStorageService do
 
       context 'when the project is already on the target storage' do
         it 'bails out and does nothing' do
-          expect do
-            subject.execute(project.repository_storage)
-          end.to raise_error(ArgumentError, /repository and source have the same storage/)
+          result = subject.execute(project.repository_storage)
+
+          expect(result[:status]).to eq(:error)
+          expect(result[:message]).to match(/repository and source have the same storage/)
         end
       end
 
       context 'when the move fails' do
         it 'unmarks the repository as read-only without updating the repository storage' do
           expect(project_repository_double).to receive(:replicate)
-            .with(project.repository.raw).and_return(false)
+            .with(project.repository.raw)
+            .and_raise(Gitlab::Git::CommandError)
           expect(GitlabShellWorker).not_to receive(:perform_async)
 
           result = subject.execute('test_second_storage')
@@ -72,7 +74,7 @@ describe Projects::UpdateRepositoryStorageService do
       context 'when the checksum does not match' do
         it 'unmarks the repository as read-only without updating the repository storage' do
           expect(project_repository_double).to receive(:replicate)
-            .with(project.repository.raw).and_return(true)
+            .with(project.repository.raw)
           expect(project_repository_double).to receive(:checksum)
             .and_return('not matching checksum')
           expect(GitlabShellWorker).not_to receive(:perform_async)
@@ -90,7 +92,7 @@ describe Projects::UpdateRepositoryStorageService do
 
         it 'leaves the pool' do
           expect(project_repository_double).to receive(:replicate)
-            .with(project.repository.raw).and_return(true)
+            .with(project.repository.raw)
           expect(project_repository_double).to receive(:checksum)
             .and_return(checksum)
 

spec/support/shared_examples/services/projects/update_repository_storage_service_shared_examples.rb

@@ -24,13 +24,11 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
     before do
       allow(project_repository_double).to receive(:replicate)
         .with(project.repository.raw)
-        .and_return(true)
       allow(project_repository_double).to receive(:checksum)
         .and_return(project_repository_checksum)
 
       allow(repository_double).to receive(:replicate)
         .with(repository.raw)
-        .and_return(true)
       allow(repository_double).to receive(:checksum)
         .and_return(repository_checksum)
     end
@@ -83,9 +81,10 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
 
   context 'when the project is already on the target storage' do
     it 'bails out and does nothing' do
-      expect do
-        subject.execute(project.repository_storage)
-      end.to raise_error(ArgumentError, /repository and source have the same storage/)
+      result = subject.execute(project.repository_storage)
+
+      expect(result[:status]).to eq(:error)
+      expect(result[:message]).to match(/repository and source have the same storage/)
     end
   end
 
@@ -93,13 +92,12 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
     it 'unmarks the repository as read-only without updating the repository storage' do
       allow(project_repository_double).to receive(:replicate)
         .with(project.repository.raw)
-        .and_return(true)
       allow(project_repository_double).to receive(:checksum)
         .and_return(project_repository_checksum)
 
       allow(repository_double).to receive(:replicate)
         .with(repository.raw)
-        .and_return(false)
+        .and_raise(Gitlab::Git::CommandError)
 
       expect(GitlabShellWorker).not_to receive(:perform_async)
 
@@ -114,12 +112,12 @@ RSpec.shared_examples 'moves repository to another storage' do |repository_type|
   context "when the checksum of the #{repository_type} repository does not match" do
     it 'unmarks the repository as read-only without updating the repository storage' do
       allow(project_repository_double).to receive(:replicate)
-        .with(project.repository.raw).and_return(true)
+        .with(project.repository.raw)
       allow(project_repository_double).to receive(:checksum)
         .and_return(project_repository_checksum)
 
       allow(repository_double).to receive(:replicate)
-        .with(repository.raw).and_return(true)
+        .with(repository.raw)
       allow(repository_double).to receive(:checksum)
         .and_return('not matching checksum')