diff options
Diffstat (limited to 'spec/models/merge_request')
-rw-r--r-- | spec/models/merge_request/cleanup_schedule_spec.rb | 133 | ||||
-rw-r--r-- | spec/models/merge_request/diff_commit_user_spec.rb | 127 |
2 files changed, 245 insertions, 15 deletions
diff --git a/spec/models/merge_request/cleanup_schedule_spec.rb b/spec/models/merge_request/cleanup_schedule_spec.rb index 925d287088b..85208f901fd 100644 --- a/spec/models/merge_request/cleanup_schedule_spec.rb +++ b/spec/models/merge_request/cleanup_schedule_spec.rb @@ -11,22 +11,125 @@ RSpec.describe MergeRequest::CleanupSchedule do it { is_expected.to validate_presence_of(:scheduled_at) } end - describe '.scheduled_merge_request_ids' do - let_it_be(:mr_cleanup_schedule_1) { create(:merge_request_cleanup_schedule, scheduled_at: 2.days.ago) } - let_it_be(:mr_cleanup_schedule_2) { create(:merge_request_cleanup_schedule, scheduled_at: 1.day.ago) } - let_it_be(:mr_cleanup_schedule_3) { create(:merge_request_cleanup_schedule, scheduled_at: 1.day.ago, completed_at: Time.current) } - let_it_be(:mr_cleanup_schedule_4) { create(:merge_request_cleanup_schedule, scheduled_at: 4.days.ago) } - let_it_be(:mr_cleanup_schedule_5) { create(:merge_request_cleanup_schedule, scheduled_at: 3.days.ago) } - let_it_be(:mr_cleanup_schedule_6) { create(:merge_request_cleanup_schedule, scheduled_at: 1.day.from_now) } - let_it_be(:mr_cleanup_schedule_7) { create(:merge_request_cleanup_schedule, scheduled_at: 5.days.ago) } - - it 'only includes incomplete schedule within the specified limit' do - expect(described_class.scheduled_merge_request_ids(4)).to eq([ - mr_cleanup_schedule_2.merge_request_id, - mr_cleanup_schedule_1.merge_request_id, - mr_cleanup_schedule_5.merge_request_id, - mr_cleanup_schedule_4.merge_request_id + describe 'state machine transitions' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule) } + + it 'sets status to unstarted by default' do + expect(cleanup_schedule).to be_unstarted + end + + describe '#run' do + it 'sets the status to running' do + cleanup_schedule.run + + expect(cleanup_schedule.reload).to be_running + end + + context 'when previous status is not unstarted' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule, :running) } + + it 'does not change status' do + expect { cleanup_schedule.run }.not_to change(cleanup_schedule, :status) + end + end + end + + describe '#retry' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule, :running) } + + it 'sets the status to unstarted' do + cleanup_schedule.retry + + expect(cleanup_schedule.reload).to be_unstarted + end + + it 'increments failed_count' do + expect { cleanup_schedule.retry }.to change(cleanup_schedule, :failed_count).by(1) + end + + context 'when previous status is not running' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule) } + + it 'does not change status' do + expect { cleanup_schedule.retry }.not_to change(cleanup_schedule, :status) + end + end + end + + describe '#complete' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule, :running) } + + it 'sets the status to completed' do + cleanup_schedule.complete + + expect(cleanup_schedule.reload).to be_completed + end + + it 'sets the completed_at' do + expect { cleanup_schedule.complete }.to change(cleanup_schedule, :completed_at) + end + + context 'when previous status is not running' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule, :completed) } + + it 'does not change status' do + expect { cleanup_schedule.complete }.not_to change(cleanup_schedule, :status) + end + end + end + + describe '#mark_as_failed' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule, :running) } + + it 'sets the status to failed' do + cleanup_schedule.mark_as_failed + + expect(cleanup_schedule.reload).to be_failed + end + + it 'increments failed_count' do + expect { cleanup_schedule.mark_as_failed }.to change(cleanup_schedule, :failed_count).by(1) + end + + context 'when previous status is not running' do + let(:cleanup_schedule) { create(:merge_request_cleanup_schedule, :failed) } + + it 'does not change status' do + expect { cleanup_schedule.mark_as_failed }.not_to change(cleanup_schedule, :status) + end + end + end + end + + describe '.scheduled_and_unstarted' do + let!(:cleanup_schedule_1) { create(:merge_request_cleanup_schedule, scheduled_at: 2.days.ago) } + let!(:cleanup_schedule_2) { create(:merge_request_cleanup_schedule, scheduled_at: 1.day.ago) } + let!(:cleanup_schedule_3) { create(:merge_request_cleanup_schedule, :completed, scheduled_at: 1.day.ago) } + let!(:cleanup_schedule_4) { create(:merge_request_cleanup_schedule, scheduled_at: 4.days.ago) } + let!(:cleanup_schedule_5) { create(:merge_request_cleanup_schedule, scheduled_at: 3.days.ago) } + let!(:cleanup_schedule_6) { create(:merge_request_cleanup_schedule, scheduled_at: 1.day.from_now) } + let!(:cleanup_schedule_7) { create(:merge_request_cleanup_schedule, :failed, scheduled_at: 5.days.ago) } + + it 'returns records that are scheduled before or on current time and unstarted (ordered by scheduled first)' do + expect(described_class.scheduled_and_unstarted).to eq([ + cleanup_schedule_2, + cleanup_schedule_1, + cleanup_schedule_5, + cleanup_schedule_4 ]) end end + + describe '.start_next' do + let!(:cleanup_schedule_1) { create(:merge_request_cleanup_schedule, :completed, scheduled_at: 1.day.ago) } + let!(:cleanup_schedule_2) { create(:merge_request_cleanup_schedule, scheduled_at: 2.days.ago) } + let!(:cleanup_schedule_3) { create(:merge_request_cleanup_schedule, :running, scheduled_at: 1.day.ago) } + let!(:cleanup_schedule_4) { create(:merge_request_cleanup_schedule, scheduled_at: 3.days.ago) } + let!(:cleanup_schedule_5) { create(:merge_request_cleanup_schedule, :failed, scheduled_at: 3.days.ago) } + + it 'finds the next scheduled and unstarted then marked it as running' do + expect(described_class.start_next).to eq(cleanup_schedule_2) + expect(cleanup_schedule_2.reload).to be_running + end + end end diff --git a/spec/models/merge_request/diff_commit_user_spec.rb b/spec/models/merge_request/diff_commit_user_spec.rb new file mode 100644 index 00000000000..08e073568f9 --- /dev/null +++ b/spec/models/merge_request/diff_commit_user_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe MergeRequest::DiffCommitUser do + describe 'validations' do + it 'requires that names are less than 512 characters long' do + expect(described_class.new(name: 'a' * 1000)).not_to be_valid + end + + it 'requires that Emails are less than 512 characters long' do + expect(described_class.new(email: 'a' * 1000)).not_to be_valid + end + + it 'requires either a name or Email' do + expect(described_class.new).not_to be_valid + end + + it 'allows setting of just a name' do + expect(described_class.new(name: 'Alice')).to be_valid + end + + it 'allows setting of just an Email' do + expect(described_class.new(email: 'alice@example.com')).to be_valid + end + + it 'allows setting of both a name and Email' do + expect(described_class.new(name: 'Alice', email: 'alice@example.com')) + .to be_valid + end + end + + describe '.prepare' do + it 'trims a value to at most 512 characters' do + expect(described_class.prepare('€' * 1_000)).to eq('€' * 512) + end + + it 'returns nil if the value is an empty string' do + expect(described_class.prepare('')).to be_nil + end + end + + describe '.find_or_create' do + it 'creates a new row if none exist' do + alice = described_class.find_or_create('Alice', 'alice@example.com') + + expect(alice.name).to eq('Alice') + expect(alice.email).to eq('alice@example.com') + end + + it 'returns an existing row if one exists' do + user1 = create(:merge_request_diff_commit_user) + user2 = described_class.find_or_create(user1.name, user1.email) + + expect(user1).to eq(user2) + end + + it 'handles concurrent inserts' do + user = create(:merge_request_diff_commit_user) + + expect(described_class) + .to receive(:find_or_create_by!) + .ordered + .with(name: user.name, email: user.email) + .and_raise(ActiveRecord::RecordNotUnique) + + expect(described_class) + .to receive(:find_or_create_by!) + .ordered + .with(name: user.name, email: user.email) + .and_return(user) + + expect(described_class.find_or_create(user.name, user.email)).to eq(user) + end + end + + describe '.bulk_find_or_create' do + it 'bulk creates missing rows and reuses existing rows' do + bob = create( + :merge_request_diff_commit_user, + name: 'Bob', + email: 'bob@example.com' + ) + + users = described_class.bulk_find_or_create( + [%w[Alice alice@example.com], %w[Bob bob@example.com]] + ) + alice = described_class.find_by(name: 'Alice') + + expect(users[%w[Alice alice@example.com]]).to eq(alice) + expect(users[%w[Bob bob@example.com]]).to eq(bob) + end + + it 'does not insert any data when all users exist' do + bob = create( + :merge_request_diff_commit_user, + name: 'Bob', + email: 'bob@example.com' + ) + + users = described_class.bulk_find_or_create([%w[Bob bob@example.com]]) + + expect(described_class).not_to receive(:insert_all) + expect(users[%w[Bob bob@example.com]]).to eq(bob) + end + + it 'handles concurrently inserted rows' do + bob = create( + :merge_request_diff_commit_user, + name: 'Bob', + email: 'bob@example.com' + ) + + input = [%w[Bob bob@example.com]] + + expect(described_class) + .to receive(:bulk_find) + .twice + .with(input) + .and_return([], [bob]) + + users = described_class.bulk_find_or_create(input) + + expect(users[%w[Bob bob@example.com]]).to eq(bob) + end + end +end |