Merge branch 'toon-first-refactor-gitaly-test' into 'master'

First iteration in refactoring Gitaly Setup in test

See merge request gitlab-org/gitlab!76789

lib/gitlab/task_helpers.rb

@@ -190,6 +190,9 @@ module Gitlab
     end
 
     def checkout_version(version, target_dir)
+      # Explicitly setting the git protocol version to v2 allows older Git binaries
+      # to do have a shallow clone obtain objects by object ID.
+      run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} config protocol.version 2])
       run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} fetch --quiet origin #{version}])
       run_command!(%W[#{Gitlab.config.git.bin_path} -C #{target_dir} checkout -f --quiet FETCH_HEAD --])
     end

lib/tasks/gitlab/gitaly.rake

@@ -37,8 +37,8 @@ namespace :gitlab do
       raise
     end
 
-    desc 'GitLab | Gitaly | Install or upgrade gitaly'
-    task :install, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args|
+    desc 'GitLab | Gitaly | Clone and checkout gitaly'
+    task :clone, [:dir, :storage_path, :repo] => :gitlab_environment do |t, args|
       warn_user_is_not_gitlab
 
       unless args.dir.present? && args.storage_path.present?
@@ -51,6 +51,11 @@ Usage: rake "gitlab:gitaly:install[/installation/dir,/storage/path]")
       version = Gitlab::GitalyClient.expected_server_version
 
       checkout_or_clone_version(version: version, repo: args.repo, target_dir: args.dir, clone_opts: %w[--depth 1])
+    end
+
+    desc 'GitLab | Gitaly | Install or upgrade gitaly'
+    task :install, [:dir, :storage_path, :repo] => [:gitlab_environment, 'gitlab:gitaly:clone'] do |t, args|
+      warn_user_is_not_gitlab
 
       storage_paths = { 'default' => args.storage_path }
       Gitlab::SetupHelper::Gitaly.create_configuration(args.dir, storage_paths)

spec/support/helpers/gitaly_setup.rb

@@ -18,8 +18,12 @@ module GitalySetup
     Logger.new($stdout, level: level, formatter: ->(_, _, _, msg) { msg })
   end
 
+  def expand_path(path)
+    File.expand_path(path, File.join(__dir__, '../../..'))
+  end
+
   def tmp_tests_gitaly_dir
-    File.expand_path('../../../tmp/tests/gitaly', __dir__)
+    expand_path('tmp/tests/gitaly')
   end
 
   def tmp_tests_gitaly_bin_dir
@@ -27,11 +31,11 @@ module GitalySetup
   end
 
   def tmp_tests_gitlab_shell_dir
-    File.expand_path('../../../tmp/tests/gitlab-shell', __dir__)
+    expand_path('tmp/tests/gitlab-shell')
   end
 
   def rails_gitlab_shell_secret
-    File.expand_path('../../../.gitlab_shell_secret', __dir__)
+    expand_path('.gitlab_shell_secret')
   end
 
   def gemfile
@@ -48,7 +52,7 @@ module GitalySetup
 
   def env
     {
-      'HOME' => File.expand_path('tmp/tests'),
+      'HOME' => expand_path('tmp/tests'),
       'GEM_PATH' => Gem.path.join(':'),
       'BUNDLE_APP_CONFIG' => File.join(gemfile_dir, '.bundle'),
       'BUNDLE_INSTALL_FLAGS' => nil,
@@ -67,7 +71,7 @@ module GitalySetup
     system('bundle config set --local retry 3', chdir: gemfile_dir)
 
     if ENV['CI']
-      bundle_path = File.expand_path('../../../vendor/gitaly-ruby', __dir__)
+      bundle_path = expand_path('vendor/gitaly-ruby')
       system('bundle', 'config', 'set', '--local', 'path', bundle_path, chdir: gemfile_dir)
     end
   end
@@ -154,7 +158,7 @@ module GitalySetup
 
     LOGGER.debug "Checking gitaly-ruby bundle...\n"
     out = ENV['CI'] ? $stdout : '/dev/null'
-    abort 'bundle check failed' unless system(env, 'bundle', 'check', out: out, chdir: File.dirname(gemfile))
+    abort 'bundle check failed' unless system(env, 'bundle', 'check', out: out, chdir: gemfile_dir)
   end
 
   def read_socket_path(service)

spec/support/helpers/test_env.rb

@@ -594,6 +594,8 @@ module TestEnv
     # Not a git SHA, so return early
     return false unless expected_version =~ ::Gitlab::Git::COMMIT_ID
 
+    return false unless Dir.exist?(component_folder)
+
     sha, exit_status = Gitlab::Popen.popen(%W(#{Gitlab.config.git.bin_path} rev-parse HEAD), component_folder)
     return false if exit_status != 0
 

spec/tasks/gitlab/gitaly_rake_spec.rb

@@ -7,26 +7,26 @@ RSpec.describe 'gitlab:gitaly namespace rake task', :silence_stdout do
     Rake.application.rake_require 'tasks/gitlab/gitaly'
   end
 
-  describe 'install' do
-    let(:repo) { 'https://gitlab.com/gitlab-org/gitaly.git' }
-    let(:clone_path) { Rails.root.join('tmp/tests/gitaly').to_s }
-    let(:storage_path) { Rails.root.join('tmp/tests/repositories').to_s }
-    let(:version) { File.read(Rails.root.join(Gitlab::GitalyClient::SERVER_VERSION_FILE)).chomp }
+  let(:repo) { 'https://gitlab.com/gitlab-org/gitaly.git' }
+  let(:clone_path) { Rails.root.join('tmp/tests/gitaly').to_s }
+  let(:storage_path) { Rails.root.join('tmp/tests/repositories').to_s }
+  let(:version) { File.read(Rails.root.join(Gitlab::GitalyClient::SERVER_VERSION_FILE)).chomp }
 
-    subject { run_rake_task('gitlab:gitaly:install', clone_path, storage_path) }
+  describe 'clone' do
+    subject { run_rake_task('gitlab:gitaly:clone', clone_path, storage_path) }
 
     context 'no dir given' do
       it 'aborts and display a help message' do
         # avoid writing task output to spec progress
         allow($stderr).to receive :write
-        expect { run_rake_task('gitlab:gitaly:install') }.to raise_error /Please specify the directory where you want to install gitaly and the path for the default storage/
+        expect { run_rake_task('gitlab:gitaly:clone') }.to raise_error /Please specify the directory where you want to install gitaly and the path for the default storage/
       end
     end
 
     context 'no storage path given' do
       it 'aborts and display a help message' do
         allow($stderr).to receive :write
-        expect { run_rake_task('gitlab:gitaly:install', clone_path) }.to raise_error /Please specify the directory where you want to install gitaly and the path for the default storage/
+        expect { run_rake_task('gitlab:gitaly:clone', clone_path) }.to raise_error /Please specify the directory where you want to install gitaly and the path for the default storage/
       end
     end
 
@@ -40,11 +40,6 @@ RSpec.describe 'gitlab:gitaly namespace rake task', :silence_stdout do
     end
 
     describe 'checkout or clone' do
-      before do
-        stub_env('CI', false)
-        expect(Dir).to receive(:chdir).with(clone_path)
-      end
-
       it 'calls checkout_or_clone_version with the right arguments' do
         expect(main_object)
           .to receive(:checkout_or_clone_version).with(version: version, repo: repo, target_dir: clone_path, clone_opts: %w[--depth 1])
@@ -52,6 +47,10 @@ RSpec.describe 'gitlab:gitaly namespace rake task', :silence_stdout do
         subject
       end
     end
+  end
+
+  describe 'install' do
+    subject { run_rake_task('gitlab:gitaly:install', clone_path, storage_path) }
 
     describe 'gmake/make' do
       before do
@@ -62,10 +61,6 @@ RSpec.describe 'gitlab:gitaly namespace rake task', :silence_stdout do
       end
 
       context 'gmake is available' do
-        before do
-          expect(main_object).to receive(:checkout_or_clone_version)
-        end
-
         it 'calls gmake in the gitaly directory' do
           expect(Gitlab::Popen).to receive(:popen)
             .with(%w[which gmake])
@@ -93,7 +88,6 @@ RSpec.describe 'gitlab:gitaly namespace rake task', :silence_stdout do
 
       context 'gmake is not available' do
         before do
-          expect(main_object).to receive(:checkout_or_clone_version)
           expect(Gitlab::Popen).to receive(:popen)
             .with(%w[which gmake])
             .and_return(['', 42])

spec/tasks/gitlab/task_helpers_spec.rb

@@ -71,6 +71,8 @@ RSpec.describe Gitlab::TaskHelpers do
 
   describe '#checkout_version' do
     it 'clones the repo in the target dir' do
+      expect(subject)
+        .to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} config protocol.version 2])
       expect(subject)
         .to receive(:run_command!).with(%W[#{Gitlab.config.git.bin_path} -C #{clone_path} fetch --quiet origin #{tag}])
       expect(subject)