diff options
author | Rui Anderson <anderson.rui@gmail.com> | 2016-04-27 21:34:42 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-06-10 15:36:54 +0300 |
commit | 07dbd6b3884c4f188b2c3f29dd7419791f1051eb (patch) | |
tree | 6c00e4b9097f62a89fcd6f506a9c7b3eb27231bc /spec | |
parent | cfc99bbd1390bc548a703fdc7857c7db5b0e7c13 (diff) |
Allow or not merge MR with failed build
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec')
-rw-r--r-- | spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb | 105 | ||||
-rw-r--r-- | spec/models/merge_request_spec.rb | 117 | ||||
-rw-r--r-- | spec/requests/api/merge_requests_spec.rb | 8 |
3 files changed, 230 insertions, 0 deletions
diff --git a/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb new file mode 100644 index 00000000000..1627aa7287a --- /dev/null +++ b/spec/features/merge_requests/only_allow_merge_if_build_succeeds.rb @@ -0,0 +1,105 @@ +require 'spec_helper' + +feature 'Only allow merge requests to be merged if the build succeeds', feature: true, js: true do + let(:user) { create(:user) } + + let(:project) { create(:project, :public) } + let(:merge_request) { create(:merge_request_with_diffs, source_project: project, author: user) } + + before do + login_as user + + project.team << [user, :master] + end + + context "project hasn't ci enabled" do + it "allows MR to be merged" do + visit_merge_request(merge_request) + expect(page).to have_button "Accept Merge Request" + end + end + + context "when project has ci enabled" do + let!(:ci_commit) { create(:ci_commit, project: project, sha: merge_request.last_commit.id, ref: merge_request.source_branch) } + let!(:ci_build) { create(:ci_build, commit: ci_commit) } + + before do + project.enable_ci + end + + context "when merge requests can only be merged if the build succeeds" do + before do + project.update_attribute(:only_allow_merge_if_build_succeeds, true) + end + + context "when ci is running" do + it "doesn't allow to merge immediately" do + ci_commit.statuses.update_all(status: :pending) + visit_merge_request(merge_request) + + expect(page).to have_button "Merge When Build Succeeds" + expect(page).to_not have_button "Select Merge Moment" + end + end + + context "when ci failed" do + it "doesn't allow MR to be merged" do + ci_commit.statuses.update_all(status: :failed) + visit_merge_request(merge_request) + + expect(page).to_not have_button "Accept Merge Request" + expect(page).to have_content("Please retry the build or push code to fix the failure.") + end + end + + context "when ci succeed" do + it "allows MR to be merged" do + ci_commit.statuses.update_all(status: :success) + visit_merge_request(merge_request) + + expect(page).to have_button "Accept Merge Request" + end + end + end + + context "when merge requests can be merged when the build failed" do + before do + project.update_attribute(:only_allow_merge_if_build_succeeds, false) + end + + context "when ci is running" do + it "allows MR to be merged immediately" do + ci_commit.statuses.update_all(status: :pending) + visit_merge_request(merge_request) + + expect(page).to have_button "Merge When Build Succeeds" + + click_button "Select Merge Moment" + expect(page).to have_content "Merge Immediately" + end + end + + context "when ci failed" do + it "allows MR to be merged" do + ci_commit.statuses.update_all(status: :failed) + visit_merge_request(merge_request) + + expect(page).to have_button "Accept Merge Request" + end + end + + context "when ci succeed" do + it "allows MR to be merged" do + ci_commit.statuses.update_all(status: :success) + visit_merge_request(merge_request) + + expect(page).to have_button "Accept Merge Request" + end + end + end + end + + def visit_merge_request(merge_request) + visit namespace_project_merge_request_path(merge_request.project.namespace, merge_request.project, merge_request) + end +end diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 1b7cbc3efda..76912eed834 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -455,4 +455,121 @@ describe MergeRequest, models: true do expect(user2.assigned_open_merge_request_count).to eq(1) end end + + describe '#check_if_can_be_merged' do + let(:project) { create(:project, only_allow_merge_if_build_succeeds: true) } + + subject { create(:merge_request, source_project: project, merge_status: :unchecked) } + + context 'when it is not broken and has no conflicts' do + it 'is marked as mergeable' do + allow(subject).to receive(:broken?) { false } + allow(project).to receive_message_chain(:repository, :can_be_merged?) { true } + + expect { subject.check_if_can_be_merged }.to change { subject.merge_status }.to('can_be_merged') + end + end + + context 'when broken' do + before { 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 + + context 'when it has conflicts' do + before do + allow(subject).to receive(:broken?) { false } + allow(project).to receive_message_chain(:repository, :can_be_merged?) { 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 + + describe '#mergeable?' do + let(:project) { create(:project, only_allow_merge_if_build_succeeds: true) } + + subject { create(:merge_request, source_project: project) } + + it "checks if merge request can be merged" do + allow(subject).to receive(:cannot_be_merged_because_build_failed?) { false } + expect(subject).to receive(:check_if_can_be_merged) + + subject.mergeable? + end + + context 'when not open' do + before { subject.close } + + it 'returns false' do + expect(subject.mergeable?).to be_falsey + end + end + + context 'when working in progress' do + before { subject.title = 'WIP MR' } + + it 'returns false' do + expect(subject.mergeable?).to be_falsey + end + end + + context 'when broken' do + before { allow(subject).to receive(:broken?) { true } } + + it 'returns false' do + expect(subject.mergeable?).to be_falsey + end + end + + context 'when failed' do + before { allow(subject).to receive(:broken?) { false } } + + context "when project settings restrict to merge only if build succeeds" do + before { allow(subject).to receive(:cannot_be_merged_because_build_failed?) { true } } + it 'returns false if project settings restrict to merge only if build succeeds' do + expect(subject.mergeable?).to be_falsey + end + end + end + end + + describe '#cannot_be_merged_because_build_failed?' do + let(:project) { create(:empty_project, only_allow_merge_if_build_succeeds: true) } + let(:commit_status) { create(:commit_status, status: 'failed', project: project) } + let(:ci_commit) { create(:ci_empty_pipeline) } + + subject { build(:merge_request, target_project: project) } + + before do + ci_commit.statuses << commit_status + allow(subject).to receive(:ci_commit) { ci_commit } + end + + it "returns true if it's only allowed to merge green build and build has been failed" do + expect(subject.cannot_be_merged_because_build_failed?).to be_truthy + end + + context 'when no ci_commit is associated' do + before do + allow(subject).to receive(:ci_commit) { nil } + end + + it 'returns false' do + expect(subject.cannot_be_merged_because_build_failed?).to be_falsey + end + end + + context "when isn't only allowed to merge green build at project settings" do + subject { build(:merge_request, target_project: build(:empty_project, only_allow_merge_if_build_succeeds: false)) } + + it 'returns false' do + expect(subject.cannot_be_merged_because_build_failed?).to be_falsey + end + end + end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9da69a913a8..91c25a0948f 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -419,6 +419,14 @@ describe API::API, api: true do expect(json_response['message']).to eq('405 Method Not Allowed') end + it "should return 405 if merge_request build is failed it's restrict to merge only when susccess" do + allow_any_instance_of(MergeRequest).to receive(:cannot_be_merged_because_build_failed?).and_return(true) + + put api("/projects/#{project.id}/merge_requests/#{merge_request.id}/merge", user) + expect(response.status).to eq(405) + expect(json_response['message']).to eq('405 Method Not Allowed') + end + it "should return 401 if user has no permissions to merge" do user2 = create(:user) project.team << [user2, :reporter] |