diff options
Diffstat (limited to 'spec/services/merge_requests')
9 files changed, 80 insertions, 76 deletions
diff --git a/spec/services/merge_requests/approval_service_spec.rb b/spec/services/merge_requests/approval_service_spec.rb index 8761aba432f..6e20c42c8f6 100644 --- a/spec/services/merge_requests/approval_service_spec.rb +++ b/spec/services/merge_requests/approval_service_spec.rb @@ -16,11 +16,7 @@ RSpec.describe MergeRequests::ApprovalService, feature_category: :code_review_wo stub_feature_flags ff_require_saml_auth_to_approve: false end - context 'with invalid approval' do - before do - allow(merge_request.approvals).to receive(:new).and_return(double(save: false)) - end - + shared_examples 'no-op call' do it 'does not reset approvals' do expect(merge_request.approvals).not_to receive(:reset) @@ -47,22 +43,34 @@ RSpec.describe MergeRequests::ApprovalService, feature_category: :code_review_wo end end + context 'with invalid approval' do + before do + allow(merge_request.approvals).to receive(:new).and_return(double(save: false)) + end + + it_behaves_like 'no-op call' + end + context 'with an already approved MR' do before do merge_request.approvals.create!(user: user) end - it 'does not create an approval' do - expect { service.execute(merge_request) }.not_to change { merge_request.approvals.size } - end + it_behaves_like 'no-op call' + end - it_behaves_like 'does not trigger GraphQL subscription mergeRequestMergeStatusUpdated' do - let(:action) { service.execute(merge_request) } - end + context 'with a merged MR' do + let(:merge_request) { create(:merge_request, :merged) } - it_behaves_like 'does not trigger GraphQL subscription mergeRequestReviewersUpdated' do - let(:action) { service.execute(merge_request) } + it_behaves_like 'no-op call' + end + + context 'user cannot update the merge request' do + before do + project.add_guest(user) end + + it_behaves_like 'no-op call' end context 'with valid approval' do @@ -115,27 +123,5 @@ RSpec.describe MergeRequests::ApprovalService, feature_category: :code_review_wo let(:action) { service.execute(merge_request) } end end - - context 'user cannot update the merge request' do - before do - project.add_guest(user) - end - - it 'does not update approvals' do - expect { service.execute(merge_request) }.not_to change { merge_request.approvals.size } - end - - it_behaves_like 'does not trigger GraphQL subscription mergeRequestMergeStatusUpdated' do - let(:action) { service.execute(merge_request) } - end - - it_behaves_like 'does not trigger GraphQL subscription mergeRequestReviewersUpdated' do - let(:action) { service.execute(merge_request) } - end - - it_behaves_like 'does not trigger GraphQL subscription mergeRequestApprovalStateUpdated' do - let(:action) { service.execute(merge_request) } - end - end end end diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index 5eb53b1bcba..416b28bff05 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -84,7 +84,7 @@ RSpec.describe MergeRequests::Conflicts::ListService, feature_category: :code_re it 'returns a falsey value when the MR has a missing revision after a force push' do merge_request = create_merge_request('conflict-resolvable') service = conflicts_service(merge_request) - allow(merge_request).to receive_message_chain(:target_branch_head, :raw, :id).and_return(Gitlab::Git::BLANK_SHA) + allow(merge_request).to receive_message_chain(:target_branch_head, :raw, :id).and_return(Gitlab::Git::SHA1_BLANK_SHA) expect(service.can_be_resolved_in_ui?).to be_falsey end diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 31b3e513a51..85a84f07094 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -10,8 +10,8 @@ RSpec.describe MergeRequests::GetUrlsService, feature_category: :code_review_wor let(:source_branch) { "merge-test" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/-/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:show_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/-/merge_requests/#{merge_request.iid}" } - let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } - let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" } + let(:new_branch_changes) { "#{Gitlab::Git::SHA1_BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } + let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::SHA1_BLANK_SHA} refs/heads/#{source_branch}" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/master" } @@ -131,7 +131,7 @@ RSpec.describe MergeRequests::GetUrlsService, feature_category: :code_review_wor context 'pushing new branch and existing branch (with merge request created) at once' do let!(:merge_request) { create(:merge_request, source_project: project, source_branch: "markdown") } - let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } + let(:new_branch_changes) { "#{Gitlab::Git::SHA1_BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/new_branch" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/markdown" } let(:changes) { "#{new_branch_changes}\n#{existing_branch_changes}" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/-/merge_requests/new?merge_request%5Bsource_branch%5D=new_branch" } 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 038977e4fd0..e34eb804a82 100644 --- a/spec/services/merge_requests/push_options_handler_service_spec.rb +++ b/spec/services/merge_requests/push_options_handler_service_spec.rb @@ -34,9 +34,9 @@ RSpec.describe MergeRequests::PushOptionsHandlerService, feature_category: :sour let(:label1) { 'mylabel1' } let(:label2) { 'mylabel2' } let(:label3) { 'mylabel3' } - let(:new_branch_changes) { "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } + let(:new_branch_changes) { "#{Gitlab::Git::SHA1_BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } let(:existing_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{source_branch}" } - let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::BLANK_SHA} refs/heads/#{source_branch}" } + let(:deleted_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 #{Gitlab::Git::SHA1_BLANK_SHA} refs/heads/#{source_branch}" } let(:default_branch_changes) { "d14d6c0abdd253381df51a723d58691b2ee1ab08 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{project.default_branch}" } let(:error_mr_required) { "A merge_request.create push option is required to create a merge request for branch #{source_branch}" } @@ -802,7 +802,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService, feature_category: :sour let(:changes) do [ new_branch_changes, - "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/feature_conflict" + "#{Gitlab::Git::SHA1_BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/feature_conflict" ] end @@ -814,7 +814,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService, feature_category: :sour let(:limit) { MergeRequests::PushOptionsHandlerService::LIMIT } let(:changes) do TestEnv::BRANCH_SHA.to_a[0..limit].map do |x| - "#{Gitlab::Git::BLANK_SHA} #{x.first} refs/heads/#{x.last}" + "#{Gitlab::Git::SHA1_BLANK_SHA} #{x.first} refs/heads/#{x.last}" end end diff --git a/spec/services/merge_requests/pushed_branches_service_spec.rb b/spec/services/merge_requests/pushed_branches_service_spec.rb index de99fb244d3..bcde2fd5165 100644 --- a/spec/services/merge_requests/pushed_branches_service_spec.rb +++ b/spec/services/merge_requests/pushed_branches_service_spec.rb @@ -37,11 +37,11 @@ RSpec.describe MergeRequests::PushedBranchesService, feature_category: :source_c end it 'returns empty result without any SQL query performed' do - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do expect(service.execute).to be_empty - end.count + end - expect(control_count).to be_zero + expect(control.count).to be_zero end end end diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index dd50dfa49e0..e2b1c91d6eb 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -712,10 +712,10 @@ RSpec.describe MergeRequests::RefreshService, feature_category: :code_review_wor it 'refreshes the merge request' do expect(refresh_service).to receive(:execute_hooks) - .with(@fork_merge_request, 'update', old_rev: Gitlab::Git::BLANK_SHA) + .with(@fork_merge_request, 'update', old_rev: Gitlab::Git::SHA1_BLANK_SHA) allow_any_instance_of(Repository).to receive(:merge_base).and_return(@oldrev) - refresh_service.execute(Gitlab::Git::BLANK_SHA, @newrev, 'refs/heads/master') + refresh_service.execute(Gitlab::Git::SHA1_BLANK_SHA, @newrev, 'refs/heads/master') reload_mrs expect(@merge_request.notes).to be_empty diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 77056cbe541..a6654989374 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -45,11 +45,11 @@ RSpec.describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_ current_user merge_request - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do subject.execute - end.count + end - expect { subject.execute }.not_to exceed_query_limit(control_count) + expect { subject.execute }.not_to exceed_query_limit(control) end end end diff --git a/spec/services/merge_requests/remove_approval_service_spec.rb b/spec/services/merge_requests/remove_approval_service_spec.rb index e4e54db5013..b0109a022eb 100644 --- a/spec/services/merge_requests/remove_approval_service_spec.rb +++ b/spec/services/merge_requests/remove_approval_service_spec.rb @@ -19,6 +19,34 @@ RSpec.describe MergeRequests::RemoveApprovalService, feature_category: :code_rev project.add_developer(user) end + shared_examples 'no-op call' do + it 'does not create an unapproval note and triggers web hook' do + expect(service).not_to receive(:execute_hooks) + expect(SystemNoteService).not_to receive(:unapprove_mr) + + execute! + end + + it 'does not track merge request unapprove action' do + expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) + .not_to receive(:track_unapprove_mr_action).with(user: user) + + execute! + end + + it_behaves_like 'does not trigger GraphQL subscription mergeRequestMergeStatusUpdated' do + let(:action) { execute! } + end + + it_behaves_like 'does not trigger GraphQL subscription mergeRequestReviewersUpdated' do + let(:action) { execute! } + end + + it_behaves_like 'does not trigger GraphQL subscription mergeRequestApprovalStateUpdated' do + let(:action) { execute! } + end + end + context 'with a user who has approved' do let!(:approval) { create(:approval, user: user, merge_request: merge_request) } let(:notification_service) { NotificationService.new } @@ -27,6 +55,12 @@ RSpec.describe MergeRequests::RemoveApprovalService, feature_category: :code_rev allow(service).to receive(:notification_service).and_return(notification_service) end + context 'when the merge request is merged' do + let(:merge_request) { create(:merge_request, :merged, source_project: project) } + + it_behaves_like 'no-op call' + end + it 'removes the approval' do expect { execute! }.to change { merge_request.approvals.size }.from(2).to(1) end @@ -60,31 +94,7 @@ RSpec.describe MergeRequests::RemoveApprovalService, feature_category: :code_rev end context 'with a user who has not approved' do - it 'does not create an unapproval note and triggers web hook' do - expect(service).not_to receive(:execute_hooks) - expect(SystemNoteService).not_to receive(:unapprove_mr) - - execute! - end - - it 'does not track merge request unapprove action' do - expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) - .not_to receive(:track_unapprove_mr_action).with(user: user) - - execute! - end - - it_behaves_like 'does not trigger GraphQL subscription mergeRequestMergeStatusUpdated' do - let(:action) { execute! } - end - - it_behaves_like 'does not trigger GraphQL subscription mergeRequestReviewersUpdated' do - let(:action) { execute! } - end - - it_behaves_like 'does not trigger GraphQL subscription mergeRequestApprovalStateUpdated' do - let(:action) { execute! } - end + it_behaves_like 'no-op call' end end end diff --git a/spec/services/merge_requests/request_review_service_spec.rb b/spec/services/merge_requests/request_review_service_spec.rb index ef96bf11e0b..a5f0d5b5c5a 100644 --- a/spec/services/merge_requests/request_review_service_spec.rb +++ b/spec/services/merge_requests/request_review_service_spec.rb @@ -71,6 +71,14 @@ RSpec.describe MergeRequests::RequestReviewService, feature_category: :code_revi service.execute(merge_request, user) end + it 'creates a sytem note' do + expect(SystemNoteService) + .to receive(:request_review) + .with(merge_request, project, current_user, user) + + service.execute(merge_request, user) + end + it_behaves_like 'triggers GraphQL subscription mergeRequestReviewersUpdated' do let(:action) { result } end |