Commit 4e342eaf authored by Sean McGivern's avatar Sean McGivern

Merge branch 'fix-merge-train-is-not-refreshed-when-aborted' into 'master'

Fix merge train is not refreshed when the system aborts/drops a merge request

See merge request gitlab-org/gitlab!19763
parents 8a55f897 84b0039d
---
title: Fix merge train is not refreshed when the system aborts/drops a merge request
merge_request: 19763
author:
type: fixed
...@@ -31,10 +31,15 @@ module AutoMerge ...@@ -31,10 +31,15 @@ module AutoMerge
end end
end end
def abort(merge_request, reason) def abort(merge_request, reason, process_next: true)
super do # Before dropping a merge request from a merge train, get the next
# merge request in order to refresh it later.
next_merge_request = merge_request.merge_train&.next
super(merge_request, reason) do
if merge_request.merge_train&.destroy if merge_request.merge_train&.destroy
SystemNoteService.abort_merge_train(merge_request, project, current_user, reason) SystemNoteService.abort_merge_train(merge_request, project, current_user, reason)
AutoMergeProcessWorker.perform_async(next_merge_request.id) if next_merge_request && process_next
end end
end end
end end
......
...@@ -19,7 +19,7 @@ module MergeTrains ...@@ -19,7 +19,7 @@ module MergeTrains
success(pipeline_created: pipeline_created.present?) success(pipeline_created: pipeline_created.present?)
rescue ProcessError => e rescue ProcessError => e
drop(e) abort(e)
end end
private private
...@@ -159,9 +159,9 @@ module MergeTrains ...@@ -159,9 +159,9 @@ module MergeTrains
params[:require_recreate] params[:require_recreate]
end end
def drop(error) def abort(error)
AutoMerge::MergeTrainService.new(project, merge_user) AutoMerge::MergeTrainService.new(project, merge_user)
.abort(merge_request, error.message) .abort(merge_request, error.message, process_next: false)
error(error.message) error(error.message)
end end
......
...@@ -174,6 +174,29 @@ describe 'Two merge requests on a merge train' do ...@@ -174,6 +174,29 @@ describe 'Two merge requests on a merge train' do
end end
end end
context 'when merge request 1 got a new commit' do
before do
oldrev = project.repository.commit('feature').sha
create_file_in_repo(project, 'refs/heads/feature', 'refs/heads/feature', 'test.txt', 'This is test')
newrev = project.repository.commit('feature').sha
MergeRequests::RefreshService.new(project, maintainer_1)
.execute(oldrev, newrev, 'refs/heads/feature')
merge_request_1.reload
merge_request_2.reload
end
it_behaves_like 'drops merge request 1 from the merge train' do
let(:system_note) do
'removed this merge request from the merge train because source branch was updated'
end
end
it_behaves_like 're-creates a pipeline for merge request 2' do
let(:target_branch_sha) { project.repository.commit('refs/heads/master').sha }
end
end
context 'when merge request 1 is not mergeable' do context 'when merge request 1 is not mergeable' do
before do before do
merge_request_1.update!(title: merge_request_1.wip_title) merge_request_1.update!(title: merge_request_1.wip_title)
......
...@@ -159,7 +159,7 @@ describe AutoMerge::MergeTrainService do ...@@ -159,7 +159,7 @@ describe AutoMerge::MergeTrainService do
end end
describe '#abort' do describe '#abort' do
subject { service.abort(merge_request, 'an error') } subject { service.abort(merge_request, 'an error', **args) }
let!(:merge_request) do let!(:merge_request) do
create(:merge_request, :on_train, create(:merge_request, :on_train,
...@@ -167,6 +167,8 @@ describe AutoMerge::MergeTrainService do ...@@ -167,6 +167,8 @@ describe AutoMerge::MergeTrainService do
target_project: project, target_branch: 'master') target_project: project, target_branch: 'master')
end end
let(:args) { {} }
it 'aborts auto merge on the merge request' do it 'aborts auto merge on the merge request' do
subject subject
...@@ -186,6 +188,30 @@ describe AutoMerge::MergeTrainService do ...@@ -186,6 +188,30 @@ describe AutoMerge::MergeTrainService do
subject subject
end end
context 'when the other merge request is following the merge request' do
let!(:merge_request_2) do
create(:merge_request, :on_train,
source_project: project, source_branch: 'signed-commits',
target_project: project, target_branch: 'master')
end
it 'processes the next merge request on the train' do
expect(AutoMergeProcessWorker).to receive(:perform_async).with(merge_request_2.id)
subject
end
context 'when process_next is false' do
let(:args) { { process_next: false } }
it 'does not process the next merge request on the train' do
expect(AutoMergeProcessWorker).not_to receive(:perform_async)
subject
end
end
end
end end
describe '#available_for?' do describe '#available_for?' do
......
...@@ -28,7 +28,7 @@ describe MergeTrains::RefreshMergeRequestService do ...@@ -28,7 +28,7 @@ describe MergeTrains::RefreshMergeRequestService do
it do it do
expect_next_instance_of(AutoMerge::MergeTrainService) do |service| expect_next_instance_of(AutoMerge::MergeTrainService) do |service|
expect(service).to receive(:abort).with(merge_request, kind_of(String)) expect(service).to receive(:abort).with(merge_request, kind_of(String), hash_including(process_next: false))
end end
subject subject
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment