diff options
Diffstat (limited to 'spec/lib/gitlab/git')
| -rw-r--r-- | spec/lib/gitlab/git/blob_spec.rb | 17 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/branch_spec.rb | 8 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/commit_spec.rb | 31 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/committer_with_hooks_spec.rb | 156 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/diff_spec.rb | 135 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/diff_stats_collection_spec.rb | 32 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/gitlab_projects_spec.rb | 321 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/hook_env_spec.rb | 20 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/hook_spec.rb | 111 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/hooks_service_spec.rb | 50 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/index_spec.rb | 239 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/popen_spec.rb | 179 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/push_spec.rb | 166 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/repository_spec.rb | 970 | ||||
| -rw-r--r-- | spec/lib/gitlab/git/user_spec.rb | 15 |
15 files changed, 583 insertions, 1867 deletions
diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index ea49502ae2e..b243f0dacae 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -4,6 +4,9 @@ require "spec_helper" describe Gitlab::Git::Blob, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:rugged) do + Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) + end describe 'initialize' do let(:blob) { Gitlab::Git::Blob.new(name: 'test') } @@ -139,9 +142,7 @@ describe Gitlab::Git::Blob, :seed_helper do it 'limits the size of a large file' do blob_size = Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE + 1 buffer = Array.new(blob_size, 0) - rugged_blob = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Rugged::Blob.from_buffer(repository.rugged, buffer.join('')) - end + rugged_blob = Rugged::Blob.from_buffer(rugged, buffer.join('')) blob = Gitlab::Git::Blob.raw(repository, rugged_blob) expect(blob.size).to eq(blob_size) @@ -156,9 +157,7 @@ describe Gitlab::Git::Blob, :seed_helper do context 'when sha references a tree' do it 'returns nil' do - tree = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.rev_parse('master^{tree}') - end + tree = rugged.rev_parse('master^{tree}') blob = Gitlab::Git::Blob.raw(repository, tree.oid) @@ -262,11 +261,7 @@ describe Gitlab::Git::Blob, :seed_helper do end describe '.batch_lfs_pointers' do - let(:tree_object) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.rev_parse('master^{tree}') - end - end + let(:tree_object) { rugged.rev_parse('master^{tree}') } let(:non_lfs_blob) do Gitlab::Git::Blob.find( diff --git a/spec/lib/gitlab/git/branch_spec.rb b/spec/lib/gitlab/git/branch_spec.rb index 79ccbb79966..0df282d0ae3 100644 --- a/spec/lib/gitlab/git/branch_spec.rb +++ b/spec/lib/gitlab/git/branch_spec.rb @@ -3,9 +3,7 @@ require "spec_helper" describe Gitlab::Git::Branch, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:rugged) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged - end + Rugged::Repository.new(File.join(TestEnv.repos_path, repository.relative_path)) end subject { repository.branches } @@ -74,9 +72,7 @@ describe Gitlab::Git::Branch, :seed_helper do Gitlab::Git.committer_hash(email: user.email, name: user.name) end let(:params) do - parents = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - [repository.rugged.head.target] - end + parents = [rugged.head.target] tree = parents.first.tree { diff --git a/spec/lib/gitlab/git/commit_spec.rb b/spec/lib/gitlab/git/commit_spec.rb index 2718a3c5e49..9ef27081f98 100644 --- a/spec/lib/gitlab/git/commit_spec.rb +++ b/spec/lib/gitlab/git/commit_spec.rb @@ -1,19 +1,17 @@ require "spec_helper" describe Gitlab::Git::Commit, :seed_helper do + include GitHelpers + let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } - let(:rugged_commit) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.lookup(SeedRepo::Commit::ID) - end + let(:rugged_repo) do + Rugged::Repository.new(File.join(TestEnv.repos_path, TEST_REPO_PATH)) end + let(:commit) { described_class.find(repository, SeedRepo::Commit::ID) } + let(:rugged_commit) { rugged_repo.lookup(SeedRepo::Commit::ID) } + describe "Commit info" do before do - repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - end - @committer = { email: 'mike@smith.com', name: "Mike Smith", @@ -26,12 +24,12 @@ describe Gitlab::Git::Commit, :seed_helper do time: Time.now } - @parents = [repo.head.target] + @parents = [rugged_repo.head.target] @gitlab_parents = @parents.map { |c| described_class.find(repository, c.oid) } @tree = @parents.first.tree sha = Rugged::Commit.create( - repo, + rugged_repo, author: @author, committer: @committer, tree: @tree, @@ -40,7 +38,7 @@ describe Gitlab::Git::Commit, :seed_helper do update_ref: "HEAD" ) - @raw_commit = repo.lookup(sha) + @raw_commit = rugged_repo.lookup(sha) @commit = described_class.find(repository, sha) end @@ -61,10 +59,7 @@ describe Gitlab::Git::Commit, :seed_helper do after do # Erase the new commit so other tests get the original repo - repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - end - repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) + rugged_repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end end @@ -120,9 +115,7 @@ describe Gitlab::Git::Commit, :seed_helper do describe '.find' do it "should return first head commit if without params" do expect(described_class.last(repository).id).to eq( - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.head.target.oid - end + rugged_repo.head.target.oid ) end diff --git a/spec/lib/gitlab/git/committer_with_hooks_spec.rb b/spec/lib/gitlab/git/committer_with_hooks_spec.rb deleted file mode 100644 index c7626058acd..00000000000 --- a/spec/lib/gitlab/git/committer_with_hooks_spec.rb +++ /dev/null @@ -1,156 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::CommitterWithHooks, :seed_helper do - # TODO https://gitlab.com/gitlab-org/gitaly/issues/1234 - skip 'needs to be moved to gitaly-ruby test suite' do - shared_examples 'calling wiki hooks' do - let(:project) { create(:project) } - let(:user) { project.owner } - let(:project_wiki) { ProjectWiki.new(project, user) } - let(:wiki) { project_wiki.wiki } - let(:options) do - { - id: user.id, - username: user.username, - name: user.name, - email: user.email, - message: 'commit message' - } - end - - subject { described_class.new(wiki, options) } - - before do - project_wiki.create_page('home', 'test content') - end - - shared_examples 'failing pre-receive hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([false, '']) - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('update') - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') - end - - it 'raises exception' do - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - end - - it 'does not create a new commit inside the repository' do - current_rev = find_current_rev - - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - - expect(current_rev).to eq find_current_rev - end - end - - shared_examples 'failing update hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([false, '']) - expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive') - end - - it 'raises exception' do - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - end - - it 'does not create a new commit inside the repository' do - current_rev = find_current_rev - - expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError) - - expect(current_rev).to eq find_current_rev - end - end - - shared_examples 'failing post-receive hook' do - before do - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([true, '']) - expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('post-receive').and_return([false, '']) - end - - it 'does not raise exception' do - expect { subject.commit }.not_to raise_error - end - - it 'creates the commit' do - current_rev = find_current_rev - - subject.commit - - expect(current_rev).not_to eq find_current_rev - end - end - - shared_examples 'when hooks call succceeds' do - let(:hook) { double(:hook) } - - it 'calls the three hooks' do - expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - expect(hook).to receive(:trigger).exactly(3).times.and_return([true, nil]) - - subject.commit - end - - it 'creates the commit' do - current_rev = find_current_rev - - subject.commit - - expect(current_rev).not_to eq find_current_rev - end - end - - context 'when creating a page' do - before do - project_wiki.create_page('index', 'test content') - end - - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end - - context 'when updating a page' do - before do - project_wiki.update_page(find_page('home'), content: 'some other content', format: :markdown) - end - - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end - - context 'when deleting a page' do - before do - project_wiki.delete_page(find_page('home')) - end - - it_behaves_like 'failing pre-receive hook' - it_behaves_like 'failing update hook' - it_behaves_like 'failing post-receive hook' - it_behaves_like 'when hooks call succceeds' - end - - def find_current_rev - wiki.gollum_wiki.repo.commits.first&.sha - end - - def find_page(name) - wiki.page(title: name) - end - end - - context 'when Gitaly is enabled' do - it_behaves_like 'calling wiki hooks' - end - - context 'when Gitaly is disabled', :disable_gitaly do - it_behaves_like 'calling wiki hooks' - end - end -end diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb index 87d9fcee39e..8a4415506c4 100644 --- a/spec/lib/gitlab/git/diff_spec.rb +++ b/spec/lib/gitlab/git/diff_spec.rb @@ -2,12 +2,24 @@ require "spec_helper" describe Gitlab::Git::Diff, :seed_helper do let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:gitaly_diff) do + Gitlab::GitalyClient::Diff.new( + from_path: '.gitmodules', + to_path: '.gitmodules', + old_mode: 0100644, + new_mode: 0100644, + from_id: '0792c58905eff3432b721f8c4a64363d8e28d9ae', + to_id: 'efd587ccb47caf5f31fc954edb21f0a713d9ecc3', + overflow_marker: false, + collapsed: false, + too_large: false, + patch: "@@ -4,3 +4,6 @@\n [submodule \"gitlab-shell\"]\n \tpath = gitlab-shell\n \turl = https://github.com/gitlabhq/gitlab-shell.git\n+[submodule \"gitlab-grack\"]\n+\tpath = gitlab-grack\n+\turl = https://gitlab.com/gitlab-org/gitlab-grack.git\n" + ) + end before do @raw_diff_hash = { diff: <<EOT.gsub(/^ {8}/, "").sub(/\n$/, ""), - --- a/.gitmodules - +++ b/.gitmodules @@ -4,3 +4,6 @@ [submodule "gitlab-shell"] \tpath = gitlab-shell @@ -26,12 +38,6 @@ EOT deleted_file: false, too_large: false } - - # TODO use a Gitaly diff object instead - @rugged_diff = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.diff("5937ac0a7beb003549fc5fd26fc247adbce4a52e^", "5937ac0a7beb003549fc5fd26fc247adbce4a52e", paths: - [".gitmodules"]).patches.first - end end describe '.new' do @@ -58,9 +64,22 @@ EOT end end - context 'using a Rugged::Patch' do + context 'using a GitalyClient::Diff' do + let(:gitaly_diff) do + Gitlab::GitalyClient::Diff.new( + to_path: ".gitmodules", + from_path: ".gitmodules", + old_mode: 0100644, + new_mode: 0100644, + from_id: '357406f3075a57708d0163752905cc1576fceacc', + to_id: '8e5177d718c561d36efde08bad36b43687ee6bf0', + patch: raw_patch + ) + end + let(:diff) { described_class.new(gitaly_diff) } + context 'with a small diff' do - let(:diff) { described_class.new(@rugged_diff) } + let(:raw_patch) { @raw_diff_hash[:diff] } it 'initializes the diff' do expect(diff.to_hash).to eq(@raw_diff_hash) @@ -72,28 +91,20 @@ EOT end context 'using a diff that is too large' do - it 'prunes the diff' do - expect_any_instance_of(String).to receive(:bytesize) - .and_return(1024 * 1024 * 1024) - - diff = described_class.new(@rugged_diff) + let(:raw_patch) { 'a' * 204800 } + it 'prunes the diff' do expect(diff.diff).to be_empty expect(diff).to be_too_large end end context 'using a collapsable diff that is too large' do - before do - # The patch total size is 200, with lines between 21 and 54. - # This is a quick-and-dirty way to test this. Ideally, a new patch is - # added to the test repo with a size that falls between the real limits. - stub_const("#{described_class}::SIZE_LIMIT", 150) - stub_const("#{described_class}::COLLAPSE_LIMIT", 100) - end + let(:raw_patch) { 'a' * 204800 } it 'prunes the diff as a large diff instead of as a collapsed diff' do - diff = described_class.new(@rugged_diff, expanded: false) + gitaly_diff.too_large = true + diff = described_class.new(gitaly_diff, expanded: false) expect(diff.diff).to be_empty expect(diff).to be_too_large @@ -101,54 +112,6 @@ EOT end end - context 'using a large binary diff' do - it 'does not prune the diff' do - expect_any_instance_of(Rugged::Diff::Delta).to receive(:binary?) - .and_return(true) - - diff = described_class.new(@rugged_diff) - - expect(diff.diff).not_to be_empty - end - end - end - - context 'using a GitalyClient::Diff' do - let(:diff) do - described_class.new( - Gitlab::GitalyClient::Diff.new( - to_path: ".gitmodules", - from_path: ".gitmodules", - old_mode: 0100644, - new_mode: 0100644, - from_id: '357406f3075a57708d0163752905cc1576fceacc', - to_id: '8e5177d718c561d36efde08bad36b43687ee6bf0', - patch: raw_patch - ) - ) - end - - context 'with a small diff' do - let(:raw_patch) { @raw_diff_hash[:diff] } - - it 'initializes the diff' do - expect(diff.to_hash).to eq(@raw_diff_hash) - end - - it 'does not prune the diff' do - expect(diff).not_to be_too_large - end - end - - context 'using a diff that is too large' do - let(:raw_patch) { 'a' * 204800 } - - it 'prunes the diff' do - expect(diff.diff).to be_empty - expect(diff).to be_too_large - end - end - context 'when the patch passed is not UTF-8-encoded' do let(:raw_patch) { @raw_diff_hash[:diff].encode(Encoding::ASCII_8BIT) } @@ -259,31 +222,37 @@ EOT end it 'leave non-binary diffs as-is' do - diff = described_class.new(@rugged_diff) + diff = described_class.new(gitaly_diff) expect(diff.json_safe_diff).to eq(diff.diff) end end describe '#submodule?' do - before do - # TODO use a Gitaly diff object instead - rugged_commit = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged.rev_parse('5937ac0a7beb003549fc5fd26fc247adbce4a52e') - end - - @diffs = rugged_commit.parents[0].diff(rugged_commit).patches + let(:gitaly_submodule_diff) do + Gitlab::GitalyClient::Diff.new( + from_path: 'gitlab-grack', + to_path: 'gitlab-grack', + old_mode: 0, + new_mode: 57344, + from_id: '0000000000000000000000000000000000000000', + to_id: '645f6c4c82fd3f5e06f67134450a570b795e55a6', + overflow_marker: false, + collapsed: false, + too_large: false, + patch: "@@ -0,0 +1 @@\n+Subproject commit 645f6c4c82fd3f5e06f67134450a570b795e55a6\n" + ) end - it { expect(described_class.new(@diffs[0]).submodule?).to eq(false) } - it { expect(described_class.new(@diffs[1]).submodule?).to eq(true) } + it { expect(described_class.new(gitaly_diff).submodule?).to eq(false) } + it { expect(described_class.new(gitaly_submodule_diff).submodule?).to eq(true) } end describe '#line_count' do it 'returns the correct number of lines' do - diff = described_class.new(@rugged_diff) + diff = described_class.new(gitaly_diff) - expect(diff.line_count).to eq(9) + expect(diff.line_count).to eq(7) end end diff --git a/spec/lib/gitlab/git/diff_stats_collection_spec.rb b/spec/lib/gitlab/git/diff_stats_collection_spec.rb new file mode 100644 index 00000000000..b07690ef39c --- /dev/null +++ b/spec/lib/gitlab/git/diff_stats_collection_spec.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +require "spec_helper" + +describe Gitlab::Git::DiffStatsCollection do + let(:stats_a) do + double(Gitaly::DiffStats, additions: 10, deletions: 15, path: 'foo') + end + + let(:stats_b) do + double(Gitaly::DiffStats, additions: 5, deletions: 1, path: 'bar') + end + + let(:diff_stats) { [stats_a, stats_b] } + let(:collection) { described_class.new(diff_stats) } + + describe '#find_by_path' do + it 'returns stats by path when found' do + expect(collection.find_by_path('foo')).to eq(stats_a) + end + + it 'returns nil when stats is not found by path' do + expect(collection.find_by_path('no-file')).to be_nil + end + end + + describe '#paths' do + it 'returns only modified paths' do + expect(collection.paths).to eq %w[foo bar] + end + end +end diff --git a/spec/lib/gitlab/git/gitlab_projects_spec.rb b/spec/lib/gitlab/git/gitlab_projects_spec.rb deleted file mode 100644 index f5d8503c30c..00000000000 --- a/spec/lib/gitlab/git/gitlab_projects_spec.rb +++ /dev/null @@ -1,321 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::GitlabProjects do - after do - TestEnv.clean_test_path - end - - around do |example| - # TODO move this spec to gitaly-ruby. GitlabProjects is not used in gitlab-ce - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - let(:project) { create(:project, :repository) } - - if $VERBOSE - let(:logger) { Logger.new(STDOUT) } - else - let(:logger) { double('logger').as_null_object } - end - - let(:tmp_repos_path) { TestEnv.repos_path } - let(:repo_name) { project.disk_path + '.git' } - let(:tmp_repo_path) { File.join(tmp_repos_path, repo_name) } - let(:gl_projects) { build_gitlab_projects(TestEnv::REPOS_STORAGE, repo_name) } - - describe '#initialize' do - it { expect(gl_projects.shard_path).to eq(tmp_repos_path) } - it { expect(gl_projects.repository_relative_path).to eq(repo_name) } - it { expect(gl_projects.repository_absolute_path).to eq(File.join(tmp_repos_path, repo_name)) } - it { expect(gl_projects.logger).to eq(logger) } - end - - describe '#push_branches' do - let(:remote_name) { 'remote-name' } - let(:branch_name) { 'master' } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} push -- #{remote_name} #{branch_name}) } - let(:force) { false } - - subject { gl_projects.push_branches(remote_name, 600, force, [branch_name]) } - - it 'executes the command' do - stub_spawn(cmd, 600, tmp_repo_path, success: true) - - is_expected.to be_truthy - end - - it 'fails' do - stub_spawn(cmd, 600, tmp_repo_path, success: false) - - is_expected.to be_falsy - end - - context 'with --force' do - let(:cmd) { %W(#{Gitlab.config.git.bin_path} push --force -- #{remote_name} #{branch_name}) } - let(:force) { true } - - it 'executes the command' do - stub_spawn(cmd, 600, tmp_repo_path, success: true) - - is_expected.to be_truthy - end - end - end - - describe '#fetch_remote' do - let(:remote_name) { 'remote-name' } - let(:branch_name) { 'master' } - let(:force) { false } - let(:prune) { true } - let(:tags) { true } - let(:args) { { force: force, tags: tags, prune: prune }.merge(extra_args) } - let(:extra_args) { {} } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --prune --tags) } - - subject { gl_projects.fetch_remote(remote_name, 600, args) } - - def stub_tempfile(name, filename, opts = {}) - chmod = opts.delete(:chmod) - file = StringIO.new - - allow(file).to receive(:close!) - allow(file).to receive(:path).and_return(name) - - expect(Tempfile).to receive(:new).with(filename).and_return(file) - expect(file).to receive(:chmod).with(chmod) if chmod - - file - end - - context 'with default args' do - it 'executes the command' do - stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) - - is_expected.to be_truthy - end - - it 'fails' do - stub_spawn(cmd, 600, tmp_repo_path, {}, success: false) - - is_expected.to be_falsy - end - end - - context 'with --force' do - let(:force) { true } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --prune --force --tags) } - - it 'executes the command with forced option' do - stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) - - is_expected.to be_truthy - end - end - - context 'with --no-tags' do - let(:tags) { false } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --prune --no-tags) } - - it 'executes the command' do - stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) - - is_expected.to be_truthy - end - end - - context 'with no prune' do - let(:prune) { false } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} fetch #{remote_name} --quiet --tags) } - - it 'executes the command' do - stub_spawn(cmd, 600, tmp_repo_path, {}, success: true) - - is_expected.to be_truthy - end - end - - describe 'with an SSH key' do - let(:extra_args) { { ssh_key: 'SSH KEY' } } - - it 'sets GIT_SSH to a custom script' do - script = stub_tempfile('scriptFile', 'gitlab-shell-ssh-wrapper', chmod: 0o755) - key = stub_tempfile('/tmp files/keyFile', 'gitlab-shell-key-file', chmod: 0o400) - - stub_spawn(cmd, 600, tmp_repo_path, { 'GIT_SSH' => 'scriptFile' }, success: true) - - is_expected.to be_truthy - - expect(script.string).to eq("#!/bin/sh\nexec ssh '-oIdentityFile=\"/tmp files/keyFile\"' '-oIdentitiesOnly=\"yes\"' \"$@\"") - expect(key.string).to eq('SSH KEY') - end - end - - describe 'with known_hosts data' do - let(:extra_args) { { known_hosts: 'KNOWN HOSTS' } } - - it 'sets GIT_SSH to a custom script' do - script = stub_tempfile('scriptFile', 'gitlab-shell-ssh-wrapper', chmod: 0o755) - key = stub_tempfile('/tmp files/knownHosts', 'gitlab-shell-known-hosts', chmod: 0o400) - - stub_spawn(cmd, 600, tmp_repo_path, { 'GIT_SSH' => 'scriptFile' }, success: true) - - is_expected.to be_truthy - - expect(script.string).to eq("#!/bin/sh\nexec ssh '-oStrictHostKeyChecking=\"yes\"' '-oUserKnownHostsFile=\"/tmp files/knownHosts\"' \"$@\"") - expect(key.string).to eq('KNOWN HOSTS') - end - end - end - - describe '#import_project' do - let(:project) { create(:project) } - let(:import_url) { TestEnv.factory_repo_path_bare } - let(:cmd) { %W(#{Gitlab.config.git.bin_path} clone --bare -- #{import_url} #{tmp_repo_path}) } - let(:timeout) { 600 } - - subject { gl_projects.import_project(import_url, timeout) } - - shared_examples 'importing repository' do - context 'success import' do - it 'imports a repo' do - expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy - - is_expected.to be_truthy - - expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_truthy - end - end - - context 'already exists' do - it "doesn't import" do - FileUtils.mkdir_p(tmp_repo_path) - - is_expected.to be_falsy - end - end - end - - describe 'logging' do - it 'imports a repo' do - message = "Importing project from <#{import_url}> to <#{tmp_repo_path}>." - expect(logger).to receive(:info).with(message) - - subject - end - end - - context 'timeout' do - it 'does not import a repo' do - stub_spawn_timeout(cmd, timeout, nil) - - message = "Importing project from <#{import_url}> to <#{tmp_repo_path}> failed." - expect(logger).to receive(:error).with(message) - - is_expected.to be_falsy - - expect(gl_projects.output).to eq("Timed out\n") - expect(File.exist?(File.join(tmp_repo_path, 'HEAD'))).to be_falsy - end - end - - it_behaves_like 'importing repository' - end - - describe '#fork_repository' do - let(:dest_repos) { TestEnv::REPOS_STORAGE } - let(:dest_repos_path) { tmp_repos_path } - let(:dest_repo_name) { File.join('@hashed', 'aa', 'bb', 'xyz.git') } - let(:dest_repo) { File.join(dest_repos_path, dest_repo_name) } - - subject { gl_projects.fork_repository(dest_repos, dest_repo_name) } - - before do - FileUtils.mkdir_p(dest_repos_path) - end - - after do - FileUtils.rm_rf(dest_repos_path) - end - - shared_examples 'forking a repository' do - it 'forks the repository' do - is_expected.to be_truthy - - expect(File.exist?(dest_repo)).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy - end - - it 'does not fork if a project of the same name already exists' do - # create a fake project at the intended destination - FileUtils.mkdir_p(dest_repo) - - is_expected.to be_falsy - end - end - - it_behaves_like 'forking a repository' - - # We seem to be stuck to having only one working Gitaly storage in tests, changing - # that is not very straight-forward so I'm leaving this test here for now till - # https://gitlab.com/gitlab-org/gitlab-ce/issues/41393 is fixed. - context 'different storages' do - let(:dest_repos) { 'alternative' } - let(:dest_repos_path) { File.join(File.dirname(tmp_repos_path), dest_repos) } - - before do - stub_storage_settings(dest_repos => { 'path' => dest_repos_path }) - end - - it 'forks the repo' do - is_expected.to be_truthy - - expect(File.exist?(dest_repo)).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'pre-receive'))).to be_truthy - expect(File.exist?(File.join(dest_repo, 'hooks', 'post-receive'))).to be_truthy - end - end - - describe 'log messages' do - describe 'successful fork' do - it do - message = "Forking repository from <#{tmp_repo_path}> to <#{dest_repo}>." - expect(logger).to receive(:info).with(message) - - subject - end - end - - describe 'failed fork due existing destination' do - it do - FileUtils.mkdir_p(dest_repo) - message = "fork-repository failed: destination repository <#{dest_repo}> already exists." - expect(logger).to receive(:error).with(message) - - subject - end - end - end - end - - def build_gitlab_projects(*args) - described_class.new( - *args, - global_hooks_path: Gitlab.config.gitlab_shell.hooks_path, - logger: logger - ) - end - - def stub_spawn(*args, success: true) - exitstatus = success ? 0 : nil - expect(gl_projects).to receive(:popen_with_timeout).with(*args) - .and_return(["output", exitstatus]) - end - - def stub_spawn_timeout(*args) - expect(gl_projects).to receive(:popen_with_timeout).with(*args) - .and_raise(Timeout::Error) - end -end diff --git a/spec/lib/gitlab/git/hook_env_spec.rb b/spec/lib/gitlab/git/hook_env_spec.rb index e6aa5ad8c90..5e49ea6da7a 100644 --- a/spec/lib/gitlab/git/hook_env_spec.rb +++ b/spec/lib/gitlab/git/hook_env_spec.rb @@ -4,11 +4,7 @@ describe Gitlab::Git::HookEnv do let(:gl_repository) { 'project-123' } describe ".set" do - context 'with RequestStore.store disabled' do - before do - allow(RequestStore).to receive(:active?).and_return(false) - end - + context 'with RequestStore disabled' do it 'does not store anything' do described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo') @@ -16,11 +12,7 @@ describe Gitlab::Git::HookEnv do end end - context 'with RequestStore.store enabled' do - before do - allow(RequestStore).to receive(:active?).and_return(true) - end - + context 'with RequestStore enabled', :request_store do it 'whitelist some `GIT_*` variables and stores them using RequestStore' do described_class.set( gl_repository, @@ -41,9 +33,8 @@ describe Gitlab::Git::HookEnv do end describe ".all" do - context 'with RequestStore.store enabled' do + context 'with RequestStore enabled', :request_store do before do - allow(RequestStore).to receive(:active?).and_return(true) described_class.set( gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo', @@ -60,7 +51,7 @@ describe Gitlab::Git::HookEnv do end describe ".to_env_hash" do - context 'with RequestStore.store enabled' do + context 'with RequestStore enabled', :request_store do using RSpec::Parameterized::TableSyntax let(:key) { 'GIT_OBJECT_DIRECTORY_RELATIVE' } @@ -76,7 +67,6 @@ describe Gitlab::Git::HookEnv do with_them do before do - allow(RequestStore).to receive(:active?).and_return(true) described_class.set(gl_repository, key.to_sym => input) end @@ -92,7 +82,7 @@ describe Gitlab::Git::HookEnv do end describe 'thread-safety' do - context 'with RequestStore.store enabled' do + context 'with RequestStore enabled', :request_store do before do allow(RequestStore).to receive(:active?).and_return(true) described_class.set(gl_repository, GIT_OBJECT_DIRECTORY_RELATIVE: 'foo') diff --git a/spec/lib/gitlab/git/hook_spec.rb b/spec/lib/gitlab/git/hook_spec.rb deleted file mode 100644 index a45c8510b15..00000000000 --- a/spec/lib/gitlab/git/hook_spec.rb +++ /dev/null @@ -1,111 +0,0 @@ -require 'spec_helper' -require 'fileutils' - -describe Gitlab::Git::Hook do - before do - # We need this because in the spec/spec_helper.rb we define it like this: - # allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil]) - allow_any_instance_of(described_class).to receive(:trigger).and_call_original - end - - around do |example| - # TODO move hook tests to gitaly-ruby. Hook will disappear from gitlab-ce - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - describe "#trigger" do - set(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw_repository } - let(:repo_path) { repository.path } - let(:hooks_dir) { File.join(repo_path, 'hooks') } - let(:user) { create(:user) } - let(:gl_id) { Gitlab::GlId.gl_id(user) } - let(:gl_username) { user.username } - - def create_hook(name) - FileUtils.mkdir_p(hooks_dir) - hook_path = File.join(hooks_dir, name) - File.open(hook_path, 'w', 0755) do |f| - f.write(<<~HOOK) - #!/bin/sh - exit 0 - HOOK - end - end - - def create_failing_hook(name) - FileUtils.mkdir_p(hooks_dir) - hook_path = File.join(hooks_dir, name) - File.open(hook_path, 'w', 0755) do |f| - f.write(<<~HOOK) - #!/bin/sh - echo 'regular message from the hook' - echo 'error message from the hook' 1>&2 - echo 'error message from the hook line 2' 1>&2 - exit 1 - HOOK - end - end - - ['pre-receive', 'post-receive', 'update'].each do |hook_name| - context "when triggering a #{hook_name} hook" do - context "when the hook is successful" do - let(:hook_path) { File.join(hooks_dir, hook_name) } - let(:gl_repository) { Gitlab::GlRepository.gl_repository(project, false) } - let(:env) do - { - 'GL_ID' => gl_id, - 'GL_USERNAME' => gl_username, - 'PWD' => repo_path, - 'GL_PROTOCOL' => 'web', - 'GL_REPOSITORY' => gl_repository - } - end - - it "returns success with no errors" do - create_hook(hook_name) - hook = described_class.new(hook_name, repository) - blank = Gitlab::Git::BLANK_SHA - ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - - if hook_name != 'update' - expect(Open3).to receive(:popen3) - .with(env, hook_path, chdir: repo_path).and_call_original - end - - status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) - expect(status).to be true - expect(errors).to be_blank - end - end - - context "when the hook is unsuccessful" do - it "returns failure with errors" do - create_failing_hook(hook_name) - hook = described_class.new(hook_name, repository) - blank = Gitlab::Git::BLANK_SHA - ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - - status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) - expect(status).to be false - expect(errors).to eq("error message from the hook\nerror message from the hook line 2\n") - end - end - end - end - - context "when the hook doesn't exist" do - it "returns success with no errors" do - hook = described_class.new('unknown_hook', repository) - blank = Gitlab::Git::BLANK_SHA - ref = Gitlab::Git::BRANCH_REF_PREFIX + 'new_branch' - - status, errors = hook.trigger(gl_id, gl_username, blank, blank, ref) - expect(status).to be true - expect(errors).to be_nil - end - end - end -end diff --git a/spec/lib/gitlab/git/hooks_service_spec.rb b/spec/lib/gitlab/git/hooks_service_spec.rb deleted file mode 100644 index 55ffced36ac..00000000000 --- a/spec/lib/gitlab/git/hooks_service_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::HooksService, :seed_helper do - let(:gl_id) { 'user-456' } - let(:gl_username) { 'janedoe' } - let(:user) { Gitlab::Git::User.new(gl_username, 'Jane Doe', 'janedoe@example.com', gl_id) } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, 'project-123') } - let(:service) { described_class.new } - let(:blankrev) { Gitlab::Git::BLANK_SHA } - let(:oldrev) { SeedRepo::Commit::PARENT_ID } - let(:newrev) { SeedRepo::Commit::ID } - let(:ref) { 'refs/heads/feature' } - - describe '#execute' do - context 'when receive hooks were successful' do - let(:hook) { double(:hook) } - - it 'calls all three hooks' do - expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook) - expect(hook).to receive(:trigger).with(gl_id, gl_username, blankrev, newrev, ref) - .exactly(3).times.and_return([true, nil]) - - service.execute(user, repository, blankrev, newrev, ref) { } - end - end - - context 'when pre-receive hook failed' do - it 'does not call post-receive hook' do - expect(service).to receive(:run_hook).with('pre-receive').and_return([false, 'hello world']) - expect(service).not_to receive(:run_hook).with('post-receive') - - expect do - service.execute(user, repository, blankrev, newrev, ref) - end.to raise_error(Gitlab::Git::PreReceiveError, 'hello world') - end - end - - context 'when update hook failed' do - it 'does not call post-receive hook' do - expect(service).to receive(:run_hook).with('pre-receive').and_return([true, nil]) - expect(service).to receive(:run_hook).with('update').and_return([false, 'hello world']) - expect(service).not_to receive(:run_hook).with('post-receive') - - expect do - service.execute(user, repository, blankrev, newrev, ref) - end.to raise_error(Gitlab::Git::PreReceiveError, 'hello world') - end - end - end -end diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb deleted file mode 100644 index c4edd6961e1..00000000000 --- a/spec/lib/gitlab/git/index_spec.rb +++ /dev/null @@ -1,239 +0,0 @@ -require 'spec_helper' - -describe Gitlab::Git::Index, :seed_helper do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - let(:index) { described_class.new(repository) } - - before do - index.read_tree(lookup('master').tree) - end - - around do |example| - # TODO move these specs to gitaly-ruby. The Index class will disappear from gitlab-ce - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - describe '#create' do - let(:options) do - { - content: 'Lorem ipsum...', - file_path: 'documents/story.txt' - } - end - - context 'when no file at that path exists' do - it 'creates the file in the index' do - index.create(options) - - entry = index.get(options[:file_path]) - - expect(entry).not_to be_nil - expect(lookup(entry[:oid]).content).to eq(options[:content]) - end - end - - context 'when a file at that path exists' do - before do - options[:file_path] = 'files/executables/ls' - end - - it 'raises an error' do - expect { index.create(options) }.to raise_error('A file with this name already exists') - end - end - - context 'when content is in base64' do - before do - options[:content] = Base64.encode64(options[:content]) - options[:encoding] = 'base64' - end - - it 'decodes base64' do - index.create(options) - - entry = index.get(options[:file_path]) - expect(lookup(entry[:oid]).content).to eq(Base64.decode64(options[:content])) - end - end - - context 'when content contains CRLF' do - before do - repository.autocrlf = :input - options[:content] = "Hello,\r\nWorld" - end - - it 'converts to LF' do - index.create(options) - - entry = index.get(options[:file_path]) - expect(lookup(entry[:oid]).content).to eq("Hello,\nWorld") - end - end - end - - describe '#create_dir' do - let(:options) do - { - file_path: 'newdir' - } - end - - context 'when no file or dir at that path exists' do - it 'creates the dir in the index' do - index.create_dir(options) - - entry = index.get(options[:file_path] + '/.gitkeep') - - expect(entry).not_to be_nil - end - end - - context 'when a file at that path exists' do - before do - options[:file_path] = 'files/executables/ls' - end - - it 'raises an error' do - expect { index.create_dir(options) }.to raise_error('A file with this name already exists') - end - end - - context 'when a directory at that path exists' do - before do - options[:file_path] = 'files/executables' - end - - it 'raises an error' do - expect { index.create_dir(options) }.to raise_error('A directory with this name already exists') - end - end - end - - describe '#update' do - let(:options) do - { - content: 'Lorem ipsum...', - file_path: 'README.md' - } - end - - context 'when no file at that path exists' do - before do - options[:file_path] = 'documents/story.txt' - end - - it 'raises an error' do - expect { index.update(options) }.to raise_error("A file with this name doesn't exist") - end - end - - context 'when a file at that path exists' do - it 'updates the file in the index' do - index.update(options) - - entry = index.get(options[:file_path]) - - expect(lookup(entry[:oid]).content).to eq(options[:content]) - end - - it 'preserves file mode' do - options[:file_path] = 'files/executables/ls' - - index.update(options) - - entry = index.get(options[:file_path]) - - expect(entry[:mode]).to eq(0100755) - end - end - end - - describe '#move' do - let(:options) do - { - content: 'Lorem ipsum...', - previous_path: 'README.md', - file_path: 'NEWREADME.md' - } - end - - context 'when no file at that path exists' do - it 'raises an error' do - options[:previous_path] = 'documents/story.txt' - - expect { index.move(options) }.to raise_error("A file with this name doesn't exist") - end - end - - context 'when a file at the new path already exists' do - it 'raises an error' do - options[:file_path] = 'CHANGELOG' - - expect { index.move(options) }.to raise_error("A file with this name already exists") - end - end - - context 'when a file at that path exists' do - it 'removes the old file in the index' do - index.move(options) - - entry = index.get(options[:previous_path]) - - expect(entry).to be_nil - end - - it 'creates the new file in the index' do - index.move(options) - - entry = index.get(options[:file_path]) - - expect(entry).not_to be_nil - expect(lookup(entry[:oid]).content).to eq(options[:content]) - end - - it 'preserves file mode' do - options[:previous_path] = 'files/executables/ls' - - index.move(options) - - entry = index.get(options[:file_path]) - - expect(entry[:mode]).to eq(0100755) - end - end - end - - describe '#delete' do - let(:options) do - { - file_path: 'README.md' - } - end - - context 'when no file at that path exists' do - before do - options[:file_path] = 'documents/story.txt' - end - - it 'raises an error' do - expect { index.delete(options) }.to raise_error("A file with this name doesn't exist") - end - end - - context 'when a file at that path exists' do - it 'removes the file in the index' do - index.delete(options) - - entry = index.get(options[:file_path]) - - expect(entry).to be_nil - end - end - end - - def lookup(revision) - repository.rugged.rev_parse(revision) - end -end diff --git a/spec/lib/gitlab/git/popen_spec.rb b/spec/lib/gitlab/git/popen_spec.rb deleted file mode 100644 index 074e66d2a5d..00000000000 --- a/spec/lib/gitlab/git/popen_spec.rb +++ /dev/null @@ -1,179 +0,0 @@ -require 'spec_helper' - -describe 'Gitlab::Git::Popen' do - let(:path) { Rails.root.join('tmp').to_s } - let(:test_string) { 'The quick brown fox jumped over the lazy dog' } - # The pipe buffer is typically 64K. This string is about 440K. - let(:spew_command) { ['bash', '-c', "for i in {1..10000}; do echo '#{test_string}' 1>&2; done"] } - - let(:klass) do - Class.new(Object) do - include Gitlab::Git::Popen - end - end - - context 'popen' do - context 'zero status' do - let(:result) { klass.new.popen(%w(ls), path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - it { expect(output).to include('tests') } - end - - context 'non-zero status' do - let(:result) { klass.new.popen(%w(cat NOTHING), path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to eq(1) } - it { expect(output).to include('No such file or directory') } - end - - context 'unsafe string command' do - it 'raises an error when it gets called with a string argument' do - expect { klass.new.popen('ls', path) }.to raise_error(RuntimeError) - end - end - - context 'with custom options' do - let(:vars) { { 'foobar' => 123, 'PWD' => path } } - let(:options) { { chdir: path } } - - it 'calls popen3 with the provided environment variables' do - expect(Open3).to receive(:popen3).with(vars, 'ls', options) - - klass.new.popen(%w(ls), path, { 'foobar' => 123 }) - end - end - - context 'use stdin' do - let(:result) { klass.new.popen(%w[cat], path) { |stdin| stdin.write 'hello' } } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - it { expect(output).to eq('hello') } - end - - context 'with lazy block' do - it 'yields a lazy io' do - expect_lazy_io = lambda do |io| - expect(io).to be_a Enumerator::Lazy - expect(io.inspect).to include('#<IO:fd') - end - - klass.new.popen(%w[ls], path, lazy_block: expect_lazy_io) - end - - it "doesn't wait for process exit" do - Timeout.timeout(2) do - klass.new.popen(%w[yes], path, lazy_block: ->(io) {}) - end - end - end - - context 'with a process that writes a lot of data to stderr' do - it 'returns zero' do - output, status = klass.new.popen(spew_command, path) - - expect(output).to include(test_string) - expect(status).to eq(0) - end - end - end - - context 'popen_with_timeout' do - let(:timeout) { 1.second } - - context 'no timeout' do - context 'zero status' do - let(:result) { klass.new.popen_with_timeout(%w(ls), timeout, path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - it { expect(output).to include('tests') } - end - - context 'multi-line string' do - let(:test_string) { "this is 1 line\n2nd line\n3rd line\n" } - let(:result) { klass.new.popen_with_timeout(['echo', test_string], timeout, path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to be_zero } - # echo adds its own line - it { expect(output).to eq(test_string + "\n") } - end - - context 'non-zero status' do - let(:result) { klass.new.popen_with_timeout(%w(cat NOTHING), timeout, path) } - let(:output) { result.first } - let(:status) { result.last } - - it { expect(status).to eq(1) } - it { expect(output).to include('No such file or directory') } - end - - context 'unsafe string command' do - it 'raises an error when it gets called with a string argument' do - expect { klass.new.popen_with_timeout('ls', timeout, path) }.to raise_error(RuntimeError) - end - end - end - - context 'timeout' do - context 'timeout' do - it "raises a Timeout::Error" do - expect { klass.new.popen_with_timeout(%w(sleep 1000), timeout, path) }.to raise_error(Timeout::Error) - end - - it "handles processes that do not shutdown correctly" do - expect { klass.new.popen_with_timeout(['bash', '-c', "trap -- '' SIGTERM; sleep 1000"], timeout, path) }.to raise_error(Timeout::Error) - end - - it 'handles process that writes a lot of data to stderr' do - output, status = klass.new.popen_with_timeout(spew_command, timeout, path) - - expect(output).to include(test_string) - expect(status).to eq(0) - end - end - - context 'timeout period' do - let(:time_taken) do - begin - start = Time.now - klass.new.popen_with_timeout(%w(sleep 1000), timeout, path) - rescue - Time.now - start - end - end - - it { expect(time_taken).to be >= timeout } - end - - context 'clean up' do - let(:instance) { klass.new } - - it 'kills the child process' do - expect(instance).to receive(:kill_process_group_for_pid).and_wrap_original do |m, *args| - # is the PID, and it should not be running at this point - m.call(*args) - - pid = args.first - begin - Process.getpgid(pid) - raise "The child process should have been killed" - rescue Errno::ESRCH - end - end - - expect { instance.popen_with_timeout(['bash', '-c', "trap -- '' SIGTERM; sleep 1000"], timeout, path) }.to raise_error(Timeout::Error) - end - end - end - end -end diff --git a/spec/lib/gitlab/git/push_spec.rb b/spec/lib/gitlab/git/push_spec.rb new file mode 100644 index 00000000000..566c8209504 --- /dev/null +++ b/spec/lib/gitlab/git/push_spec.rb @@ -0,0 +1,166 @@ +require 'spec_helper' + +describe Gitlab::Git::Push do + set(:project) { create(:project, :repository) } + + let(:oldrev) { project.commit('HEAD~2').id } + let(:newrev) { project.commit.id } + let(:ref) { 'refs/heads/some-branch' } + + subject { described_class.new(project, oldrev, newrev, ref) } + + describe '#branch_name' do + context 'when it is a branch push' do + let(:ref) { 'refs/heads/my-branch' } + + it 'returns branch name' do + expect(subject.branch_name).to eq 'my-branch' + end + end + + context 'when it is a tag push' do + let(:ref) { 'refs/tags/my-branch' } + + it 'returns nil' do + expect(subject.branch_name).to be_nil + end + end + end + + describe '#branch_push?' do + context 'when pushing a branch ref' do + let(:ref) { 'refs/heads/my-branch' } + + it { is_expected.to be_branch_push } + end + + context 'when it is a tag push' do + let(:ref) { 'refs/tags/my-tag' } + + it { is_expected.not_to be_branch_push } + end + end + + describe '#branch_updated?' do + context 'when it is a branch push with correct old and new revisions' do + it { is_expected.to be_branch_updated } + end + + context 'when it is not a branch push' do + let(:ref) { 'refs/tags/my-tag' } + + it { is_expected.not_to be_branch_updated } + end + + context 'when old revision is blank' do + let(:oldrev) { Gitlab::Git::BLANK_SHA } + + it { is_expected.not_to be_branch_updated } + end + + context 'when it is not a branch push' do + let(:newrev) { Gitlab::Git::BLANK_SHA } + + it { is_expected.not_to be_branch_updated } + end + + context 'when oldrev is nil' do + let(:oldrev) { nil } + + it { is_expected.not_to be_branch_updated } + end + end + + describe '#force_push?' do + context 'when old revision is an ancestor of the new revision' do + let(:oldrev) { 'HEAD~3' } + let(:newrev) { 'HEAD~1' } + + it { is_expected.not_to be_force_push } + end + + context 'when old revision is not an ancestor of the new revision' do + let(:oldrev) { 'HEAD~3' } + let(:newrev) { '123456' } + + it { is_expected.to be_force_push } + end + end + + describe '#branch_added?' do + context 'when old revision is defined' do + it { is_expected.not_to be_branch_added } + end + + context 'when old revision is not defined' do + let(:oldrev) { Gitlab::Git::BLANK_SHA } + + it { is_expected.to be_branch_added } + end + end + + describe '#branch_removed?' do + context 'when new revision is defined' do + it { is_expected.not_to be_branch_removed } + end + + context 'when new revision is not defined' do + let(:newrev) { Gitlab::Git::BLANK_SHA } + + it { is_expected.to be_branch_removed } + end + end + + describe '#modified_paths' do + context 'when a push is a branch update' do + let(:newrev) { '498214d' } + let(:oldrev) { '281d3a7' } + + it 'returns modified paths' do + expect(subject.modified_paths).to eq ['bar/branch-test.txt', + 'files/js/commit.coffee', + 'with space/README.md'] + end + end + + context 'when a push is not a branch update' do + let(:oldrev) { Gitlab::Git::BLANK_SHA } + + it 'raises an error' do + expect { subject.modified_paths }.to raise_error(ArgumentError) + end + end + end + + describe '#oldrev' do + context 'when a valid oldrev is provided' do + it 'returns oldrev' do + expect(subject.oldrev).to eq oldrev + end + end + + context 'when a nil valud is provided' do + let(:oldrev) { nil } + + it 'returns blank SHA' do + expect(subject.oldrev).to eq Gitlab::Git::BLANK_SHA + end + end + end + + describe '#newrev' do + context 'when valid newrev is provided' do + it 'returns newrev' do + expect(subject.newrev).to eq newrev + end + end + + context 'when a nil valud is provided' do + let(:newrev) { nil } + + it 'returns blank SHA' do + expect(subject.newrev).to eq Gitlab::Git::BLANK_SHA + end + end + end +end diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb index 17348b01006..51eb997a325 100644 --- a/spec/lib/gitlab/git/repository_spec.rb +++ b/spec/lib/gitlab/git/repository_spec.rb @@ -19,7 +19,10 @@ describe Gitlab::Git::Repository, :seed_helper do end end + let(:mutable_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } + let(:repository_path) { File.join(TestEnv.repos_path, repository.relative_path) } + let(:repository_rugged) { Rugged::Repository.new(repository_path) } let(:storage_path) { TestEnv.repos_path } let(:user) { build(:user) } @@ -71,7 +74,6 @@ describe Gitlab::Git::Repository, :seed_helper do describe "Respond to" do subject { repository } - it { is_expected.to respond_to(:rugged) } it { is_expected.to respond_to(:root_ref) } it { is_expected.to respond_to(:tags) } end @@ -91,57 +93,6 @@ describe Gitlab::Git::Repository, :seed_helper do end end - describe "#rugged" do - describe 'when storage is broken', :broken_storage do - it 'raises a storage exception when storage is not available' do - broken_repo = described_class.new('broken', 'a/path.git', '') - - expect { broken_repo.rugged }.to raise_error(Gitlab::Git::Storage::Inaccessible) - end - end - - it 'raises a no repository exception when there is no repo' do - broken_repo = described_class.new('default', 'a/path.git', '') - - expect do - Gitlab::GitalyClient::StorageSettings.allow_disk_access { broken_repo.rugged } - end.to raise_error(Gitlab::Git::Repository::NoRepository) - end - - describe 'alternates keyword argument' do - context 'with no Git env stored' do - before do - allow(Gitlab::Git::HookEnv).to receive(:all).and_return({}) - end - - it "is passed an empty array" do - expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: []) - - repository_rugged - end - end - - context 'with absolute and relative Git object dir envvars stored' do - before do - allow(Gitlab::Git::HookEnv).to receive(:all).and_return({ - 'GIT_OBJECT_DIRECTORY_RELATIVE' => './objects/foo', - 'GIT_ALTERNATE_OBJECT_DIRECTORIES_RELATIVE' => ['./objects/bar', './objects/baz'], - 'GIT_OBJECT_DIRECTORY' => 'ignored', - 'GIT_ALTERNATE_OBJECT_DIRECTORIES' => %w[ignored ignored], - 'GIT_OTHER' => 'another_env' - }) - end - - it "is passed the relative object dir envvars after being converted to absolute ones" do - alternates = %w[foo bar baz].map { |d| File.join(repository_path, './objects', d) } - expect(Rugged::Repository).to receive(:new).with(repository_path, alternates: alternates) - - repository_rugged - end - end - end - end - describe '#branch_names' do subject { repository.branch_names } @@ -284,7 +235,6 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#submodule_url_for' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } let(:ref) { 'master' } def submodule_url(path) @@ -322,21 +272,12 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#commit_count' do - shared_examples 'simple commit counting' do - it { expect(repository.commit_count("master")).to eq(25) } - it { expect(repository.commit_count("feature")).to eq(9) } - it { expect(repository.commit_count("does-not-exist")).to eq(0) } - end + it { expect(repository.commit_count("master")).to eq(25) } + it { expect(repository.commit_count("feature")).to eq(9) } + it { expect(repository.commit_count("does-not-exist")).to eq(0) } - context 'when Gitaly commit_count feature is enabled' do - it_behaves_like 'simple commit counting' - it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :commit_count do - subject { repository.commit_count('master') } - end - end - - context 'when Gitaly commit_count feature is disabled', :skip_gitaly_mock do - it_behaves_like 'simple commit counting' + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :commit_count do + subject { repository.commit_count('master') } end end @@ -345,7 +286,7 @@ describe Gitlab::Git::Repository, :seed_helper do it { expect(repository.has_local_branches?).to eq(true) } context 'mutable' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:repository) { mutable_repository } after do ensure_seeds @@ -378,118 +319,82 @@ describe Gitlab::Git::Repository, :seed_helper do end describe "#delete_branch" do - shared_examples "deleting a branch" do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - after do - ensure_seeds - end - - it "removes the branch from the repo" do - branch_name = "to-be-deleted-soon" + let(:repository) { mutable_repository } - repository.create_branch(branch_name) - expect(repository_rugged.branches[branch_name]).not_to be_nil + after do + ensure_seeds + end - repository.delete_branch(branch_name) - expect(repository_rugged.branches[branch_name]).to be_nil - end + it "removes the branch from the repo" do + branch_name = "to-be-deleted-soon" - context "when branch does not exist" do - it "raises a DeleteBranchError exception" do - expect { repository.delete_branch("this-branch-does-not-exist") }.to raise_error(Gitlab::Git::Repository::DeleteBranchError) - end - end - end + repository.create_branch(branch_name) + expect(repository_rugged.branches[branch_name]).not_to be_nil - context "when Gitaly delete_branch is enabled" do - it_behaves_like "deleting a branch" + repository.delete_branch(branch_name) + expect(repository_rugged.branches[branch_name]).to be_nil end - context "when Gitaly delete_branch is disabled", :skip_gitaly_mock do - it_behaves_like "deleting a branch" + context "when branch does not exist" do + it "raises a DeleteBranchError exception" do + expect { repository.delete_branch("this-branch-does-not-exist") }.to raise_error(Gitlab::Git::Repository::DeleteBranchError) + end end end describe "#create_branch" do - shared_examples 'creating a branch' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - after do - ensure_seeds - end - - it "should create a new branch" do - expect(repository.create_branch('new_branch', 'master')).not_to be_nil - end + let(:repository) { mutable_repository } - it "should create a new branch with the right name" do - expect(repository.create_branch('another_branch', 'master').name).to eq('another_branch') - end + after do + ensure_seeds + end - it "should fail if we create an existing branch" do - repository.create_branch('duplicated_branch', 'master') - expect {repository.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") - end + it "should create a new branch" do + expect(repository.create_branch('new_branch', 'master')).not_to be_nil + end - it "should fail if we create a branch from a non existing ref" do - expect {repository.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") - end + it "should create a new branch with the right name" do + expect(repository.create_branch('another_branch', 'master').name).to eq('another_branch') end - context 'when Gitaly create_branch feature is enabled' do - it_behaves_like 'creating a branch' + it "should fail if we create an existing branch" do + repository.create_branch('duplicated_branch', 'master') + expect {repository.create_branch('duplicated_branch', 'master')}.to raise_error("Branch duplicated_branch already exists") end - context 'when Gitaly create_branch feature is disabled', :skip_gitaly_mock do - it_behaves_like 'creating a branch' + it "should fail if we create a branch from a non existing ref" do + expect {repository.create_branch('branch_based_in_wrong_ref', 'master_2_the_revenge')}.to raise_error("Invalid reference master_2_the_revenge") end end describe '#delete_refs' do - shared_examples 'deleting refs' do - let(:repo) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - def repo_rugged - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repo.rugged - end - end - - after do - ensure_seeds - end - - it 'deletes the ref' do - repo.delete_refs('refs/heads/feature') + let(:repository) { mutable_repository } - expect(repo_rugged.references['refs/heads/feature']).to be_nil - end + after do + ensure_seeds + end - it 'deletes all refs' do - refs = %w[refs/heads/wip refs/tags/v1.1.0] - repo.delete_refs(*refs) + it 'deletes the ref' do + repository.delete_refs('refs/heads/feature') - refs.each do |ref| - expect(repo_rugged.references[ref]).to be_nil - end - end + expect(repository_rugged.references['refs/heads/feature']).to be_nil + end - it 'does not fail when deleting an empty list of refs' do - expect { repo.delete_refs(*[]) }.not_to raise_error - end + it 'deletes all refs' do + refs = %w[refs/heads/wip refs/tags/v1.1.0] + repository.delete_refs(*refs) - it 'raises an error if it failed' do - expect { repo.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) + refs.each do |ref| + expect(repository_rugged.references[ref]).to be_nil end end - context 'when Gitaly delete_refs feature is enabled' do - it_behaves_like 'deleting refs' + it 'does not fail when deleting an empty list of refs' do + expect { repository.delete_refs(*[]) }.not_to raise_error end - context 'when Gitaly delete_refs feature is disabled', :disable_gitaly do - it_behaves_like 'deleting refs' + it 'raises an error if it failed' do + expect { repository.delete_refs('refs\heads\fix') }.to raise_error(Gitlab::Git::Repository::GitError) end end @@ -542,44 +447,63 @@ describe Gitlab::Git::Repository, :seed_helper do Gitlab::Shell.new.remove_repository('default', 'my_project') end - shared_examples 'repository mirror fetching' do - it 'fetches a repository as a mirror remote' do + it 'fetches a repository as a mirror remote' do + subject + + expect(refs(new_repository_path)).to eq(refs(repository_path)) + end + + context 'with keep-around refs' do + let(:sha) { SeedRepo::Commit::ID } + let(:keep_around_ref) { "refs/keep-around/#{sha}" } + let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } + + before do + repository_rugged.references.create(keep_around_ref, sha, force: true) + repository_rugged.references.create(tmp_ref, sha, force: true) + end + + it 'includes the temporary and keep-around refs' do subject - expect(refs(new_repository_path)).to eq(refs(repository_path)) + expect(refs(new_repository_path)).to include(keep_around_ref) + expect(refs(new_repository_path)).to include(tmp_ref) end + end - context 'with keep-around refs' do - let(:sha) { SeedRepo::Commit::ID } - let(:keep_around_ref) { "refs/keep-around/#{sha}" } - let(:tmp_ref) { "refs/tmp/#{SecureRandom.hex}" } + def new_repository_path + File.join(TestEnv.repos_path, new_repository.relative_path) + end + end - before do - repository_rugged.references.create(keep_around_ref, sha, force: true) - repository_rugged.references.create(tmp_ref, sha, force: true) - end + describe '#find_remote_root_ref' do + it 'gets the remote root ref from GitalyClient' do + expect_any_instance_of(Gitlab::GitalyClient::RemoteService) + .to receive(:find_remote_root_ref).and_call_original - it 'includes the temporary and keep-around refs' do - subject + expect(repository.find_remote_root_ref('origin')).to eq 'master' + end - expect(refs(new_repository_path)).to include(keep_around_ref) - expect(refs(new_repository_path)).to include(tmp_ref) - end - end + it 'returns UTF-8' do + expect(repository.find_remote_root_ref('origin')).to be_utf8 end - context 'with gitaly enabled' do - it_behaves_like 'repository mirror fetching' + it 'returns nil when remote name is nil' do + expect_any_instance_of(Gitlab::GitalyClient::RemoteService) + .not_to receive(:find_remote_root_ref) + + expect(repository.find_remote_root_ref(nil)).to be_nil end - context 'with gitaly enabled', :skip_gitaly_mock do - it_behaves_like 'repository mirror fetching' + it 'returns nil when remote name is empty' do + expect_any_instance_of(Gitlab::GitalyClient::RemoteService) + .not_to receive(:find_remote_root_ref) + + expect(repository.find_remote_root_ref('')).to be_nil end - def new_repository_path - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - new_repository.path - end + it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RemoteService, :find_remote_root_ref do + subject { repository.find_remote_root_ref('origin') } end end @@ -595,18 +519,16 @@ describe Gitlab::Git::Repository, :seed_helper do Gitlab::Git::Commit.find(repository, @rename_commit_id) end - before(:context) do + before do # Add new commits so that there's a renamed file in the commit history - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - @commit_with_old_name_id = new_commit_edit_old_file(repo).oid - @rename_commit_id = new_commit_move_file(repo).oid - @commit_with_new_name_id = new_commit_edit_new_file(repo).oid + @commit_with_old_name_id = new_commit_edit_old_file(repository_rugged).oid + @rename_commit_id = new_commit_move_file(repository_rugged).oid + @commit_with_new_name_id = new_commit_edit_new_file(repository_rugged).oid end - after(:context) do + after do # Erase our commits so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - repo.references.update("refs/heads/master", SeedRepo::LastCommit::ID) + repository_rugged.references.update("refs/heads/master", SeedRepo::LastCommit::ID) end context "where 'follow' == true" do @@ -887,25 +809,15 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#merge_base' do - shared_examples '#merge_base' do - where(:from, :to, :result) do - '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' - '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' - '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | 'foobar' | nil - 'foobar' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | nil - end - - with_them do - it { expect(repository.merge_base(from, to)).to eq(result) } - end - end - - context 'with gitaly' do - it_behaves_like '#merge_base' + where(:from, :to, :result) do + '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' | '570e7b2abdd848b95f2f578043fc23bd6f6fd24d' + '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | 'foobar' | nil + 'foobar' | '40f4a7a617393735a95a0bb67b08385bc1e7c66d' | nil end - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#merge_base' + with_them do + it { expect(repository.merge_base(from, to)).to eq(result) } end end @@ -997,54 +909,6 @@ describe Gitlab::Git::Repository, :seed_helper do end end - describe '#autocrlf' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.rugged.config['core.autocrlf'] = true - end - - around do |example| - # OK because autocrlf is only used in gitaly-ruby - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - it 'return the value of the autocrlf option' do - expect(@repo.autocrlf).to be(true) - end - - after(:all) do - @repo.rugged.config.delete('core.autocrlf') - end - end - - describe '#autocrlf=' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - @repo.rugged.config['core.autocrlf'] = false - end - - around do |example| - # OK because autocrlf= is only used in gitaly-ruby - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - example.run - end - end - - it 'should set the autocrlf option to the provided option' do - @repo.autocrlf = :input - - File.open(File.join(SEED_STORAGE_PATH, TEST_MUTABLE_REPO_PATH, 'config')) do |config_file| - expect(config_file.read).to match('autocrlf = input') - end - end - - after(:all) do - @repo.rugged.config.delete('core.autocrlf') - end - end - describe '#find_branch' do it 'should return a Branch for master' do branch = repository.find_branch('master') @@ -1086,12 +950,10 @@ describe Gitlab::Git::Repository, :seed_helper do subject { repository.branches } context 'with local and remote branches' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } before do - create_remote_branch(repository, 'joe', 'remote_branch', 'master') + create_remote_branch('joe', 'remote_branch', 'master') repository.create_branch('local_branch', 'master') end @@ -1114,12 +976,10 @@ describe Gitlab::Git::Repository, :seed_helper do end context 'with local and remote branches' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } before do - create_remote_branch(repository, 'joe', 'remote_branch', 'master') + create_remote_branch('joe', 'remote_branch', 'master') repository.create_branch('local_branch', 'master') end @@ -1144,57 +1004,81 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#merged_branch_names' do - shared_examples 'finding merged branch names' do - context 'when branch names are passed' do - it 'only returns the names we are asking' do - names = repository.merged_branch_names(%w[merge-test]) + context 'when branch names are passed' do + it 'only returns the names we are asking' do + names = repository.merged_branch_names(%w[merge-test]) - expect(names).to contain_exactly('merge-test') - end + expect(names).to contain_exactly('merge-test') + end - it 'does not return unmerged branch names' do - names = repository.merged_branch_names(%w[feature]) + it 'does not return unmerged branch names' do + names = repository.merged_branch_names(%w[feature]) - expect(names).to be_empty - end + expect(names).to be_empty end + end - context 'when no root ref is available' do - it 'returns empty list' do - project = create(:project, :empty_repo) + context 'when no root ref is available' do + it 'returns empty list' do + project = create(:project, :empty_repo) - names = project.repository.merged_branch_names(%w[feature]) + names = project.repository.merged_branch_names(%w[feature]) - expect(names).to be_empty - end + expect(names).to be_empty end + end - context 'when no branch names are specified' do - before do - repository.create_branch('identical', 'master') - end + context 'when no branch names are specified' do + before do + repository.create_branch('identical', 'master') + end - after do - ensure_seeds - end + after do + ensure_seeds + end - it 'returns all merged branch names except for identical one' do - names = repository.merged_branch_names + it 'returns all merged branch names except for identical one' do + names = repository.merged_branch_names - expect(names).to include('merge-test') - expect(names).to include('fix-mode') - expect(names).not_to include('feature') - expect(names).not_to include('identical') - end + expect(names).to include('merge-test') + expect(names).to include('fix-mode') + expect(names).not_to include('feature') + expect(names).not_to include('identical') end end + end + + describe '#diff_stats' do + let(:left_commit_id) { 'feature' } + let(:right_commit_id) { 'master' } + + it 'returns a DiffStatsCollection' do + collection = repository.diff_stats(left_commit_id, right_commit_id) + + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + end + + it 'yields Gitaly::DiffStats objects' do + collection = repository.diff_stats(left_commit_id, right_commit_id) + + expect(collection.to_a).to all(be_a(Gitaly::DiffStats)) + end + + it 'returns no Gitaly::DiffStats when SHAs are invalid' do + collection = repository.diff_stats('foo', 'bar') - context 'when Gitaly merged_branch_names feature is enabled' do - it_behaves_like 'finding merged branch names' + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + expect(collection.to_a).to be_empty end - context 'when Gitaly merged_branch_names feature is disabled', :disable_gitaly do - it_behaves_like 'finding merged branch names' + it 'returns no Gitaly::DiffStats when there is a nil SHA' do + collection = repository.diff_stats(nil, 'master') + + expect(collection).to be_a(Gitlab::Git::DiffStatsCollection) + expect(collection).to be_a(Enumerable) + expect(collection.to_a).to be_empty end end @@ -1311,98 +1195,68 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#ref_exists?' do - shared_examples 'checks the existence of refs' do - it 'returns true for an existing tag' do - expect(repository.ref_exists?('refs/heads/master')).to eq(true) - end - - it 'returns false for a non-existing tag' do - expect(repository.ref_exists?('refs/tags/THIS_TAG_DOES_NOT_EXIST')).to eq(false) - end - - it 'raises an ArgumentError for an empty string' do - expect { repository.ref_exists?('') }.to raise_error(ArgumentError) - end + it 'returns true for an existing tag' do + expect(repository.ref_exists?('refs/heads/master')).to eq(true) + end - it 'raises an ArgumentError for an invalid ref' do - expect { repository.ref_exists?('INVALID') }.to raise_error(ArgumentError) - end + it 'returns false for a non-existing tag' do + expect(repository.ref_exists?('refs/tags/THIS_TAG_DOES_NOT_EXIST')).to eq(false) end - context 'when Gitaly ref_exists feature is enabled' do - it_behaves_like 'checks the existence of refs' + it 'raises an ArgumentError for an empty string' do + expect { repository.ref_exists?('') }.to raise_error(ArgumentError) end - context 'when Gitaly ref_exists feature is disabled', :skip_gitaly_mock do - it_behaves_like 'checks the existence of refs' + it 'raises an ArgumentError for an invalid ref' do + expect { repository.ref_exists?('INVALID') }.to raise_error(ArgumentError) end end describe '#tag_exists?' do - shared_examples 'checks the existence of tags' do - it 'returns true for an existing tag' do - tag = repository.tag_names.first - - expect(repository.tag_exists?(tag)).to eq(true) - end + it 'returns true for an existing tag' do + tag = repository.tag_names.first - it 'returns false for a non-existing tag' do - expect(repository.tag_exists?('v9000')).to eq(false) - end + expect(repository.tag_exists?(tag)).to eq(true) end - context 'when Gitaly ref_exists_tags feature is enabled' do - it_behaves_like 'checks the existence of tags' - end - - context 'when Gitaly ref_exists_tags feature is disabled', :skip_gitaly_mock do - it_behaves_like 'checks the existence of tags' + it 'returns false for a non-existing tag' do + expect(repository.tag_exists?('v9000')).to eq(false) end end describe '#branch_exists?' do - shared_examples 'checks the existence of branches' do - it 'returns true for an existing branch' do - expect(repository.branch_exists?('master')).to eq(true) - end - - it 'returns false for a non-existing branch' do - expect(repository.branch_exists?('kittens')).to eq(false) - end - - it 'returns false when using an invalid branch name' do - expect(repository.branch_exists?('.bla')).to eq(false) - end + it 'returns true for an existing branch' do + expect(repository.branch_exists?('master')).to eq(true) end - context 'when Gitaly ref_exists_branches feature is enabled' do - it_behaves_like 'checks the existence of branches' + it 'returns false for a non-existing branch' do + expect(repository.branch_exists?('kittens')).to eq(false) end - context 'when Gitaly ref_exists_branches feature is disabled', :skip_gitaly_mock do - it_behaves_like 'checks the existence of branches' + it 'returns false when using an invalid branch name' do + expect(repository.branch_exists?('.bla')).to eq(false) end end describe '#local_branches' do - before(:all) do - @repo = Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') + let(:repository) { mutable_repository } + + before do + create_remote_branch('joe', 'remote_branch', 'master') + repository.create_branch('local_branch', 'master') end - after(:all) do + after do ensure_seeds end it 'returns the local branches' do - create_remote_branch(@repo, 'joe', 'remote_branch', 'master') - @repo.create_branch('local_branch', 'master') - - expect(@repo.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) - expect(@repo.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) + expect(repository.local_branches.any? { |branch| branch.name == 'remote_branch' }).to eq(false) + expect(repository.local_branches.any? { |branch| branch.name == 'local_branch' }).to eq(true) end it 'returns a Branch with UTF-8 fields' do - branches = @repo.local_branches.to_a + branches = repository.local_branches.to_a expect(branches.size).to be > 0 branches.each do |branch| expect(branch.name).to be_utf8 @@ -1413,11 +1267,11 @@ describe Gitlab::Git::Repository, :seed_helper do it 'gets the branches from GitalyClient' do expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(:local_branches) .and_return([]) - @repo.local_branches + repository.local_branches end it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :local_branches do - subject { @repo.local_branches } + subject { repository.local_branches } end end @@ -1471,56 +1325,9 @@ describe Gitlab::Git::Repository, :seed_helper do end end - describe '#with_repo_branch_commit' do - context 'when comparing with the same repository' do - let(:start_repository) { repository } - - context 'when the branch exists' do - let(:start_branch_name) { 'master' } - - it 'yields the commit' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) - end - end - - context 'when the branch does not exist' do - let(:start_branch_name) { 'definitely-not-master' } - - it 'yields nil' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(nil) - end - end - end - - context 'when comparing with another repository' do - let(:start_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - - context 'when the branch exists' do - let(:start_branch_name) { 'master' } - - it 'yields the commit' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(an_instance_of(Gitlab::Git::Commit)) - end - end - - context 'when the branch does not exist' do - let(:start_branch_name) { 'definitely-not-master' } - - it 'yields nil' do - expect { |b| repository.with_repo_branch_commit(start_repository, start_branch_name, &b) } - .to yield_with_args(nil) - end - end - end - end - describe '#fetch_source_branch!' do let(:local_ref) { 'refs/merge-requests/1/head' } - let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '') } - let(:source_repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } + let(:source_repository) { mutable_repository } after do ensure_seeds @@ -1529,7 +1336,8 @@ describe Gitlab::Git::Repository, :seed_helper do context 'when the branch exists' do context 'when the commit does not exist locally' do let(:source_branch) { 'new-branch-for-fetch-source-branch' } - let(:source_rugged) { Gitlab::GitalyClient::StorageSettings.allow_disk_access { source_repository.rugged } } + let(:source_path) { File.join(TestEnv.repos_path, source_repository.relative_path) } + let(:source_rugged) { Rugged::Repository.new(source_path) } let(:new_oid) { new_commit_edit_old_file(source_rugged).oid } before do @@ -1567,29 +1375,19 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#rm_branch' do - shared_examples "user deleting a branch" do - let(:project) { create(:project, :repository) } - let(:repository) { project.repository.raw } - let(:branch_name) { "to-be-deleted-soon" } - - before do - project.add_developer(user) - repository.create_branch(branch_name) - end + let(:project) { create(:project, :repository) } + let(:repository) { project.repository.raw } + let(:branch_name) { "to-be-deleted-soon" } - it "removes the branch from the repo" do - repository.rm_branch(branch_name, user: user) - - expect(repository_rugged.branches[branch_name]).to be_nil - end + before do + project.add_developer(user) + repository.create_branch(branch_name) end - context "when Gitaly user_delete_branch is enabled" do - it_behaves_like "user deleting a branch" - end + it "removes the branch from the repo" do + repository.rm_branch(branch_name, user: user) - context "when Gitaly user_delete_branch is disabled", :skip_gitaly_mock do - it_behaves_like "user deleting a branch" + expect(repository_rugged.branches[branch_name]).to be_nil end end @@ -1651,8 +1449,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#set_config' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - let(:rugged) { repository_rugged } + let(:repository) { mutable_repository } let(:entries) do { 'test.foo1' => 'bla bla', @@ -1664,19 +1461,18 @@ describe Gitlab::Git::Repository, :seed_helper do it 'can set config settings' do expect(repository.set_config(entries)).to be_nil - expect(rugged.config['test.foo1']).to eq('bla bla') - expect(rugged.config['test.foo2']).to eq('1234') - expect(rugged.config['test.foo3']).to eq('true') + expect(repository_rugged.config['test.foo1']).to eq('bla bla') + expect(repository_rugged.config['test.foo2']).to eq('1234') + expect(repository_rugged.config['test.foo3']).to eq('true') end after do - entries.keys.each { |k| rugged.config.delete(k) } + entries.keys.each { |k| repository_rugged.config.delete(k) } end end describe '#delete_config' do - let(:repository) { Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') } - let(:rugged) { repository_rugged } + let(:repository) { mutable_repository } let(:entries) do { 'test.foo1' => 'bla bla', @@ -1687,21 +1483,19 @@ describe Gitlab::Git::Repository, :seed_helper do it 'can delete config settings' do entries.each do |key, value| - rugged.config[key] = value + repository_rugged.config[key] = value end expect(repository.delete_config(*%w[does.not.exist test.foo1 test.foo2])).to be_nil - config_keys = rugged.config.each_key.to_a + config_keys = repository_rugged.config.each_key.to_a expect(config_keys).not_to include('test.foo1') expect(config_keys).not_to include('test.foo2') end end describe '#merge' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } let(:source_sha) { '913c66a37b4a45b9769037c55c2d238bd0942d2e' } let(:target_branch) { 'test-merge-target-branch' } @@ -1713,46 +1507,34 @@ describe Gitlab::Git::Repository, :seed_helper do ensure_seeds end - shared_examples '#merge' do - it 'can perform a merge' do - merge_commit_id = nil - result = repository.merge(user, source_sha, target_branch, 'Test merge') do |commit_id| - merge_commit_id = commit_id - end - - expect(result.newrev).to eq(merge_commit_id) - expect(result.repo_created).to eq(false) - expect(result.branch_created).to eq(false) + it 'can perform a merge' do + merge_commit_id = nil + result = repository.merge(user, source_sha, target_branch, 'Test merge') do |commit_id| + merge_commit_id = commit_id end - it 'returns nil if there was a concurrent branch update' do - concurrent_update_id = '33f3729a45c02fc67d00adb1b8bca394b0e761d9' - result = repository.merge(user, source_sha, target_branch, 'Test merge') do - # This ref update should make the merge fail - repository.write_ref(Gitlab::Git::BRANCH_REF_PREFIX + target_branch, concurrent_update_id) - end - - # This 'nil' signals that the merge was not applied - expect(result).to be_nil + expect(result.newrev).to eq(merge_commit_id) + expect(result.repo_created).to eq(false) + expect(result.branch_created).to eq(false) + end - # Our concurrent ref update should not have been undone - expect(repository.find_branch(target_branch).target).to eq(concurrent_update_id) + it 'returns nil if there was a concurrent branch update' do + concurrent_update_id = '33f3729a45c02fc67d00adb1b8bca394b0e761d9' + result = repository.merge(user, source_sha, target_branch, 'Test merge') do + # This ref update should make the merge fail + repository.write_ref(Gitlab::Git::BRANCH_REF_PREFIX + target_branch, concurrent_update_id) end - end - context 'with gitaly' do - it_behaves_like '#merge' - end + # This 'nil' signals that the merge was not applied + expect(result).to be_nil - context 'without gitaly', :skip_gitaly_mock do - it_behaves_like '#merge' + # Our concurrent ref update should not have been undone + expect(repository.find_branch(target_branch).target).to eq(concurrent_update_id) end end describe '#ff_merge' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } let(:branch_head) { '6d394385cf567f80a8fd85055db1ab4c5295806f' } let(:source_sha) { 'cfe32cf61b73a0d5e9f13e774abde7ff789b1660' } let(:target_branch) { 'test-ff-target-branch' } @@ -1815,9 +1597,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe '#delete_all_refs_except' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end + let(:repository) { mutable_repository } before do repository.write_ref("refs/delete/a", "0b4bc9a49b562e85de7cc9e834518ea6828729b9") @@ -1841,12 +1621,7 @@ describe Gitlab::Git::Repository, :seed_helper do end describe 'remotes' do - let(:repository) do - Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '') - end - let(:rugged) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access { repository.rugged } - end + let(:repository) { mutable_repository } let(:remote_name) { 'my-remote' } let(:url) { 'http://my-repo.git' } @@ -1857,88 +1632,47 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#add_remote' do let(:mirror_refmap) { '+refs/*:refs/*' } - shared_examples 'add_remote' do - it 'added the remote' do - begin - rugged.remotes.delete(remote_name) - rescue Rugged::ConfigError - end - - repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) - - expect(rugged.remotes[remote_name]).not_to be_nil - expect(rugged.config["remote.#{remote_name}.mirror"]).to eq('true') - expect(rugged.config["remote.#{remote_name}.prune"]).to eq('true') - expect(rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) + it 'added the remote' do + begin + repository_rugged.remotes.delete(remote_name) + rescue Rugged::ConfigError end - end - context 'using Gitaly' do - it_behaves_like 'add_remote' - end + repository.add_remote(remote_name, url, mirror_refmap: mirror_refmap) - context 'with Gitaly disabled', :disable_gitaly do - it_behaves_like 'add_remote' + expect(repository_rugged.remotes[remote_name]).not_to be_nil + expect(repository_rugged.config["remote.#{remote_name}.mirror"]).to eq('true') + expect(repository_rugged.config["remote.#{remote_name}.prune"]).to eq('true') + expect(repository_rugged.config["remote.#{remote_name}.fetch"]).to eq(mirror_refmap) end end describe '#remove_remote' do - shared_examples 'remove_remote' do - it 'removes the remote' do - rugged.remotes.create(remote_name, url) + it 'removes the remote' do + repository_rugged.remotes.create(remote_name, url) - repository.remove_remote(remote_name) + repository.remove_remote(remote_name) - expect(rugged.remotes[remote_name]).to be_nil - end - end - - context 'using Gitaly' do - it_behaves_like 'remove_remote' - end - - context 'with Gitaly disabled', :disable_gitaly do - it_behaves_like 'remove_remote' + expect(repository_rugged.remotes[remote_name]).to be_nil end end end - describe '#gitlab_projects' do - subject { repository.gitlab_projects } - - it do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - expect(subject.shard_path).to eq(storage_path) - end - end - it { expect(subject.repository_relative_path).to eq(repository.relative_path) } - end - describe '#bundle_to_disk' do - shared_examples 'bundling to disk' do - let(:save_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } - - after do - FileUtils.rm_rf(save_path) - end - - it 'saves a bundle to disk' do - repository.bundle_to_disk(save_path) + let(:save_path) { File.join(Dir.tmpdir, "repo-#{SecureRandom.hex}.bundle") } - success = system( - *%W(#{Gitlab.config.git.bin_path} -C #{repository_path} bundle verify #{save_path}), - [:out, :err] => '/dev/null' - ) - expect(success).to be true - end + after do + FileUtils.rm_rf(save_path) end - context 'when Gitaly bundle_to_disk feature is enabled' do - it_behaves_like 'bundling to disk' - end + it 'saves a bundle to disk' do + repository.bundle_to_disk(save_path) - context 'when Gitaly bundle_to_disk feature is disabled', :disable_gitaly do - it_behaves_like 'bundling to disk' + success = system( + *%W(#{Gitlab.config.git.bin_path} -C #{repository_path} bundle verify #{save_path}), + [:out, :err] => '/dev/null' + ) + expect(success).to be true end end @@ -1975,7 +1709,7 @@ describe Gitlab::Git::Repository, :seed_helper do describe '#checksum' do it 'calculates the checksum for non-empty repo' do - expect(repository.checksum).to eq '4be7d24ce7e8d845502d599b72d567d23e6a40c0' + expect(repository.checksum).to eq '51d0a9662681f93e1fee547a6b7ba2bcaf716059' end it 'returns 0000000000000000000000000000000000000000 for an empty repo' do @@ -2013,138 +1747,41 @@ describe Gitlab::Git::Repository, :seed_helper do end end - context 'gitlab_projects commands' do - let(:gitlab_projects) { repository.gitlab_projects } - let(:timeout) { Gitlab.config.gitlab_shell.git_timeout } - - describe '#push_remote_branches' do - subject do - repository.push_remote_branches('downstream-remote', ['master']) - end - - it 'executes the command' do - expect(gitlab_projects).to receive(:push_branches) - .with('downstream-remote', timeout, true, ['master']) - .and_return(true) - - is_expected.to be_truthy - end - - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:push_branches) - .with('downstream-remote', timeout, true, ['master']) - .and_return(false) - - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end - end - - describe '#delete_remote_branches' do - subject do - repository.delete_remote_branches('downstream-remote', ['master']) - end - - it 'executes the command' do - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(true) - - is_expected.to be_truthy - end - - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(false) - - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end - end + describe '#clean_stale_repository_files' do + let(:worktree_path) { File.join(repository_path, 'worktrees', 'delete-me') } - describe '#delete_remote_branches' do - subject do - repository.delete_remote_branches('downstream-remote', ['master']) - end + it 'cleans up the files' do + create_worktree = %W[git -C #{repository_path} worktree add --detach #{worktree_path} master] + raise 'preparation failed' unless system(*create_worktree, err: '/dev/null') - it 'executes the command' do - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(true) + FileUtils.touch(worktree_path, mtime: Time.now - 8.hours) + # git rev-list --all will fail in git 2.16 if HEAD is pointing to a non-existent object, + # but the HEAD must be 40 characters long or git will ignore it. + File.write(File.join(worktree_path, 'HEAD'), Gitlab::Git::BLANK_SHA) - is_expected.to be_truthy - end + # git 2.16 fails with "fatal: bad object HEAD" + expect(rev_list_all).to be false - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(false) + repository.clean_stale_repository_files - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end + expect(rev_list_all).to be true + expect(File.exist?(worktree_path)).to be_falsey end - describe '#clean_stale_repository_files' do - let(:worktree_path) { File.join(repository_path, 'worktrees', 'delete-me') } - - it 'cleans up the files' do - create_worktree = %W[git -C #{repository_path} worktree add --detach #{worktree_path} master] - raise 'preparation failed' unless system(*create_worktree, err: '/dev/null') - - FileUtils.touch(worktree_path, mtime: Time.now - 8.hours) - # git rev-list --all will fail in git 2.16 if HEAD is pointing to a non-existent object, - # but the HEAD must be 40 characters long or git will ignore it. - File.write(File.join(worktree_path, 'HEAD'), Gitlab::Git::BLANK_SHA) - - # git 2.16 fails with "fatal: bad object HEAD" - expect(rev_list_all).to be false - - repository.clean_stale_repository_files - - expect(rev_list_all).to be true - expect(File.exist?(worktree_path)).to be_falsey - end - - def rev_list_all - system(*%W[git -C #{repository_path} rev-list --all], out: '/dev/null', err: '/dev/null') - end - - it 'increments a counter upon an error' do - expect(repository.gitaly_repository_client).to receive(:cleanup).and_raise(Gitlab::Git::CommandError) - - counter = double(:counter) - - expect(counter).to receive(:increment) - expect(Gitlab::Metrics).to receive(:counter).with(:failed_repository_cleanup_total, - 'Number of failed repository cleanup events').and_return(counter) - - repository.clean_stale_repository_files - end + def rev_list_all + system(*%W[git -C #{repository_path} rev-list --all], out: '/dev/null', err: '/dev/null') end - describe '#delete_remote_branches' do - subject do - repository.delete_remote_branches('downstream-remote', ['master']) - end + it 'increments a counter upon an error' do + expect(repository.gitaly_repository_client).to receive(:cleanup).and_raise(Gitlab::Git::CommandError) - it 'executes the command' do - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(true) + counter = double(:counter) - is_expected.to be_truthy - end + expect(counter).to receive(:increment) + expect(Gitlab::Metrics).to receive(:counter).with(:failed_repository_cleanup_total, + 'Number of failed repository cleanup events').and_return(counter) - it 'raises an error if the command fails' do - allow(gitlab_projects).to receive(:output) { 'error' } - expect(gitlab_projects).to receive(:delete_remote_branches) - .with('downstream-remote', ['master']) - .and_return(false) - - expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error') - end + repository.clean_stale_repository_files end end @@ -2187,13 +1824,11 @@ describe Gitlab::Git::Repository, :seed_helper do end context 'when the diff contains a rename' do - let(:repo) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged } - let(:end_sha) { new_commit_move_file(repo).oid } + let(:end_sha) { new_commit_move_file(repository_rugged).oid } after do # Erase our commits so other tests get the original repo - repo = Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '').rugged - repo.references.update('refs/heads/master', SeedRepo::LastCommit::ID) + repository_rugged.references.update('refs/heads/master', SeedRepo::LastCommit::ID) end it 'does not include the renamed file in the sparse checkout' do @@ -2240,10 +1875,9 @@ describe Gitlab::Git::Repository, :seed_helper do end end - def create_remote_branch(repository, remote_name, branch_name, source_branch_name) + def create_remote_branch(remote_name, branch_name, source_branch_name) source_branch = repository.branches.find { |branch| branch.name == source_branch_name } - rugged = repository_rugged - rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) + repository_rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_branch.dereferenced_target.sha) end # Build the options hash that's passed to Rugged::Commit#create @@ -2321,16 +1955,4 @@ describe Gitlab::Git::Repository, :seed_helper do line.split("\t").last end end - - def repository_rugged - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.rugged - end - end - - def repository_path - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - repository.path - end - end end diff --git a/spec/lib/gitlab/git/user_spec.rb b/spec/lib/gitlab/git/user_spec.rb index 99d850e1df9..d9d338206f8 100644 --- a/spec/lib/gitlab/git/user_spec.rb +++ b/spec/lib/gitlab/git/user_spec.rb @@ -22,10 +22,19 @@ describe Gitlab::Git::User do end describe '.from_gitlab' do - let(:user) { build(:user) } - subject { described_class.from_gitlab(user) } + context 'when no commit_email has been set' do + let(:user) { build(:user, email: 'alice@example.com', commit_email: nil) } + subject { described_class.from_gitlab(user) } - it { expect(subject).to eq(described_class.new(user.username, user.name, user.email, 'user-')) } + it { expect(subject).to eq(described_class.new(user.username, user.name, user.email, 'user-')) } + end + + context 'when commit_email has been set' do + let(:user) { build(:user, email: 'alice@example.com', commit_email: 'bob@example.com') } + subject { described_class.from_gitlab(user) } + + it { expect(subject).to eq(described_class.new(user.username, user.name, user.commit_email, 'user-')) } + end end describe '#==' do |