diff options
Diffstat (limited to 'spec/services/merge_requests')
8 files changed, 102 insertions, 68 deletions
diff --git a/spec/services/merge_requests/approval_service_spec.rb b/spec/services/merge_requests/approval_service_spec.rb index 0846ec7f50e..da6492aca95 100644 --- a/spec/services/merge_requests/approval_service_spec.rb +++ b/spec/services/merge_requests/approval_service_spec.rb @@ -33,9 +33,17 @@ RSpec.describe MergeRequests::ApprovalService do service.execute(merge_request) end + it_behaves_like 'does not trigger GraphQL subscription mergeRequestMergeStatusUpdated' do + let(:action) { service.execute(merge_request) } + end + it 'does not publish MergeRequests::ApprovedEvent' do expect { service.execute(merge_request) }.not_to publish_event(MergeRequests::ApprovedEvent) end + + it_behaves_like 'does not trigger GraphQL subscription mergeRequestReviewersUpdated' do + let(:action) { service.execute(merge_request) } + end end context 'with an already approved MR' do @@ -46,6 +54,14 @@ RSpec.describe MergeRequests::ApprovalService do it 'does not create an approval' 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 end context 'with valid approval' do @@ -67,6 +83,14 @@ RSpec.describe MergeRequests::ApprovalService do .to publish_event(MergeRequests::ApprovedEvent) .with(current_user_id: user.id, merge_request_id: merge_request.id) end + + it_behaves_like 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do + let(:action) { service.execute(merge_request) } + end + + it_behaves_like 'triggers GraphQL subscription mergeRequestReviewersUpdated' do + let(:action) { service.execute(merge_request) } + end end context 'user cannot update the merge request' do @@ -77,6 +101,14 @@ RSpec.describe MergeRequests::ApprovalService do 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 end end end diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 6a6f01e6a95..4f27ff30da7 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -93,7 +93,7 @@ RSpec.describe MergeRequests::BuildService do shared_examples 'with a Default.md template' do let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } - let(:project) { create(:project, :custom_repo, files: files ) } + let(:project) { create(:project, :custom_repo, files: files) } it 'the template description is preferred' do expect(merge_request.description).to eq('Default template contents') @@ -306,7 +306,7 @@ RSpec.describe MergeRequests::BuildService do context 'a Default.md template is defined' do let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } - let(:project) { create(:project, :custom_repo, files: files ) } + let(:project) { create(:project, :custom_repo, files: files) } it 'appends the closing description to a Default.md template' do expected_description = ['Default template contents', closing_message].compact.join("\n\n") @@ -386,7 +386,7 @@ RSpec.describe MergeRequests::BuildService do context 'a Default.md template is defined' do let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } - let(:project) { create(:project, :custom_repo, files: files ) } + let(:project) { create(:project, :custom_repo, files: files) } it 'keeps the description from the initial params' do expect(merge_request.description).to eq(description) @@ -425,7 +425,7 @@ RSpec.describe MergeRequests::BuildService do context 'a Default.md template is defined' do let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } - let(:project) { create(:project, :custom_repo, files: files ) } + let(:project) { create(:project, :custom_repo, files: files) } it 'appends the closing description to a Default.md template' do expected_description = ['Default template contents', closing_message].compact.join("\n\n") @@ -486,7 +486,7 @@ RSpec.describe MergeRequests::BuildService do context 'a Default.md template is defined' do let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } - let(:project) { create(:project, :custom_repo, files: files ) } + let(:project) { create(:project, :custom_repo, files: files) } it 'appends the closing description to a Default.md template' do expected_description = ['Default template contents', closing_message].compact.join("\n\n") @@ -715,7 +715,7 @@ RSpec.describe MergeRequests::BuildService do context 'when a Default template is found' do context 'when its contents cannot be retrieved' do let(:files) { { '.gitlab/merge_request_templates/OtherTemplate.md' => 'Other template contents' } } - let(:project) { create(:project, :custom_repo, files: files ) } + let(:project) { create(:project, :custom_repo, files: files) } it 'does not modify the merge request description' do allow(TemplateFinder).to receive(:all_template_names).and_return({ @@ -732,7 +732,7 @@ RSpec.describe MergeRequests::BuildService do context 'when its contents can be retrieved' do let(:files) { { '.gitlab/merge_request_templates/Default.md' => 'Default template contents' } } - let(:project) { create(:project, :custom_repo, files: files ) } + let(:project) { create(:project, :custom_repo, files: files) } it 'modifies the merge request description' do merge_request.description = nil diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index 0bc8258af42..da8e8d944d6 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -336,6 +336,12 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it_behaves_like 'reviewer_ids filter' do let(:execute) { service.execute } end + + context 'when called in a transaction' do + it 'does not raise an error' do + expect { MergeRequest.transaction { described_class.new(project: project, current_user: user, params: opts).execute } }.not_to raise_error + end + end end it_behaves_like 'issuable record that supports quick actions' do @@ -495,15 +501,40 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do project.add_developer(user) end - it 'creates the merge request', :sidekiq_might_not_need_inline do - expect_next_instance_of(MergeRequest) do |instance| - expect(instance).to receive(:eager_fetch_ref!).and_call_original + context 'when async_merge_request_diff_creation is enabled' do + before do + stub_feature_flags(async_merge_request_diff_creation: true) end - merge_request = described_class.new(project: project, current_user: user, params: opts).execute + it 'creates the merge request', :sidekiq_inline do + expect_next_instance_of(MergeRequest) do |instance| + expect(instance).not_to receive(:eager_fetch_ref!) + end - expect(merge_request).to be_persisted - expect(merge_request.iid).to be > 0 + merge_request = described_class.new(project: project, current_user: user, params: opts).execute + + expect(merge_request).to be_persisted + expect(merge_request.iid).to be > 0 + expect(merge_request.merge_request_diff).not_to be_empty + end + end + + context 'when async_merge_request_diff_creation is disabled' do + before do + stub_feature_flags(async_merge_request_diff_creation: false) + end + + it 'creates the merge request' do + expect_next_instance_of(MergeRequest) do |instance| + expect(instance).to receive(:eager_fetch_ref!).and_call_original + end + + merge_request = described_class.new(project: project, current_user: user, params: opts).execute + + expect(merge_request).to be_persisted + expect(merge_request.iid).to be > 0 + expect(merge_request.merge_request_diff).not_to be_empty + end end it 'does not create the merge request when the target project is archived' do diff --git a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb index cf34923795e..c56b38bccc1 100644 --- a/spec/services/merge_requests/mergeability/run_checks_service_spec.rb +++ b/spec/services/merge_requests/mergeability/run_checks_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe MergeRequests::Mergeability::RunChecksService do +RSpec.describe MergeRequests::Mergeability::RunChecksService, :clean_gitlab_redis_cache do subject(:run_checks) { described_class.new(merge_request: merge_request, params: {}) } describe '#execute' do @@ -104,18 +104,6 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do expect(execute.success?).to eq(true) end end - - context 'when mergeability_caching is turned off' do - before do - stub_feature_flags(mergeability_caching: false) - end - - it 'does not call the results store' do - expect(Gitlab::MergeRequests::Mergeability::ResultsStore).not_to receive(:new) - - expect(execute.success?).to eq(true) - end - end end end @@ -161,11 +149,11 @@ RSpec.describe MergeRequests::Mergeability::RunChecksService do let_it_be(:merge_request) { create(:merge_request) } context 'when the execute method has been executed' do - before do - run_checks.execute - end - context 'when all the checks succeed' do + before do + run_checks.execute + end + it 'returns nil' do expect(failure_reason).to eq(nil) end diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index c24b83e21a6..ee23238314e 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -190,14 +190,6 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar target_branch: 'conflict-start') end - it 'does not change the merge ref HEAD' do - expect(merge_request.merge_ref_head).to be_nil - - subject - - expect(merge_request.reload.merge_ref_head).not_to be_nil - end - it 'returns ServiceResponse.error and keeps merge status as cannot_be_merged' do result = subject @@ -351,27 +343,5 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar end end end - - context 'merge with conflicts' do - it 'calls MergeToRefService with true allow_conflicts param' do - expect(MergeRequests::MergeToRefService).to receive(:new) - .with(project: project, current_user: merge_request.author, params: { allow_conflicts: true }).and_call_original - - subject - end - - context 'when display_merge_conflicts_in_diff is disabled' do - before do - stub_feature_flags(display_merge_conflicts_in_diff: false) - end - - it 'calls MergeToRefService with false allow_conflicts param' do - expect(MergeRequests::MergeToRefService).to receive(:new) - .with(project: project, current_user: merge_request.author, params: { allow_conflicts: false }).and_call_original - - subject - end - end - 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 5a319e90a68..7b38f0d1c45 100644 --- a/spec/services/merge_requests/remove_approval_service_spec.rb +++ b/spec/services/merge_requests/remove_approval_service_spec.rb @@ -45,6 +45,14 @@ RSpec.describe MergeRequests::RemoveApprovalService do execute! end + + it_behaves_like 'triggers GraphQL subscription mergeRequestMergeStatusUpdated' do + let(:action) { execute! } + end + + it_behaves_like 'triggers GraphQL subscription mergeRequestReviewersUpdated' do + let(:action) { execute! } + end end context 'with a user who has not approved' do @@ -61,6 +69,14 @@ RSpec.describe MergeRequests::RemoveApprovalService do 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 end end end diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index 9210242a11e..471bb03f18c 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -3,8 +3,6 @@ require 'spec_helper' RSpec.describe MergeRequests::SquashService do - include GitHelpers - let(:service) { described_class.new(project: project, current_user: user, params: { merge_request: merge_request }) } let(:user) { project.first_owner } let(:project) { create(:project, :repository) } @@ -109,11 +107,10 @@ RSpec.describe MergeRequests::SquashService do end it 'has the same diff as the merge request, but a different SHA' do - rugged = rugged_repo(project.repository) - mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha) - squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha) + mr_diff = project.repository.diff(merge_request.diff_base_sha, merge_request.diff_head_sha) + squash_diff = project.repository.diff(merge_request.diff_start_sha, squash_sha) - expect(squash_diff.patch.length).to eq(mr_diff.patch.length) + expect(squash_diff.size).to eq(mr_diff.size) expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha) end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 1d67574b06d..da78f86c7c8 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -794,7 +794,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do end it "does not try to mark as unchecked if it's already unchecked" do - expect(merge_request).to receive(:unchecked?).and_return(true) + allow(merge_request).to receive(:unchecked?).twice.and_return(true) expect(merge_request).not_to receive(:mark_as_unchecked) update_merge_request({ target_branch: "target" }) @@ -1148,7 +1148,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do end end - include_examples 'issuable update service' do + it_behaves_like 'issuable update service' do let(:open_issuable) { merge_request } let(:closed_issuable) { create(:closed_merge_request, source_project: project) } end |