diff options
Diffstat (limited to 'spec/lib/gitlab/gitaly_client')
| -rw-r--r-- | spec/lib/gitlab/gitaly_client/commit_service_spec.rb | 115 | ||||
| -rw-r--r-- | spec/lib/gitlab/gitaly_client/operation_service_spec.rb | 230 |
2 files changed, 249 insertions, 96 deletions
diff --git a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb index 92860c9232f..3a34d39c722 100644 --- a/spec/lib/gitlab/gitaly_client/commit_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/commit_service_spec.rb @@ -340,17 +340,12 @@ RSpec.describe Gitlab::GitalyClient::CommitService do let(:revisions) { [revision] } let(:gitaly_commits) { create_list(:gitaly_commit, 3) } let(:expected_commits) { gitaly_commits.map { |c| Gitlab::Git::Commit.new(repository, c) }} - let(:filter_quarantined_commits) { false } subject do - client.list_new_commits(revisions, allow_quarantine: allow_quarantine) + client.list_new_commits(revisions) end shared_examples 'a #list_all_commits message' do - before do - stub_feature_flags(filter_quarantined_commits: filter_quarantined_commits) - end - it 'sends a list_all_commits message' do expected_repository = repository.gitaly_repository.dup expected_repository.git_alternate_object_directories = Google::Protobuf::RepeatedField.new(:string) @@ -360,29 +355,25 @@ RSpec.describe Gitlab::GitalyClient::CommitService do .with(gitaly_request_with_params(repository: expected_repository), kind_of(Hash)) .and_return([Gitaly::ListAllCommitsResponse.new(commits: gitaly_commits)]) - if filter_quarantined_commits - # The object directory of the repository must not be set so that we - # don't use the quarantine directory. - objects_exist_repo = repository.gitaly_repository.dup - objects_exist_repo.git_object_directory = "" - - # The first request contains the repository, the second request the - # commit IDs we want to check for existence. - objects_exist_request = [ - gitaly_request_with_params(repository: objects_exist_repo), - gitaly_request_with_params(revisions: gitaly_commits.map(&:id)) - ] - - objects_exist_response = Gitaly::CheckObjectsExistResponse.new(revisions: revision_existence.map do - |rev, exists| Gitaly::CheckObjectsExistResponse::RevisionExistence.new(name: rev, exists: exists) - end) - - expect(service).to receive(:check_objects_exist) - .with(objects_exist_request, kind_of(Hash)) - .and_return([objects_exist_response]) - else - expect(service).not_to receive(:check_objects_exist) - end + # The object directory of the repository must not be set so that we + # don't use the quarantine directory. + objects_exist_repo = repository.gitaly_repository.dup + objects_exist_repo.git_object_directory = "" + + # The first request contains the repository, the second request the + # commit IDs we want to check for existence. + objects_exist_request = [ + gitaly_request_with_params(repository: objects_exist_repo), + gitaly_request_with_params(revisions: gitaly_commits.map(&:id)) + ] + + objects_exist_response = Gitaly::CheckObjectsExistResponse.new(revisions: revision_existence.map do + |rev, exists| Gitaly::CheckObjectsExistResponse::RevisionExistence.new(name: rev, exists: exists) + end) + + expect(service).to receive(:check_objects_exist) + .with(objects_exist_request, kind_of(Hash)) + .and_return([objects_exist_response]) end expect(subject).to eq(expected_commits) @@ -418,49 +409,31 @@ RSpec.describe Gitlab::GitalyClient::CommitService do } end - context 'with allowed quarantine' do - let(:allow_quarantine) { true } - - context 'without commit filtering' do - it_behaves_like 'a #list_all_commits message' - end - - context 'with commit filtering' do - let(:filter_quarantined_commits) { true } - - context 'reject commits which exist in target repository' do - let(:revision_existence) { gitaly_commits.to_h { |c| [c.id, true] } } - let(:expected_commits) { [] } - - it_behaves_like 'a #list_all_commits message' - end - - context 'keep commits which do not exist in target repository' do - let(:revision_existence) { gitaly_commits.to_h { |c| [c.id, false] } } + context 'reject commits which exist in target repository' do + let(:revision_existence) { gitaly_commits.to_h { |c| [c.id, true] } } + let(:expected_commits) { [] } - it_behaves_like 'a #list_all_commits message' - end + it_behaves_like 'a #list_all_commits message' + end - context 'mixed existing and nonexisting commits' do - let(:revision_existence) do - { - gitaly_commits[0].id => true, - gitaly_commits[1].id => false, - gitaly_commits[2].id => true - } - end + context 'keep commits which do not exist in target repository' do + let(:revision_existence) { gitaly_commits.to_h { |c| [c.id, false] } } - let(:expected_commits) { [Gitlab::Git::Commit.new(repository, gitaly_commits[1])] } + it_behaves_like 'a #list_all_commits message' + end - it_behaves_like 'a #list_all_commits message' - end + context 'mixed existing and nonexisting commits' do + let(:revision_existence) do + { + gitaly_commits[0].id => true, + gitaly_commits[1].id => false, + gitaly_commits[2].id => true + } end - end - context 'with disallowed quarantine' do - let(:allow_quarantine) { false } + let(:expected_commits) { [Gitlab::Git::Commit.new(repository, gitaly_commits[1])] } - it_behaves_like 'a #list_commits message' + it_behaves_like 'a #list_all_commits message' end end @@ -472,17 +445,7 @@ RSpec.describe Gitlab::GitalyClient::CommitService do } end - context 'with allowed quarantine' do - let(:allow_quarantine) { true } - - it_behaves_like 'a #list_commits message' - end - - context 'with disallowed quarantine' do - let(:allow_quarantine) { false } - - it_behaves_like 'a #list_commits message' - end + it_behaves_like 'a #list_commits message' end end diff --git a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb index 0c04863f466..4320c5460da 100644 --- a/spec/lib/gitlab/gitaly_client/operation_service_spec.rb +++ b/spec/lib/gitlab/gitaly_client/operation_service_spec.rb @@ -170,6 +170,65 @@ RSpec.describe Gitlab::GitalyClient::OperationService do Gitlab::Git::PreReceiveError, "something failed") end end + + context 'with a custom hook error' do + let(:stdout) { nil } + let(:stderr) { nil } + let(:error_message) { "error_message" } + let(:custom_hook_error) do + new_detailed_error( + GRPC::Core::StatusCodes::PERMISSION_DENIED, + error_message, + Gitaly::UserDeleteBranchError.new( + custom_hook: Gitaly::CustomHookError.new( + stdout: stdout, + stderr: stderr, + hook_type: Gitaly::CustomHookError::HookType::HOOK_TYPE_PRERECEIVE + ))) + end + + shared_examples 'a failed branch deletion' do + it 'raises a PreRecieveError' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_delete_branch).with(request, kind_of(Hash)) + .and_raise(custom_hook_error) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(Gitlab::Git::PreReceiveError) + expect(error.message).to eq(expected_message) + expect(error.raw_message).to eq(expected_raw_message) + end + end + end + + context 'when details contain stderr' do + let(:stderr) { "something" } + let(:stdout) { "GL-HOOK-ERR: stdout is overridden by stderr" } + let(:expected_message) { error_message } + let(:expected_raw_message) { stderr } + + it_behaves_like 'a failed branch deletion' + end + + context 'when details contain stdout' do + let(:stderr) { " \n" } + let(:stdout) { "something" } + let(:expected_message) { error_message } + let(:expected_raw_message) { stdout } + + it_behaves_like 'a failed branch deletion' + end + end + + context 'with a non-detailed error' do + it 'raises a GRPC error' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_delete_branch).with(request, kind_of(Hash)) + .and_raise(GRPC::Internal.new('non-detailed error')) + + expect { subject }.to raise_error(GRPC::Internal) + end + end end describe '#user_merge_branch' do @@ -212,6 +271,82 @@ RSpec.describe Gitlab::GitalyClient::OperationService do end end + context 'with a custom hook error' do + let(:stdout) { nil } + let(:stderr) { nil } + let(:error_message) { "error_message" } + let(:custom_hook_error) do + new_detailed_error( + GRPC::Core::StatusCodes::PERMISSION_DENIED, + error_message, + Gitaly::UserMergeBranchError.new( + custom_hook: Gitaly::CustomHookError.new( + stdout: stdout, + stderr: stderr, + hook_type: Gitaly::CustomHookError::HookType::HOOK_TYPE_PRERECEIVE + ))) + end + + shared_examples 'a failed merge' do + it 'raises a PreRecieveError' do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_merge_branch).with(kind_of(Enumerator), kind_of(Hash)) + .and_raise(custom_hook_error) + + expect { subject }.to raise_error do |error| + expect(error).to be_a(Gitlab::Git::PreReceiveError) + expect(error.message).to eq(expected_message) + expect(error.raw_message).to eq(expected_raw_message) + end + end + end + + context 'when details contain stderr without prefix' do + let(:stderr) { "something" } + let(:stdout) { "GL-HOOK-ERR: stdout is overridden by stderr" } + let(:expected_message) { error_message } + let(:expected_raw_message) { stderr } + + it_behaves_like 'a failed merge' + end + + context 'when details contain stderr with prefix' do + let(:stderr) { "GL-HOOK-ERR: something" } + let(:stdout) { "GL-HOOK-ERR: stdout is overridden by stderr" } + let(:expected_message) { "something" } + let(:expected_raw_message) { stderr } + + it_behaves_like 'a failed merge' + end + + context 'when details contain stdout without prefix' do + let(:stderr) { " \n" } + let(:stdout) { "something" } + let(:expected_message) { error_message } + let(:expected_raw_message) { stdout } + + it_behaves_like 'a failed merge' + end + + context 'when details contain stdout with prefix' do + let(:stderr) { " \n" } + let(:stdout) { "GL-HOOK-ERR: something" } + let(:expected_message) { "something" } + let(:expected_raw_message) { stdout } + + it_behaves_like 'a failed merge' + end + + context 'when details contain no stderr or stdout' do + let(:stderr) { " \n" } + let(:stdout) { "\n \n" } + let(:expected_message) { error_message } + let(:expected_raw_message) { "\n \n" } + + it_behaves_like 'a failed merge' + end + end + context 'with an exception without the detailed error' do let(:permission_error) do GRPC::PermissionDenied.new @@ -340,6 +475,15 @@ RSpec.describe Gitlab::GitalyClient::OperationService do end end + shared_examples '#user_cherry_pick with a gRPC error' do + it 'raises an exception' do + expect_any_instance_of(Gitaly::OperationService::Stub).to receive(:user_cherry_pick) + .and_raise(raised_error) + + expect { subject }.to raise_error(expected_error, expected_error_message) + end + end + describe '#user_cherry_pick' do let(:response_class) { Gitaly::UserCherryPickResponse } @@ -354,13 +498,74 @@ RSpec.describe Gitlab::GitalyClient::OperationService do ) end - before do - expect_any_instance_of(Gitaly::OperationService::Stub) - .to receive(:user_cherry_pick).with(kind_of(Gitaly::UserCherryPickRequest), kind_of(Hash)) - .and_return(response) + context 'when errors are not raised but returned in the response' do + before do + expect_any_instance_of(Gitaly::OperationService::Stub) + .to receive(:user_cherry_pick).with(kind_of(Gitaly::UserCherryPickRequest), kind_of(Hash)) + .and_return(response) + end + + it_behaves_like 'cherry pick and revert errors' end - it_behaves_like 'cherry pick and revert errors' + context 'when AccessCheckError is raised' do + let(:raised_error) do + new_detailed_error( + GRPC::Core::StatusCodes::INTERNAL, + 'something failed', + Gitaly::UserCherryPickError.new( + access_check: Gitaly::AccessCheckError.new( + error_message: 'something went wrong' + ))) + end + + let(:expected_error) { Gitlab::Git::PreReceiveError } + let(:expected_error_message) { "something went wrong" } + + it_behaves_like '#user_cherry_pick with a gRPC error' + end + + context 'when NotAncestorError is raised' do + let(:raised_error) do + new_detailed_error( + GRPC::Core::StatusCodes::FAILED_PRECONDITION, + 'Branch diverged', + Gitaly::UserCherryPickError.new( + target_branch_diverged: Gitaly::NotAncestorError.new + ) + ) + end + + let(:expected_error) { Gitlab::Git::CommitError } + let(:expected_error_message) { 'branch diverged' } + + it_behaves_like '#user_cherry_pick with a gRPC error' + end + + context 'when MergeConflictError is raised' do + let(:raised_error) do + new_detailed_error( + GRPC::Core::StatusCodes::FAILED_PRECONDITION, + 'Conflict', + Gitaly::UserCherryPickError.new( + cherry_pick_conflict: Gitaly::MergeConflictError.new + ) + ) + end + + let(:expected_error) { Gitlab::Git::Repository::CreateTreeError } + let(:expected_error_message) { } + + it_behaves_like '#user_cherry_pick with a gRPC error' + end + + context 'when a non-detailed gRPC error is raised' do + let(:raised_error) { GRPC::Internal.new('non-detailed error') } + let(:expected_error) { GRPC::Internal } + let(:expected_error_message) { } + + it_behaves_like '#user_cherry_pick with a gRPC error' + end end describe '#user_revert' do @@ -489,21 +694,6 @@ RSpec.describe Gitlab::GitalyClient::OperationService do expect(subject).to eq(squash_sha) end - context "when git_error is present" do - let(:response) do - Gitaly::UserSquashResponse.new(git_error: "something failed") - end - - it "raises a GitError exception" do - expect_any_instance_of(Gitaly::OperationService::Stub) - .to receive(:user_squash).with(request, kind_of(Hash)) - .and_return(response) - - expect { subject }.to raise_error( - Gitlab::Git::Repository::GitError, "something failed") - end - end - shared_examples '#user_squash with an error' do it 'raises a GitError exception' do expect_any_instance_of(Gitaly::OperationService::Stub) |