Merge branch 'retry-git-remove-commands-in-qa-tests' into 'master'

Retry remote Git operation in QA tests

Closes #217459

See merge request gitlab-org/gitlab!31755

qa/qa/git/repository.rb

@@ -12,6 +12,8 @@ module QA
   module Git
     class Repository
       include Scenario::Actable
+      include Support::Repeater
+
       RepositoryCommandError = Class.new(StandardError)
 
       attr_writer :use_lfs, :gpg_key_id
@@ -58,8 +60,8 @@ module QA
       end
 
       def clone(opts = '')
-        clone_result = run("git clone #{opts} #{uri} ./")
-        return clone_result.response unless clone_result.success
+        clone_result = run("git clone #{opts} #{uri} ./", max_attempts: 3)
+        return clone_result.response unless clone_result.success?
 
         enable_lfs_result = enable_lfs if use_lfs?
 
@@ -92,7 +94,7 @@ module QA
 
         if use_lfs?
           git_lfs_track_result = run(%Q{git lfs track #{name} --lockable})
-          return git_lfs_track_result.response unless git_lfs_track_result.success
+          return git_lfs_track_result.response unless git_lfs_track_result.success?
         end
 
         git_add_result = run(%Q{git add #{name}})
@@ -101,11 +103,11 @@ module QA
       end
 
       def delete_tag(tag_name)
-        run(%Q{git push origin --delete #{tag_name}}).to_s
+        run(%Q{git push origin --delete #{tag_name}}, max_attempts: 3).to_s
       end
 
       def commit(message)
-        run(%Q{git commit -m "#{message}"}).to_s
+        run(%Q{git commit -m "#{message}"}, max_attempts: 3).to_s
       end
 
       def commit_with_gpg(message)
@@ -113,7 +115,7 @@ module QA
       end
 
       def push_changes(branch = 'master')
-        run("git push #{uri} #{branch}").to_s
+        run("git push #{uri} #{branch}", max_attempts: 3).to_s
       end
 
       def merge(branch)
@@ -164,8 +166,8 @@ module QA
       def fetch_supported_git_protocol
         # ls-remote is one command known to respond to Git protocol v2 so we use
         # it to get output including the version reported via Git tracing
-        output = run("git ls-remote #{uri}", "GIT_TRACE_PACKET=1")
-        output.response[/git< version (\d+)/, 1] || 'unknown'
+        result = run("git ls-remote #{uri}", env: "GIT_TRACE_PACKET=1", max_attempts: 3)
+        result.response[/git< version (\d+)/, 1] || 'unknown'
       end
 
       def try_add_credentials_to_netrc
@@ -182,9 +184,12 @@ module QA
 
       alias_method :use_lfs?, :use_lfs
 
-      Result = Struct.new(:success, :response) do
-        alias_method :success?, :success
+      Result = Struct.new(:command, :exitstatus, :response) do
         alias_method :to_s, :response
+
+        def success?
+          exitstatus.zero?
+        end
       end
 
       def add_credentials?
@@ -209,19 +214,26 @@ module QA
         touch_gitconfig_result.to_s + git_lfs_install_result.to_s
       end
 
-      def run(command_str, *extra_env)
-        command = [env_vars, *extra_env, command_str, '2>&1'].compact.join(' ')
-        Runtime::Logger.debug "Git: pwd=[#{Dir.pwd}], command=[#{command}]"
+      def run(command_str, env: [], max_attempts: 1)
+        command = [env_vars, *env, command_str, '2>&1'].compact.join(' ')
+        result = nil
 
-        output, status = Open3.capture2e(command)
-        output.chomp!
-        Runtime::Logger.debug "Git: output=[#{output}], exitstatus=[#{status.exitstatus}]"
+        repeat_until(max_attempts: max_attempts, raise_on_failure: false) do
+          Runtime::Logger.debug "Git: pwd=[#{Dir.pwd}], command=[#{command}]"
+          output, status = Open3.capture2e(command)
+          output.chomp!
+          Runtime::Logger.debug "Git: output=[#{output}], exitstatus=[#{status.exitstatus}]"
+
+          result = Result.new(command, status.exitstatus, output)
+
+          result.success?
+        end
 
-        unless status.success?
-          raise RepositoryCommandError, "The command #{command} failed (#{status.exitstatus}) with the following output:\n#{output}"
+        unless result.success?
+          raise RepositoryCommandError, "The command #{result.command} failed (#{result.exitstatus}) with the following output:\n#{result.response}"
         end
 
-        Result.new(status.exitstatus == 0, output)
+        result
       end
 
       def default_credentials

qa/spec/git/repository_spec.rb

@@ -3,55 +3,119 @@
 describe QA::Git::Repository do
   include Helpers::StubENV
 
-  shared_context 'git directory' do
-    let(:repository) { described_class.new }
+  shared_context 'unresolvable git directory' do
+    let(:repo_uri) { 'http://foo/bar.git' }
+    let(:repo_uri_with_credentials) { 'http://root@foo/bar.git' }
+    let(:repository) { described_class.new.tap { |r| r.uri = repo_uri } }
     let(:tmp_git_dir) { Dir.mktmpdir }
     let(:tmp_netrc_dir) { Dir.mktmpdir }
 
     before do
       stub_env('GITLAB_USERNAME', 'root')
-      cd_empty_temp_directory
-      set_bad_uri
-
       allow(repository).to receive(:tmp_home_dir).and_return(tmp_netrc_dir)
     end
 
+    around do |example|
+      FileUtils.cd(tmp_git_dir) do
+        example.run
+      end
+    end
+
     after do
-      # Switch to a safe dir before deleting tmp dirs to avoid dir access errors
-      FileUtils.cd __dir__
       FileUtils.remove_entry_secure(tmp_git_dir, true)
       FileUtils.remove_entry_secure(tmp_netrc_dir, true)
     end
+  end
 
-    def cd_empty_temp_directory
-      FileUtils.cd tmp_git_dir
+  shared_examples 'command with retries' do
+    let(:extra_args) { {} }
+    let(:result_output) { +'Command successful' }
+    let(:result) { described_class::Result.new(any_args, 0, result_output) }
+    let(:command_return) { result_output }
+
+    context 'when command is successful' do
+      it 'returns the #run command Result output' do
+        expect(repository).to receive(:run).with(command, extra_args.merge(max_attempts: 3)).and_return(result)
+
+        expect(call_method).to eq(command_return)
+      end
     end
 
-    def set_bad_uri
-      repository.uri = 'http://foo/bar.git'
+    context 'when command is not successful the first time' do
+      context 'and retried command is successful' do
+        it 'retries the command twice and returns the successful #run command Result output' do
+          expect(Open3).to receive(:capture2e).and_return([+'', double(exitstatus: 1)]).twice
+          expect(Open3).to receive(:capture2e).and_return([result_output, double(exitstatus: 0)])
+
+          expect(call_method).to eq(command_return)
+        end
+      end
+
+      context 'and retried command is not successful after 3 attempts' do
+        it 'raises a RepositoryCommandError exception' do
+          expect(Open3).to receive(:capture2e).and_return([+'FAILURE', double(exitstatus: 42)]).exactly(3).times
+
+          expect { call_method }.to raise_error(described_class::RepositoryCommandError, /The command .* failed \(42\) with the following output:\nFAILURE/)
+        end
+      end
     end
   end
 
   context 'with default credentials' do
-    include_context 'git directory' do
+    include_context 'unresolvable git directory' do
       before do
         repository.use_default_credentials
       end
     end
 
     describe '#clone' do
-      it 'is unable to resolve host' do
-        expect { repository.clone }.to raise_error(described_class::RepositoryCommandError, /The command .* failed \(128\) with the following output/)
+      let(:opts) { '' }
+      let(:call_method) { repository.clone }
+      let(:command) { "git clone #{opts} #{repo_uri_with_credentials} ./" }
+
+      context 'when no opts is given' do
+        it_behaves_like 'command with retries'
+      end
+
+      context 'when opts is given' do
+        let(:opts) { '--depth 1' }
+
+        it_behaves_like 'command with retries' do
+          let(:call_method) { repository.clone(opts) }
+        end
+      end
+    end
+
+    describe '#shallow_clone' do
+      it_behaves_like 'command with retries' do
+        let(:call_method) { repository.shallow_clone }
+        let(:command) { "git clone --depth 1 #{repo_uri_with_credentials} ./" }
+      end
+    end
+
+    describe '#delete_tag' do
+      it_behaves_like 'command with retries' do
+        let(:tag_name) { 'v1.0' }
+        let(:call_method) { repository.delete_tag(tag_name) }
+        let(:command) { "git push origin --delete #{tag_name}" }
       end
     end
 
     describe '#push_changes' do
-      before do
-        `git init` # need a repo to push from
+      let(:branch) { 'master' }
+      let(:call_method) { repository.push_changes }
+      let(:command) { "git push #{repo_uri_with_credentials} #{branch}" }
+
+      context 'when no branch is given' do
+        it_behaves_like 'command with retries'
       end
 
-      it 'fails to push changes' do
-        expect { repository.push_changes }.to raise_error(described_class::RepositoryCommandError, /The command .* failed \(1\) with the following output/)
+      context 'when branch is given' do
+        let(:branch) { 'my-branch' }
+
+        it_behaves_like 'command with retries' do
+          let(:call_method) { repository.push_changes(branch) }
+        end
       end
     end
 
@@ -59,6 +123,7 @@ describe QA::Git::Repository do
       [0, 1, 2].each do |version|
         it "configures git to use protocol version #{version}" do
           expect(repository).to receive(:run).with("git config protocol.version #{version}")
+
           repository.git_protocol = version
         end
       end
@@ -69,21 +134,31 @@ describe QA::Git::Repository do
     end
 
     describe '#fetch_supported_git_protocol' do
-      result = Struct.new(:response)
+      let(:call_method) { repository.fetch_supported_git_protocol }
+
+      it_behaves_like 'command with retries' do
+        let(:command) { "git ls-remote #{repo_uri_with_credentials}" }
+        let(:result_output) { +'packet: git< version 2' }
+        let(:command_return) { '2' }
+        let(:extra_args) { { env: "GIT_TRACE_PACKET=1" } }
+      end
 
       it "reports the detected version" do
-        expect(repository).to receive(:run).and_return(result.new("packet: git< version 2"))
-        expect(repository.fetch_supported_git_protocol).to eq('2')
+        expect(repository).to receive(:run).and_return(described_class::Result.new(any_args, 0, "packet: git< version 2"))
+
+        expect(call_method).to eq('2')
       end
 
       it 'reports unknown if version is unknown' do
-        expect(repository).to receive(:run).and_return(result.new("packet: git< version -1"))
-        expect(repository.fetch_supported_git_protocol).to eq('unknown')
+        expect(repository).to receive(:run).and_return(described_class::Result.new(any_args, 0, "packet: git< version -1"))
+
+        expect(call_method).to eq('unknown')
       end
 
       it 'reports unknown if content does not identify a version' do
-        expect(repository).to receive(:run).and_return(result.new("foo"))
-        expect(repository.fetch_supported_git_protocol).to eq('unknown')
+        expect(repository).to receive(:run).and_return(described_class::Result.new(any_args, 0, "foo"))
+
+        expect(call_method).to eq('unknown')
       end
     end
 
@@ -96,7 +171,7 @@ describe QA::Git::Repository do
   end
 
   context 'with specific credentials' do
-    include_context 'git directory'
+    include_context 'unresolvable git directory'
 
     context 'before setting credentials' do
       it 'does not add credentials to .netrc' do