summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacob Vosmaer (GitLab) <jacob@gitlab.com>2018-07-03 09:12:03 +0000
committerDouwe Maan <douwe@gitlab.com>2018-07-03 09:12:03 +0000
commit15aadc665f266e8e974aded0fe1e0c7f1a9eb0fb (patch)
tree7cc2d811dfcb6f42fcfcf1fd6f126280a1e4606f
parent7e84f353b9c7ef79829923bd5888af5acf0aa8b6 (diff)
downloadgitlab-ce-15aadc665f266e8e974aded0fe1e0c7f1a9eb0fb.tar.gz
Make OperationService RPC's mandatory
-rw-r--r--GITALY_SERVER_VERSION2
-rw-r--r--lib/gitlab/git/repository.rb329
-rw-r--r--lib/gitlab/gitaly_client/operation_service.rb4
-rw-r--r--spec/features/tags/master_deletes_tag_spec.rb27
-rw-r--r--spec/lib/gitlab/git/repository_spec.rb145
-rw-r--r--spec/models/repository_spec.rb156
-rw-r--r--spec/services/files/update_service_spec.rb12
-rw-r--r--spec/services/merge_requests/rebase_service_spec.rb38
-rw-r--r--spec/services/merge_requests/squash_service_spec.rb45
9 files changed, 162 insertions, 596 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index 8b27ad70f93..068bced3785 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.109.0
+0.110.0
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 7c3b91f6efb..706aa7343be 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -648,18 +648,14 @@ module Gitlab
end
def add_branch(branch_name, user:, target:)
- gitaly_operation_client.user_create_branch(branch_name, user, target)
- rescue GRPC::FailedPrecondition => ex
- raise InvalidRef, ex
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_create_branch(branch_name, user, target)
+ end
end
def add_tag(tag_name, user:, target:, message: nil)
- gitaly_migrate(:operation_user_add_tag, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
- if is_enabled
- gitaly_add_tag(tag_name, user: user, target: target, message: message)
- else
- rugged_add_tag(tag_name, user: user, target: target, message: message)
- end
+ wrapped_gitaly_errors do
+ gitaly_operation_client.add_tag(tag_name, user, target, message)
end
end
@@ -668,22 +664,14 @@ module Gitlab
end
def rm_branch(branch_name, user:)
- gitaly_migrate(:operation_user_delete_branch, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
- if is_enabled
- gitaly_operations_client.user_delete_branch(branch_name, user)
- else
- OperationService.new(user, self).rm_branch(find_branch(branch_name))
- end
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_delete_branch(branch_name, user)
end
end
def rm_tag(tag_name, user:)
- gitaly_migrate(:operation_user_delete_tag) do |is_enabled|
- if is_enabled
- gitaly_operations_client.rm_tag(tag_name, user)
- else
- Gitlab::Git::OperationService.new(user, self).rm_tag(find_tag(tag_name))
- end
+ wrapped_gitaly_errors do
+ gitaly_operation_client.rm_tag(tag_name, user)
end
end
@@ -692,72 +680,29 @@ module Gitlab
end
def merge(user, source_sha, target_branch, message, &block)
- gitaly_migrate(:operation_user_merge_branch) do |is_enabled|
- if is_enabled
- gitaly_operation_client.user_merge_branch(user, source_sha, target_branch, message, &block)
- else
- rugged_merge(user, source_sha, target_branch, message, &block)
- end
- end
- end
-
- def rugged_merge(user, source_sha, target_branch, message)
- committer = Gitlab::Git.committer_hash(email: user.email, name: user.name)
-
- OperationService.new(user, self).with_branch(target_branch) do |start_commit|
- our_commit = start_commit.sha
- their_commit = source_sha
-
- raise 'Invalid merge target' unless our_commit
- raise 'Invalid merge source' unless their_commit
-
- merge_index = rugged.merge_commits(our_commit, their_commit)
- break if merge_index.conflicts?
-
- options = {
- parents: [our_commit, their_commit],
- tree: merge_index.write_tree(rugged),
- message: message,
- author: committer,
- committer: committer
- }
-
- commit_id = create_commit(options)
-
- yield commit_id
-
- commit_id
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_merge_branch(user, source_sha, target_branch, message, &block)
end
- rescue Gitlab::Git::CommitError # when merge_index.conflicts?
- nil
end
def ff_merge(user, source_sha, target_branch)
- gitaly_migrate(:operation_user_ff_branch) do |is_enabled|
- if is_enabled
- gitaly_ff_merge(user, source_sha, target_branch)
- else
- rugged_ff_merge(user, source_sha, target_branch)
- end
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_ff_branch(user, source_sha, target_branch)
end
end
def revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
- gitaly_migrate(:revert, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
- args = {
- user: user,
- commit: commit,
- branch_name: branch_name,
- message: message,
- start_branch_name: start_branch_name,
- start_repository: start_repository
- }
+ args = {
+ user: user,
+ commit: commit,
+ branch_name: branch_name,
+ message: message,
+ start_branch_name: start_branch_name,
+ start_repository: start_repository
+ }
- if is_enabled
- gitaly_operations_client.user_revert(args)
- else
- rugged_revert(args)
- end
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_revert(args)
end
end
@@ -775,21 +720,17 @@ module Gitlab
end
def cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
- gitaly_migrate(:cherry_pick) do |is_enabled|
- args = {
- user: user,
- commit: commit,
- branch_name: branch_name,
- message: message,
- start_branch_name: start_branch_name,
- start_repository: start_repository
- }
+ args = {
+ user: user,
+ commit: commit,
+ branch_name: branch_name,
+ message: message,
+ start_branch_name: start_branch_name,
+ start_repository: start_repository
+ }
- if is_enabled
- gitaly_operations_client.user_cherry_pick(args)
- else
- rugged_cherry_pick(args)
- end
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_cherry_pick(args)
end
end
@@ -1113,20 +1054,12 @@ module Gitlab
end
def rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:)
- gitaly_migrate(:rebase) do |is_enabled|
- if is_enabled
- gitaly_rebase(user, rebase_id,
- branch: branch,
- branch_sha: branch_sha,
- remote_repository: remote_repository,
- remote_branch: remote_branch)
- else
- git_rebase(user, rebase_id,
- branch: branch,
- branch_sha: branch_sha,
- remote_repository: remote_repository,
- remote_branch: remote_branch)
- end
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_rebase(user, rebase_id,
+ branch: branch,
+ branch_sha: branch_sha,
+ remote_repository: remote_repository,
+ remote_branch: remote_branch)
end
end
@@ -1137,13 +1070,9 @@ module Gitlab
end
def squash(user, squash_id, branch:, start_sha:, end_sha:, author:, message:)
- gitaly_migrate(:squash) do |is_enabled|
- if is_enabled
- gitaly_operation_client.user_squash(user, squash_id, branch,
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_squash(user, squash_id, branch,
start_sha, end_sha, author, message)
- else
- git_squash(user, squash_id, branch, start_sha, end_sha, author, message)
- end
end
end
@@ -1189,15 +1118,10 @@ module Gitlab
author_email: nil, author_name: nil,
start_branch_name: nil, start_repository: self)
- gitaly_migrate(:operation_user_commit_files) do |is_enabled|
- if is_enabled
- gitaly_operation_client.user_commit_files(user, branch_name,
+ wrapped_gitaly_errors do
+ gitaly_operation_client.user_commit_files(user, branch_name,
message, actions, author_email, author_name,
start_branch_name, start_repository)
- else
- rugged_multi_action(user, branch_name, message, actions,
- author_email, author_name, start_branch_name, start_repository)
- end
end
end
# rubocop:enable Metrics/ParameterLists
@@ -1217,10 +1141,6 @@ module Gitlab
Gitlab::GitalyClient::Util.repository(@storage, @relative_path, @gl_repository)
end
- def gitaly_operations_client
- @gitaly_operations_client ||= Gitlab::GitalyClient::OperationService.new(self)
- end
-
def gitaly_ref_client
@gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self)
end
@@ -1760,33 +1680,6 @@ module Gitlab
false
end
- def gitaly_add_tag(tag_name, user:, target:, message: nil)
- gitaly_operations_client.add_tag(tag_name, user, target, message)
- end
-
- def rugged_add_tag(tag_name, user:, target:, message: nil)
- target_object = Ref.dereference_object(lookup(target))
- raise InvalidRef.new("target not found: #{target}") unless target_object
-
- user = Gitlab::Git::User.from_gitlab(user) unless user.respond_to?(:gl_id)
-
- options = nil # Use nil, not the empty hash. Rugged cares about this.
- if message
- options = {
- message: message,
- tagger: Gitlab::Git.committer_hash(email: user.email, name: user.name)
- }
- end
-
- Gitlab::Git::OperationService.new(user, self).add_tag(tag_name, target_object.oid, options)
-
- find_tag(tag_name)
- rescue Rugged::ReferenceError => ex
- raise InvalidRef, ex
- rescue Rugged::TagError
- raise TagExistsError
- end
-
def rugged_create_branch(ref, start_point)
rugged_ref = rugged.branches.create(ref, start_point)
target_commit = Gitlab::Git::Commit.find(self, rugged_ref.target)
@@ -1831,28 +1724,6 @@ module Gitlab
end
end
- def rugged_revert(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
- OperationService.new(user, self).with_branch(
- branch_name,
- start_branch_name: start_branch_name,
- start_repository: start_repository
- ) do |start_commit|
-
- Gitlab::Git.check_namespace!(commit, start_repository)
-
- revert_tree_id = check_revert_content(commit, start_commit.sha)
- raise CreateTreeError unless revert_tree_id
-
- committer = user_to_committer(user)
-
- create_commit(message: message,
- author: committer,
- committer: committer,
- tree: revert_tree_id,
- parents: [start_commit.sha])
- end
- end
-
def rugged_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
OperationService.new(user, self).with_branch(
branch_name,
@@ -1892,71 +1763,6 @@ module Gitlab
tree_id
end
- def gitaly_rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:)
- gitaly_operation_client.user_rebase(user, rebase_id,
- branch: branch,
- branch_sha: branch_sha,
- remote_repository: remote_repository,
- remote_branch: remote_branch)
- end
-
- def git_rebase(user, rebase_id, branch:, branch_sha:, remote_repository:, remote_branch:)
- rebase_path = worktree_path(REBASE_WORKTREE_PREFIX, rebase_id)
- env = git_env_for_user(user)
-
- if remote_repository.is_a?(RemoteRepository)
- env.merge!(remote_repository.fetch_env)
- remote_repo_path = GITALY_INTERNAL_URL
- else
- remote_repo_path = remote_repository.path
- end
-
- with_worktree(rebase_path, branch, env: env) do
- run_git!(
- %W(pull --rebase #{remote_repo_path} #{remote_branch}),
- chdir: rebase_path, env: env
- )
-
- rebase_sha = run_git!(%w(rev-parse HEAD), chdir: rebase_path, env: env).strip
-
- update_branch(branch, user: user, newrev: rebase_sha, oldrev: branch_sha)
-
- rebase_sha
- end
- end
-
- def git_squash(user, squash_id, branch, start_sha, end_sha, author, message)
- squash_path = worktree_path(SQUASH_WORKTREE_PREFIX, squash_id)
- env = git_env_for_user(user).merge(
- 'GIT_AUTHOR_NAME' => author.name,
- 'GIT_AUTHOR_EMAIL' => author.email
- )
- diff_range = "#{start_sha}...#{end_sha}"
- diff_files = run_git!(
- %W(diff --name-only --diff-filter=ar --binary #{diff_range})
- ).chomp
-
- with_worktree(squash_path, branch, sparse_checkout_files: diff_files, env: env) do
- # Apply diff of the `diff_range` to the worktree
- diff = run_git!(%W(diff --binary #{diff_range}))
- run_git!(%w(apply --index --whitespace=nowarn), chdir: squash_path, env: env) do |stdin|
- stdin.binmode
- stdin.write(diff)
- end
-
- # Commit the `diff_range` diff
- run_git!(%W(commit --no-verify --message #{message}), chdir: squash_path, env: env)
-
- # Return the squash sha. May print a warning for ambiguous refs, but
- # we can ignore that with `--quiet` and just take the SHA, if present.
- # HEAD here always refers to the current HEAD commit, even if there is
- # another ref called HEAD.
- run_git!(
- %w(rev-parse --quiet --verify HEAD), chdir: squash_path, env: env
- ).chomp
- end
- end
-
def local_fetch_ref(source_path, source_ref:, target_ref:)
args = %W(fetch --no-tags -f #{source_path} #{source_ref}:#{target_ref})
run_git(args)
@@ -1968,22 +1774,6 @@ module Gitlab
run_git(args, env: source_repository.fetch_env)
end
- def gitaly_ff_merge(user, source_sha, target_branch)
- gitaly_operations_client.user_ff_branch(user, source_sha, target_branch)
- rescue GRPC::FailedPrecondition => e
- raise CommitError, e
- end
-
- def rugged_ff_merge(user, source_sha, target_branch)
- OperationService.new(user, self).with_branch(target_branch) do |our_commit|
- raise ArgumentError, 'Invalid merge target' unless our_commit
-
- source_sha
- end
- rescue Rugged::ReferenceError, InvalidRef
- raise ArgumentError, 'Invalid merge source'
- end
-
def rugged_add_remote(remote_name, url, mirror_refmap)
rugged.remotes.create(remote_name, url)
@@ -2035,39 +1825,6 @@ module Gitlab
remove_remote(remote_name)
end
- def rugged_multi_action(
- user, branch_name, message, actions, author_email, author_name,
- start_branch_name, start_repository)
-
- OperationService.new(user, self).with_branch(
- branch_name,
- start_branch_name: start_branch_name,
- start_repository: start_repository
- ) do |start_commit|
- index = Gitlab::Git::Index.new(self)
- parents = []
-
- if start_commit
- index.read_tree(start_commit.rugged_commit.tree)
- parents = [start_commit.sha]
- end
-
- actions.each { |opts| index.apply(opts.delete(:action), opts) }
-
- committer = user_to_committer(user)
- author = Gitlab::Git.committer_hash(email: author_email, name: author_name) || committer
- options = {
- tree: index.write_tree,
- message: message,
- parents: parents,
- author: author,
- committer: committer
- }
-
- create_commit(options)
- end
- end
-
def fetch_remote(remote_name = 'origin', env: nil)
run_git(['fetch', remote_name], env: env).last.zero?
end
diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb
index e9d4bb4c4b6..c04183a348f 100644
--- a/lib/gitlab/gitaly_client/operation_service.rb
+++ b/lib/gitlab/gitaly_client/operation_service.rb
@@ -64,6 +64,8 @@ module Gitlab
target_commit = Gitlab::Git::Commit.decorate(@repository, branch.target_commit)
Gitlab::Git::Branch.new(@repository, branch.name, target_commit.id, target_commit)
+ rescue GRPC::FailedPrecondition => ex
+ raise Gitlab::Git::Repository::InvalidRef, ex
end
def user_delete_branch(branch_name, user)
@@ -133,6 +135,8 @@ module Gitlab
request
).branch_update
Gitlab::Git::OperationService::BranchUpdate.from_gitaly(branch_update)
+ rescue GRPC::FailedPrecondition => e
+ raise Gitlab::Git::CommitError, e
end
def user_cherry_pick(user:, commit:, branch_name:, message:, start_branch_name:, start_repository:)
diff --git a/spec/features/tags/master_deletes_tag_spec.rb b/spec/features/tags/master_deletes_tag_spec.rb
index 9981bfa4609..1d4df2c55a7 100644
--- a/spec/features/tags/master_deletes_tag_spec.rb
+++ b/spec/features/tags/master_deletes_tag_spec.rb
@@ -35,30 +35,15 @@ feature 'Master deletes tag' do
end
context 'when pre-receive hook fails', :js do
- context 'when Gitaly operation_user_delete_tag feature is enabled' do
- before do
- allow_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rm_tag)
- .and_raise(Gitlab::Git::PreReceiveError, 'Do not delete tags')
- end
-
- scenario 'shows the error message' do
- delete_first_tag
-
- expect(page).to have_content('Do not delete tags')
- end
+ before do
+ allow_any_instance_of(Gitlab::GitalyClient::OperationService).to receive(:rm_tag)
+ .and_raise(Gitlab::Git::PreReceiveError, 'Do not delete tags')
end
- context 'when Gitaly operation_user_delete_tag feature is disabled', :skip_gitaly_mock do
- before do
- allow_any_instance_of(Gitlab::Git::HooksService).to receive(:execute)
- .and_raise(Gitlab::Git::PreReceiveError, 'Do not delete tags')
- end
-
- scenario 'shows the error message' do
- delete_first_tag
+ scenario 'shows the error message' do
+ delete_first_tag
- expect(page).to have_content('Do not delete tags')
- end
+ expect(page).to have_content('Do not delete tags')
end
end
diff --git a/spec/lib/gitlab/git/repository_spec.rb b/spec/lib/gitlab/git/repository_spec.rb
index 6ec4b90d70c..615faa4e7c9 100644
--- a/spec/lib/gitlab/git/repository_spec.rb
+++ b/spec/lib/gitlab/git/repository_spec.rb
@@ -1971,21 +1971,15 @@ describe Gitlab::Git::Repository, seed_helper: true do
end
end
- context 'with gitaly' do
- it "calls Gitaly's OperationService" do
- expect_any_instance_of(Gitlab::GitalyClient::OperationService)
- .to receive(:user_ff_branch).with(user, source_sha, target_branch)
- .and_return(nil)
+ it "calls Gitaly's OperationService" do
+ expect_any_instance_of(Gitlab::GitalyClient::OperationService)
+ .to receive(:user_ff_branch).with(user, source_sha, target_branch)
+ .and_return(nil)
- subject
- end
-
- it_behaves_like '#ff_merge'
+ subject
end
- context 'without gitaly', :skip_gitaly_mock do
- it_behaves_like '#ff_merge'
- end
+ it_behaves_like '#ff_merge'
end
describe '#delete_all_refs_except' do
@@ -2308,92 +2302,95 @@ describe Gitlab::Git::Repository, seed_helper: true do
expect { subject }.to raise_error(Gitlab::Git::CommandError, 'error')
end
end
+ end
- describe '#squash' do
- let(:squash_id) { '1' }
- let(:branch_name) { 'fix' }
- let(:start_sha) { '4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6' }
- let(:end_sha) { '12d65c8dd2b2676fa3ac47d955accc085a37a9c1' }
+ describe '#squash' do
+ let(:squash_id) { '1' }
+ let(:branch_name) { 'fix' }
+ let(:start_sha) { '4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6' }
+ let(:end_sha) { '12d65c8dd2b2676fa3ac47d955accc085a37a9c1' }
- subject do
- opts = {
- branch: branch_name,
- start_sha: start_sha,
- end_sha: end_sha,
- author: user,
- message: 'Squash commit message'
- }
+ subject do
+ opts = {
+ branch: branch_name,
+ start_sha: start_sha,
+ end_sha: end_sha,
+ author: user,
+ message: 'Squash commit message'
+ }
- repository.squash(user, squash_id, opts)
+ repository.squash(user, squash_id, opts)
+ end
+
+ # Should be ported to gitaly-ruby rspec suite https://gitlab.com/gitlab-org/gitaly/issues/1234
+ skip 'sparse checkout' do
+ let(:expected_files) { %w(files files/js files/js/application.js) }
+
+ it 'checks out only the files in the diff' do
+ allow(repository).to receive(:with_worktree).and_wrap_original do |m, *args|
+ m.call(*args) do
+ worktree_path = args[0]
+ files_pattern = File.join(worktree_path, '**', '*')
+ expected = expected_files.map do |path|
+ File.expand_path(path, worktree_path)
+ end
+
+ expect(Dir[files_pattern]).to eq(expected)
+ end
+ end
+
+ subject
end
- context 'sparse checkout', :skip_gitaly_mock do
- let(:expected_files) { %w(files files/js files/js/application.js) }
+ 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 }
- it 'checks out only the files in the diff' 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)
+ end
+
+ it 'does not include the renamed file in the sparse checkout' do
allow(repository).to receive(:with_worktree).and_wrap_original do |m, *args|
m.call(*args) do
worktree_path = args[0]
files_pattern = File.join(worktree_path, '**', '*')
- expected = expected_files.map do |path|
- File.expand_path(path, worktree_path)
- end
- expect(Dir[files_pattern]).to eq(expected)
+ expect(Dir[files_pattern]).not_to include('CHANGELOG')
+ expect(Dir[files_pattern]).not_to include('encoding/CHANGELOG')
end
end
subject
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 }
-
- 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)
- end
-
- it 'does not include the renamed file in the sparse checkout' do
- allow(repository).to receive(:with_worktree).and_wrap_original do |m, *args|
- m.call(*args) do
- worktree_path = args[0]
- files_pattern = File.join(worktree_path, '**', '*')
-
- expect(Dir[files_pattern]).not_to include('CHANGELOG')
- expect(Dir[files_pattern]).not_to include('encoding/CHANGELOG')
- end
- end
-
- subject
- end
- end
end
+ end
- context 'with an ASCII-8BIT diff', :skip_gitaly_mock do
- let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+✓ testme\n ======\n \n Sample repo for testing gitlab features\n" }
+ # Should be ported to gitaly-ruby rspec suite https://gitlab.com/gitlab-org/gitaly/issues/1234
+ skip 'with an ASCII-8BIT diff' do
+ let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+✓ testme\n ======\n \n Sample repo for testing gitlab features\n" }
- it 'applies a ASCII-8BIT diff' do
- allow(repository).to receive(:run_git!).and_call_original
- allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT'))
+ it 'applies a ASCII-8BIT diff' do
+ allow(repository).to receive(:run_git!).and_call_original
+ allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT'))
- expect(subject).to match(/\h{40}/)
- end
+ expect(subject).to match(/\h{40}/)
end
+ end
- context 'with trailing whitespace in an invalid patch', :skip_gitaly_mock do
- let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+ \n ====== \n \n Sample repo for testing gitlab features\n" }
+ # Should be ported to gitaly-ruby rspec suite https://gitlab.com/gitlab-org/gitaly/issues/1234
+ skip 'with trailing whitespace in an invalid patch' do
+ let(:diff) { "diff --git a/README.md b/README.md\nindex faaf198..43c5edf 100644\n--- a/README.md\n+++ b/README.md\n@@ -1,4 +1,4 @@\n-testme\n+ \n ====== \n \n Sample repo for testing gitlab features\n" }
- it 'does not include whitespace warnings in the error' do
- allow(repository).to receive(:run_git!).and_call_original
- allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT'))
+ it 'does not include whitespace warnings in the error' do
+ allow(repository).to receive(:run_git!).and_call_original
+ allow(repository).to receive(:run_git!).with(%W(diff --binary #{start_sha}...#{end_sha})).and_return(diff.force_encoding('ASCII-8BIT'))
- expect { subject }.to raise_error do |error|
- expect(error).to be_a(described_class::GitError)
- expect(error.message).not_to include('trailing whitespace')
- end
+ expect { subject }.to raise_error do |error|
+ expect(error).to be_a(described_class::GitError)
+ expect(error.message).not_to include('trailing whitespace')
end
end
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index cfa78c4472c..d060ab923d1 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -1861,155 +1861,61 @@ describe Repository do
describe '#add_tag' do
let(:user) { build_stubbed(:user) }
- shared_examples 'adding tag' do
- context 'with a valid target' do
- it 'creates the tag' do
- repository.add_tag(user, '8.5', 'master', 'foo')
-
- tag = repository.find_tag('8.5')
- expect(tag).to be_present
- expect(tag.message).to eq('foo')
- expect(tag.dereferenced_target.id).to eq(repository.commit('master').id)
- end
-
- it 'returns a Gitlab::Git::Tag object' do
- tag = repository.add_tag(user, '8.5', 'master', 'foo')
-
- expect(tag).to be_a(Gitlab::Git::Tag)
- end
- end
+ context 'with a valid target' do
+ it 'creates the tag' do
+ repository.add_tag(user, '8.5', 'master', 'foo')
- context 'with an invalid target' do
- it 'returns false' do
- expect(repository.add_tag(user, '8.5', 'bar', 'foo')).to be false
- end
+ tag = repository.find_tag('8.5')
+ expect(tag).to be_present
+ expect(tag.message).to eq('foo')
+ expect(tag.dereferenced_target.id).to eq(repository.commit('master').id)
end
- end
-
- context 'when Gitaly operation_user_add_tag feature is enabled' do
- it_behaves_like 'adding tag'
- end
-
- context 'when Gitaly operation_user_add_tag feature is disabled', :disable_gitaly do
- it_behaves_like 'adding tag'
-
- it 'passes commit SHA to pre-receive and update hooks and tag SHA to post-receive hook' do
- pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', project)
- update_hook = Gitlab::Git::Hook.new('update', project)
- post_receive_hook = Gitlab::Git::Hook.new('post-receive', project)
-
- allow(Gitlab::Git::Hook).to receive(:new)
- .and_return(pre_receive_hook, update_hook, post_receive_hook)
-
- allow(pre_receive_hook).to receive(:trigger).and_call_original
- allow(update_hook).to receive(:trigger).and_call_original
- allow(post_receive_hook).to receive(:trigger).and_call_original
+ it 'returns a Gitlab::Git::Tag object' do
tag = repository.add_tag(user, '8.5', 'master', 'foo')
- commit_sha = repository.commit('master').id
- tag_sha = tag.target
+ expect(tag).to be_a(Gitlab::Git::Tag)
+ end
+ end
- expect(pre_receive_hook).to have_received(:trigger)
- .with(anything, anything, anything, commit_sha, anything)
- expect(update_hook).to have_received(:trigger)
- .with(anything, anything, anything, commit_sha, anything)
- expect(post_receive_hook).to have_received(:trigger)
- .with(anything, anything, anything, tag_sha, anything)
+ context 'with an invalid target' do
+ it 'returns false' do
+ expect(repository.add_tag(user, '8.5', 'bar', 'foo')).to be false
end
end
end
describe '#rm_branch' do
- shared_examples "user deleting a branch" do
- it 'removes a branch' do
- expect(repository).to receive(:before_remove_branch)
- expect(repository).to receive(:after_remove_branch)
+ it 'removes a branch' do
+ expect(repository).to receive(:before_remove_branch)
+ expect(repository).to receive(:after_remove_branch)
- repository.rm_branch(user, 'feature')
- end
+ repository.rm_branch(user, 'feature')
end
- context 'with gitaly enabled' do
- it_behaves_like "user deleting a branch"
-
- context 'when pre hooks failed' do
- before do
- allow_any_instance_of(Gitlab::GitalyClient::OperationService)
- .to receive(:user_delete_branch).and_raise(Gitlab::Git::PreReceiveError)
- end
-
- it 'gets an error and does not delete the branch' do
- expect do
- repository.rm_branch(user, 'feature')
- end.to raise_error(Gitlab::Git::PreReceiveError)
-
- expect(repository.find_branch('feature')).not_to be_nil
- end
- end
- end
-
- context 'with gitaly disabled', :disable_gitaly do
- it_behaves_like "user deleting a branch"
-
- let(:old_rev) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' } # git rev-parse feature
- let(:blank_sha) { '0000000000000000000000000000000000000000' }
-
- context 'when pre hooks were successful' do
- it 'runs without errors' do
- expect_any_instance_of(Gitlab::Git::HooksService).to receive(:execute)
- .with(git_user, repository.raw_repository, old_rev, blank_sha, 'refs/heads/feature')
-
- expect { repository.rm_branch(user, 'feature') }.not_to raise_error
- end
-
- it 'deletes the branch' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([true, nil])
-
- expect { repository.rm_branch(user, 'feature') }.not_to raise_error
-
- expect(repository.find_branch('feature')).to be_nil
- end
+ context 'when pre hooks failed' do
+ before do
+ allow_any_instance_of(Gitlab::GitalyClient::OperationService)
+ .to receive(:user_delete_branch).and_raise(Gitlab::Git::PreReceiveError)
end
- context 'when pre hooks failed' do
- it 'gets an error' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
-
- expect do
- repository.rm_branch(user, 'feature')
- end.to raise_error(Gitlab::Git::PreReceiveError)
- end
-
- it 'does not delete the branch' do
- allow_any_instance_of(Gitlab::Git::Hook).to receive(:trigger).and_return([false, ''])
+ it 'gets an error and does not delete the branch' do
+ expect do
+ repository.rm_branch(user, 'feature')
+ end.to raise_error(Gitlab::Git::PreReceiveError)
- expect do
- repository.rm_branch(user, 'feature')
- end.to raise_error(Gitlab::Git::PreReceiveError)
- expect(repository.find_branch('feature')).not_to be_nil
- end
+ expect(repository.find_branch('feature')).not_to be_nil
end
end
end
describe '#rm_tag' do
- shared_examples 'removing tag' do
- it 'removes a tag' do
- expect(repository).to receive(:before_remove_tag)
+ it 'removes a tag' do
+ expect(repository).to receive(:before_remove_tag)
- repository.rm_tag(build_stubbed(:user), 'v1.1.0')
-
- expect(repository.find_tag('v1.1.0')).to be_nil
- end
- end
-
- context 'when Gitaly operation_user_delete_tag feature is enabled' do
- it_behaves_like 'removing tag'
- end
+ repository.rm_tag(build_stubbed(:user), 'v1.1.0')
- context 'when Gitaly operation_user_delete_tag feature is disabled', :skip_gitaly_mock do
- it_behaves_like 'removing tag'
+ expect(repository.find_tag('v1.1.0')).to be_nil
end
end
diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb
index 16bfbdf3089..eaee89fb1a5 100644
--- a/spec/services/files/update_service_spec.rb
+++ b/spec/services/files/update_service_spec.rb
@@ -71,17 +71,5 @@ describe Files::UpdateService do
expect(results.data).to eq(new_contents)
end
end
-
- context 'with gitaly disabled', :skip_gitaly_mock do
- context 'when target branch is different than source branch' do
- let(:branch_name) { "#{project.default_branch}-new" }
-
- it 'fires hooks only once' do
- expect(Gitlab::Git::HooksService).to receive(:new).once.and_call_original
-
- subject.execute
- end
- end
- end
end
end
diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb
index 757c31ab692..4daa25f8cf2 100644
--- a/spec/services/merge_requests/rebase_service_spec.rb
+++ b/spec/services/merge_requests/rebase_service_spec.rb
@@ -36,9 +36,9 @@ describe MergeRequests::RebaseService do
end
end
- context 'when unexpected error occurs', :disable_gitaly do
+ context 'when unexpected error occurs' do
before do
- allow(repository).to receive(:run_git!).and_raise('Something went wrong')
+ allow(repository).to receive(:gitaly_operation_client).and_raise('Something went wrong')
end
it 'saves a generic error message' do
@@ -53,9 +53,9 @@ describe MergeRequests::RebaseService do
end
end
- context 'with git command failure', :disable_gitaly do
+ context 'with git command failure' do
before do
- allow(repository).to receive(:run_git!).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong')
+ allow(repository).to receive(:gitaly_operation_client).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong')
end
it 'saves a generic error message' do
@@ -71,7 +71,7 @@ describe MergeRequests::RebaseService do
end
context 'valid params' do
- shared_examples 'successful rebase' do
+ describe 'successful rebase' do
before do
service.execute(merge_request)
end
@@ -97,26 +97,8 @@ describe MergeRequests::RebaseService do
end
end
- context 'when Gitaly rebase feature is enabled' do
- it_behaves_like 'successful rebase'
- end
-
- context 'when Gitaly rebase feature is disabled', :disable_gitaly do
- it_behaves_like 'successful rebase'
- end
-
- context 'git commands', :disable_gitaly do
- it 'sets GL_REPOSITORY env variable when calling git commands' do
- expect(repository).to receive(:popen).exactly(3)
- .with(anything, anything, hash_including('GL_REPOSITORY'), anything)
- .and_return(['', 0])
-
- service.execute(merge_request)
- end
- end
-
context 'fork' do
- shared_examples 'successful fork rebase' do
+ describe 'successful fork rebase' do
let(:forked_project) do
fork_project(project, user, repository: true)
end
@@ -140,14 +122,6 @@ describe MergeRequests::RebaseService do
expect(parent_sha).to eq(target_branch_sha)
end
end
-
- context 'when Gitaly rebase feature is enabled' do
- it_behaves_like 'successful fork rebase'
- end
-
- context 'when Gitaly rebase feature is disabled', :disable_gitaly do
- it_behaves_like 'successful fork rebase'
- end
end
end
end
diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb
index ded17fa92a4..8ab09412f55 100644
--- a/spec/services/merge_requests/squash_service_spec.rb
+++ b/spec/services/merge_requests/squash_service_spec.rb
@@ -124,51 +124,6 @@ describe MergeRequests::SquashService do
message: a_string_including('squash'))
end
end
-
- context 'with Gitaly disabled', :skip_gitaly_mock do
- stages = {
- 'add worktree for squash' => 'worktree',
- 'configure sparse checkout' => 'config',
- 'get files in diff' => 'diff --name-only',
- 'check out target branch' => 'checkout',
- 'apply patch' => 'diff --binary',
- 'commit squashed changes' => 'commit',
- 'get SHA of squashed commit' => 'rev-parse'
- }
-
- stages.each do |stage, command|
- context "when the #{stage} stage fails" do
- before do
- git_command = a_collection_containing_exactly(
- a_string_starting_with("#{Gitlab.config.git.bin_path} #{command}")
- ).or(
- a_collection_starting_with([Gitlab.config.git.bin_path] + command.split)
- )
-
- allow(repository).to receive(:popen).and_return(['', 0])
- allow(repository).to receive(:popen).with(git_command, anything, anything, anything).and_return([error, 1])
- end
-
- it 'logs the stage and output' do
- expect(service).to receive(:log_error).with(log_error)
- expect(service).to receive(:log_error).with(error)
-
- service.execute(merge_request)
- end
-
- it 'returns an error' do
- expect(service.execute(merge_request)).to match(status: :error,
- message: a_string_including('squash'))
- end
-
- it 'cleans up the temporary directory' do
- expect(File.exist?(squash_dir_path)).to be(false)
-
- service.execute(merge_request)
- end
- end
- end
- end
end
context 'when any other exception is thrown' do