diff options
author | Luke Duncalfe <lduncalfe@eml.cc> | 2019-04-08 08:07:53 +0300 |
---|---|---|
committer | Luke Duncalfe <lduncalfe@eml.cc> | 2019-04-09 01:57:01 +0300 |
commit | e73f537cb5097e85849110bafe184cb89e3bbc22 (patch) | |
tree | c711e4148007708c5dcb9fb45eab68092b8a9503 /spec/services | |
parent | 68f189ad23d7a384f40caa152d263fdf1465b30a (diff) |
Refactor PushOptionsHandlerService from review
Exceptions are no longer raised, instead all errors encountered are
added to the errors property.
MergeRequests::BuildService is used to generate attributes of a new
merge request.
Code moved from Api::Internal to Api::Helpers::InternalHelpers.
Diffstat (limited to 'spec/services')
-rw-r--r-- | spec/services/merge_requests/push_options_handler_service_spec.rb | 73 |
1 files changed, 39 insertions, 34 deletions
diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb index eafe7a34617..af52feef24d 100644 --- a/spec/services/merge_requests/push_options_handler_service_spec.rb +++ b/spec/services/merge_requests/push_options_handler_service_spec.rb @@ -3,10 +3,13 @@ require 'spec_helper' describe MergeRequests::PushOptionsHandlerService do + include ProjectForksHelper + let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } + let(:forked_project) { fork_project(project, user, repository: true) } let(:service) { described_class.new(project, user, changes, push_options) } - let(:source_branch) { 'test' } + let(:source_branch) { 'fix' } let(:target_branch) { 'feature' } let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } @@ -38,21 +41,25 @@ describe MergeRequests::PushOptionsHandlerService do expect(last_mr.assignee).to eq(user) end - it 'sets the title and description from the first non-merge commit' do - commits = project.repository.commits('master', limit: 5) + context 'when project has been forked' do + let(:forked_project) { fork_project(project, user, repository: true) } + let(:service) { described_class.new(forked_project, user, changes, push_options) } - expect(Gitlab::Git::Commit).to receive(:between).at_least(:once).and_return(commits) + before do + allow(forked_project).to receive(:empty_repo?).and_return(false) + end - service.execute + it 'sets the correct source project' do + service.execute - merge_commit = commits.first - non_merge_commit = commits.second + expect(last_mr.source_project).to eq(forked_project) + end - expect(merge_commit.merge_commit?).to eq(true) - expect(non_merge_commit.merge_commit?).to eq(false) + it 'sets the correct target project' do + service.execute - expect(last_mr.title).to eq(non_merge_commit.title) - expect(last_mr.description).to eq(non_merge_commit.description) + expect(last_mr.target_project).to eq(project) + end end end @@ -271,7 +278,7 @@ describe MergeRequests::PushOptionsHandlerService do let(:changes) do [ new_branch_changes, - "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/second-branch" + "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/feature_conflict" ] end @@ -287,11 +294,10 @@ describe MergeRequests::PushOptionsHandlerService do end end - it 'throws an error' do - expect { service.execute }.to raise_error( - MergeRequests::PushOptionsHandlerService::Error, - "Too many branches pushed (#{limit + 1} were pushed, limit is #{limit})" - ) + it 'records an error' do + service.execute + + expect(service.errors).to eq(["Too many branches pushed (#{limit + 1} were pushed, limit is #{limit})"]) end end end @@ -308,11 +314,10 @@ describe MergeRequests::PushOptionsHandlerService do let(:push_options) { { create: true } } let(:changes) { new_branch_changes } - it 'throws an error' do - expect { service.execute }.to raise_error( - MergeRequests::PushOptionsHandlerService::Error, - 'User is required' - ) + it 'records an error' do + service.execute + + expect(service.errors).to eq(['User is required']) end end @@ -320,10 +325,12 @@ describe MergeRequests::PushOptionsHandlerService do let(:push_options) { { create: true } } let(:changes) { new_branch_changes } - it 'throws an error' do + it 'records an error' do Members::DestroyService.new(user).execute(ProjectMember.find_by!(user_id: user.id)) - expect { service.execute }.to raise_error(Gitlab::Access::AccessDeniedError) + service.execute + + expect(service.errors).to eq(['User access was denied']) end end @@ -331,11 +338,10 @@ describe MergeRequests::PushOptionsHandlerService do let(:push_options) { { create: true, target: 'my-branch' } } let(:changes) { new_branch_changes } - it 'throws an error' do - expect { service.execute }.to raise_error( - MergeRequests::PushOptionsHandlerService::Error, - 'Branch my-branch does not exist' - ) + it 'records an error' do + service.execute + + expect(service.errors).to eq(['Branch my-branch does not exist']) end end @@ -343,13 +349,12 @@ describe MergeRequests::PushOptionsHandlerService do let(:push_options) { { create: true } } let(:changes) { new_branch_changes } - it 'throws an error' do + it 'records an error' do expect(project).to receive(:merge_requests_enabled?).and_return(false) - expect { service.execute }.to raise_error( - MergeRequests::PushOptionsHandlerService::Error, - 'Merge requests are not enabled for project' - ) + service.execute + + expect(service.errors).to eq(["Merge requests are not enabled for project #{project.full_path}"]) end end |