Commit cfb7018a authored by Shinya Maeda's avatar Shinya Maeda

Fix AutoMergeProcessWorker raises an error for merged MRs

This commit fixes the noise of AutoMergeProcessWorker
parent 9fadae07
---
title: Fix RefreshMergeRequestsService raises an exception and unnecessary sidekiq retry
merge_request: 22262
author:
type: fixed
......@@ -81,7 +81,7 @@ module EE
end
def on_train?
merge_train.present?
merge_train&.active?
end
def allows_multiple_assignees?
......
......@@ -3,6 +3,8 @@
class MergeTrain < ApplicationRecord
include AfterCommitQueue
ACTIVE_STATUSES = %w[created stale fresh].freeze
belongs_to :target_project, class_name: "Project"
belongs_to :merge_request, inverse_of: :merge_train
belongs_to :user
......@@ -59,7 +61,7 @@ class MergeTrain < ApplicationRecord
end
def active_statuses
statuses.values_at(:created, :stale, :fresh)
statuses.values_at(*ACTIVE_STATUSES)
end
def merged_statuses
......@@ -117,6 +119,10 @@ class MergeTrain < ApplicationRecord
merge_request.cleanup_refs(only: :train)
end
def active?
ACTIVE_STATUSES.include?(status)
end
private
def refresh_async
......
......@@ -33,8 +33,9 @@ module MergeTrains
end
if result[:status] == :finished && result[:new_items].present?
first_merge_request = MergeTrain.first_in_train_from(result[:new_items])
AutoMergeProcessWorker.perform_async(first_merge_request.id)
MergeTrain.first_in_train_from(result[:new_items]).try do |merge_request|
AutoMergeProcessWorker.perform_async(merge_request.id)
end
end
end
......
......@@ -11,6 +11,7 @@ FactoryBot.modify do
trait :on_train do
transient do
train_creator { author }
status { 'created' }
end
auto_merge_enabled { true }
......@@ -18,7 +19,8 @@ FactoryBot.modify do
merge_user { train_creator }
after :create do |merge_request, evaluator|
merge_request.create_merge_train(user: evaluator.train_creator,
merge_request.create_merge_train(status: evaluator.status,
user: evaluator.train_creator,
target_project: merge_request.target_project,
target_branch: merge_request.target_branch)
end
......
......@@ -745,6 +745,14 @@ describe MergeRequest do
it { is_expected.to be_truthy }
end
context 'when the merge request was on a merge train' do
let(:merge_request) do
create(:merge_request, :on_train, status: 'merged', source_project: project, target_project: project)
end
it { is_expected.to be_falsy }
end
context 'when the merge request is not on a merge train' do
let(:merge_request) do
create(:merge_request, source_project: project, target_project: project)
......
......@@ -569,6 +569,22 @@ describe MergeTrain do
end
end
describe '#active?' do
subject { merge_train.active? }
context 'when status is created' do
let(:merge_train) { create(:merge_train, :created) }
it { is_expected.to eq(true) }
end
context 'when status is merged' do
let(:merge_train) { create(:merge_train, :merged) }
it { is_expected.to eq(false) }
end
end
def create_merge_request_on_train(target_project: project, target_branch: 'master', source_project: project, source_branch: 'feature')
create(:merge_request,
:on_train,
......
......@@ -112,6 +112,19 @@ describe MergeTrains::RefreshMergeRequestsService do
end
end
context 'when merge request 1 was on a merge train' do
before do
allow(merge_request_1.merge_train).to receive(:cleanup_ref)
merge_request_1.merge_train.merged!
end
it 'does not refresh' do
expect(refresh_service_1).not_to receive(:execute).with(merge_request_1)
subject
end
end
context 'when the other thread has already been processing the merge train' do
let(:lock_key) { "batch_pop_queueing:lock:merge_trains:#{merge_request.target_project_id}:#{merge_request.target_branch}" }
......@@ -177,6 +190,19 @@ describe MergeTrains::RefreshMergeRequestsService do
subject
end
context 'when merge request 1 has already been merged' do
before do
allow(merge_request_1.merge_train).to receive(:cleanup_ref)
merge_request_1.merge_train.merged!
end
it 'does not refresh the merge request 1' do
expect(AutoMergeProcessWorker).not_to receive(:perform_async).with(merge_request_1.id)
subject
end
end
end
end
end
......
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