Merge branch '24616-mr-shows-the-build-for-this-merge-request-failed-although-builds-still-running' into 'master'
Resolve "MR shows "The build for this merge request failed" although builds still running" Closes #24616 See merge request !7579
This commit is contained in:
commit
e33a9bee68
|
@ -686,7 +686,7 @@ class MergeRequest < ActiveRecord::Base
|
|||
def mergeable_ci_state?
|
||||
return true unless project.only_allow_merge_if_build_succeeds?
|
||||
|
||||
!pipeline || pipeline.success?
|
||||
!pipeline || pipeline.success? || pipeline.skipped?
|
||||
end
|
||||
|
||||
def environments
|
||||
|
|
|
@ -23,7 +23,7 @@
|
|||
= render 'projects/merge_requests/widget/open/merge_when_build_succeeds'
|
||||
- elsif !@merge_request.can_be_merged_by?(current_user)
|
||||
= render 'projects/merge_requests/widget/open/not_allowed'
|
||||
- elsif !@merge_request.mergeable_ci_state?
|
||||
- elsif !@merge_request.mergeable_ci_state? && (@pipeline.failed? || @pipeline.canceled?)
|
||||
= render 'projects/merge_requests/widget/open/build_failed'
|
||||
- elsif !@merge_request.mergeable_discussions_state?
|
||||
= render 'projects/merge_requests/widget/open/unresolved_discussions'
|
||||
|
|
|
@ -0,0 +1,4 @@
|
|||
---
|
||||
title: Fix a wrong "The build for this merge request failed" message
|
||||
merge_request: 7579
|
||||
author:
|
|
@ -1,8 +1,8 @@
|
|||
require 'spec_helper'
|
||||
|
||||
feature 'Only allow merge requests to be merged if the build succeeds', feature: true do
|
||||
let(:project) { create(:project, :public) }
|
||||
let(:merge_request) { create(:merge_request_with_diffs, source_project: project) }
|
||||
let(:merge_request) { create(:merge_request_with_diffs) }
|
||||
let(:project) { merge_request.target_project }
|
||||
|
||||
before do
|
||||
login_as merge_request.author
|
||||
|
@ -19,7 +19,13 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
|
|||
end
|
||||
|
||||
context 'when project has CI enabled' do
|
||||
let(:pipeline) { create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch) }
|
||||
given!(:pipeline) do
|
||||
create(:ci_empty_pipeline,
|
||||
project: project,
|
||||
sha: merge_request.diff_head_sha,
|
||||
ref: merge_request.source_branch,
|
||||
status: status)
|
||||
end
|
||||
|
||||
context 'when merge requests can only be merged if the build succeeds' do
|
||||
before do
|
||||
|
@ -27,7 +33,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
|
|||
end
|
||||
|
||||
context 'when CI is running' do
|
||||
before { pipeline.update_column(:status, :running) }
|
||||
given(:status) { :running }
|
||||
|
||||
it 'does not allow to merge immediately' do
|
||||
visit_merge_request(merge_request)
|
||||
|
@ -38,7 +44,18 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
|
|||
end
|
||||
|
||||
context 'when CI failed' do
|
||||
before { pipeline.update_column(:status, :failed) }
|
||||
given(:status) { :failed }
|
||||
|
||||
it 'does not allow MR to be merged' do
|
||||
visit_merge_request(merge_request)
|
||||
|
||||
expect(page).not_to have_button 'Accept Merge Request'
|
||||
expect(page).to have_content('Please retry the build or push a new commit to fix the failure.')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when CI canceled' do
|
||||
given(:status) { :canceled }
|
||||
|
||||
it 'does not allow MR to be merged' do
|
||||
visit_merge_request(merge_request)
|
||||
|
@ -49,7 +66,17 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
|
|||
end
|
||||
|
||||
context 'when CI succeeded' do
|
||||
before { pipeline.update_column(:status, :success) }
|
||||
given(:status) { :success }
|
||||
|
||||
it 'allows MR to be merged' do
|
||||
visit_merge_request(merge_request)
|
||||
|
||||
expect(page).to have_button 'Accept Merge Request'
|
||||
end
|
||||
end
|
||||
|
||||
context 'when CI skipped' do
|
||||
given(:status) { :skipped }
|
||||
|
||||
it 'allows MR to be merged' do
|
||||
visit_merge_request(merge_request)
|
||||
|
@ -65,7 +92,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
|
|||
end
|
||||
|
||||
context 'when CI is running' do
|
||||
before { pipeline.update_column(:status, :running) }
|
||||
given(:status) { :running }
|
||||
|
||||
it 'allows MR to be merged immediately', js: true do
|
||||
visit_merge_request(merge_request)
|
||||
|
@ -78,7 +105,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
|
|||
end
|
||||
|
||||
context 'when CI failed' do
|
||||
before { pipeline.update_column(:status, :failed) }
|
||||
given(:status) { :failed }
|
||||
|
||||
it 'allows MR to be merged' do
|
||||
visit_merge_request(merge_request)
|
||||
|
@ -88,7 +115,7 @@ feature 'Only allow merge requests to be merged if the build succeeds', feature:
|
|||
end
|
||||
|
||||
context 'when CI succeeded' do
|
||||
before { pipeline.update_column(:status, :success) }
|
||||
given(:status) { :success }
|
||||
|
||||
it 'allows MR to be merged' do
|
||||
visit_merge_request(merge_request)
|
|
@ -856,13 +856,31 @@ describe MergeRequest, models: true do
|
|||
context 'when it is only allowed to merge when build is green' do
|
||||
context 'and a failed pipeline is associated' do
|
||||
before do
|
||||
pipeline.statuses << create(:commit_status, status: 'failed', project: project)
|
||||
pipeline.update(status: 'failed')
|
||||
allow(subject).to receive(:pipeline) { pipeline }
|
||||
end
|
||||
|
||||
it { expect(subject.mergeable_ci_state?).to be_falsey }
|
||||
end
|
||||
|
||||
context 'and a successful pipeline is associated' do
|
||||
before do
|
||||
pipeline.update(status: 'success')
|
||||
allow(subject).to receive(:pipeline) { pipeline }
|
||||
end
|
||||
|
||||
it { expect(subject.mergeable_ci_state?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'and a skipped pipeline is associated' do
|
||||
before do
|
||||
pipeline.update(status: 'skipped')
|
||||
allow(subject).to receive(:pipeline) { pipeline }
|
||||
end
|
||||
|
||||
it { expect(subject.mergeable_ci_state?).to be_truthy }
|
||||
end
|
||||
|
||||
context 'when no pipeline is associated' do
|
||||
before do
|
||||
allow(subject).to receive(:pipeline) { nil }
|
||||
|
|
Loading…
Reference in New Issue