diff options
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/merge_requests/conflicts_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/features/merge_request/user_resolves_conflicts_spec.rb | 2 | ||||
-rw-r--r-- | spec/mailers/notify_spec.rb | 40 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 138 | ||||
-rw-r--r-- | spec/presenters/merge_request_presenter_spec.rb | 35 | ||||
-rw-r--r-- | spec/services/merge_requests/conflicts/list_service_spec.rb | 2 | ||||
-rw-r--r-- | spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb | 26 | ||||
-rw-r--r-- | spec/services/notification_service_spec.rb | 26 | ||||
-rw-r--r-- | spec/services/todo_service_spec.rb | 25 |
10 files changed, 238 insertions, 60 deletions
diff --git a/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb b/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb index 2d7647a6e12..397cc79bde4 100644 --- a/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/conflicts_controller_spec.rb @@ -5,7 +5,7 @@ describe Projects::MergeRequests::ConflictsController do let(:user) { project.owner } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request_with_conflicts) do - create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) do |mr| + create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project, merge_status: :unchecked) do |mr| mr.mark_as_unmergeable end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index c8cc6b374f6..d3042be9e8b 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -7,7 +7,7 @@ describe Projects::MergeRequestsController do let(:user) { project.owner } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request_with_conflicts) do - create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project) do |mr| + create(:merge_request, source_branch: 'conflict-resolvable', target_branch: 'conflict-start', source_project: project, merge_status: :unchecked) do |mr| mr.mark_as_unmergeable end end diff --git a/spec/features/merge_request/user_resolves_conflicts_spec.rb b/spec/features/merge_request/user_resolves_conflicts_spec.rb index 25a57f7d379..59aa90fc86f 100644 --- a/spec/features/merge_request/user_resolves_conflicts_spec.rb +++ b/spec/features/merge_request/user_resolves_conflicts_spec.rb @@ -10,7 +10,7 @@ describe 'Merge request > User resolves conflicts', :js do end def create_merge_request(source_branch) - create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start', source_project: project) do |mr| + create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start', source_project: project, merge_status: :unchecked) do |mr| mr.mark_as_unmergeable end end diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 8a52c151cc4..f310a6854d5 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -390,6 +390,46 @@ describe Notify do end end + describe 'that are unmergeable' do + set(:merge_request) do + create(:merge_request, :conflict, + source_project: project, + target_project: project, + author: current_user, + assignee: assignee, + description: 'Awesome description') + end + + subject { described_class.merge_request_unmergeable_email(recipient.id, merge_request.id) } + + it_behaves_like 'a multiple recipients email' + it_behaves_like 'an answer to an existing thread with reply-by-email enabled' do + let(:model) { merge_request } + end + it_behaves_like 'it should show Gmail Actions View Merge request link' + it_behaves_like 'an unsubscribeable thread' + + it 'is sent as the merge request author' do + sender = subject.header[:from].addrs[0] + expect(sender.display_name).to eq(merge_request.author.name) + expect(sender.address).to eq(gitlab_sender) + end + + it 'has the correct subject and body' do + reasons = %w[foo bar] + + allow_any_instance_of(MergeRequestPresenter).to receive(:unmergeable_reasons).and_return(reasons) + aggregate_failures do + is_expected.to have_referable_subject(merge_request, reply: true) + is_expected.to have_body_text(project_merge_request_path(project, merge_request)) + is_expected.to have_body_text('reasons:') + reasons.each do |reason| + is_expected.to have_body_text(reason) + end + end + end + end + shared_examples 'a push to an existing merge request' do let(:push_user) { create(:user) } diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 04379e7d2c3..92e33a64d26 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -1245,38 +1245,50 @@ describe MergeRequest do describe '#check_if_can_be_merged' do let(:project) { create(:project, only_allow_merge_if_pipeline_succeeds: true) } - subject { create(:merge_request, source_project: project, merge_status: :unchecked) } + shared_examples 'checking if can be merged' do + context 'when it is not broken and has no conflicts' do + before do + allow(subject).to receive(:broken?) { false } + allow(project.repository).to receive(:can_be_merged?).and_return(true) + end - context 'when it is not broken and has no conflicts' do - before do - allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?).and_return(true) + it 'is marked as mergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') + end end - it 'is marked as mergeable' do - expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') - end - end + context 'when broken' do + before do + allow(subject).to receive(:broken?) { true } + end - context 'when broken' do - before do - allow(subject).to receive(:broken?) { true } + it 'becomes unmergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') + end end - it 'becomes unmergeable' do - expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') + context 'when it has conflicts' do + before do + allow(subject).to receive(:broken?) { false } + allow(project.repository).to receive(:can_be_merged?).and_return(false) + end + + it 'becomes unmergeable' do + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') + end end end - context 'when it has conflicts' do - before do - allow(subject).to receive(:broken?) { false } - allow(project.repository).to receive(:can_be_merged?).and_return(false) - end + context 'when merge_status is unchecked' do + subject { create(:merge_request, source_project: project, merge_status: :unchecked) } - it 'becomes unmergeable' do - expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('cannot_be_merged') - end + it_behaves_like 'checking if can be merged' + end + + context 'when merge_status is unchecked' do + subject { create(:merge_request, source_project: project, merge_status: :cannot_be_merged_recheck) } + + it_behaves_like 'checking if can be merged' end end @@ -2064,6 +2076,53 @@ describe MergeRequest do expect(subject.merge_jid).to be_nil end end + + describe 'transition to cannot_be_merged' do + let(:notification_service) { double(:notification_service) } + let(:todo_service) { double(:todo_service) } + + subject { create(:merge_request, merge_status: :unchecked) } + + before do + allow(NotificationService).to receive(:new).and_return(notification_service) + allow(TodoService).to receive(:new).and_return(todo_service) + end + + it 'notifies, but does not notify again if rechecking still results in cannot_be_merged' do + expect(notification_service).to receive(:merge_request_unmergeable).with(subject).once + expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).once + + subject.mark_as_unmergeable + subject.mark_as_unchecked + subject.mark_as_unmergeable + end + + it 'notifies whenever merge request is newly unmergeable' do + expect(notification_service).to receive(:merge_request_unmergeable).with(subject).twice + expect(todo_service).to receive(:merge_request_became_unmergeable).with(subject).twice + + subject.mark_as_unmergeable + subject.mark_as_unchecked + subject.mark_as_mergeable + subject.mark_as_unchecked + subject.mark_as_unmergeable + end + end + + describe 'check_state?' do + it 'indicates whether MR is still checking for mergeability' do + state_machine = described_class.state_machines[:merge_status] + check_states = [:unchecked, :cannot_be_merged_recheck] + + check_states.each do |merge_status| + expect(state_machine.check_state?(merge_status)).to be true + end + + (state_machine.states.map(&:name) - check_states).each do |merge_status| + expect(state_machine.check_state?(merge_status)).to be false + end + end + end end describe '#should_be_rebased?' do @@ -2195,4 +2254,39 @@ describe MergeRequest do expect(merge_request.can_allow_maintainer_to_push?(user)).to be_truthy end end + + describe '#merge_participants' do + it 'contains author' do + expect(subject.merge_participants).to eq([subject.author]) + end + + describe 'when merge_when_pipeline_succeeds? is true' do + describe 'when merge user is author' do + let(:user) { create(:user) } + subject do + create(:merge_request, + merge_when_pipeline_succeeds: true, + merge_user: user, + author: user) + end + + it 'contains author only' do + expect(subject.merge_participants).to eq([subject.author]) + end + end + + describe 'when merge user and author are different users' do + let(:merge_user) { create(:user) } + subject do + create(:merge_request, + merge_when_pipeline_succeeds: true, + merge_user: merge_user) + end + + it 'contains author and merge user' do + expect(subject.merge_participants).to eq([subject.author, merge_user]) + end + end + end + end end diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb index e3b37739e8e..d5fb4a7742c 100644 --- a/spec/presenters/merge_request_presenter_spec.rb +++ b/spec/presenters/merge_request_presenter_spec.rb @@ -70,6 +70,41 @@ describe MergeRequestPresenter do end end + describe "#unmergeable_reasons" do + let(:presenter) { described_class.new(resource, current_user: user) } + + before do + # Mergeable base state + allow(resource).to receive(:has_no_commits?).and_return(false) + allow(resource).to receive(:source_branch_exists?).and_return(true) + allow(resource).to receive(:target_branch_exists?).and_return(true) + allow(resource.project.repository).to receive(:can_be_merged?).and_return(true) + end + + it "handles mergeable request" do + expect(presenter.unmergeable_reasons).to eq([]) + end + + it "handles no commit" do + allow(resource).to receive(:has_no_commits?).and_return(true) + + expect(presenter.unmergeable_reasons).to eq(["no commits"]) + end + + it "handles branches missing" do + allow(resource).to receive(:source_branch_exists?).and_return(false) + allow(resource).to receive(:target_branch_exists?).and_return(false) + + expect(presenter.unmergeable_reasons).to eq(["source branch is missing", "target branch is missing"]) + end + + it "handles merge conflict" do + allow(resource.project.repository).to receive(:can_be_merged?).and_return(false) + + expect(presenter.unmergeable_reasons).to eq(["has merge conflicts"]) + end + end + describe '#conflict_resolution_path' do let(:project) { create :project } let(:user) { 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 837b8a56d12..d57852615d9 100644 --- a/spec/services/merge_requests/conflicts/list_service_spec.rb +++ b/spec/services/merge_requests/conflicts/list_service_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe MergeRequests::Conflicts::ListService do describe '#can_be_resolved_in_ui?' do def create_merge_request(source_branch) - create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start') do |mr| + create(:merge_request, source_branch: source_branch, target_branch: 'conflict-start', merge_status: :unchecked) do |mr| mr.mark_as_unmergeable end end diff --git a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb index 240aa638f79..8838742a637 100644 --- a/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb +++ b/spec/services/merge_requests/merge_when_pipeline_succeeds_service_spec.rb @@ -112,32 +112,6 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do service.trigger(unrelated_pipeline) end end - - context 'when the merge request is not mergeable' do - let(:mr_conflict) do - create(:merge_request, merge_when_pipeline_succeeds: true, merge_user: user, - source_branch: 'master', target_branch: 'feature-conflict', - source_project: project, target_project: project) - end - - let(:conflict_pipeline) do - create(:ci_pipeline, project: project, ref: mr_conflict.source_branch, - sha: mr_conflict.diff_head_sha, status: 'success', - head_pipeline_of: mr_conflict) - end - - it 'does not merge the merge request' do - expect(MergeWorker).not_to receive(:perform_async) - - service.trigger(conflict_pipeline) - end - - it 'creates todos for unmergeability' do - expect_any_instance_of(TodoService).to receive(:merge_request_became_unmergeable).with(mr_conflict) - - service.trigger(conflict_pipeline) - end - end end describe "#cancel" do diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 5f28bc123f3..831678b949d 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -1224,6 +1224,32 @@ describe NotificationService, :mailer do end end + describe '#merge_request_unmergeable' do + it "sends email to merge request author" do + notification.merge_request_unmergeable(merge_request) + + should_email(merge_request.author) + expect(email_recipients.size).to eq(1) + end + + describe 'when merge_when_pipeline_succeeds is true' do + before do + merge_request.update_attributes( + merge_when_pipeline_succeeds: true, + merge_user: create(:user) + ) + end + + it "sends email to merge request author and merge_user" do + notification.merge_request_unmergeable(merge_request) + + should_email(merge_request.author) + should_email(merge_request.merge_user) + expect(email_recipients.size).to eq(2) + end + end + end + describe '#closed_merge_request' do before do update_custom_notification(:close_merge_request, @u_guest_custom, resource: project) diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb index 562b89e6767..9a51c873b30 100644 --- a/spec/services/todo_service_spec.rb +++ b/spec/services/todo_service_spec.rb @@ -721,17 +721,18 @@ describe TodoService do end describe '#merge_request_build_failed' do - it 'creates a pending todo for the merge request author' do - service.merge_request_build_failed(mr_unassigned) + let(:merge_participants) { [mr_unassigned.author, admin] } - should_create_todo(user: author, target: mr_unassigned, action: Todo::BUILD_FAILED) + before do + allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants) end - it 'creates a pending todo for merge_user' do - mr_unassigned.update(merge_when_pipeline_succeeds: true, merge_user: admin) + it 'creates a pending todo for each merge_participant' do service.merge_request_build_failed(mr_unassigned) - should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::BUILD_FAILED) + merge_participants.each do |participant| + should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::BUILD_FAILED) + end end end @@ -747,11 +748,19 @@ describe TodoService do end describe '#merge_request_became_unmergeable' do - it 'creates a pending todo for a merge_user' do + let(:merge_participants) { [admin, create(:user)] } + + before do + allow(mr_unassigned).to receive(:merge_participants).and_return(merge_participants) + end + + it 'creates a pending todo for each merge_participant' do mr_unassigned.update(merge_when_pipeline_succeeds: true, merge_user: admin) service.merge_request_became_unmergeable(mr_unassigned) - should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::UNMERGEABLE) + merge_participants.each do |participant| + should_create_todo(user: participant, author: participant, target: mr_unassigned, action: Todo::UNMERGEABLE) + end end end |