diff --git a/.travis.yml b/.travis.yml index 59c7185a6..daa7ad98e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,7 +8,7 @@ cache: bundler services: - redis-server script: - - if [[ "${STYLE}" = "true" ]]; then bundle exec rubocop; fi; + - if [[ "${STYLE}" = "true" ]]; then bundle exec rubocop - P; fi; - bundle exec rspec spec - if [[ "${STYLE}" = "true" ]]; then CODECLIMATE_REPO_TOKEN=88e524e8f638efe690def7a6e2c72b1a9db5cdfa74548921b734d609a5858ee5 bundle exec codeclimate-test-reporter; fi; rvm: diff --git a/CHANGELOG.md b/CHANGELOG.md index 70bee6d07..f99cd08d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +## v5.0.10 + +- Cleans up test setup and make tests more readable +- Allow raising all errors +- Log previously hidden errors +- Revert the changes of v5.0.5 (8a4b7648b8b0ee5d7dc1f5f5a036f41d52ad9a42) +- Allow name errors in unique args method to be raised + ## v5.0.9 - [#229](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/#229) Use HSCAN for expiring keys - [#232](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/#232) Fix testing helper diff --git a/lib/sidekiq-unique-jobs.rb b/lib/sidekiq-unique-jobs.rb index 212e19d14..e61026148 100644 --- a/lib/sidekiq-unique-jobs.rb +++ b/lib/sidekiq-unique-jobs.rb @@ -29,6 +29,7 @@ def config default_run_lock_expiration: 60, default_lock: :while_executing, redis_test_mode: :redis, # :mock + raise_unique_args_errors: false, ) end @@ -55,12 +56,19 @@ def namespace end # Attempt to constantize a string worker_class argument, always - # failing back to the original argument. + # failing back to the original argument when the constant can't be found + # + # raises an error for other errors def worker_class_constantize(worker_class) return worker_class unless worker_class.is_a?(String) Object.const_get(worker_class) - rescue NameError - worker_class + rescue NameError => ex + case ex.message + when /uninitialized constant/ + worker_class + else + raise + end end def mocked? diff --git a/lib/sidekiq_unique_jobs/cli.rb b/lib/sidekiq_unique_jobs/cli.rb index e226efab3..2223d8621 100644 --- a/lib/sidekiq_unique_jobs/cli.rb +++ b/lib/sidekiq_unique_jobs/cli.rb @@ -44,18 +44,18 @@ def console console_class.start end - private - - def logger - SidekiqUniqueJobs.logger - end - - def console_class - require 'pry' - Pry - rescue LoadError - require 'irb' - IRB + no_commands do + def logger + SidekiqUniqueJobs.logger + end + + def console_class + require 'pry' + Pry + rescue LoadError + require 'irb' + IRB + end end end end diff --git a/lib/sidekiq_unique_jobs/lock/while_executing.rb b/lib/sidekiq_unique_jobs/lock/while_executing.rb index 2fac30cd5..0e25733b4 100644 --- a/lib/sidekiq_unique_jobs/lock/while_executing.rb +++ b/lib/sidekiq_unique_jobs/lock/while_executing.rb @@ -3,8 +3,6 @@ module SidekiqUniqueJobs module Lock class WhileExecuting - MUTEX = Mutex.new - def self.synchronize(item, redis_pool = nil) new(item, redis_pool).synchronize { yield } end @@ -13,19 +11,19 @@ def initialize(item, redis_pool = nil) @item = item @redis_pool = redis_pool @unique_digest = "#{create_digest}:run" + @mutex = Mutex.new end def synchronize - MUTEX.lock - sleep 0.1 until locked? - - yield + @mutex.synchronize do + sleep 0.1 until locked? + yield + end rescue Sidekiq::Shutdown logger.fatal { "the unique_key: #{@unique_digest} needs to be unlocked manually" } raise ensure SidekiqUniqueJobs.connection(@redis_pool) { |conn| conn.del @unique_digest } - MUTEX.unlock end def locked? diff --git a/lib/sidekiq_unique_jobs/unique_args.rb b/lib/sidekiq_unique_jobs/unique_args.rb index 8bd6b0f11..235196d22 100644 --- a/lib/sidekiq_unique_jobs/unique_args.rb +++ b/lib/sidekiq_unique_jobs/unique_args.rb @@ -9,7 +9,6 @@ module SidekiqUniqueJobs class UniqueArgs CLASS_NAME = 'SidekiqUniqueJobs::UniqueArgs' extend Forwardable - include Normalizer def_delegators :SidekiqUniqueJobs, :config, :worker_class_constantize def_delegators :Sidekiq, :logger @@ -62,9 +61,13 @@ def unique_args(args) logger.debug { "#{__method__} : unique arguments disabled" } args end - rescue NameError - # fallback to not filtering args when class can't be instantiated - return args + rescue NameError => ex + logger.error "#{__method__}(#{args}) : failed with (#{ex.message})" + logger.error ex + + raise if config.raise_unique_args_errors + + args end def unique_on_all_queues? @@ -114,6 +117,7 @@ def filter_by_proc(args) logger.warn { "#{__method__} : unique_args_method is nil. Returning (#{args})" } return args end + filter_args = unique_args_method.call(args) logger.debug { "#{__method__} : #{args} -> #{filter_args}" } filter_args diff --git a/lib/sidekiq_unique_jobs/util.rb b/lib/sidekiq_unique_jobs/util.rb index 88a7118cc..31e484d83 100644 --- a/lib/sidekiq_unique_jobs/util.rb +++ b/lib/sidekiq_unique_jobs/util.rb @@ -1,14 +1,16 @@ # frozen_string_literal: true module SidekiqUniqueJobs - module Util - SCAN_PATTERN ||= '*' - DEFAULT_COUNT ||= 1_000 - KEYS_METHOD ||= 'keys' - SCAN_METHOD ||= 'scan' - EXPIRE_BATCH_SIZE ||= 100 + module Util # rubocop:disable Metrics/ModuleLength + COUNT = 'COUNT' + DEFAULT_COUNT = 1_000 + EXPIRE_BATCH_SIZE = 100 + MATCH = 'MATCH' + KEYS_METHOD = 'keys' + SCAN_METHOD = 'scan' + SCAN_PATTERN = '*' - module_function + extend self # rubocop:disable Style/ModuleFunction def keys(pattern = SCAN_PATTERN, count = DEFAULT_COUNT) send("keys_by_#{redis_keys_method}", pattern, count) @@ -26,7 +28,7 @@ def del(pattern = SCAN_PATTERN, count = 0, dry_run = true) keys, time = timed { keys(pattern, count) } logger.debug { "#{keys.size} matching keys found in #{time} sec." } keys = dry_run(keys) - logger.debug { "#{keys.size} matching keys after postprocessing" } + logger.debug { "#{keys.size} matching keys after post-processing" } unless dry_run logger.debug { "deleting #{keys}..." } _, time = timed { batch_delete(keys) } @@ -41,23 +43,33 @@ def unique_hash end end - def expire + def expire # rubocop:disable Metrics/MethodLength removed_keys = {} connection do |conn| cursor = '0' - cursor, jobs = conn.hscan(SidekiqUniqueJobs::HASH_KEY, [cursor, 'MATCH', '*', 'COUNT', EXPIRE_BATCH_SIZE]) - jobs.each do |job_array| - jid, unique_key = job_array - next if conn.get(unique_key) - conn.hdel(SidekiqUniqueJobs::HASH_KEY, jid) - removed_keys[jid] = unique_key - end + loop do + cursor, jobs = get_jobs(conn, cursor) + jobs.each do |job_array| + jid, unique_key = job_array + + next if conn.get(unique_key) + conn.hdel(SidekiqUniqueJobs::HASH_KEY, jid) + removed_keys[jid] = unique_key + end - break if cursor == '0' + break if cursor == '0' + end end + removed_keys end + private + + def get_jobs(conn, cursor) + conn.hscan(SidekiqUniqueJobs::HASH_KEY, [cursor, MATCH, SCAN_PATTERN, COUNT, EXPIRE_BATCH_SIZE]) + end + def keys_by_scan(pattern, count) connection { |conn| conn.scan_each(match: prefix(pattern), count: count).to_a } end diff --git a/spec/jobs/custom_queue_job_with_filter_method.rb b/spec/jobs/custom_queue_job_with_filter_method.rb index 77455897c..86f1bd391 100644 --- a/spec/jobs/custom_queue_job_with_filter_method.rb +++ b/spec/jobs/custom_queue_job_with_filter_method.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative 'custom_queue_job' + class CustomQueueJobWithFilterMethod < CustomQueueJob sidekiq_options unique: :until_executed, unique_args: :args_filter diff --git a/spec/lib/sidekiq_unique_jobs/cli_spec.rb b/spec/lib/sidekiq_unique_jobs/cli_spec.rb index 0a8062a91..ae8594682 100644 --- a/spec/lib/sidekiq_unique_jobs/cli_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/cli_spec.rb @@ -176,9 +176,7 @@ specify do expect(Object).to receive(:include).with(SidekiqUniqueJobs::Util).and_return(true) - console = double(:console) - allow_any_instance_of(SidekiqUniqueJobs::Cli).to receive(:console_class).and_return(console) - allow(console).to receive(:start) + allow(Pry).to receive(:start).and_return(true) expect(output).to eq(expected) end end diff --git a/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb b/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb index 8b98366fd..745b307f7 100644 --- a/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/client/middleware_spec.rb @@ -29,6 +29,7 @@ def digest_for(item) end Sidekiq::Simulator.process_queue(:notify_worker) do + sleep 1 Sidekiq.redis do |conn| wait(10).for { conn.llen('queue:notify_worker') }.to eq(0) end @@ -48,6 +49,7 @@ def digest_for(item) end Sidekiq::Simulator.process_queue(:not_default) do + sleep(1) Sidekiq.redis do |conn| expect(conn.llen('queue:default')).to eq(1) wait(10).for { conn.llen('queue:not_default') }.to eq(0) @@ -60,6 +62,7 @@ def digest_for(item) end Sidekiq::Simulator.process_queue(:default) do + sleep(1) Sidekiq.redis do |conn| expect(conn.llen('queue:not_default')).to eq(0) wait(10).for { conn.llen('queue:default') }.to eq(0) diff --git a/spec/lib/sidekiq_unique_jobs/run_lock_timeout_calculator_spec.rb b/spec/lib/sidekiq_unique_jobs/run_lock_timeout_calculator_spec.rb index 0180d38c5..3574f99b2 100644 --- a/spec/lib/sidekiq_unique_jobs/run_lock_timeout_calculator_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/run_lock_timeout_calculator_spec.rb @@ -3,36 +3,42 @@ require 'spec_helper' RSpec.describe SidekiqUniqueJobs::RunLockTimeoutCalculator do - shared_context 'generic unscheduled job' do - subject { described_class.new('class' => 'JustAWorker') } - end + subject { calculator } - describe 'public api' do - it_behaves_like 'generic unscheduled job' do - it { is_expected.to respond_to(:seconds) } - end - end + let(:calculator) { described_class.new(args) } + let(:args) { { 'class' => 'JustAWorker' } } + + it { is_expected.to respond_to(:seconds) } describe '.for_item' do + subject { described_class.for_item('WAT') } + it 'initializes a new calculator' do expect(described_class).to receive(:new).with('WAT') - described_class.for_item('WAT') + subject end end describe '#seconds' do - context 'using default run_lock_expiration' do - subject { described_class.new(nil) } - before { allow(subject).to receive(:worker_class_run_lock_expiration).and_return(9) } + subject { calculator.seconds } + + before do + allow(calculator).to receive(:worker_class_run_lock_expiration) + .and_return(expiration) + end + + context 'when worker_class_run_lock_expiration is configured' do + let(:args) { nil } + let(:expiration) { 9 } - its(:seconds) { is_expected.to eq(9) } + it { is_expected.to eq(9) } end - context 'using specified sidekiq option run_lock_expiration' do - subject { described_class.new(nil) } - before { allow(subject).to receive(:worker_class_run_lock_expiration).and_return(nil) } + context 'when worker_class_run_lock_expiration is not configured' do + let(:args) { nil } + let(:expiration) { nil } - its(:seconds) { is_expected.to eq(60) } + it { is_expected.to eq(60) } end end end diff --git a/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb b/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb index e49c7fe0d..79ad191e1 100644 --- a/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/script_mock_spec.rb @@ -10,11 +10,6 @@ end RSpec.describe SidekiqUniqueJobs::ScriptMock, ruby_ver: '>= 2.4.1' do - MD5_DIGEST ||= 'unique' - UNIQUE_KEY ||= 'uniquejobs:unique' - JID ||= 'fuckit' - ANOTHER_JID ||= 'anotherjid' - before do SidekiqUniqueJobs.configure do |config| config.redis_test_mode = :mock @@ -39,52 +34,76 @@ end end - subject { SidekiqUniqueJobs::Scripts } - - def lock_for(seconds = 1, jid = JID, key = UNIQUE_KEY) - subject.call(:acquire_lock, nil, keys: [key], argv: [jid, seconds]) + shared_context 'shared SciptMock setup' do + let(:redis_pool) { nil } + let(:unique_key) { 'uniquejobs:unique' } + let(:jid) { 'fuckit' } + let(:another_jid) { 'anotherjid' } + let(:seconds) { 1 } end - def unlock(key = UNIQUE_KEY, jid = JID) - subject.call(:release_lock, nil, keys: [key], argv: [jid]) + def acquire_lock(custom_jid = nil) + described_class.acquire_lock( + redis_pool, + keys: [unique_key], + argv: [custom_jid || jid, seconds], + ) end describe '.acquire_lock' do + subject { acquire_lock } + + include_context 'shared SciptMock setup' + context 'when job is unique' do - specify { expect(lock_for).to eq(1) } - specify do - expect(lock_for(1)).to eq(1) + it do + expect(acquire_lock).to eq(1) expect(SidekiqUniqueJobs) - .to have_key(UNIQUE_KEY) + .to have_key(unique_key) .for_seconds(1) .with_value('fuckit') sleep 1 - expect(lock_for).to eq(1) + expect(acquire_lock).to eq(1) end + end - context 'when job is locked' do - before { expect(lock_for(10)).to eq(1) } - specify { expect(lock_for(5, ANOTHER_JID)).to eq(0) } - end + context 'when job is not unique' do + let(:seconds) { 10 } + before { expect(acquire_lock(another_jid)).to eq(1) } + + it { is_expected.to eq(0) } end + end - describe '.release_lock' do - context 'when job is locked by another jid' do - before { expect(lock_for(10, ANOTHER_JID)).to eq(1) } - specify { expect(unlock).to eq(0) } - after { unlock(UNIQUE_KEY, ANOTHER_JID) } - end + def release_lock(custom_jid = nil) + described_class.release_lock( + redis_pool, + keys: [unique_key], + argv: [custom_jid || jid, seconds], + ) + end - context 'when job is not locked at all' do - specify { expect(unlock).to eq(-1) } - end + describe '.release_lock' do + subject { release_lock } - context 'when job is locked by the same jid' do - specify do - expect(lock_for(10)).to eq(1) - expect(unlock).to eq(1) - end - end + include_context 'shared SciptMock setup' + + context 'when job is locked by another jid' do + before { expect(acquire_lock(another_jid)).to eq(1) } + after { expect(release_lock(another_jid)).to eq(1) } + + it { is_expected.to eq(0) } + end + + context 'when job is not locked at all' do + specify { expect(release_lock).to eq(-1) } + end + + context 'when job is locked by the same jid' do + let(:seconds) { 10 } + before { expect(acquire_lock).to eq(1) } + + it { is_expected.to eq(1) } end end end diff --git a/spec/lib/sidekiq_unique_jobs/scripts_spec.rb b/spec/lib/sidekiq_unique_jobs/scripts_spec.rb index c72fee2b3..d47f1f09a 100644 --- a/spec/lib/sidekiq_unique_jobs/scripts_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/scripts_spec.rb @@ -2,61 +2,55 @@ require 'spec_helper' RSpec.describe SidekiqUniqueJobs::Scripts do - MD5_DIGEST ||= 'unique' - UNIQUE_KEY ||= 'uniquejobs:unique' - JID ||= 'fuckit' - ANOTHER_JID ||= 'anotherjid' + subject { SidekiqUniqueJobs::Scripts } - context 'class methods' do - subject { SidekiqUniqueJobs::Scripts } + it { is_expected.to respond_to(:call).with(3).arguments } + it { is_expected.to respond_to(:logger) } + it { is_expected.to respond_to(:connection).with(1).arguments } + it { is_expected.to respond_to(:script_source).with(1).arguments } + it { is_expected.to respond_to(:script_path).with(1).arguments } - it { is_expected.to respond_to(:call).with(3).arguments } - it { is_expected.to respond_to(:logger) } - it { is_expected.to respond_to(:connection).with(1).arguments } - it { is_expected.to respond_to(:script_source).with(1).arguments } - it { is_expected.to respond_to(:script_path).with(1).arguments } - - describe '.logger' do - its(:logger) { is_expected.to eq(Sidekiq.logger) } - end - - describe '.call' do - let(:jid) { 'abcefab' } - let(:unique_key) { 'uniquejobs:abcefab' } - let(:max_lock_time) { 1 } - let(:options) { { keys: [unique_key], argv: [jid, max_lock_time] } } - let(:scriptsha) { 'abcdefab' } - let(:script_name) { :acquire_lock } - let(:error_message) { 'Some interesting error' } - - subject { described_class.call(script_name, nil, options) } + describe '.logger' do + subject { described_class.logger } + it { is_expected.to eq(Sidekiq.logger) } + end - context 'when conn.evalsha raises Redis::CommandError' do - before do - call_count = 0 - allow(described_class).to receive(:internal_call).with(script_name, nil, options) do - call_count += 1 - (call_count == 1) ? raise(Redis::CommandError, error_message) : 1 - end + describe '.call' do + subject { described_class.call(script_name, nil, options) } + + let(:jid) { 'abcefab' } + let(:unique_key) { 'uniquejobs:abcefab' } + let(:max_lock_time) { 1 } + let(:options) { { keys: [unique_key], argv: [jid, max_lock_time] } } + let(:scriptsha) { 'abcdefab' } + let(:script_name) { :acquire_lock } + let(:error_message) { 'Some interesting error' } + + context 'when conn.evalsha raises Redis::CommandError' do + before do + call_count = 0 + allow(described_class).to receive(:internal_call).with(script_name, nil, options) do + call_count += 1 + (call_count == 1) ? raise(Redis::CommandError, error_message) : 1 end + end - specify do - expect(described_class::SCRIPT_SHAS).not_to receive(:delete).with(script_name) - expect(described_class).to receive(:internal_call).with(script_name, nil, options).once - expect { subject }.to raise_error( - SidekiqUniqueJobs::ScriptError, - "Problem compiling #{script_name}. Invalid LUA syntax?", - ) - end + specify do + expect(described_class::SCRIPT_SHAS).not_to receive(:delete).with(script_name) + expect(described_class).to receive(:internal_call).with(script_name, nil, options).once + expect { subject }.to raise_error( + SidekiqUniqueJobs::ScriptError, + "Problem compiling #{script_name}. Invalid LUA syntax?", + ) + end - context 'when error message is No matching script' do - let(:error_message) { 'NOSCRIPT No matching script. Please use EVAL.' } + context 'when error message is No matching script' do + let(:error_message) { 'NOSCRIPT No matching script. Please use EVAL.' } - specify do - expect(described_class::SCRIPT_SHAS).to receive(:delete).with(script_name) - expect(described_class).to receive(:internal_call).with(script_name, nil, options).twice - expect { subject }.not_to raise_error - end + specify do + expect(described_class::SCRIPT_SHAS).to receive(:delete).with(script_name) + expect(described_class).to receive(:internal_call).with(script_name, nil, options).twice + expect { subject }.not_to raise_error end end end diff --git a/spec/lib/sidekiq_unique_jobs/timeout_calculator_spec.rb b/spec/lib/sidekiq_unique_jobs/timeout_calculator_spec.rb index 6b93721c7..ffc47cef0 100644 --- a/spec/lib/sidekiq_unique_jobs/timeout_calculator_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/timeout_calculator_spec.rb @@ -3,16 +3,13 @@ require 'spec_helper' RSpec.describe SidekiqUniqueJobs::TimeoutCalculator do - shared_context 'undefined worker class' do - subject { described_class.new('class' => 'test') } - end - - shared_context 'item not scheduled' do - subject { described_class.new('class' => 'MyUniqueJob') } - end + let(:calculator) { described_class.new('class' => worker_class, 'at' => schedule_time) } + let(:worker_class) { 'MyUniqueJob' } + let(:schedule_time) { nil } describe 'public api' do subject { described_class.new(nil) } + it { is_expected.to respond_to(:time_until_scheduled) } it { is_expected.to respond_to(:worker_class_queue_lock_expiration) } it { is_expected.to respond_to(:worker_class_run_lock_expiration) } @@ -28,45 +25,56 @@ end describe '#time_until_scheduled' do - it_behaves_like 'item not scheduled' do - its(:time_until_scheduled) { is_expected.to eq(0) } + subject { calculator.time_until_scheduled } + + context 'when not scheduled' do + it { is_expected.to eq(0) } end - subject { described_class.new('class' => 'MyUniqueJob', 'at' => schedule_time) } - let(:schedule_time) { Time.now.utc.to_i + 24 * 60 * 60 } - let(:now_in_utc) { Time.now.utc.to_i } + context 'when scheduled' do + let(:schedule_time) { Time.now.utc.to_i + 24 * 60 * 60 } + let(:now_in_utc) { Time.now.utc.to_i } - its(:time_until_scheduled) do - Timecop.travel(Time.at(now_in_utc)) do - is_expected.to be_within(1).of(schedule_time - now_in_utc) + it do + Timecop.travel(Time.at(now_in_utc)) do + is_expected.to be_within(1).of(schedule_time - now_in_utc) + end end end end describe '#worker_class_queue_lock_expiration' do - it_behaves_like 'undefined worker class' do - its(:worker_class_queue_lock_expiration) { is_expected.to eq(nil) } - end + subject { calculator.worker_class_queue_lock_expiration } - subject { described_class.new('class' => 'MyUniqueJob') } - its(:worker_class_queue_lock_expiration) { is_expected.to eq(7_200) } + it { is_expected.to eq(7_200) } end describe '#worker_class_run_lock_expiration' do - it_behaves_like 'undefined worker class' do - its(:worker_class_run_lock_expiration) { is_expected.to eq(nil) } - end + subject { calculator.worker_class_run_lock_expiration } + let(:worker_class) { 'LongRunningJob' } - subject { described_class.new('class' => 'LongRunningJob') } - its(:worker_class_run_lock_expiration) { is_expected.to eq(7_200) } + it { is_expected.to eq(7_200) } end describe '#worker_class' do - it_behaves_like 'undefined worker class' do - its(:worker_class) { is_expected.to eq('test') } + subject { calculator.worker_class } + + let(:worker_class) { 'MyUniqueJob' } + + it { is_expected.to eq(MyUniqueJob) } + + context 'when worker class is a constant' do + let(:worker_class) { 'MissingWorker' } + + it { is_expected.to eq('MissingWorker') } end - subject { described_class.new('class' => 'MyJob') } - its(:worker_class) { is_expected.to eq(MyJob) } + context 'when worker class is not a constant' do + let(:worker_class) { 'missing_worker' } + + it do + expect { subject }.to raise_error(NameError, 'wrong constant name missing_worker') + end + end end end diff --git a/spec/lib/sidekiq_unique_jobs/unique_args_spec.rb b/spec/lib/sidekiq_unique_jobs/unique_args_spec.rb index 94fa0a1d7..18986c366 100644 --- a/spec/lib/sidekiq_unique_jobs/unique_args_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/unique_args_spec.rb @@ -4,43 +4,45 @@ RSpec.describe SidekiqUniqueJobs::UniqueArgs do let(:item) { { 'class' => 'UntilExecutedJob', 'queue' => 'myqueue', 'args' => [[1, 2]] } } - subject { described_class.new(item) } + let(:unique_args) { described_class.new(item) } describe '#unique_digest' do + subject { unique_args.unique_digest } + context 'when args are empty' do let(:item) { { 'class' => 'WithoutArgumentJob', 'args' => [] } } - let(:another_subject) { described_class.new(item) } + let(:another_unique_args) { described_class.new(item) } context 'with the same unique args' do it 'equals to unique_digest for that item' do - expect(subject.unique_digest).to eq(another_subject.unique_digest) + expect(subject).to eq(another_unique_args.unique_digest) end end end shared_examples 'unique digest' do - subject { described_class.new(item_options) } context 'given another item' do - let(:another_subject) { described_class.new(another_item) } + let(:another_unique_args) { described_class.new(another_item) } context 'with the same unique args' do - let(:another_item) { item_options.merge('args' => [1, 2, 'type' => 'it']) } + let(:another_item) { item.merge('args' => [1, 2, 'type' => 'it']) } + it 'equals to unique_digest for that item' do - expect(subject.unique_digest).to eq(another_subject.unique_digest) + expect(subject).to eq(another_unique_args.unique_digest) end end context 'with different unique args' do - let(:another_item) { item_options.merge('args' => [1, 3, 'type' => 'that']) } + let(:another_item) { item.merge('args' => [1, 3, 'type' => 'that']) } it 'differs from unique_digest for that item' do - expect(subject.unique_digest).not_to eq(another_subject.unique_digest) + expect(subject).not_to eq(another_unique_args.unique_digest) end end end end context 'when unique_args is a proc' do - let(:item_options) do + let(:item) do { 'class' => 'MyUniqueJobWithFilterProc', 'queue' => 'customqueue', @@ -52,7 +54,7 @@ end context 'when unique_args is a symbol' do - let(:item_options) do + let(:item) do { 'class' => 'MyUniqueJobWithFilterMethod', 'queue' => 'customqueue', @@ -65,101 +67,121 @@ end describe '#digestable_hash' do + subject { unique_args.digestable_hash } + + let(:expected_hash) do + { 'class' => 'UntilExecutedJob', 'queue' => 'myqueue', 'unique_args' => [[1, 2]] } + end + + it { is_expected.to eq(expected_hash) } + with_global_config(unique_args_enabled: true) do with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_on_all_queues: true) do - its(:digestable_hash) do - is_expected.to eq('class' => 'UntilExecutedJob', - 'unique_args' => [[1, 2]]) - end + let(:expected_hash) { { 'class' => 'UntilExecutedJob', 'unique_args' => [[1, 2]] } } + it { is_expected.to eq(expected_hash) } end with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_across_workers: true) do - its(:digestable_hash) do - is_expected.to eq('queue' => 'myqueue', - 'unique_args' => [[1, 2]]) - end - end + let(:expected_hash) { { 'queue' => 'myqueue', 'unique_args' => [[1, 2]] } } - its(:digestable_hash) do - is_expected.to eq('class' => 'UntilExecutedJob', - 'queue' => 'myqueue', - 'unique_args' => [[1, 2]]) + it { is_expected.to eq(expected_hash) } end end end describe '#unique_args_enabled?' do + subject { unique_args.unique_args_enabled? } + with_default_worker_options(unique: :until_executed, unique_args: ->(args) { args[1]['test'] }) do with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args) do - its(:unique_args_enabled?) { is_expected.to eq(:unique_args) } + it { is_expected.to eq(:unique_args) } end with_sidekiq_options_for(UntilExecutedJob, unique_args: false) do - its(:unique_args_enabled?) { is_expected.to be_a(Proc) } + it { is_expected.to be_a(Proc) } + + context 'when raise_unique_args_errors is true' do + before { SidekiqUniqueJobs.config.raise_unique_args_errors = true } + after { SidekiqUniqueJobs.config.raise_unique_args_errors = false } + + it { expect { subject }.to raise_error(NoMethodError, "undefined method `[]' for nil:NilClass") } + end + + context 'when raise_unique_args_errors is false' do + it { is_expected.to be_a(Proc) } + end end end with_default_worker_options(unique: false, unique_args: nil) do with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args) do - its(:unique_args_enabled?) { is_expected.to eq(:unique_args) } + it { is_expected.to eq(:unique_args) } end with_sidekiq_options_for(UntilExecutedJob, unique_args: false) do - its(:unique_args_enabled?) { is_expected.to be_falsy } + it { is_expected.to be_falsy } + end + + with_sidekiq_options_for('MissingWorker', unique_args: true) do + it { is_expected.to be_falsy } end - its(:unique_args_enabled?) { is_expected.to be_falsy } + it { is_expected.to be_falsy } end end describe '#unique_on_all_queues?' do + subject { unique_args.unique_on_all_queues? } + with_global_config(unique_args_enabled: true) do - its(:unique_on_all_queues?) { is_expected.to eq(nil) } + it { is_expected.to eq(nil) } with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_on_all_queues: true) do - its(:unique_on_all_queues?) { is_expected.to eq(true) } + it { is_expected.to eq(true) } end with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_on_all_queues: false) do - its(:unique_on_all_queues?) { is_expected.to be_falsy } + it { is_expected.to be_falsy } end end with_global_config(unique_args_enabled: false) do - its(:unique_on_all_queues?) { is_expected.to eq(nil) } + it { is_expected.to eq(nil) } with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_on_all_queues: false) do - its(:unique_on_all_queues?) { is_expected.to eq(false) } + it { is_expected.to eq(false) } end with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_on_all_queues: true) do - its(:unique_on_all_queues?) { is_expected.to eq(true) } + it { is_expected.to eq(true) } end end end describe '#unique_across_workers?' do + subject { unique_args.unique_across_workers? } + with_global_config(unique_args_enabled: true) do - its(:unique_across_workers?) { is_expected.to eq(nil) } + it { is_expected.to eq(nil) } with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_across_workers: true) do - its(:unique_across_workers?) { is_expected.to eq(true) } + it { is_expected.to eq(true) } end with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_across_workers: false) do - its(:unique_across_workers?) { is_expected.to be_falsy } + it { is_expected.to be_falsy } end end with_global_config(unique_args_enabled: false) do - its(:unique_across_workers?) { is_expected.to eq(nil) } + it { is_expected.to eq(nil) } with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_across_workers: false) do - its(:unique_across_workers?) { is_expected.to eq(false) } + it { is_expected.to eq(false) } end with_sidekiq_options_for(UntilExecutedJob, unique_args: :unique_args, unique_across_workers: true) do - its(:unique_across_workers?) { is_expected.to eq(true) } + it { is_expected.to eq(true) } end end end @@ -202,7 +224,7 @@ end describe '#filter_by_symbol' do - let(:unique_args) { described_class.new(item) } + subject { unique_args.filter_by_symbol(args) } context 'when filter is a working symbol' do let(:item) do @@ -212,13 +234,14 @@ end let(:args) { ['name', 2, 'whatever' => nil, 'type' => 'test'] } - subject { unique_args.filter_by_symbol(args) } it 'returns the value of the provided class method' do expected = %w[name test] + expect(unique_args.logger).to receive(:debug) do |&block| expect(block.call).to eq("filter_by_symbol : filtered_args(#{args}) => #{expected}") end + expect(subject).to eq(expected) end end @@ -231,17 +254,12 @@ end let(:args) { [1] } - subject { unique_args.filter_by_symbol(args) } - - it 'returns the value of the provided class method' do - expect(subject).to eq(args) - end + it { is_expected.to eq(args) } context 'when provided nil' do let(:args) { [] } - it 'returns the value of the provided class method' do - expect(subject).to eq(args) - end + + it { is_expected.to eq(args) } end end @@ -251,18 +269,16 @@ 'queue' => 'myqueue', 'args' => [[1, 2]] } end - let(:args) { ['name', 2, 'whatever' => nil, 'type' => 'test'] } - subject { unique_args.filter_by_symbol(args) } - it 'returns the value of the provided class method' do + before do expect(unique_args.logger) .to receive(:fatal) .with('filter_by_symbol : UniqueJobWithoutUniqueArgsParameter\'s unique_args needs at least one argument') expect(unique_args.logger).to receive(:fatal).with a_kind_of(ArgumentError) - - expect(subject).to eq(args) end + + it { is_expected.to eq(args) } end context 'when @worker_class does not respond_to unique_args_method' do @@ -271,11 +287,9 @@ 'queue' => 'myqueue', 'args' => [[1, 2]] } end - let(:args) { ['name', 2, 'whatever' => nil, 'type' => 'test'] } - subject { unique_args.filter_by_symbol(args) } - it 'returns the value of the provided class method' do + before do expect(unique_args.logger).to receive(:warn) do |&block| expect(block.call) .to eq( @@ -283,9 +297,9 @@ "Returning (#{args})", ) end - - expect(subject).to eq(args) end + + it { is_expected.to eq(args) } end context 'when workers unique_args method returns nil' do @@ -294,17 +308,15 @@ 'queue' => 'myqueue', 'args' => [[1, 2]] } end - let(:args) { ['name', 2, 'whatever' => nil, 'type' => 'test'] } - subject { unique_args.filter_by_symbol(args) } - it 'returns the value of the provided class method' do + before do expect(unique_args.logger).to receive(:debug) do |&block| expect(block.call).to eq("filter_by_symbol : unique_args(#{args}) => ") end - - expect(subject).to eq(nil) end + + it { is_expected.to eq(nil) } end end end diff --git a/spec/lib/sidekiq_unique_jobs/util_spec.rb b/spec/lib/sidekiq_unique_jobs/util_spec.rb index d80369309..43a5657e3 100644 --- a/spec/lib/sidekiq_unique_jobs/util_spec.rb +++ b/spec/lib/sidekiq_unique_jobs/util_spec.rb @@ -59,25 +59,26 @@ def acquire_lock end describe '.prefix' do - before do - allow(SidekiqUniqueJobs.config).to receive(:unique_prefix).and_return('test-uniqueness') - end + subject { described_class.send(:prefix, key) } - it 'returns a prefixed key' do - expect(described_class.prefix('key')).to eq('test-uniqueness:key') - end + let(:key) { 'key' } - context 'when .unique_prefix is nil?' do - it 'does not prefix with unique_prefix' do - allow(SidekiqUniqueJobs.config).to receive(:unique_prefix).and_return(nil) - expect(described_class.prefix('key')).to eq('key') + context 'when prefix is configured' do + before { allow(SidekiqUniqueJobs.config).to receive(:unique_prefix).and_return('test-uniqueness') } + + it { is_expected.to eq('test-uniqueness:key') } + + context 'when key is already prefixed' do + let(:key) { 'test-uniqueness:key' } + + it { is_expected.to eq('test-uniqueness:key') } end end - context 'when key is already prefixed' do - it 'does not add another prefix' do - expect(described_class.prefix('test-uniqueness:key')).to eq('test-uniqueness:key') - end + context 'when .unique_prefix is nil?' do + before { allow(SidekiqUniqueJobs.config).to receive(:unique_prefix).and_return(nil) } + + it { is_expected.to eq('key') } end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index da5219424..e76baea1d 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -39,6 +39,9 @@ Dir[File.join(File.dirname(__FILE__), 'support', '**', '*.rb')].each { |f| require f } RSpec.configure do |config| # rubocop:disable BlockLength + config.define_derived_metadata do |meta| + meta[:aggregate_failures] = true + end config.expect_with :rspec do |expectations| expectations.include_chain_clauses_in_custom_matcher_descriptions = true end diff --git a/spec/support/unique_macros.rb b/spec/support/unique_macros.rb index 5d1a29230..6c013fff2 100644 --- a/spec/support/unique_macros.rb +++ b/spec/support/unique_macros.rb @@ -3,7 +3,6 @@ module SidekiqUniqueJobs module RSpec module InstanceMethods - # enable versioning for specific blocks (at instance-level) def with_global_config(config) was_config = SidekiqUniqueJobs.config SidekiqUniqueJobs.configure(config) @@ -27,6 +26,7 @@ def with_default_worker_options(options) was_options = Sidekiq.default_worker_options Sidekiq.default_worker_options.clear Sidekiq.default_worker_options = options + yield ensure Sidekiq.default_worker_options.clear @@ -44,7 +44,6 @@ def with_sidekiq_options_for(worker_class, options = {}, &block) end end - # enable versioning for specific blocks (at class-level) def with_global_config(config = {}, &block) context "with global configuration #{config}" do around(:each) do |ex| @@ -54,7 +53,6 @@ def with_global_config(config = {}, &block) end end - # enable versioning for specific blocks (at class-level) def with_default_worker_options(config = {}, &block) context "with default sidekiq options #{config}" do around(:each) do |ex|