diff options
Diffstat (limited to 'spec/services/merge_requests')
32 files changed, 333 insertions, 61 deletions
diff --git a/spec/services/merge_requests/add_context_service_spec.rb b/spec/services/merge_requests/add_context_service_spec.rb index d4e95c2f1ea..58ed91218d1 100644 --- a/spec/services/merge_requests/add_context_service_spec.rb +++ b/spec/services/merge_requests/add_context_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::AddContextService do +RSpec.describe MergeRequests::AddContextService do let(:project) { create(:project, :repository) } let(:admin) { create(:admin) } let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: admin) } diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb index 0cec1e7be22..3c81ad6722d 100644 --- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb +++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::AddTodoWhenBuildFailsService do +RSpec.describe MergeRequests::AddTodoWhenBuildFailsService do let(:user) { create(:user) } let(:project) { create(:project, :repository) } let(:sha) { '1234567890abcdef1234567890abcdef12345678' } diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb index 4aefe5f7dae..840b7bc0a1c 100644 --- a/spec/services/merge_requests/after_create_service_spec.rb +++ b/spec/services/merge_requests/after_create_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::AfterCreateService do +RSpec.describe MergeRequests::AfterCreateService do let_it_be(:merge_request) { create(:merge_request) } subject(:after_create_service) do diff --git a/spec/services/merge_requests/approval_service_spec.rb b/spec/services/merge_requests/approval_service_spec.rb new file mode 100644 index 00000000000..124501f17d5 --- /dev/null +++ b/spec/services/merge_requests/approval_service_spec.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::ApprovalService do + describe '#execute' do + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + let(:project) { merge_request.project } + let!(:todo) { create(:todo, user: user, project: project, target: merge_request) } + + subject(:service) { described_class.new(project, user) } + + before do + project.add_developer(user) + end + + context 'with invalid approval' do + before do + allow(merge_request.approvals).to receive(:new).and_return(double(save: false)) + end + + it 'does not create an approval note' do + expect(SystemNoteService).not_to receive(:approve_mr) + + service.execute(merge_request) + end + + it 'does not mark pending todos as done' do + service.execute(merge_request) + + expect(todo.reload).to be_pending + end + end + + context 'with valid approval' do + it 'creates an approval note and marks pending todos as done' do + expect(SystemNoteService).to receive(:approve_mr).with(merge_request, user) + expect(merge_request.approvals).to receive(:reset) + + service.execute(merge_request) + + expect(todo.reload).to be_done + end + + it 'creates approve MR event' do + expect_next_instance_of(EventCreateService) do |instance| + expect(instance).to receive(:approve_mr) + .with(merge_request, user) + end + + service.execute(merge_request) + end + + context 'with remaining approvals' do + it 'fires an approval webhook' do + expect(service).to receive(:execute_hooks).with(merge_request, 'approved') + + service.execute(merge_request) + end + 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 + end + end +end diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index c0b57b9092d..6398e8c533e 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::AssignIssuesService do +RSpec.describe MergeRequests::AssignIssuesService do let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } let(:issue) { create(:issue, project: project) } diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 9b358839c06..f99be26927d 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -describe MergeRequests::BuildService do +RSpec.describe MergeRequests::BuildService do using RSpec::Parameterized::TableSyntax include RepoHelpers include ProjectForksHelper @@ -189,8 +189,8 @@ describe MergeRequests::BuildService do it_behaves_like 'allows the merge request to be created' - it 'adds a WIP prefix to the merge request title' do - expect(merge_request.title).to eq('WIP: Feature branch') + it 'adds a Draft prefix to the merge request title' do + expect(merge_request.title).to eq('Draft: Feature branch') end end diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 0e51de48fb1..e518e439a84 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::CloseService do +RSpec.describe MergeRequests::CloseService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } diff --git a/spec/services/merge_requests/conflicts/list_service_spec.rb b/spec/services/merge_requests/conflicts/list_service_spec.rb index 13d69307084..14133731e37 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::Conflicts::ListService do +RSpec.describe MergeRequests::Conflicts::ListService do describe '#can_be_resolved_in_ui?' do def create_merge_request(source_branch, target_branch = 'conflict-start') create(:merge_request, source_branch: source_branch, target_branch: target_branch, merge_status: :unchecked) do |mr| @@ -30,6 +30,7 @@ describe MergeRequests::Conflicts::ListService do it 'returns a falsey value when one of the MR branches is missing' do merge_request = create_merge_request('conflict-resolvable') merge_request.project.repository.rm_branch(merge_request.author, 'conflict-resolvable') + merge_request.clear_memoized_source_branch_exists expect(conflicts_service(merge_request).can_be_resolved_in_ui?).to be_falsey end diff --git a/spec/services/merge_requests/conflicts/resolve_service_spec.rb b/spec/services/merge_requests/conflicts/resolve_service_spec.rb index 74f20094081..c4d50124ca9 100644 --- a/spec/services/merge_requests/conflicts/resolve_service_spec.rb +++ b/spec/services/merge_requests/conflicts/resolve_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::Conflicts::ResolveService do +RSpec.describe MergeRequests::Conflicts::ResolveService do include ProjectForksHelper let(:user) { create(:user) } let(:project) { create(:project, :public, :repository) } diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb index fb1bb308170..fa70ad8c559 100644 --- a/spec/services/merge_requests/create_from_issue_service_spec.rb +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::CreateFromIssueService do +RSpec.describe MergeRequests::CreateFromIssueService do include ProjectForksHelper let(:project) { create(:project, :repository) } @@ -163,10 +163,10 @@ describe MergeRequests::CreateFromIssueService do expect(result[:merge_request].milestone_id).to eq(milestone_id) end - it 'sets the merge request title to: "WIP: Resolves "$issue-title"' do + it 'sets the merge request title to: "Draft: Resolves "$issue-title"' do result = service.execute - expect(result[:merge_request].title).to eq("WIP: Resolve \"#{issue.title}\"") + expect(result[:merge_request].title).to eq("Draft: Resolve \"#{issue.title}\"") end end @@ -193,10 +193,10 @@ describe MergeRequests::CreateFromIssueService do it_behaves_like 'a service that creates a merge request from an issue' - it 'sets the merge request title to: "WIP: $issue-branch-name', :sidekiq_might_not_need_inline do + it 'sets the merge request title to: "Draft: $issue-branch-name', :sidekiq_might_not_need_inline do result = service.execute - expect(result[:merge_request].title).to eq("WIP: #{issue.to_branch_name.titleize.humanize}") + expect(result[:merge_request].title).to eq("Draft: #{issue.to_branch_name.titleize.humanize}") end end end diff --git a/spec/services/merge_requests/create_pipeline_service_spec.rb b/spec/services/merge_requests/create_pipeline_service_spec.rb index 9eb28759061..db46bd37eea 100644 --- a/spec/services/merge_requests/create_pipeline_service_spec.rb +++ b/spec/services/merge_requests/create_pipeline_service_spec.rb @@ -2,10 +2,13 @@ require 'spec_helper' -describe MergeRequests::CreatePipelineService do +RSpec.describe MergeRequests::CreatePipelineService do + include ProjectForksHelper + let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } - let(:service) { described_class.new(project, user, params) } + let(:service) { described_class.new(project, actor, params) } + let(:actor) { user } let(:params) { {} } before do @@ -26,11 +29,13 @@ describe MergeRequests::CreatePipelineService do let(:merge_request) do create(:merge_request, source_branch: 'feature', - source_project: project, + source_project: source_project, target_branch: 'master', target_project: project) end + let(:source_project) { project } + it 'creates a detached merge request pipeline' do expect { subject }.to change { Ci::Pipeline.count }.by(1) @@ -42,6 +47,50 @@ describe MergeRequests::CreatePipelineService do expect(subject.source).to eq('merge_request_event') end + context 'with fork merge request' do + let_it_be(:forked_project) { fork_project(project, nil, repository: true, target_project: create(:project, :private, :repository)) } + let(:source_project) { forked_project } + + context 'when actor has permission to create pipelines in target project' do + let(:actor) { user } + + it 'creates a pipeline in the target project' do + expect(subject.project).to eq(project) + end + + context 'when ci_allow_to_create_merge_request_pipelines_in_target_project feature flag is disabled' do + before do + stub_feature_flags(ci_allow_to_create_merge_request_pipelines_in_target_project: false) + end + + it 'creates a pipeline in the source project' do + expect(subject.project).to eq(source_project) + end + end + end + + context 'when actor has permission to create pipelines in forked project' do + let(:actor) { fork_user } + let(:fork_user) { create(:user) } + + before do + source_project.add_developer(fork_user) + end + + it 'creates a pipeline in the source project' do + expect(subject.project).to eq(source_project) + end + end + + context 'when actor does not have permission to create pipelines' do + let(:actor) { create(:user) } + + it 'returns nothing' do + expect(subject.full_error_messages).to include('Insufficient permissions to create a new pipeline') + end + end + end + context 'when service is called multiple times' do it 'creates a pipeline once' do expect do diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index bb40c399b6e..a8661f027e8 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do +RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do include ProjectForksHelper let(:project) { create(:project, :repository) } @@ -216,11 +216,12 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do target_project.add_maintainer(user) end - it 'create legacy detached merge request pipeline for fork merge request' do + it 'create detached merge request pipeline for fork merge request' do merge_request.reload - expect(merge_request.actual_head_pipeline) - .to be_legacy_detached_merge_request_pipeline + head_pipeline = merge_request.actual_head_pipeline + expect(head_pipeline).to be_detached_merge_request_pipeline + expect(head_pipeline.project).to eq(target_project) end end @@ -339,13 +340,14 @@ describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end end - it_behaves_like 'new issuable record that supports quick actions' do + it_behaves_like 'issuable record that supports quick actions' do let(:default_params) do { source_branch: 'feature', target_branch: 'master' } end + let(:issuable) { described_class.new(project, user, params).execute } end context 'Quick actions' do diff --git a/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb b/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb index 2adf808619d..377615bbc6f 100644 --- a/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb +++ b/spec/services/merge_requests/delete_non_latest_diffs_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::DeleteNonLatestDiffsService, :clean_gitlab_redis_shared_state do +RSpec.describe MergeRequests::DeleteNonLatestDiffsService, :clean_gitlab_redis_shared_state do let(:merge_request) { create(:merge_request) } let!(:subject) { described_class.new(merge_request) } diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index 415b351e13a..c3da02273a4 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::FfMergeService do +RSpec.describe MergeRequests::FfMergeService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:merge_request) do diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 8cc627b64d9..053752626dc 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -2,7 +2,7 @@ require "spec_helper" -describe MergeRequests::GetUrlsService do +RSpec.describe MergeRequests::GetUrlsService do include ProjectForksHelper let(:project) { create(:project, :public, :repository) } diff --git a/spec/services/merge_requests/link_lfs_objects_service_spec.rb b/spec/services/merge_requests/link_lfs_objects_service_spec.rb index f07cf13e4f2..c1765e3a2ab 100644 --- a/spec/services/merge_requests/link_lfs_objects_service_spec.rb +++ b/spec/services/merge_requests/link_lfs_objects_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::LinkLfsObjectsService, :sidekiq_inline do +RSpec.describe MergeRequests::LinkLfsObjectsService, :sidekiq_inline do include ProjectForksHelper include RepoHelpers diff --git a/spec/services/merge_requests/merge_orchestration_service_spec.rb b/spec/services/merge_requests/merge_orchestration_service_spec.rb index c50f20d7703..67dbb5a1a01 100644 --- a/spec/services/merge_requests/merge_orchestration_service_spec.rb +++ b/spec/services/merge_requests/merge_orchestration_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::MergeOrchestrationService do +RSpec.describe MergeRequests::MergeOrchestrationService do let_it_be(:maintainer) { create(:user) } let(:merge_params) { { sha: merge_request.diff_head_sha } } let(:user) { maintainer } diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 2274d917527..11e341994f7 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::MergeService do +RSpec.describe MergeRequests::MergeService do let_it_be(:user) { create(:user) } let_it_be(:user2) { create(:user) } let(:merge_request) { create(:merge_request, :simple, author: user2, assignees: [user2]) } @@ -360,6 +360,25 @@ describe MergeRequests::MergeService do expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) end + context 'when squashing is required' do + before do + merge_request.update!(source_branch: 'master', target_branch: 'feature') + merge_request.target_project.project_setting.squash_always! + end + + it 'raises an error if squashing is not done' do + error_message = 'requires squashing commits' + + service.execute(merge_request) + + expect(merge_request).to be_open + + expect(merge_request.merge_commit_sha).to be_nil + expect(merge_request.merge_error).to include(error_message) + expect(Gitlab::AppLogger).to have_received(:error).with(a_string_matching(error_message)) + end + end + context 'when squashing' do before do merge_request.update!(source_branch: 'master', target_branch: 'feature') diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index 596d46f3c43..b482e8d6724 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::MergeToRefService do +RSpec.describe MergeRequests::MergeToRefService do shared_examples_for 'MergeService for target ref' do it 'target_ref has the same state of target branch' do repo = merge_request.target_project.repository diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index 45519ddf3d3..543da46f883 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shared_state do +RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shared_state do shared_examples_for 'unmergeable merge request' do it 'updates or keeps merge status as cannot_be_merged' do subject diff --git a/spec/services/merge_requests/migrate_external_diffs_service_spec.rb b/spec/services/merge_requests/migrate_external_diffs_service_spec.rb index 233b944624f..6ea8626ba73 100644 --- a/spec/services/merge_requests/migrate_external_diffs_service_spec.rb +++ b/spec/services/merge_requests/migrate_external_diffs_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::MigrateExternalDiffsService do +RSpec.describe MergeRequests::MigrateExternalDiffsService do let(:merge_request) { create(:merge_request) } let(:diff) { merge_request.merge_request_diff } diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index fff6ddf3928..a51a896ca96 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -2,11 +2,13 @@ require 'spec_helper' -describe MergeRequests::PostMergeService do +RSpec.describe MergeRequests::PostMergeService do let(:user) { create(:user) } let(:merge_request) { create(:merge_request, assignees: [user]) } let(:project) { merge_request.project } + subject { described_class.new(project, user).execute(merge_request) } + before do project.add_maintainer(user) end @@ -19,10 +21,7 @@ describe MergeRequests::PostMergeService do 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) + expect { subject }.to change { project.open_merge_requests_count }.from(1).to(0) end it 'updates metrics' do @@ -35,7 +34,7 @@ describe MergeRequests::PostMergeService do expect(metrics_service).to receive(:merge) - described_class.new(project, user, {}).execute(merge_request) + subject end it 'deletes non-latest diffs' do @@ -45,7 +44,7 @@ describe MergeRequests::PostMergeService do .to receive(:new).with(merge_request) .and_return(diff_removal_service) - described_class.new(project, user, {}).execute(merge_request) + subject expect(diff_removal_service).to have_received(:execute) end @@ -56,21 +55,63 @@ describe MergeRequests::PostMergeService do issue = create(:issue, project: project) allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue]) - expect_next_instance_of(Issues::CloseService) do |service| - allow(service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError) + expect_next_instance_of(Issues::CloseService) do |close_service| + allow(close_service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError) end - expect { described_class.new(project, user).execute(merge_request) }.to raise_error(RuntimeError) + expect { subject }.to raise_error(RuntimeError) expect(merge_request.reload).to be_merged end it 'clean up environments for the merge request' do - expect_next_instance_of(Ci::StopEnvironmentsService) do |service| - expect(service).to receive(:execute_for_merge_request).with(merge_request) + expect_next_instance_of(Ci::StopEnvironmentsService) do |stop_environment_service| + expect(stop_environment_service).to receive(:execute_for_merge_request).with(merge_request) end - described_class.new(project, user).execute(merge_request) + subject + end + + context 'when the merge request has review apps' do + it 'cancels all review app deployments' do + pipeline = create(:ci_pipeline, + source: :merge_request_event, + merge_request: merge_request, + project: project, + sha: merge_request.diff_head_sha, + merge_requests_as_head_pipeline: [merge_request]) + + review_env_a = create(:environment, project: project, state: :available, name: 'review/a') + review_env_b = create(:environment, project: project, state: :available, name: 'review/b') + review_env_c = create(:environment, project: project, state: :stopped, name: 'review/c') + deploy_env = create(:environment, project: project, state: :available, name: 'deploy') + + review_job_a1 = create(:ci_build, :with_deployment, :start_review_app, + pipeline: pipeline, project: project, environment: review_env_a.name) + review_job_a2 = create(:ci_build, :with_deployment, :start_review_app, + pipeline: pipeline, project: project, environment: review_env_a.name) + finished_review_job_a = create(:ci_build, :with_deployment, :start_review_app, + pipeline: pipeline, project: project, status: :success, environment: review_env_a.name) + review_job_b1 = create(:ci_build, :with_deployment, :start_review_app, + pipeline: pipeline, project: project, environment: review_env_b.name) + review_job_b2 = create(:ci_build, :start_review_app, + pipeline: pipeline, project: project, environment: review_env_b.name) + review_job_c1 = create(:ci_build, :with_deployment, :start_review_app, + pipeline: pipeline, project: project, environment: review_env_c.name) + deploy_job = create(:ci_build, :with_deployment, :deploy_to_production, + pipeline: pipeline, project: project, environment: deploy_env.name) + + subject + + expect(review_job_a1.reload.canceled?).to be true + expect(review_job_a2.reload.canceled?).to be true + expect(finished_review_job_a.reload.status).to eq "success" + expect(finished_review_job_a.reload.canceled?).to be false + expect(review_job_b1.reload.canceled?).to be true + expect(review_job_b2.reload.canceled?).to be false + expect(review_job_c1.reload.canceled?).to be false + expect(deploy_job.reload.canceled?).to be false + end end end end 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 420c8513c72..55f92d6bd0a 100644 --- a/spec/services/merge_requests/push_options_handler_service_spec.rb +++ b/spec/services/merge_requests/push_options_handler_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::PushOptionsHandlerService do +RSpec.describe MergeRequests::PushOptionsHandlerService do include ProjectForksHelper let(:user) { create(:user) } diff --git a/spec/services/merge_requests/pushed_branches_service_spec.rb b/spec/services/merge_requests/pushed_branches_service_spec.rb index 7b5d505f4d9..6e9c77bd3b6 100644 --- a/spec/services/merge_requests/pushed_branches_service_spec.rb +++ b/spec/services/merge_requests/pushed_branches_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::PushedBranchesService do +RSpec.describe MergeRequests::PushedBranchesService do let(:project) { create(:project) } let!(:service) { described_class.new(project, nil, changes: pushed_branches) } diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index 69d555f838d..2e525f2ed01 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::RebaseService do +RSpec.describe MergeRequests::RebaseService do include ProjectForksHelper let(:user) { create(:user) } diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index e60ff6eb98a..18c4cef7087 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::RefreshService do +RSpec.describe MergeRequests::RefreshService do include ProjectForksHelper include ProjectHelpers @@ -225,12 +225,13 @@ describe MergeRequests::RefreshService do context 'when service runs on forked project' do let(:project) { @fork_project } - it 'creates legacy detached merge request pipeline for fork merge request', :sidekiq_might_not_need_inline do + it 'creates detached merge request pipeline for fork merge request', :sidekiq_inline do expect { subject } .to change { @fork_merge_request.pipelines_for_merge_request.count }.by(1) - expect(@fork_merge_request.pipelines_for_merge_request.first) - .to be_legacy_detached_merge_request_pipeline + merge_request_pipeline = @fork_merge_request.pipelines_for_merge_request.first + expect(merge_request_pipeline).to be_detached_merge_request_pipeline + expect(merge_request_pipeline.project).to eq(@project) end end diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index d2444af1b0f..3d5b65207e6 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_caching do +RSpec.describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_caching do let(:current_user) { create(:user) } let(:merge_request) { create(:merge_request) } let(:subject) { described_class.new(merge_request, current_user) } @@ -34,10 +34,8 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin context 'cache clearing' do it 'clears the cache for older diffs on the merge request' do - old_diff = merge_request.merge_request_diff - old_cache_key = old_diff.diffs_collection.cache_key - - expect_any_instance_of(Redis).to receive(:del).with(old_cache_key).and_call_original + expect_any_instance_of(Redis).to receive(:del).once.and_call_original + expect(Rails.cache).to receive(:delete).once.and_call_original subject.execute end diff --git a/spec/services/merge_requests/remove_approval_service_spec.rb b/spec/services/merge_requests/remove_approval_service_spec.rb new file mode 100644 index 00000000000..40da928e832 --- /dev/null +++ b/spec/services/merge_requests/remove_approval_service_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequests::RemoveApprovalService do + describe '#execute' do + let(:user) { create(:user) } + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project) } + let!(:existing_approval) { create(:approval, merge_request: merge_request) } + + subject(:service) { described_class.new(project, user) } + + def execute! + service.execute(merge_request) + end + + before do + project.add_developer(user) + end + + context 'with a user who has approved' do + let!(:approval) { create(:approval, user: user, merge_request: merge_request) } + + it 'removes the approval' do + expect { execute! }.to change { merge_request.approvals.size }.from(2).to(1) + end + + it 'creates an unapproval note and triggers web hook' do + expect(service).to receive(:execute_hooks).with(merge_request, 'unapproved') + expect(SystemNoteService).to receive(:unapprove_mr) + + execute! + end + 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 + end + end +end diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index 3807c44b01f..0066834180e 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::ReopenService do +RSpec.describe MergeRequests::ReopenService do let(:user) { create(:user) } let(:user2) { create(:user) } let(:guest) { create(:user) } diff --git a/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb index 29896db58ac..874cf66659a 100644 --- a/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb +++ b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::ResolvedDiscussionNotificationService do +RSpec.describe MergeRequests::ResolvedDiscussionNotificationService do let(:merge_request) { create(:merge_request) } let(:user) { create(:user) } let(:project) { merge_request.project } diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index a53314ed737..1ec1dc0f6eb 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::SquashService do +RSpec.describe MergeRequests::SquashService do include GitHelpers let(:service) { described_class.new(project, user, { merge_request: merge_request }) } @@ -131,6 +131,42 @@ describe MergeRequests::SquashService do include_examples 'the squash succeeds' end + context 'when squashing is disabled by default on the project' do + # Squashing is disabled by default, but it should still allow you + # to squash-and-merge if selected through the UI + let(:merge_request) { merge_request_with_only_new_files } + + before do + merge_request.project.project_setting.squash_default_off! + end + + include_examples 'the squash succeeds' + end + + context 'when squashing is forbidden on the project' do + let(:merge_request) { merge_request_with_only_new_files } + + before do + merge_request.project.project_setting.squash_never! + end + + it 'raises a squash error' do + expect(service.execute).to match( + status: :error, + message: a_string_including('does not allow squashing commits when merge requests are accepted')) + end + end + + context 'when squashing is enabled by default on the project' do + let(:merge_request) { merge_request_with_only_new_files } + + before do + merge_request.project.project_setting.squash_always! + end + + include_examples 'the squash succeeds' + end + context 'when squashing with files too large to display' do let(:merge_request) { merge_request_with_large_files } diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 2b934b24757..c3433c8c9d2 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe MergeRequests::UpdateService, :mailer do +RSpec.describe MergeRequests::UpdateService, :mailer do include ProjectForksHelper let(:group) { create(:group, :public) } @@ -737,5 +737,10 @@ describe MergeRequests::UpdateService, :mailer do .to change { merge_request.reload.force_remove_source_branch? }.from(nil).to(true) end end + + it_behaves_like 'issuable record that supports quick actions' do + let(:existing_merge_request) { create(:merge_request, source_project: project) } + let(:issuable) { described_class.new(project, user, params).execute(existing_merge_request) } + end end end |