From dd35c1d556ea5809ef2aefd8ec5741d7b4d83372 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Thu, 14 Sep 2017 17:26:03 +0300 Subject: Add spec for FfMergeService --- .../merge_requests/ff_merge_service_spec.rb | 84 ++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 spec/services/merge_requests/ff_merge_service_spec.rb (limited to 'spec/services') diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb new file mode 100644 index 00000000000..aaabf3ed2b0 --- /dev/null +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -0,0 +1,84 @@ +require 'spec_helper' + +describe MergeRequests::FfMergeService do + let(:user) { create(:user) } + let(:user2) { create(:user) } + let(:merge_request) do + create(:merge_request, + source_branch: 'flatten-dir', + target_branch: 'improve/awesome', + assignee: user2) + end + let(:project) { merge_request.project } + + before do + project.team << [user, :master] + project.team << [user2, :developer] + end + + describe '#execute' do + context 'valid params' do + let(:service) { described_class.new(project, user, {}) } + + before do + allow(service).to receive(:execute_hooks) + + perform_enqueued_jobs do + service.execute(merge_request) + end + end + + it "does not create merge commit" do + source_branch_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha + target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha + expect(source_branch_sha).to eq(target_branch_sha) + end + + it { expect(merge_request).to be_valid } + it { expect(merge_request).to be_merged } + + it 'sends email to user2 about merge of new merge_request' do + email = ActionMailer::Base.deliveries.last + expect(email.to.first).to eq(user2.email) + expect(email.subject).to include(merge_request.title) + end + + it 'creates system note about merge_request merge' do + note = merge_request.notes.last + expect(note.note).to include 'merged' + end + end + + context "error handling" do + let(:service) { described_class.new(project, user, commit_message: 'Awesome message') } + + before do + allow(Rails.logger).to receive(:error) + end + + it 'logs and saves error if there is an exception' do + error_message = 'error message' + + allow(service).to receive(:repository).and_raise("error message") + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + + it 'logs and saves error if there is an PreReceiveError exception' do + error_message = 'error message' + + allow(service).to receive(:repository).and_raise(Gitlab::Git::HooksService::PreReceiveError, error_message) + allow(service).to receive(:execute_hooks) + + service.execute(merge_request) + + expect(merge_request.merge_error).to include(error_message) + expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message)) + end + end + end +end -- cgit v1.2.1 From 8f47d484dab12df982655c3c05305bce7624914d Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 25 Sep 2017 16:22:00 +0200 Subject: Extract pipeline chain builder classes from service --- spec/services/ci/create_pipeline_service_spec.rb | 99 ------------------------ 1 file changed, 99 deletions(-) (limited to 'spec/services') diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 4c2ff08039c..5b959b9a142 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -482,103 +482,4 @@ describe Ci::CreatePipelineService do end end - describe '#allowed_to_create?' do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:ref) { 'master' } - - subject do - described_class.new(project, user, ref: ref) - .send(:allowed_to_create?) - end - - context 'when user is a developer' do - before do - project.add_developer(user) - end - - it { is_expected.to be_truthy } - - context 'when the branch is protected' do - let!(:protected_branch) do - create(:protected_branch, project: project, name: ref) - end - - it { is_expected.to be_falsey } - - context 'when developers are allowed to merge' do - let!(:protected_branch) do - create(:protected_branch, - :developers_can_merge, - project: project, - name: ref) - end - - it { is_expected.to be_truthy } - end - end - - context 'when the tag is protected' do - let(:ref) { 'v1.0.0' } - - let!(:protected_tag) do - create(:protected_tag, project: project, name: ref) - end - - it { is_expected.to be_falsey } - - context 'when developers are allowed to create the tag' do - let!(:protected_tag) do - create(:protected_tag, - :developers_can_create, - project: project, - name: ref) - end - - it { is_expected.to be_truthy } - end - end - end - - context 'when user is a master' do - before do - project.add_master(user) - end - - it { is_expected.to be_truthy } - - context 'when the branch is protected' do - let!(:protected_branch) do - create(:protected_branch, project: project, name: ref) - end - - it { is_expected.to be_truthy } - end - - context 'when the tag is protected' do - let(:ref) { 'v1.0.0' } - - let!(:protected_tag) do - create(:protected_tag, project: project, name: ref) - end - - it { is_expected.to be_truthy } - - context 'when no one can create the tag' do - let!(:protected_tag) do - create(:protected_tag, - :no_one_can_create, - project: project, - name: ref) - end - - it { is_expected.to be_falsey } - end - end - end - - context 'when owner cannot create pipeline' do - it { is_expected.to be_falsey } - end - end end -- cgit v1.2.1 From 7cfaccd6edba11e43963bbd4dcb5c2bb3c71d9f4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 26 Sep 2017 09:38:16 +0200 Subject: Fix code style offenses in pipeline create services --- spec/services/ci/create_pipeline_service_spec.rb | 1 - 1 file changed, 1 deletion(-) (limited to 'spec/services') diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 5b959b9a142..6ee75b8fc23 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -481,5 +481,4 @@ describe Ci::CreatePipelineService do end end end - end -- cgit v1.2.1 From 057a8b709346a89e2ccdfe6e9b352ce5f93e71c7 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 26 Sep 2017 15:01:16 +0200 Subject: Add test for head pipeline assignment when skipped Closes gitlab-org/gitlab-ce#34415 --- spec/services/ci/create_pipeline_service_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'spec/services') diff --git a/spec/services/ci/create_pipeline_service_spec.rb b/spec/services/ci/create_pipeline_service_spec.rb index 6ee75b8fc23..eb6e683cc23 100644 --- a/spec/services/ci/create_pipeline_service_spec.rb +++ b/spec/services/ci/create_pipeline_service_spec.rb @@ -133,6 +133,26 @@ describe Ci::CreatePipelineService do expect(merge_request.reload.head_pipeline).to eq head_pipeline end end + + context 'when pipeline has been skipped' do + before do + allow_any_instance_of(Ci::Pipeline) + .to receive(:git_commit_message) + .and_return('some commit [ci skip]') + end + + it 'updates merge request head pipeline' do + merge_request = create(:merge_request, source_branch: 'master', + target_branch: 'feature', + source_project: project) + + head_pipeline = execute_service + + expect(head_pipeline).to be_skipped + expect(head_pipeline).to be_persisted + expect(merge_request.reload.head_pipeline).to eq head_pipeline + end + end end context 'auto-cancel enabled' do -- cgit v1.2.1 From e38dc10c09ac6e6d7752d2bb9eeb0c3feab9765a Mon Sep 17 00:00:00 2001 From: Oswaldo Ferreira Date: Wed, 27 Sep 2017 17:38:23 -0300 Subject: Clean merge_jid whenever necessary on the merge process MergeRequest#merge_jid should be cleaned up whenever we hit a known error on MergeService#execute. This way we can keep track if the MR is really "ongoing" or "stuck" --- spec/services/merge_requests/merge_service_spec.rb | 23 +++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) (limited to 'spec/services') diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index b60136064b7..80213d093f1 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -13,20 +13,21 @@ describe MergeRequests::MergeService do describe '#execute' do context 'MergeRequest#merge_jid' do + let(:service) do + described_class.new(project, user, commit_message: 'Awesome message') + end + before do merge_request.update_column(:merge_jid, 'hash-123') end it 'is cleaned when no error is raised' do - service = described_class.new(project, user, commit_message: 'Awesome message') - service.execute(merge_request) expect(merge_request.reload.merge_jid).to be_nil end it 'is cleaned when expected error is raised' do - service = described_class.new(project, user, commit_message: 'Awesome message') allow(service).to receive(:commit).and_raise(described_class::MergeError) service.execute(merge_request) @@ -34,6 +35,22 @@ describe MergeRequests::MergeService do expect(merge_request.reload.merge_jid).to be_nil end + it 'is cleaned when merge request is not mergeable' do + allow(merge_request).to receive(:mergeable?).and_return(false) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is cleaned when no source is found' do + allow(merge_request).to receive(:diff_head_sha).and_return(nil) + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + it 'is not cleaned when unexpected error is raised' do service = described_class.new(project, user, commit_message: 'Awesome message') allow(service).to receive(:commit).and_raise(StandardError) -- cgit v1.2.1 From f2e9ef102713344938e425d68d1a017403a710e0 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 20 Sep 2017 13:36:00 +0200 Subject: fix specs --- spec/services/emails/create_service_spec.rb | 2 +- spec/services/emails/destroy_service_spec.rb | 2 +- spec/services/users/update_service_spec.rb | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'spec/services') diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 641d5538de8..1c8b81c222c 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -4,7 +4,7 @@ describe Emails::CreateService do let(:user) { create(:user) } let(:opts) { { email: 'new@email.com' } } - subject(:service) { described_class.new(user, opts) } + subject(:service) { described_class.new(user, user, opts) } describe '#execute' do it 'creates an email with valid attributes' do diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 1f4294dd905..138c3ff33b0 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -4,7 +4,7 @@ describe Emails::DestroyService do let!(:user) { create(:user) } let!(:email) { create(:email, user: user) } - subject(:service) { described_class.new(user, email: email.email) } + subject(:service) { described_class.new(user, user, email: email.email) } describe '#execute' do it 'removes an email' do diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 6ee35a33b2d..707f83b3359 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -31,13 +31,13 @@ describe Users::UpdateService do end def update_user(user, opts) - described_class.new(user, opts).execute + described_class.new(user, user, opts).execute end end describe '#execute!' do it 'updates the name' do - service = described_class.new(user, name: 'New Name') + service = described_class.new(user, user, name: 'New Name') expect(service).not_to receive(:notify_new_user) result = service.execute! @@ -55,7 +55,7 @@ describe Users::UpdateService do it 'fires system hooks when a new user is saved' do system_hook_service = spy(:system_hook_service) user = build(:user) - service = described_class.new(user, name: 'John Doe') + service = described_class.new(user, user, name: 'John Doe') expect(service).to receive(:notify_new_user).and_call_original expect(service).to receive(:system_hook_service).and_return(system_hook_service) @@ -65,7 +65,7 @@ describe Users::UpdateService do end def update_user(user, opts) - described_class.new(user, opts).execute! + described_class.new(user, user, opts).execute! end end end -- cgit v1.2.1 From 67d06dee30379fc93be196e2cf509660d1661aea Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 27 Sep 2017 11:48:33 +0200 Subject: refactor users update service --- spec/services/users/update_service_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'spec/services') diff --git a/spec/services/users/update_service_spec.rb b/spec/services/users/update_service_spec.rb index 707f83b3359..f8d4a47b212 100644 --- a/spec/services/users/update_service_spec.rb +++ b/spec/services/users/update_service_spec.rb @@ -31,13 +31,13 @@ describe Users::UpdateService do end def update_user(user, opts) - described_class.new(user, user, opts).execute + described_class.new(user, opts.merge(user: user)).execute end end describe '#execute!' do it 'updates the name' do - service = described_class.new(user, user, name: 'New Name') + service = described_class.new(user, user: user, name: 'New Name') expect(service).not_to receive(:notify_new_user) result = service.execute! @@ -55,7 +55,7 @@ describe Users::UpdateService do it 'fires system hooks when a new user is saved' do system_hook_service = spy(:system_hook_service) user = build(:user) - service = described_class.new(user, user, name: 'John Doe') + service = described_class.new(user, user: user, name: 'John Doe') expect(service).to receive(:notify_new_user).and_call_original expect(service).to receive(:system_hook_service).and_return(system_hook_service) @@ -65,7 +65,7 @@ describe Users::UpdateService do end def update_user(user, opts) - described_class.new(user, user, opts).execute! + described_class.new(user, opts.merge(user: user)).execute! end end end -- cgit v1.2.1 From 1dcb7111107ed5a6b6258613d804b8da56af8b35 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 27 Sep 2017 16:39:10 +0200 Subject: refactor emails service --- spec/services/emails/create_service_spec.rb | 4 ++-- spec/services/emails/destroy_service_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/services') diff --git a/spec/services/emails/create_service_spec.rb b/spec/services/emails/create_service_spec.rb index 1c8b81c222c..75812c17309 100644 --- a/spec/services/emails/create_service_spec.rb +++ b/spec/services/emails/create_service_spec.rb @@ -2,9 +2,9 @@ require 'spec_helper' describe Emails::CreateService do let(:user) { create(:user) } - let(:opts) { { email: 'new@email.com' } } + let(:opts) { { email: 'new@email.com', user: user } } - subject(:service) { described_class.new(user, user, opts) } + subject(:service) { described_class.new(user, opts) } describe '#execute' do it 'creates an email with valid attributes' do diff --git a/spec/services/emails/destroy_service_spec.rb b/spec/services/emails/destroy_service_spec.rb index 138c3ff33b0..7726fc0ef81 100644 --- a/spec/services/emails/destroy_service_spec.rb +++ b/spec/services/emails/destroy_service_spec.rb @@ -4,7 +4,7 @@ describe Emails::DestroyService do let!(:user) { create(:user) } let!(:email) { create(:email, user: user) } - subject(:service) { described_class.new(user, user, email: email.email) } + subject(:service) { described_class.new(user, user: user, email: email.email) } describe '#execute' do it 'removes an email' do -- cgit v1.2.1 From f4de14d71f425dc14ee5837d96f4e9f42c7cc239 Mon Sep 17 00:00:00 2001 From: Gabriel Mazetto Date: Wed, 6 Sep 2017 07:16:26 +0200 Subject: Add support to migrate existing projects to Hashed Storage async --- .../hashed_storage_migration_service_spec.rb | 74 ++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 spec/services/projects/hashed_storage_migration_service_spec.rb (limited to 'spec/services') diff --git a/spec/services/projects/hashed_storage_migration_service_spec.rb b/spec/services/projects/hashed_storage_migration_service_spec.rb new file mode 100644 index 00000000000..1b61207b550 --- /dev/null +++ b/spec/services/projects/hashed_storage_migration_service_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' + +describe Projects::HashedStorageMigrationService do + let(:gitlab_shell) { Gitlab::Shell.new } + let(:project) { create(:project, :empty_repo, :wiki_repo) } + let(:service) { described_class.new(project) } + let(:legacy_storage) { Storage::LegacyProject.new(project) } + let(:hashed_storage) { Storage::HashedProject.new(project) } + + describe '#execute' do + before do + allow(service).to receive(:gitlab_shell) { gitlab_shell } + end + + context 'when succeeds' do + it 'renames project and wiki repositories' do + service.execute + + expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_truthy + expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy + end + + it 'updates project to be hashed and not readonly' do + service.execute + + expect(project.hashed_storage?).to be_truthy + expect(project.repository_read_only).to be_falsey + end + + it 'move operation is called for both repositories' do + expect_move_repository(project.disk_path, hashed_storage.disk_path) + expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki") + + service.execute + end + end + + context 'when one move fails' do + it 'rollsback repositories to original name' do + from_name = project.disk_path + to_name = hashed_storage.disk_path + allow(service).to receive(:move_repository).and_call_original + allow(service).to receive(:move_repository).with(from_name, to_name).once { false } # will disable first move only + + expect(service).to receive(:rollback_folder_move).and_call_original + + service.execute + + expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_falsey + expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey + end + + context 'when rollback fails' do + before do + from_name = legacy_storage.disk_path + to_name = hashed_storage.disk_path + + hashed_storage.ensure_storage_path_exists + gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name) + end + + it 'does not try to move nil repository over hashed' do + expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki") + + service.execute + end + end + end + + def expect_move_repository(from_name, to_name) + expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name).and_call_original + end + end +end -- cgit v1.2.1 From dc32128de976d864ca143c4b56fa1b45531e277e Mon Sep 17 00:00:00 2001 From: Sean McGivern Date: Thu, 28 Sep 2017 17:09:35 +0100 Subject: Clear merge requests counter cache after merge Before this change, the MR counter in the sidebar would be wrong if an MR had been merged since the last update, but not opened or closed, as merging did not trigger a counter cache update. --- spec/services/merge_requests/post_merge_service_spec.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'spec/services') diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index a37cdab8928..d2bd05d921f 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -11,5 +11,16 @@ describe MergeRequests::PostMergeService do describe '#execute' do it_behaves_like 'cache counters invalidator' + + it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do + # Cache the counter before the MR changed state. + project.open_merge_requests_count + merge_request.update!(state: 'merged') + + service = described_class.new(project, user, {}) + + expect { service.execute(merge_request) } + .to change { project.open_merge_requests_count }.from(1).to(0) + end end end -- cgit v1.2.1 From 4f5be9ec7bfd4813eaac630c9232cd5335a5076b Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Mon, 25 Sep 2017 22:01:04 +0200 Subject: Migrate Gitlab::Git::Repository#add_tag to Gitaly Closes gitaly#601 --- spec/services/tags/create_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/services') diff --git a/spec/services/tags/create_service_spec.rb b/spec/services/tags/create_service_spec.rb index 57013b54560..e7e9080b6b0 100644 --- a/spec/services/tags/create_service_spec.rb +++ b/spec/services/tags/create_service_spec.rb @@ -28,7 +28,7 @@ describe Tags::CreateService do it 'returns an error' do expect(repository).to receive(:add_tag) .with(user, 'v1.1.0', 'master', 'Foo') - .and_raise(Rugged::TagError) + .and_raise(Gitlab::Git::Repository::TagExistsError) response = service.execute('v1.1.0', 'master', 'Foo') -- cgit v1.2.1 From e5fecc3a377c458e49751e3d2eacfb52972e59c6 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 28 Sep 2017 19:07:22 +0200 Subject: Create repositories via Gitaly --- spec/services/projects/create_service_spec.rb | 5 +++-- spec/services/projects/fork_service_spec.rb | 5 +++-- spec/services/projects/transfer_service_spec.rb | 7 +++++-- spec/services/projects/update_service_spec.rb | 5 +++-- 4 files changed, 14 insertions(+), 8 deletions(-) (limited to 'spec/services') diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 5da634e2fb1..c2ec805ea99 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -156,10 +156,11 @@ describe Projects::CreateService, '#execute' do } end - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do - gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") end after do diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index a6e0364d44c..fa9d6969830 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -76,10 +76,11 @@ describe Projects::ForkService do end context 'repository already exists' do - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do - gitlab_shell.add_repository(repository_storage_path, "#{@to_user.namespace.full_path}/#{@from_project.path}") + gitlab_shell.add_repository(repository_storage, "#{@to_user.namespace.full_path}/#{@from_project.path}") end after do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index a14ed526f68..2459f371a91 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -121,11 +121,14 @@ describe Projects::TransferService do end context 'namespace which contains orphan repository with same projects path name' do - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do group.add_owner(user) - gitlab_shell.add_repository(repository_storage_path, "#{group.full_path}/#{project.path}") + unless gitlab_shell.add_repository(repository_storage, "#{group.full_path}/#{project.path}") + raise 'failed to add repository' + end @result = transfer_project(project, user, group) end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index c551083ac90..4873e967535 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -149,10 +149,11 @@ describe Projects::UpdateService, '#execute' do end context 'when renaming a project' do - let(:repository_storage_path) { Gitlab.config.repositories.storages['default']['path'] } + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } before do - gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") end after do -- cgit v1.2.1 From bac29160302549c3c651991bf839b304a9e1c8b4 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Fri, 29 Sep 2017 17:20:56 -0700 Subject: Fix gitlab-rake gitlab:import:repos task Because of a change in GitLab 9.5.4 to prevent users from assuming control of a repository already on disk, the import task broke. Imports would fail with the message, "There is already a repository with that name on disk". This change skips the validation when the import is done from the command-line. Closes #37682 --- spec/services/projects/create_service_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'spec/services') diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index c2ec805ea99..35f0c85b0ec 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -209,6 +209,15 @@ describe Projects::CreateService, '#execute' do end end + context 'when skip_disk_validation is used' do + it 'sets the project attribute' do + opts[:skip_disk_validation] = true + project = create_project(user, opts) + + expect(project.skip_disk_validation).to be_truthy + end + end + def create_project(user, opts) Projects::CreateService.new(user, opts).execute end -- cgit v1.2.1 From 47978e9781916bc10301c714acea9f692d11d934 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 21 Sep 2017 18:04:09 -0300 Subject: Doesn't check if path exists on disk when renaming a hashed project --- spec/services/projects/create_service_spec.rb | 67 ++++++++++++++++++++------- spec/services/projects/update_service_spec.rb | 42 ++++++++++++----- 2 files changed, 79 insertions(+), 30 deletions(-) (limited to 'spec/services') diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 35f0c85b0ec..3aed4aa56d1 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -149,6 +149,9 @@ describe Projects::CreateService, '#execute' do end context 'when another repository already exists on disk' do + let(:repository_storage) { 'default' } + let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } + let(:opts) do { name: 'Existing', @@ -156,31 +159,59 @@ describe Projects::CreateService, '#execute' do } end - let(:repository_storage) { 'default' } - let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } + context 'with legacy storage' do + before do + gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end - before do - gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") - end + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end - after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") - end + it 'does not allow to create a project when path matches existing repository on disk' do + project = create_project(user, opts) - it 'does not allow to create project with same path' do - project = create_project(user, opts) + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end - expect(project).to respond_to(:errors) - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + it 'does not allow to import project when path matches existing repository on disk' do + project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) + + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end end - it 'does not allow to import a project with the same path' do - project = create_project(user, opts.merge({ import_url: 'https://gitlab.com/gitlab-org/gitlab-test.git' })) + context 'with hashed storage' do + let(:hash) { '6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + let(:hashed_path) { '@hashed/6b/86/6b86b273ff34fce19d6b804eff5a3f5747ada4eaa22f1d49c01e52ddb7875b4b' } + + before do + stub_application_setting(hashed_storage_enabled: true) + allow(Digest::SHA2).to receive(:hexdigest) { hash } + end + + before do + gitlab_shell.add_repository(repository_storage_path, hashed_path) + end + + after do + gitlab_shell.remove_repository(repository_storage_path, hashed_path) + end + + it 'does not allow to create a project when path matches existing repository on disk' do + project = create_project(user, opts) - expect(project).to respond_to(:errors) - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + expect(project).not_to be_persisted + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base].first).to match('There is already a repository with that name on disk') + end end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 4873e967535..2f86ffad378 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -152,22 +152,40 @@ describe Projects::UpdateService, '#execute' do let(:repository_storage) { 'default' } let(:repository_storage_path) { Gitlab.config.repositories.storages[repository_storage]['path'] } - before do - gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") - end + context 'with legacy storage' do + before do + gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end - after do - gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + after do + gitlab_shell.remove_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + end + + it 'does not allow renaming when new path matches existing repository on disk' do + result = update_project(project, admin, path: 'existing') + + expect(result).to include(status: :error) + expect(result[:message]).to match('There is already a repository with that name on disk') + expect(project).not_to be_valid + expect(project.errors.messages).to have_key(:base) + expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') + end end - it 'does not allow renaming when new path matches existing repository on disk' do - result = update_project(project, admin, path: 'existing') + context 'with hashed storage' do + let(:project) { create(:project, :repository, creator: user, namespace: user.namespace) } - expect(result).to include(status: :error) - expect(result[:message]).to match('There is already a repository with that name on disk') - expect(project).not_to be_valid - expect(project.errors.messages).to have_key(:base) - expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') + before do + stub_application_setting(hashed_storage_enabled: true) + end + + it 'does not check if new path matches existing repository on disk' do + expect(project).not_to receive(:repository_with_same_path_already_exists?) + + result = update_project(project, admin, path: 'existing') + + expect(result).to include(status: :success) + end end end -- cgit v1.2.1 From 624d54d2a8ada8bbf4aaa9cdbd5aa44e06ee076e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 2 Oct 2017 15:21:39 +0200 Subject: Fix specs for project creation and update services --- spec/services/projects/create_service_spec.rb | 4 ++-- spec/services/projects/update_service_spec.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'spec/services') diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 3aed4aa56d1..2cc4643777e 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -161,7 +161,7 @@ describe Projects::CreateService, '#execute' do context 'with legacy storage' do before do - gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") end after do @@ -197,7 +197,7 @@ describe Projects::CreateService, '#execute' do end before do - gitlab_shell.add_repository(repository_storage_path, hashed_path) + gitlab_shell.add_repository(repository_storage, hashed_path) end after do diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 2f86ffad378..d400304622e 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -154,7 +154,7 @@ describe Projects::UpdateService, '#execute' do context 'with legacy storage' do before do - gitlab_shell.add_repository(repository_storage_path, "#{user.namespace.full_path}/existing") + gitlab_shell.add_repository(repository_storage, "#{user.namespace.full_path}/existing") end after do -- cgit v1.2.1 From 5dd26d4e5a5a27ca93e6d55b4261c42f4f70e762 Mon Sep 17 00:00:00 2001 From: "Jacob Vosmaer (GitLab)" Date: Tue, 3 Oct 2017 16:58:33 +0000 Subject: Hide Gollum inside Gitlab::Git::Wiki --- spec/services/projects/create_service_spec.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'spec/services') diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 2cc4643777e..dc89fdebce7 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -76,9 +76,8 @@ describe Projects::CreateService, '#execute' do context 'wiki_enabled true creates wiki repository directory' do it do project = create_project(user, opts) - path = ProjectWiki.new(project, user).send(:path_to_repo) - expect(File.exist?(path)).to be_truthy + expect(wiki_repo(project).exists?).to be_truthy end end @@ -86,11 +85,15 @@ describe Projects::CreateService, '#execute' do it do opts[:wiki_enabled] = false project = create_project(user, opts) - path = ProjectWiki.new(project, user).send(:path_to_repo) - expect(File.exist?(path)).to be_falsey + expect(wiki_repo(project).exists?).to be_falsey end end + + def wiki_repo(project) + relative_path = ProjectWiki.new(project).disk_path + '.git' + Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar') + end end context 'builds_enabled global setting' do -- cgit v1.2.1