diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2017-08-28 16:41:05 -0300 |
---|---|---|
committer | Oswaldo Ferreira <oswaldo@gitlab.com> | 2017-08-28 16:41:05 -0300 |
commit | 86f43912fcb8dbf8604b0054d020acd12071d5b2 (patch) | |
tree | 2003939f9427528b2d98889428514c7490f66ed1 | |
parent | ffc576d7df8d9dd53806d48b1870d11785e8d2a7 (diff) | |
download | gitlab-ce-86f43912fcb8dbf8604b0054d020acd12071d5b2.tar.gz |
Cleans merge_jid when possible on MergeService36114-stuck-mrs-job-follow-up
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 9 | ||||
-rw-r--r-- | spec/services/merge_requests/merge_service_spec.rb | 33 |
2 files changed, 34 insertions, 8 deletions
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index aac1e7e19c3..b2b6c5627fb 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -26,14 +26,13 @@ module MergeRequests merge_request.in_locked_state do if commit after_merge + clean_merge_jid success end end rescue MergeError => e + clean_merge_jid log_merge_error(e.message, save_message_on_model: true) - ensure - # Make sure to clean up merge_jid in the end of the merge process. - merge_request.update_column(:merge_jid, nil) end private @@ -73,6 +72,10 @@ module MergeRequests end end + def clean_merge_jid + merge_request.update_column(:merge_jid, nil) + end + def branch_deletion_user @merge_request.force_remove_source_branch? ? @merge_request.author : current_user end diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index 0a007bdfc3b..b60136064b7 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -12,13 +12,36 @@ describe MergeRequests::MergeService do end describe '#execute' do - it 'cleans up merge_jid from MergeRequest' do - merge_request.update_column(:merge_jid, 'hash-123') - service = described_class.new(project, user, commit_message: 'Awesome message') + context 'MergeRequest#merge_jid' do + before do + merge_request.update_column(:merge_jid, 'hash-123') + end + + it 'is cleaned when no error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end - service.execute(merge_request) + it 'is cleaned when expected error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + allow(service).to receive(:commit).and_raise(described_class::MergeError) - expect(merge_request.reload.merge_jid).to be_nil + service.execute(merge_request) + + expect(merge_request.reload.merge_jid).to be_nil + end + + it 'is not cleaned when unexpected error is raised' do + service = described_class.new(project, user, commit_message: 'Awesome message') + allow(service).to receive(:commit).and_raise(StandardError) + + expect { service.execute(merge_request) }.to raise_error(StandardError) + + expect(merge_request.reload.merge_jid).to be_present + end end context 'valid params' do |