Speed improvement for `AccountsStatusesCleanupScheduler` spec (#25406)

lolsob-rspec
Matt Jankowski 2023-06-14 03:56:11 -04:00 committed by GitHub
parent 943c99f780
commit 8c7da82c64
2 changed files with 43 additions and 42 deletions

View File

@ -56,7 +56,6 @@ Lint/AmbiguousBlockAssociation:
- 'spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb' - 'spec/controllers/settings/two_factor_authentication/otp_authentication_controller_spec.rb'
- 'spec/services/activitypub/process_status_update_service_spec.rb' - 'spec/services/activitypub/process_status_update_service_spec.rb'
- 'spec/services/post_status_service_spec.rb' - 'spec/services/post_status_service_spec.rb'
- 'spec/workers/scheduler/accounts_statuses_cleanup_scheduler_spec.rb'
# Configuration parameters: AllowComments, AllowEmptyLambdas. # Configuration parameters: AllowComments, AllowEmptyLambdas.
Lint/EmptyBlock: Lint/EmptyBlock:
@ -359,7 +358,6 @@ RSpec/LetSetup:
- 'spec/services/suspend_account_service_spec.rb' - 'spec/services/suspend_account_service_spec.rb'
- 'spec/services/unallow_domain_service_spec.rb' - 'spec/services/unallow_domain_service_spec.rb'
- 'spec/services/unsuspend_account_service_spec.rb' - 'spec/services/unsuspend_account_service_spec.rb'
- 'spec/workers/scheduler/accounts_statuses_cleanup_scheduler_spec.rb'
- 'spec/workers/scheduler/user_cleanup_scheduler_spec.rb' - 'spec/workers/scheduler/user_cleanup_scheduler_spec.rb'
RSpec/MessageChain: RSpec/MessageChain:

View File

@ -12,11 +12,6 @@ describe Scheduler::AccountsStatusesCleanupScheduler do
let!(:account5) { Fabricate(:account, domain: nil) } let!(:account5) { Fabricate(:account, domain: nil) }
let!(:remote) { Fabricate(:account) } let!(:remote) { Fabricate(:account) }
let!(:policy1) { Fabricate(:account_statuses_cleanup_policy, account: account1) }
let!(:policy2) { Fabricate(:account_statuses_cleanup_policy, account: account3) }
let!(:policy3) { Fabricate(:account_statuses_cleanup_policy, account: account4, enabled: false) }
let!(:policy4) { Fabricate(:account_statuses_cleanup_policy, account: account5) }
let(:queue_size) { 0 } let(:queue_size) { 0 }
let(:queue_latency) { 0 } let(:queue_latency) { 0 }
let(:process_set_stub) do let(:process_set_stub) do
@ -29,33 +24,12 @@ describe Scheduler::AccountsStatusesCleanupScheduler do
end end
before do before do
queue_stub = double queue_stub = instance_double(Sidekiq::Queue, size: queue_size, latency: queue_latency)
allow(queue_stub).to receive(:size).and_return(queue_size)
allow(queue_stub).to receive(:latency).and_return(queue_latency)
allow(Sidekiq::Queue).to receive(:new).and_return(queue_stub) allow(Sidekiq::Queue).to receive(:new).and_return(queue_stub)
allow(Sidekiq::ProcessSet).to receive(:new).and_return(process_set_stub) allow(Sidekiq::ProcessSet).to receive(:new).and_return(process_set_stub)
sidekiq_stats_stub = double sidekiq_stats_stub = instance_double(Sidekiq::Stats)
allow(Sidekiq::Stats).to receive(:new).and_return(sidekiq_stats_stub) allow(Sidekiq::Stats).to receive(:new).and_return(sidekiq_stats_stub)
# Create a bunch of old statuses
10.times do
Fabricate(:status, account: account1, created_at: 3.years.ago)
Fabricate(:status, account: account2, created_at: 3.years.ago)
Fabricate(:status, account: account3, created_at: 3.years.ago)
Fabricate(:status, account: account4, created_at: 3.years.ago)
Fabricate(:status, account: account5, created_at: 3.years.ago)
Fabricate(:status, account: remote, created_at: 3.years.ago)
end
# Create a bunch of newer statuses
5.times do
Fabricate(:status, account: account1, created_at: 3.minutes.ago)
Fabricate(:status, account: account2, created_at: 3.minutes.ago)
Fabricate(:status, account: account3, created_at: 3.minutes.ago)
Fabricate(:status, account: account4, created_at: 3.minutes.ago)
Fabricate(:status, account: remote, created_at: 3.minutes.ago)
end
end end
describe '#under_load?' do describe '#under_load?' do
@ -101,23 +75,45 @@ describe Scheduler::AccountsStatusesCleanupScheduler do
end end
describe '#perform' do describe '#perform' do
before do
# Policies for the accounts
Fabricate(:account_statuses_cleanup_policy, account: account1)
Fabricate(:account_statuses_cleanup_policy, account: account3)
Fabricate(:account_statuses_cleanup_policy, account: account4, enabled: false)
Fabricate(:account_statuses_cleanup_policy, account: account5)
# Create a bunch of old statuses
4.times do
Fabricate(:status, account: account1, created_at: 3.years.ago)
Fabricate(:status, account: account2, created_at: 3.years.ago)
Fabricate(:status, account: account3, created_at: 3.years.ago)
Fabricate(:status, account: account4, created_at: 3.years.ago)
Fabricate(:status, account: account5, created_at: 3.years.ago)
Fabricate(:status, account: remote, created_at: 3.years.ago)
end
# Create a bunch of newer statuses
Fabricate(:status, account: account1, created_at: 3.minutes.ago)
Fabricate(:status, account: account2, created_at: 3.minutes.ago)
Fabricate(:status, account: account3, created_at: 3.minutes.ago)
Fabricate(:status, account: account4, created_at: 3.minutes.ago)
Fabricate(:status, account: remote, created_at: 3.minutes.ago)
end
context 'when the budget is lower than the number of toots to delete' do context 'when the budget is lower than the number of toots to delete' do
it 'deletes as many statuses as the given budget' do it 'deletes the appropriate statuses' do
expect { subject.perform }.to change(Status, :count).by(-subject.compute_budget) expect(Status.count).to be > (subject.compute_budget) # Data check
end
it 'does not delete from accounts with no cleanup policy' do expect { subject.perform }
expect { subject.perform }.to_not change { account2.statuses.count } .to change(Status, :count).by(-subject.compute_budget) # Cleanable statuses
end .and (not_change { account2.statuses.count }) # No cleanup policy for account
.and(not_change { account4.statuses.count }) # Disabled cleanup policy
it 'does not delete from accounts with disabled cleanup policies' do
expect { subject.perform }.to_not change { account4.statuses.count }
end end
it 'eventually deletes every deletable toot given enough runs' do it 'eventually deletes every deletable toot given enough runs' do
stub_const 'Scheduler::AccountsStatusesCleanupScheduler::MAX_BUDGET', 4 stub_const 'Scheduler::AccountsStatusesCleanupScheduler::MAX_BUDGET', 4
expect { 10.times { subject.perform } }.to change(Status, :count).by(-30) expect { 3.times { subject.perform } }.to change(Status, :count).by(-cleanable_statuses_count)
end end
it 'correctly round-trips between users across several runs' do it 'correctly round-trips between users across several runs' do
@ -128,7 +124,7 @@ describe Scheduler::AccountsStatusesCleanupScheduler do
.to change(Status, :count).by(-3 * 3) .to change(Status, :count).by(-3 * 3)
.and change { account1.statuses.count } .and change { account1.statuses.count }
.and change { account3.statuses.count } .and change { account3.statuses.count }
.and change { account5.statuses.count } .and(change { account5.statuses.count })
end end
context 'when given a big budget' do context 'when given a big budget' do
@ -140,7 +136,7 @@ describe Scheduler::AccountsStatusesCleanupScheduler do
it 'correctly handles looping in a single run' do it 'correctly handles looping in a single run' do
expect(subject.compute_budget).to eq(400) expect(subject.compute_budget).to eq(400)
expect { subject.perform }.to change(Status, :count).by(-30) expect { subject.perform }.to change(Status, :count).by(-cleanable_statuses_count)
end end
end end
@ -157,6 +153,13 @@ describe Scheduler::AccountsStatusesCleanupScheduler do
expect { subject.perform }.to_not change(Status, :count) expect { subject.perform }.to_not change(Status, :count)
end end
end end
def cleanable_statuses_count
Status
.where(account_id: [account1, account3, account5]) # Accounts with enabled policies
.where('created_at < ?', 2.weeks.ago) # Policy defaults is 2.weeks
.count
end
end end
end end
end end