diff options
author | Nick Thomas <nick@gitlab.com> | 2019-06-21 19:56:47 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-07-04 10:50:55 +0300 |
commit | 381468d0cc6e5b528a4b2207c0a534569035a73f (patch) | |
tree | 2ffc9e9062fef50a7cca8dfd8d0b5733e8cf4c9d /spec/models/merge_request_spec.rb | |
parent | 9ef0c8559de925d0a72a3fe421d95209c2b81d8f (diff) |
Allow asynchronous rebase operations to be monitored
This MR introduces tracking of the `rebase_jid` for merge requests. As
with `merge_ongoing?`, `rebase_in_progress?` will now return true if a
rebase is proceeding in sidekiq.
After one release, we should remove the Gitaly-based lookup of rebases.
It is much better to track this kind of thing via the database.
Diffstat (limited to 'spec/models/merge_request_spec.rb')
-rw-r--r-- | spec/models/merge_request_spec.rb | 115 |
1 files changed, 91 insertions, 24 deletions
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index a2547755510..fe6d68aff3f 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -7,6 +7,8 @@ describe MergeRequest do include ProjectForksHelper include ReactiveCachingHelpers + using RSpec::Parameterized::TableSyntax + subject { create(:merge_request) } describe 'associations' do @@ -1996,6 +1998,47 @@ describe MergeRequest do end end + describe '#rebase_async' do + let(:merge_request) { create(:merge_request) } + let(:user_id) { double(:user_id) } + let(:rebase_jid) { 'rebase-jid' } + + subject(:execute) { merge_request.rebase_async(user_id) } + + it 'atomically enqueues a RebaseWorker job and updates rebase_jid' do + expect(RebaseWorker) + .to receive(:perform_async) + .with(merge_request.id, user_id) + .and_return(rebase_jid) + + expect(merge_request).to receive(:lock!).and_call_original + + execute + + expect(merge_request.rebase_jid).to eq(rebase_jid) + end + + it 'refuses to enqueue a job if a rebase is in progress' do + merge_request.update_column(:rebase_jid, rebase_jid) + + expect(RebaseWorker).not_to receive(:perform_async) + expect(Gitlab::SidekiqStatus) + .to receive(:running?) + .with(rebase_jid) + .and_return(true) + + expect { execute }.to raise_error(ActiveRecord::StaleObjectError) + end + + it 'refuses to enqueue a job if the MR is not open' do + merge_request.update_column(:state, 'foo') + + expect(RebaseWorker).not_to receive(:perform_async) + + expect { execute }.to raise_error(ActiveRecord::StaleObjectError) + end + end + describe '#mergeable?' do let(:project) { create(:project) } @@ -2946,40 +2989,64 @@ describe MergeRequest do end describe '#rebase_in_progress?' do - shared_examples 'checking whether a rebase is in progress' do - let(:repo_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - subject.source_project.repository.path - end - end - let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } + where(:rebase_jid, :jid_valid, :result) do + 'foo' | true | true + 'foo' | false | false + '' | true | false + nil | true | false + end - before do - system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) + with_them do + let(:merge_request) { create(:merge_request) } + + subject { merge_request.rebase_in_progress? } + + it do + # Stub out the legacy gitaly implementation + allow(merge_request).to receive(:gitaly_rebase_in_progress?) { false } + + allow(Gitlab::SidekiqStatus).to receive(:running?).with(rebase_jid) { jid_valid } + + merge_request.rebase_jid = rebase_jid + + is_expected.to eq(result) end + end + end - it 'returns true when there is a current rebase directory' do - expect(subject.rebase_in_progress?).to be_truthy + describe '#gitaly_rebase_in_progress?' do + let(:repo_path) do + Gitlab::GitalyClient::StorageSettings.allow_disk_access do + subject.source_project.repository.path end + end + let(:rebase_path) { File.join(repo_path, "gitlab-worktree", "rebase-#{subject.id}") } + + before do + system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{rebase_path} master)) + end - it 'returns false when there is no rebase directory' do - FileUtils.rm_rf(rebase_path) + it 'returns true when there is a current rebase directory' do + expect(subject.rebase_in_progress?).to be_truthy + end - expect(subject.rebase_in_progress?).to be_falsey - end + it 'returns false when there is no rebase directory' do + FileUtils.rm_rf(rebase_path) - it 'returns false when the rebase directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, rebase_path) + expect(subject.rebase_in_progress?).to be_falsey + end - expect(subject.rebase_in_progress?).to be_falsey - end + it 'returns false when the rebase directory has expired' do + time = 20.minutes.ago.to_time + File.utime(time, time, rebase_path) - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) + expect(subject.rebase_in_progress?).to be_falsey + end - expect(subject.rebase_in_progress?).to be_falsey - end + it 'returns false when the source project has been removed' do + allow(subject).to receive(:source_project).and_return(nil) + + expect(subject.rebase_in_progress?).to be_falsey end end |