From 0cc21f1dedf36111ac59842b1db5355d70c316e8 Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Tue, 15 Oct 2024 08:54:56 -0400 Subject: [PATCH] Reduce factory creation (132 -> 40) in lib/vacuum/* specs (#32498) --- spec/lib/vacuum/access_tokens_vacuum_spec.rb | 34 +++++++------------ spec/lib/vacuum/backups_vacuum_spec.rb | 13 +++---- spec/lib/vacuum/feeds_vacuum_spec.rb | 4 +-- .../vacuum/media_attachments_vacuum_spec.rb | 4 +-- spec/lib/vacuum/preview_cards_vacuum_spec.rb | 22 ++++++------ spec/lib/vacuum/statuses_vacuum_spec.rb | 22 +++++------- 6 files changed, 41 insertions(+), 58 deletions(-) diff --git a/spec/lib/vacuum/access_tokens_vacuum_spec.rb b/spec/lib/vacuum/access_tokens_vacuum_spec.rb index 54760c41bd..8768f6b2dc 100644 --- a/spec/lib/vacuum/access_tokens_vacuum_spec.rb +++ b/spec/lib/vacuum/access_tokens_vacuum_spec.rb @@ -14,32 +14,24 @@ RSpec.describe Vacuum::AccessTokensVacuum do let!(:expired_access_grant) { Fabricate(:access_grant, expires_in: 59.minutes.to_i, created_at: 1.hour.ago) } let!(:active_access_grant) { Fabricate(:access_grant) } - before do + it 'deletes revoked/expired access tokens and revoked/expired grants, but preserves active tokens/grants' do subject.perform - end - it 'deletes revoked access tokens' do - expect { revoked_access_token.reload }.to raise_error ActiveRecord::RecordNotFound - end + expect { revoked_access_token.reload } + .to raise_error ActiveRecord::RecordNotFound + expect { expired_access_token.reload } + .to raise_error ActiveRecord::RecordNotFound - it 'deletes expired access tokens' do - expect { expired_access_token.reload }.to raise_error ActiveRecord::RecordNotFound - end + expect { revoked_access_grant.reload } + .to raise_error ActiveRecord::RecordNotFound + expect { expired_access_grant.reload } + .to raise_error ActiveRecord::RecordNotFound - it 'deletes revoked access grants' do - expect { revoked_access_grant.reload }.to raise_error ActiveRecord::RecordNotFound - end + expect { active_access_token.reload } + .to_not raise_error - it 'deletes expired access grants' do - expect { expired_access_grant.reload }.to raise_error ActiveRecord::RecordNotFound - end - - it 'does not delete active access tokens' do - expect { active_access_token.reload }.to_not raise_error - end - - it 'does not delete active access grants' do - expect { active_access_grant.reload }.to_not raise_error + expect { active_access_grant.reload } + .to_not raise_error end end end diff --git a/spec/lib/vacuum/backups_vacuum_spec.rb b/spec/lib/vacuum/backups_vacuum_spec.rb index 867dbe4020..4a025352cb 100644 --- a/spec/lib/vacuum/backups_vacuum_spec.rb +++ b/spec/lib/vacuum/backups_vacuum_spec.rb @@ -11,16 +11,13 @@ RSpec.describe Vacuum::BackupsVacuum do let!(:expired_backup) { Fabricate(:backup, created_at: (retention_period + 1.day).ago) } let!(:current_backup) { Fabricate(:backup) } - before do + it 'deletes backups past the retention period but preserves those within the period' do subject.perform - end - it 'deletes backups past the retention period' do - expect { expired_backup.reload }.to raise_error ActiveRecord::RecordNotFound - end - - it 'does not delete backups within the retention period' do - expect { current_backup.reload }.to_not raise_error + expect { expired_backup.reload } + .to raise_error ActiveRecord::RecordNotFound + expect { current_backup.reload } + .to_not raise_error end end end diff --git a/spec/lib/vacuum/feeds_vacuum_spec.rb b/spec/lib/vacuum/feeds_vacuum_spec.rb index ede1e3c360..38459a558f 100644 --- a/spec/lib/vacuum/feeds_vacuum_spec.rb +++ b/spec/lib/vacuum/feeds_vacuum_spec.rb @@ -14,11 +14,11 @@ RSpec.describe Vacuum::FeedsVacuum do redis.zadd(feed_key_for(active_user), 1, 1) redis.zadd(feed_key_for(inactive_user, 'reblogs'), 2, 2) redis.sadd(feed_key_for(inactive_user, 'reblogs:2'), 3) - - subject.perform end it 'clears feeds of inactive users and lists' do + subject.perform + expect(redis.zcard(feed_key_for(inactive_user))).to eq 0 expect(redis.zcard(feed_key_for(active_user))).to eq 1 expect(redis.exists?(feed_key_for(inactive_user, 'reblogs'))).to be false diff --git a/spec/lib/vacuum/media_attachments_vacuum_spec.rb b/spec/lib/vacuum/media_attachments_vacuum_spec.rb index 1039c36cea..f7749038cb 100644 --- a/spec/lib/vacuum/media_attachments_vacuum_spec.rb +++ b/spec/lib/vacuum/media_attachments_vacuum_spec.rb @@ -17,9 +17,9 @@ RSpec.describe Vacuum::MediaAttachmentsVacuum do let!(:old_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 10.days.ago) } let!(:new_unattached_media) { Fabricate(:media_attachment, account_id: nil, created_at: 1.hour.ago) } - before { subject.perform } - it 'handles attachments based on metadata details' do + subject.perform + expect(old_remote_media.reload.file) # Remote and past retention period .to be_blank expect(old_local_media.reload.file) # Local and past retention diff --git a/spec/lib/vacuum/preview_cards_vacuum_spec.rb b/spec/lib/vacuum/preview_cards_vacuum_spec.rb index 9dbdf0bc2f..caeedd3269 100644 --- a/spec/lib/vacuum/preview_cards_vacuum_spec.rb +++ b/spec/lib/vacuum/preview_cards_vacuum_spec.rb @@ -15,24 +15,22 @@ RSpec.describe Vacuum::PreviewCardsVacuum do before do old_preview_card.statuses << Fabricate(:status) new_preview_card.statuses << Fabricate(:status) + end + it 'handles preview card cleanup' do subject.perform - end - it 'deletes cache of preview cards last updated before the retention period' do - expect(old_preview_card.reload.image).to be_blank - end + expect(old_preview_card.reload.image) # last updated before retention period + .to be_blank - it 'does not delete cache of preview cards last updated within the retention period' do - expect(new_preview_card.reload.image).to_not be_blank - end + expect(new_preview_card.reload.image) # last updated within the retention period + .to_not be_blank - it 'does not delete attached preview cards' do - expect(new_preview_card.reload).to be_persisted - end + expect(new_preview_card.reload) # Keep attached preview cards + .to be_persisted - it 'does not delete orphaned preview cards in the retention period' do - expect(orphaned_preview_card.reload).to be_persisted + expect(orphaned_preview_card.reload) # keep orphaned cards in the retention period + .to be_persisted end end end diff --git a/spec/lib/vacuum/statuses_vacuum_spec.rb b/spec/lib/vacuum/statuses_vacuum_spec.rb index d5c0139506..1fff864879 100644 --- a/spec/lib/vacuum/statuses_vacuum_spec.rb +++ b/spec/lib/vacuum/statuses_vacuum_spec.rb @@ -15,24 +15,20 @@ RSpec.describe Vacuum::StatusesVacuum do let!(:local_status_old) { Fabricate(:status, created_at: (retention_period + 2.days).ago) } let!(:local_status_recent) { Fabricate(:status, created_at: (retention_period - 2.days).ago) } - before do + it 'deletes remote statuses past the retention period and keeps others' do subject.perform - end - it 'deletes remote statuses past the retention period' do - expect { remote_status_old.reload }.to raise_error ActiveRecord::RecordNotFound - end + expect { remote_status_old.reload } + .to raise_error ActiveRecord::RecordNotFound - it 'does not delete local statuses past the retention period' do - expect { local_status_old.reload }.to_not raise_error - end + expect { local_status_old.reload } + .to_not raise_error - it 'does not delete remote statuses within the retention period' do - expect { remote_status_recent.reload }.to_not raise_error - end + expect { remote_status_recent.reload } + .to_not raise_error - it 'does not delete local statuses within the retention period' do - expect { local_status_recent.reload }.to_not raise_error + expect { local_status_recent.reload } + .to_not raise_error end end end