From d88a8a2b0d5a864220e7ca612a73433fb61aa1e7 Mon Sep 17 00:00:00 2001 From: Allison Browne <abrowne@gitlab.com> Date: Thu, 5 Sep 2024 13:10:05 -0400 Subject: [PATCH] Fix bug where car left after branch deletion When the merge train strategy is 'Add to train when pipeline succeeds' it's possible to abort the merge after it's been added to the train. When this happens gitlab currently neglets to destroy the train and cancel the related pipeline. Instead it only disables the auto merge. Changelog: fixed EE: true --- ...ge_train_when_pipeline_succeeds_service.rb | 13 ++++++-- ...ain_when_pipeline_succeeds_service_spec.rb | 30 +++++++++++++++---- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/ee/app/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service.rb b/ee/app/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service.rb index 7000b36ed2dc23d8..42944a309fd9dd28 100644 --- a/ee/app/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service.rb +++ b/ee/app/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service.rb @@ -25,8 +25,17 @@ def cancel(merge_request) end def abort(merge_request, reason) - super do - SystemNoteService.abort_add_to_merge_train_when_pipeline_succeeds(merge_request, project, current_user, reason) + # If the merge request is already on a merge train, we need to destroy the car + # i.e. If the target branch is deleted which causes an abort with this strategy, + # after the pipeline succeeded and was added + # + if merge_request.merge_train_car + AutoMerge::MergeTrainService.new(project, current_user).abort(merge_request, reason) + # Before the pipeline succeeds and was added to the merge train + else + super do + SystemNoteService.abort_add_to_merge_train_when_pipeline_succeeds(merge_request, project, current_user, reason) + end end end diff --git a/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb b/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb index fbf53890872ad4b7..29c8e72f99dc5def 100644 --- a/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb +++ b/ee/spec/services/auto_merge/add_to_merge_train_when_pipeline_succeeds_service_spec.rb @@ -123,14 +123,32 @@ let(:merge_request) { create(:merge_request, :add_to_merge_train_when_pipeline_succeeds, merge_user: user) } - it 'aborts auto merge' do - expect(SystemNoteService) - .to receive(:abort_add_to_merge_train_when_pipeline_succeeds) - .with(merge_request, project, user, 'an error') + context 'without merge train car' do + it 'disables the auto-merge' do + expect(SystemNoteService) + .to receive(:abort_add_to_merge_train_when_pipeline_succeeds) + .with(merge_request, project, user, 'an error') - subject + subject - expect(merge_request).not_to be_auto_merge_enabled + expect(merge_request).not_to be_auto_merge_enabled + end + end + + context 'with merge train car' do + let(:merge_train_car) { create(:merge_train_car, merge_request: merge_request, target_project: project) } + + it 'aborts by destroying the running train car and canceling the pipeline' do + expect(merge_train_car).not_to be_nil + expect(SystemNoteService) + .to receive(:abort_merge_train) + .with(merge_request, project, user, 'an error') + + subject + + expect(merge_request).not_to be_auto_merge_enabled + expect(merge_request.reload.merge_train_car).to be_nil + end end end -- GitLab