Fix merge requst relationships with pipeline in MWPSService
MWPSService currently uses the old pipeline lookup method. It searches related merge requests with pipeline.ref, however, this doesn't work for attached/detached merge request pipelines.
This commit is contained in:
parent
b54228ad3d
commit
fa73f4ee19
6 changed files with 77 additions and 17 deletions
|
@ -7,7 +7,7 @@ module MergeRequests
|
|||
def execute(commit_status)
|
||||
return if commit_status.allow_failure? || commit_status.retried?
|
||||
|
||||
commit_status_merge_requests(commit_status) do |merge_request|
|
||||
pipeline_merge_requests(commit_status.pipeline) do |merge_request|
|
||||
todo_service.merge_request_build_failed(merge_request)
|
||||
end
|
||||
end
|
||||
|
@ -16,7 +16,7 @@ module MergeRequests
|
|||
# build is retried
|
||||
#
|
||||
def close(commit_status)
|
||||
commit_status_merge_requests(commit_status) do |merge_request|
|
||||
pipeline_merge_requests(commit_status.pipeline) do |merge_request|
|
||||
todo_service.merge_request_build_retried(merge_request)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -99,22 +99,11 @@ module MergeRequests
|
|||
# rubocop: enable CodeReuse/ActiveRecord
|
||||
|
||||
def pipeline_merge_requests(pipeline)
|
||||
merge_requests_for(pipeline.ref).each do |merge_request|
|
||||
pipeline.all_merge_requests.opened.each do |merge_request|
|
||||
next unless pipeline == merge_request.head_pipeline
|
||||
|
||||
yield merge_request
|
||||
end
|
||||
end
|
||||
|
||||
def commit_status_merge_requests(commit_status)
|
||||
merge_requests_for(commit_status.ref).each do |merge_request|
|
||||
pipeline = merge_request.head_pipeline
|
||||
|
||||
next unless pipeline
|
||||
next unless pipeline.sha == commit_status.sha
|
||||
|
||||
yield merge_request
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
title: Fix MWPS does not work for merge request pipelines
|
||||
merge_request: 26906
|
||||
author:
|
||||
type: fixed
|
|
@ -3216,7 +3216,7 @@ describe Ci::Build do
|
|||
it 'does not try to create a todo' do
|
||||
project.add_developer(user)
|
||||
|
||||
expect(service).not_to receive(:commit_status_merge_requests)
|
||||
expect(service).not_to receive(:pipeline_merge_requests)
|
||||
|
||||
subject.drop!
|
||||
end
|
||||
|
@ -3252,7 +3252,23 @@ describe Ci::Build do
|
|||
end
|
||||
|
||||
context 'when build is not configured to be retried' do
|
||||
subject { create(:ci_build, :running, project: project, user: user) }
|
||||
subject { create(:ci_build, :running, project: project, user: user, pipeline: pipeline) }
|
||||
|
||||
let(:pipeline) do
|
||||
create(:ci_pipeline,
|
||||
project: project,
|
||||
ref: 'feature',
|
||||
sha: merge_request.diff_head_sha,
|
||||
merge_requests_as_head_pipeline: [merge_request])
|
||||
end
|
||||
|
||||
let(:merge_request) do
|
||||
create(:merge_request, :opened,
|
||||
source_branch: 'feature',
|
||||
source_project: project,
|
||||
target_branch: 'master',
|
||||
target_project: project)
|
||||
end
|
||||
|
||||
it 'does not retry build' do
|
||||
expect(described_class).not_to receive(:retry)
|
||||
|
@ -3271,7 +3287,10 @@ describe Ci::Build do
|
|||
it 'creates a todo' do
|
||||
project.add_developer(user)
|
||||
|
||||
expect(service).to receive(:commit_status_merge_requests)
|
||||
expect_next_instance_of(TodoService) do |todo_service|
|
||||
expect(todo_service)
|
||||
.to receive(:merge_request_build_failed).with(merge_request)
|
||||
end
|
||||
|
||||
subject.drop!
|
||||
end
|
||||
|
|
|
@ -77,6 +77,22 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
|
|||
service.execute(commit_status)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when build belongs to a merge request pipeline' do
|
||||
let(:pipeline) do
|
||||
create(:ci_pipeline, source: :merge_request_event,
|
||||
ref: merge_request.merge_ref_path,
|
||||
merge_request: merge_request,
|
||||
merge_requests_as_head_pipeline: [merge_request])
|
||||
end
|
||||
|
||||
let(:commit_status) { create(:ci_build, ref: merge_request.merge_ref_path, pipeline: pipeline) }
|
||||
|
||||
it 'notifies the todo service' do
|
||||
expect(todo_service).to receive(:merge_request_build_failed).with(merge_request)
|
||||
service.execute(commit_status)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#close' do
|
||||
|
@ -106,6 +122,22 @@ describe MergeRequests::AddTodoWhenBuildFailsService do
|
|||
service.close(commit_status)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when build belongs to a merge request pipeline' do
|
||||
let(:pipeline) do
|
||||
create(:ci_pipeline, source: :merge_request_event,
|
||||
ref: merge_request.merge_ref_path,
|
||||
merge_request: merge_request,
|
||||
merge_requests_as_head_pipeline: [merge_request])
|
||||
end
|
||||
|
||||
let(:commit_status) { create(:ci_build, ref: merge_request.merge_ref_path, pipeline: pipeline) }
|
||||
|
||||
it 'notifies the todo service' do
|
||||
expect(todo_service).to receive(:merge_request_build_retried).with(merge_request)
|
||||
service.close(commit_status)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#close_all' do
|
||||
|
|
|
@ -112,6 +112,21 @@ describe MergeRequests::MergeWhenPipelineSucceedsService do
|
|||
service.trigger(unrelated_pipeline)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when pipeline is merge request pipeline' do
|
||||
let(:pipeline) do
|
||||
create(:ci_pipeline, :success,
|
||||
source: :merge_request_event,
|
||||
ref: mr_merge_if_green_enabled.merge_ref_path,
|
||||
merge_request: mr_merge_if_green_enabled,
|
||||
merge_requests_as_head_pipeline: [mr_merge_if_green_enabled])
|
||||
end
|
||||
|
||||
it 'merges the associated merge request' do
|
||||
expect(MergeWorker).to receive(:perform_async)
|
||||
service.trigger(pipeline)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe "#cancel" do
|
||||
|
|
Loading…
Reference in a new issue