Commit 972311f9 authored by Harsh Chouraria's avatar Harsh Chouraria

Address review comments by tigerwnz

Refactors the test to share the namespace and feature flag
context

Improves the batch calculation to be more readable than using
fdiv
parent bba3204b
......@@ -20,10 +20,7 @@ class ClearSharedRunnersMinutesWorker # rubocop:disable Scalability/IdempotentWo
start_id = Namespace.minimum(:id)
last_id = Namespace.maximum(:id)
# Uses float division to avoid a ZeroDivisionError
# when namespace ID range is lower than BATCH_SIZE
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/330068
batches = (last_id - start_id).fdiv(BATCH_SIZE)
batches = [(last_id - start_id) / BATCH_SIZE, 1].max
execution_offset = (TIME_SPREAD / batches).to_i
(start_id..last_id).step(BATCH_SIZE).with_index do |batch_start_id, batch_index|
......
......@@ -133,7 +133,7 @@ RSpec.describe ClearSharedRunnersMinutesWorker do
end
end
context 'when ci_parallel_minutes_reset feature flag is enabled, with batch size lower than namespace count' do
context 'when ci_parallel_minutes_reset feature flag is enabled' do
subject { worker.perform }
before do
......@@ -142,40 +142,33 @@ RSpec.describe ClearSharedRunnersMinutesWorker do
[2, 3, 4, 5, 7, 8, 10, 14].each do |id|
create(:namespace, id: id)
end
# Test with a batch size lower than count of namespaces
stub_const("#{described_class}::BATCH_SIZE", 3)
end
it 'runs a worker per batch', :aggregate_failures do
# Spread evenly accross 8 hours (28,800 seconds)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(0.seconds, 2, 4)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(7200.seconds, 5, 7)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(14400.seconds, 8, 10)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(21600.seconds, 11, 13)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(28800.seconds, 14, 16)
subject
end
end
context 'with batch size lower than count of namespaces' do
before do
stub_const("#{described_class}::BATCH_SIZE", 3)
end
context 'when ci_parallel_minutes_reset feature flag is enabled, with batch size higher than namespace count' do
subject { worker.perform }
it 'runs a worker per batch', :aggregate_failures do
# Spreads evenly accross 8 hours (28,800 seconds)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(0.seconds, 2, 4)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(7200.seconds, 5, 7)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(14400.seconds, 8, 10)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(21600.seconds, 11, 13)
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(28800.seconds, 14, 16)
before do
stub_feature_flags(ci_parallel_minutes_reset: true)
[2, 3, 4, 5, 7, 8, 10, 14].each do |id|
create(:namespace, id: id)
subject
end
# Use the default BATCH_SIZE (100_000)
end
it 'runs the worker in a single batch', :aggregate_failures do
# Runs a single batch, immediately
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(0.seconds, 2, 100_001)
context 'with batch size higher than count of namespaces' do
# Uses default BATCH_SIZE
it 'runs the worker in a single batch', :aggregate_failures do
# Runs a single batch, immediately
expect(Ci::BatchResetMinutesWorker).to receive(:perform_in).with(0.seconds, 2, 100001)
subject
subject
end
end
end
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment