diff options
Diffstat (limited to 'spec/lib/gitlab/sidekiq_middleware/duplicate_jobs')
5 files changed, 138 insertions, 203 deletions
diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb index 98350fb9b8e..4d12e4b3f6f 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/client_spec.rb @@ -3,79 +3,84 @@ require 'spec_helper' RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Client, :clean_gitlab_redis_queues do - let(:worker_class) do - Class.new do - def self.name - 'TestDeduplicationWorker' - end + shared_context 'deduplication worker class' do |strategy, including_scheduled| + let(:worker_class) do + Class.new do + def self.name + 'TestDeduplicationWorker' + end + + include ApplicationWorker + + deduplicate strategy, including_scheduled: including_scheduled - include ApplicationWorker + include ApplicationWorker - def perform(*args) + def perform(*args) + end end end - end - before do - stub_const('TestDeduplicationWorker', worker_class) + before do + stub_const('TestDeduplicationWorker', worker_class) + end end - describe '#call' do - it 'adds a correct duplicate tag to the jobs', :aggregate_failures do - TestDeduplicationWorker.bulk_perform_async([['args1'], ['args2'], ['args1']]) + shared_examples 'client duplicate job' do |strategy| + describe '#call' do + include_context 'deduplication worker class', strategy, false - job1, job2, job3 = TestDeduplicationWorker.jobs - - expect(job1['duplicate-of']).to be_nil - expect(job2['duplicate-of']).to be_nil - expect(job3['duplicate-of']).to eq(job1['jid']) - end - - context 'without scheduled deduplication' do - it "does not mark a job that's scheduled in the future as a duplicate" do - TestDeduplicationWorker.perform_async('args1') - TestDeduplicationWorker.perform_at(1.day.from_now, 'args1') - TestDeduplicationWorker.perform_in(3.hours, 'args1') + it 'adds a correct duplicate tag to the jobs', :aggregate_failures do + TestDeduplicationWorker.bulk_perform_async([['args1'], ['args2'], ['args1']]) - duplicates = TestDeduplicationWorker.jobs.map { |job| job['duplicate-of'] } + job1, job2, job3 = TestDeduplicationWorker.jobs - expect(duplicates).to all(be_nil) + expect(job1['duplicate-of']).to be_nil + expect(job2['duplicate-of']).to be_nil + expect(job3['duplicate-of']).to eq(job1['jid']) end - end - - context 'with scheduled deduplication' do - let(:scheduled_worker_class) do - Class.new do - def self.name - 'TestDeduplicationWorker' - end - include ApplicationWorker + context 'without scheduled deduplication' do + it "does not mark a job that's scheduled in the future as a duplicate" do + TestDeduplicationWorker.perform_async('args1') + TestDeduplicationWorker.perform_at(1.day.from_now, 'args1') + TestDeduplicationWorker.perform_in(3.hours, 'args1') - deduplicate :until_executing, including_scheduled: true + duplicates = TestDeduplicationWorker.jobs.map { |job| job['duplicate-of'] } - def perform(*args) - end + expect(duplicates).to all(be_nil) end end - before do - stub_const('TestDeduplicationWorker', scheduled_worker_class) - end + context 'with scheduled deduplication' do + include_context 'deduplication worker class', strategy, true - it 'adds a correct duplicate tag to the jobs', :aggregate_failures do - TestDeduplicationWorker.perform_async('args1') - TestDeduplicationWorker.perform_at(1.day.from_now, 'args1') - TestDeduplicationWorker.perform_in(3.hours, 'args1') - TestDeduplicationWorker.perform_in(3.hours, 'args2') + before do + stub_const('TestDeduplicationWorker', worker_class) + end - job1, job2, job3, job4 = TestDeduplicationWorker.jobs + it 'adds a correct duplicate tag to the jobs', :aggregate_failures do + TestDeduplicationWorker.perform_async('args1') + TestDeduplicationWorker.perform_at(1.day.from_now, 'args1') + TestDeduplicationWorker.perform_in(3.hours, 'args1') + TestDeduplicationWorker.perform_in(3.hours, 'args2') - expect(job1['duplicate-of']).to be_nil - expect(job2['duplicate-of']).to eq(job1['jid']) - expect(job3['duplicate-of']).to eq(job1['jid']) - expect(job4['duplicate-of']).to be_nil + job1, job2, job3, job4 = TestDeduplicationWorker.jobs + + expect(job1['duplicate-of']).to be_nil + expect(job2['duplicate-of']).to eq(job1['jid']) + expect(job3['duplicate-of']).to eq(job1['jid']) + expect(job4['duplicate-of']).to be_nil + end end end end + + context 'with until_executing strategy' do + it_behaves_like 'client duplicate job', :until_executing + end + + context 'with until_executed strategy' do + it_behaves_like 'client duplicate job', :until_executed + end end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/server_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/server_spec.rb index 3f75d867936..09548d21106 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/server_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/server_spec.rb @@ -3,39 +3,71 @@ require 'spec_helper' RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Server, :clean_gitlab_redis_queues do - let(:worker_class) do - Class.new do - def self.name - 'TestDeduplicationWorker' + shared_context 'server duplicate job' do |strategy| + let(:worker_class) do + Class.new do + def self.name + 'TestDeduplicationWorker' + end + + include ApplicationWorker + + deduplicate strategy + + def perform(*args) + self.class.work + end + + def self.work + end end + end - include ApplicationWorker + before do + stub_const('TestDeduplicationWorker', worker_class) + end - def perform(*args) + around do |example| + with_sidekiq_server_middleware do |chain| + chain.add described_class + Sidekiq::Testing.inline! { example.run } end end end - before do - stub_const('TestDeduplicationWorker', worker_class) - end + context 'with until_executing strategy' do + include_context 'server duplicate job', :until_executing - around do |example| - with_sidekiq_server_middleware do |chain| - chain.add described_class - Sidekiq::Testing.inline! { example.run } + describe '#call' do + it 'removes the stored job from redis before execution' do + bare_job = { 'class' => 'TestDeduplicationWorker', 'args' => ['hello'] } + job_definition = Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob.new(bare_job.dup, 'test_deduplication') + + expect(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob) + .to receive(:new).with(a_hash_including(bare_job), 'test_deduplication') + .and_return(job_definition).twice # once in client middleware + + expect(job_definition).to receive(:delete!).ordered.and_call_original + expect(TestDeduplicationWorker).to receive(:work).ordered.and_call_original + + TestDeduplicationWorker.perform_async('hello') + end end end - describe '#call' do - it 'removes the stored job from redis' do + context 'with until_executed strategy' do + include_context 'server duplicate job', :until_executed + + it 'removes the stored job from redis after execution' do bare_job = { 'class' => 'TestDeduplicationWorker', 'args' => ['hello'] } job_definition = Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob.new(bare_job.dup, 'test_deduplication') expect(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob) .to receive(:new).with(a_hash_including(bare_job), 'test_deduplication') .and_return(job_definition).twice # once in client middleware - expect(job_definition).to receive(:delete!).and_call_original + + expect(TestDeduplicationWorker).to receive(:work).ordered.and_call_original + expect(job_definition).to receive(:delete!).ordered.and_call_original TestDeduplicationWorker.perform_async('hello') end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb new file mode 100644 index 00000000000..b3d463b6f6b --- /dev/null +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executed_spec.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuted do + it_behaves_like 'deduplicating jobs when scheduling', :until_executed do + describe '#perform' do + let(:proc) { -> {} } + + it 'deletes the lock after executing' do + expect(proc).to receive(:call).ordered + expect(fake_duplicate_job).to receive(:delete!).ordered + + strategy.perform({}) do + proc.call + end + end + end + end +end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb index 10b18052e9a..d45b6c5fcd1 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies/until_executing_spec.rb @@ -1,146 +1,20 @@ # frozen_string_literal: true -require 'fast_spec_helper' +require 'spec_helper' RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies::UntilExecuting do - let(:fake_duplicate_job) do - instance_double(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob) - end - - subject(:strategy) { described_class.new(fake_duplicate_job) } - - describe '#schedule' do - before do - allow(Gitlab::SidekiqLogging::DeduplicationLogger.instance).to receive(:log) - end - - it 'checks for duplicates before yielding' do - expect(fake_duplicate_job).to receive(:scheduled?).twice.ordered.and_return(false) - expect(fake_duplicate_job).to( - receive(:check!) - .with(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DUPLICATE_KEY_TTL) - .ordered - .and_return('a jid')) - expect(fake_duplicate_job).to receive(:duplicate?).ordered.and_return(false) - - expect { |b| strategy.schedule({}, &b) }.to yield_control - end - - it 'checks worker options for scheduled jobs' do - expect(fake_duplicate_job).to receive(:scheduled?).ordered.and_return(true) - expect(fake_duplicate_job).to receive(:options).ordered.and_return({}) - expect(fake_duplicate_job).not_to receive(:check!) - - expect { |b| strategy.schedule({}, &b) }.to yield_control - end - - context 'job marking' do - it 'adds the jid of the existing job to the job hash' do - allow(fake_duplicate_job).to receive(:scheduled?).and_return(false) - allow(fake_duplicate_job).to receive(:check!).and_return('the jid') - allow(fake_duplicate_job).to receive(:droppable?).and_return(true) - allow(fake_duplicate_job).to receive(:options).and_return({}) - job_hash = {} + it_behaves_like 'deduplicating jobs when scheduling', :until_executing do + describe '#perform' do + let(:proc) { -> {} } - expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) - expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') + it 'deletes the lock before executing' do + expect(fake_duplicate_job).to receive(:delete!).ordered + expect(proc).to receive(:call).ordered - strategy.schedule(job_hash) {} - - expect(job_hash).to include('duplicate-of' => 'the jid') - end - - context 'scheduled jobs' do - let(:time_diff) { 1.minute } - - context 'scheduled in the past' do - it 'adds the jid of the existing job to the job hash' do - allow(fake_duplicate_job).to receive(:scheduled?).twice.and_return(true) - allow(fake_duplicate_job).to receive(:scheduled_at).and_return(Time.now - time_diff) - allow(fake_duplicate_job).to receive(:options).and_return({ including_scheduled: true }) - allow(fake_duplicate_job).to( - receive(:check!) - .with(Gitlab::SidekiqMiddleware::DuplicateJobs::DuplicateJob::DUPLICATE_KEY_TTL) - .and_return('the jid')) - allow(fake_duplicate_job).to receive(:droppable?).and_return(true) - job_hash = {} - - expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) - expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') - - strategy.schedule(job_hash) {} - - expect(job_hash).to include('duplicate-of' => 'the jid') - end + strategy.perform({}) do + proc.call end - - context 'scheduled in the future' do - it 'adds the jid of the existing job to the job hash' do - freeze_time do - allow(fake_duplicate_job).to receive(:scheduled?).twice.and_return(true) - allow(fake_duplicate_job).to receive(:scheduled_at).and_return(Time.now + time_diff) - allow(fake_duplicate_job).to receive(:options).and_return({ including_scheduled: true }) - allow(fake_duplicate_job).to( - receive(:check!).with(time_diff.to_i).and_return('the jid')) - allow(fake_duplicate_job).to receive(:droppable?).and_return(true) - job_hash = {} - - expect(fake_duplicate_job).to receive(:duplicate?).and_return(true) - expect(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') - - strategy.schedule(job_hash) {} - - expect(job_hash).to include('duplicate-of' => 'the jid') - end - end - end - end - end - - context "when the job is droppable" do - before do - allow(fake_duplicate_job).to receive(:scheduled?).and_return(false) - allow(fake_duplicate_job).to receive(:check!).and_return('the jid') - allow(fake_duplicate_job).to receive(:duplicate?).and_return(true) - allow(fake_duplicate_job).to receive(:options).and_return({}) - allow(fake_duplicate_job).to receive(:existing_jid).and_return('the jid') - allow(fake_duplicate_job).to receive(:droppable?).and_return(true) - end - - it 'drops the job' do - schedule_result = nil - - expect(fake_duplicate_job).to receive(:droppable?).and_return(true) - - expect { |b| schedule_result = strategy.schedule({}, &b) }.not_to yield_control - expect(schedule_result).to be(false) - end - - it 'logs that the job was dropped' do - fake_logger = instance_double(Gitlab::SidekiqLogging::DeduplicationLogger) - - expect(Gitlab::SidekiqLogging::DeduplicationLogger).to receive(:instance).and_return(fake_logger) - expect(fake_logger).to receive(:log).with(a_hash_including({ 'jid' => 'new jid' }), 'dropped until executing', {}) - - strategy.schedule({ 'jid' => 'new jid' }) {} - end - - it 'logs the deduplication options of the worker' do - fake_logger = instance_double(Gitlab::SidekiqLogging::DeduplicationLogger) - - expect(Gitlab::SidekiqLogging::DeduplicationLogger).to receive(:instance).and_return(fake_logger) - allow(fake_duplicate_job).to receive(:options).and_return({ foo: :bar }) - expect(fake_logger).to receive(:log).with(a_hash_including({ 'jid' => 'new jid' }), 'dropped until executing', { foo: :bar }) - - strategy.schedule({ 'jid' => 'new jid' }) {} end end end - - describe '#perform' do - it 'deletes the lock before executing' do - expect(fake_duplicate_job).to receive(:delete!).ordered - expect { |b| strategy.perform({}, &b) }.to yield_control - end - end end diff --git a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies_spec.rb b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies_spec.rb index 84856238aab..e35d779f334 100644 --- a/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies_spec.rb +++ b/spec/lib/gitlab/sidekiq_middleware/duplicate_jobs/strategies_spec.rb @@ -8,6 +8,10 @@ RSpec.describe Gitlab::SidekiqMiddleware::DuplicateJobs::Strategies do expect(described_class.for(:until_executing)).to eq(described_class::UntilExecuting) end + it 'returns the right class for `until_executed`' do + expect(described_class.for(:until_executed)).to eq(described_class::UntilExecuted) + end + it 'returns the right class for `none`' do expect(described_class.for(:none)).to eq(described_class::None) end |