From 7787ffc069424a4d685a6f36afc176bfebc359ad Mon Sep 17 00:00:00 2001 From: Mikael Henriksson Date: Sun, 24 Nov 2013 11:36:27 +0100 Subject: [PATCH] Add mock_redis when testing in fake mode close #24 close #25 close #18 --- .gitignore | 1 + .rspec | 3 + Gemfile | 10 +- Rakefile | 12 +-- .../middleware/client/unique_jobs.rb | 95 ++++++++++++------- lib/sidekiq-unique-jobs/testing.rb | 6 ++ sidekiq-unique-jobs.gemspec | 7 +- .../test_client.rb => spec/lib/client_spec.rb | 55 ++++++----- spec/lib/sidekiq_testing_enabled_spec.rb | 34 +++++++ .../lib/unlock_order_spec.rb | 8 +- spec/spec_helper.rb | 29 ++++++ spec/support/my_worker.rb | 13 +++ spec/support/sidekiq_meta.rb | 15 +++ spec/support/unique_worker.rb | 13 +++ test/helper.rb | 16 ---- test/lib/sidekiq/another_test_client.rb | 28 ------ 16 files changed, 230 insertions(+), 115 deletions(-) create mode 100644 .rspec create mode 100644 lib/sidekiq-unique-jobs/testing.rb rename test/lib/sidekiq/test_client.rb => spec/lib/client_spec.rb (62%) create mode 100644 spec/lib/sidekiq_testing_enabled_spec.rb rename test/lib/sidekiq/test_unlock_ordering.rb => spec/lib/unlock_order_spec.rb (92%) create mode 100644 spec/spec_helper.rb create mode 100644 spec/support/my_worker.rb create mode 100644 spec/support/sidekiq_meta.rb create mode 100644 spec/support/unique_worker.rb delete mode 100644 test/helper.rb delete mode 100644 test/lib/sidekiq/another_test_client.rb diff --git a/.gitignore b/.gitignore index 4a61edd46..a8320c484 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ Gemfile.lock .ruby-version +.idea/ diff --git a/.rspec b/.rspec new file mode 100644 index 000000000..fa3fde71f --- /dev/null +++ b/.rspec @@ -0,0 +1,3 @@ +--color +--format progress +--order random \ No newline at end of file diff --git a/Gemfile b/Gemfile index 814690c9c..05ed44532 100644 --- a/Gemfile +++ b/Gemfile @@ -1,2 +1,10 @@ source 'http://rubygems.org' -gemspec \ No newline at end of file +gemspec + +group :development do + gem 'sidekiq', github: 'mperham/sidekiq' +end + +gem 'pry' +gem 'pry-stack_explorer' +gem 'pry-debugger' \ No newline at end of file diff --git a/Rakefile b/Rakefile index 8f732b802..5c788e2bc 100644 --- a/Rakefile +++ b/Rakefile @@ -1,11 +1,7 @@ #!/usr/bin/env rake require "bundler/gem_tasks" -require 'rake/testtask' -Rake::TestTask.new(:test) do |test| - test.libs << 'test' - #SO MUCH NOISE - #test.warning = true - test.pattern = 'test/**/test_*.rb' -end +require 'rspec/core/rake_task' -task :default => :test \ No newline at end of file +RSpec::Core::RakeTask.new(:spec) + +task :default => :spec \ No newline at end of file diff --git a/lib/sidekiq-unique-jobs/middleware/client/unique_jobs.rb b/lib/sidekiq-unique-jobs/middleware/client/unique_jobs.rb index 2dd398ca8..d44ddda15 100644 --- a/lib/sidekiq-unique-jobs/middleware/client/unique_jobs.rb +++ b/lib/sidekiq-unique-jobs/middleware/client/unique_jobs.rb @@ -4,49 +4,55 @@ module SidekiqUniqueJobs module Middleware module Client class UniqueJobs - def call(worker_class, item, queue) - - klass = worker_class_constantize(worker_class) + attr_reader :item, :worker_class - enabled = klass.get_sidekiq_options['unique'] || item['unique'] - unique_job_expiration = klass.get_sidekiq_options['unique_job_expiration'] + def call(worker_class, item, queue) + @worker_class = worker_class_constantize(worker_class) + @item = item - if enabled + if unique_enabled? + yield if unique? + else + yield + end + end - payload_hash = SidekiqUniqueJobs::PayloadHelper.get_payload(item['class'], item['queue'], item['args']) + def unique? + if testing_enabled? + unique_for_connection?(SidekiqUniqueJobs.redis_mock) + else + Sidekiq.redis do |conn| + unique_for_connection?(conn) + end + end + end - unique = false + def unique_for_connection?(conn) + unique = false + conn.watch(payload_hash) - Sidekiq.redis do |conn| + if conn.get(payload_hash).to_i == 1 || + (conn.get(payload_hash).to_i == 2 && item['at']) + # if the job is already queued, or is already scheduled and + # we're trying to schedule again, abort + conn.unwatch + else + # if the job was previously scheduled and is now being queued, + # or we've never seen it before + expires_at = unique_job_expiration || SidekiqUniqueJobs::Config.default_expiration + expires_at = ((Time.at(item['at']) - Time.now.utc) + expires_at).to_i if item['at'] - conn.watch(payload_hash) - - if conn.get(payload_hash).to_i == 1 || - (conn.get(payload_hash).to_i == 2 && item['at']) - # if the job is already queued, or is already scheduled and - # we're trying to schedule again, abort - conn.unwatch - else - # if the job was previously scheduled and is now being queued, - # or we've never seen it before - expires_at = unique_job_expiration || SidekiqUniqueJobs::Config.default_expiration - expires_at = ((Time.at(item['at']) - Time.now.utc) + expires_at).to_i if item['at'] - - unique = conn.multi do - # set value of 2 for scheduled jobs, 1 for queued jobs. - conn.setex(payload_hash, expires_at, item['at'] ? 2 : 1) - end - end + unique = conn.multi do + # set value of 2 for scheduled jobs, 1 for queued jobs. + conn.setex(payload_hash, expires_at, item['at'] ? 2 : 1) end - yield if unique - else - yield end + unique end protected - # Attempt to constantize a string worker_class argument, always + # Attempt to constantize a string worker_class argument, always # failing back to the original argument. def worker_class_constantize(worker_class) if worker_class.is_a?(String) @@ -56,7 +62,32 @@ def worker_class_constantize(worker_class) end end + private + + def payload_hash + SidekiqUniqueJobs::PayloadHelper.get_payload(item['class'], item['queue'], item['args']) + end + + # When sidekiq/testing is loaded, the Sidekiq::Testing constant is + # present and testing is enabled. + def testing_enabled? + if Sidekiq.const_defined?('Testing') && Sidekiq::Testing.enabled? + require 'sidekiq-unique-jobs/testing' + return true + end + + false + end + + def unique_enabled? + worker_class.get_sidekiq_options['unique'] || item['unique'] + end + + def unique_job_expiration + worker_class.get_sidekiq_options['unique_job_expiration'] + end + end end end -end +end \ No newline at end of file diff --git a/lib/sidekiq-unique-jobs/testing.rb b/lib/sidekiq-unique-jobs/testing.rb new file mode 100644 index 000000000..55f40c49c --- /dev/null +++ b/lib/sidekiq-unique-jobs/testing.rb @@ -0,0 +1,6 @@ +require 'mock_redis' +module SidekiqUniqueJobs + def self.redis_mock + @redis_mock ||= MockRedis.new + end +end \ No newline at end of file diff --git a/sidekiq-unique-jobs.gemspec b/sidekiq-unique-jobs.gemspec index e58a18a7e..e8aa03fc5 100644 --- a/sidekiq-unique-jobs.gemspec +++ b/sidekiq-unique-jobs.gemspec @@ -15,10 +15,9 @@ Gem::Specification.new do |gem| gem.require_paths = ["lib"] gem.version = SidekiqUniqueJobs::VERSION gem.add_dependency 'sidekiq', '~> 2.6' - gem.add_development_dependency 'minitest', '~> 3' - gem.add_development_dependency 'sinatra' - gem.add_development_dependency 'slim' - gem.add_development_dependency 'rake' + gem.add_dependency 'mock_redis' + gem.add_development_dependency 'rspec', '>= 2' + gem.add_development_dependency 'rspec-sidekiq' gem.add_development_dependency 'activesupport', '~> 3' gem.add_development_dependency 'simplecov' end \ No newline at end of file diff --git a/test/lib/sidekiq/test_client.rb b/spec/lib/client_spec.rb similarity index 62% rename from test/lib/sidekiq/test_client.rb rename to spec/lib/client_spec.rb index ade8b9c3a..37880394d 100644 --- a/test/lib/sidekiq/test_client.rb +++ b/spec/lib/client_spec.rb @@ -1,11 +1,11 @@ -require 'helper' +require 'spec_helper' require 'celluloid' require 'sidekiq/worker' require "sidekiq-unique-jobs" require 'sidekiq/scheduled' require 'sidekiq-unique-jobs/middleware/server/unique_jobs' -class TestClient < MiniTest::Unit::TestCase +describe "Client" do describe 'with real redis' do before do Sidekiq.redis = REDIS @@ -27,19 +27,22 @@ def run(x) it 'does not push duplicate messages when configured for unique only' do QueueWorker.sidekiq_options :unique => true - 10.times { Sidekiq::Client.push('class' => TestClient::QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) } - assert_equal 1, Sidekiq.redis {|c| c.llen("queue:customqueue") } + 10.times { Sidekiq::Client.push('class' => QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) } + result = Sidekiq.redis {|c| c.llen("queue:customqueue") } + expect(result).to eq 1 end it 'does not queue duplicates when when calling delay' do 10.times { PlainClass.delay(unique: true, queue: 'customqueue').run(1) } - assert_equal 1, Sidekiq.redis {|c| c.llen("queue:customqueue") } + result = Sidekiq.redis {|c| c.llen("queue:customqueue") } + expect(result).to eq 1 end it 'does not schedule duplicates when calling perform_in' do QueueWorker.sidekiq_options :unique => true 10.times { QueueWorker.perform_in(60, [1, 2]) } - assert_equal 1, Sidekiq.redis { |c| c.zcount("schedule", -1, Time.now.to_f + 2 * 60) } + result = Sidekiq.redis { |c| c.zcount("schedule", -1, Time.now.to_f + 2 * 60) } + expect(result).to eq 1 end it 'enqueues previously scheduled job' do @@ -47,26 +50,31 @@ def run(x) QueueWorker.perform_in(60 * 60, 1, 2) # time passes and the job is pulled off the schedule: - Sidekiq::Client.push('class' => TestClient::QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) + Sidekiq::Client.push('class' => QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) - assert_equal 1, Sidekiq.redis {|c| c.llen("queue:customqueue") } + result = Sidekiq.redis {|c| c.llen("queue:customqueue") } + expect(result).to eq 1 end it 'sets an expiration when provided by sidekiq options' do one_hour_expiration = 60 * 60 QueueWorker.sidekiq_options :unique => true, :unique_job_expiration => one_hour_expiration - Sidekiq::Client.push('class' => TestClient::QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) + Sidekiq::Client.push('class' => QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) - payload_hash = SidekiqUniqueJobs::PayloadHelper.get_payload("TestClient::QueueWorker", "customqueue", [1, 2]) + payload_hash = SidekiqUniqueJobs::PayloadHelper.get_payload("QueueWorker", "customqueue", [1, 2]) actual_expires_at = Sidekiq.redis {|c| c.ttl(payload_hash) } - assert_in_delta one_hour_expiration, actual_expires_at, 2 + result = Sidekiq.redis {|c| c.llen("queue:customqueue") } + expect(actual_expires_at).to be_within(2).of(one_hour_expiration) end it 'does push duplicate messages when not configured for unique only' do QueueWorker.sidekiq_options :unique => false - 10.times { Sidekiq::Client.push('class' => TestClient::QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) } - assert_equal 10, Sidekiq.redis {|c| c.llen("queue:customqueue") } + 10.times { Sidekiq::Client.push('class' => QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) } + expect(Sidekiq.redis {|c| c.llen("queue:customqueue") }).to eq 10 + + result = Sidekiq.redis {|c| c.llen("queue:customqueue") } + expect(result).to eq 10 end describe 'when unique_args is defined' do @@ -87,22 +95,25 @@ class QueueWorkerWithFilterProc < QueueWorker end it 'does not push duplicate messages based on args filter method' do - assert TestClient::QueueWorkerWithFilterMethod.respond_to?(:args_filter) - assert_equal :args_filter, TestClient::QueueWorkerWithFilterMethod.get_sidekiq_options['unique_args'] + expect(QueueWorkerWithFilterMethod).to respond_to(:args_filter) + expect(QueueWorkerWithFilterMethod.get_sidekiq_options['unique_args']).to eq :args_filter + for i in (0..10).to_a - Sidekiq::Client.push('class' => TestClient::QueueWorkerWithFilterMethod, 'queue' => 'customqueue', 'args' => [1, i]) + Sidekiq::Client.push('class' => QueueWorkerWithFilterMethod, 'queue' => 'customqueue', 'args' => [1, i]) end - assert_equal 1, Sidekiq.redis {|c| c.llen("queue:customqueue") } + result = Sidekiq.redis {|c| c.llen("queue:customqueue") } + expect(result).to eq 1 end it 'does not push duplicate messages based on args filter proc' do - assert_kind_of Proc, TestClient::QueueWorkerWithFilterProc.get_sidekiq_options['unique_args'] + expect(QueueWorkerWithFilterProc.get_sidekiq_options['unique_args']).to be_a(Proc) 10.times do - Sidekiq::Client.push('class' => TestClient::QueueWorkerWithFilterProc, 'queue' => 'customqueue', 'args' => [ 1, {:random => rand(), :name => "foobar"} ]) + Sidekiq::Client.push('class' => QueueWorkerWithFilterProc, 'queue' => 'customqueue', 'args' => [ 1, {:random => rand(), :name => "foobar"} ]) end - assert_equal 1, Sidekiq.redis {|c| c.llen("queue:customqueue") } + result = Sidekiq.redis {|c| c.llen("queue:customqueue") } + expect(result).to eq 1 end end @@ -116,12 +127,12 @@ class QueueWorkerWithFilterProc < QueueWorker expected_expires_at = (Time.at(at) - Time.now.utc) + SidekiqUniqueJobs::Config.default_expiration QueueWorker.perform_in(at, 'mike') - payload_hash = SidekiqUniqueJobs::PayloadHelper.get_payload("TestClient::QueueWorker", "customqueue", ['mike']) + payload_hash = SidekiqUniqueJobs::PayloadHelper.get_payload("QueueWorker", "customqueue", ['mike']) # deconstruct this into a time format we can use to get a decent delta for actual_expires_at = Sidekiq.redis {|c| c.ttl(payload_hash) } - assert_in_delta expected_expires_at, actual_expires_at, 2 + expect(actual_expires_at).to be_within(2).of(expected_expires_at) end end end diff --git a/spec/lib/sidekiq_testing_enabled_spec.rb b/spec/lib/sidekiq_testing_enabled_spec.rb new file mode 100644 index 000000000..1f7711f66 --- /dev/null +++ b/spec/lib/sidekiq_testing_enabled_spec.rb @@ -0,0 +1,34 @@ +require 'spec_helper' +require 'sidekiq/worker' +require "sidekiq-unique-jobs" +require 'sidekiq/scheduled' +require 'sidekiq-unique-jobs/middleware/server/unique_jobs' + +describe "When Sidekiq::Testing is enabled" do + describe 'when set to :fake!', sidekiq: :fake do + context "with unique worker" do + it "does not push duplicate messages" do + param = 'work' + expect(UniqueWorker).to have_enqueued_jobs(0) + UniqueWorker.perform_async(param) + expect(UniqueWorker).to have_enqueued_jobs(1) + expect(UniqueWorker).to have_enqueued_job(param) + UniqueWorker.perform_async(param) + expect(UniqueWorker).to have_enqueued_jobs(1) + end + end + + context "with non-unique worker" do + + it "pushes duplicates messages" do + param = 'work' + expect(MyWorker).to have_enqueued_jobs(0) + MyWorker.perform_async(param) + expect(MyWorker).to have_enqueued_jobs(1) + expect(MyWorker).to have_enqueued_job(param) + MyWorker.perform_async(param) + expect(MyWorker).to have_enqueued_jobs(2) + end + end + end +end diff --git a/test/lib/sidekiq/test_unlock_ordering.rb b/spec/lib/unlock_order_spec.rb similarity index 92% rename from test/lib/sidekiq/test_unlock_ordering.rb rename to spec/lib/unlock_order_spec.rb index 1f29e1a50..a39cc1199 100644 --- a/test/lib/sidekiq/test_unlock_ordering.rb +++ b/spec/lib/unlock_order_spec.rb @@ -1,8 +1,8 @@ -require 'helper' +require 'spec_helper' require 'sidekiq/worker' require 'sidekiq-unique-jobs/middleware/server/unique_jobs' -class TestUnlockOrdering < MiniTest::Unit::TestCase +describe "Unlock order" do QUEUE = 'unlock_ordering' class BeforeYieldOrderingWorker @@ -41,7 +41,7 @@ def perform end end - assert_nil result + expect(result).to eq nil end end @@ -56,7 +56,7 @@ def perform end end - assert_equal '1', result + expect(result).to eq '1' end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb new file mode 100644 index 000000000..115c47497 --- /dev/null +++ b/spec/spec_helper.rb @@ -0,0 +1,29 @@ +$TESTING = true + +begin + require 'pry' +rescue LoadError +end + +require 'rspec/autorun' +require 'rspec' + +require 'celluloid/test' +require 'sidekiq' +require 'sidekiq/util' +require 'sidekiq-unique-jobs' +Sidekiq.logger.level = Logger::ERROR + +require 'sidekiq/testing' +require 'rspec-sidekiq' + +Sidekiq::Testing.disable! + +require 'sidekiq/redis_connection' +redis_url = ENV['REDIS_URL'] || 'redis://localhost/15' +REDIS = Sidekiq::RedisConnection.create(:url => redis_url, :namespace => 'testy') + +Dir[File.join(File.dirname(__FILE__), 'support', '**', '*.rb')].each { |f| require f } +RSpec.configure do |config| + config.treat_symbols_as_metadata_keys_with_true_values = true +end \ No newline at end of file diff --git a/spec/support/my_worker.rb b/spec/support/my_worker.rb new file mode 100644 index 000000000..6a6c2c3d3 --- /dev/null +++ b/spec/support/my_worker.rb @@ -0,0 +1,13 @@ +class MyWorker + include Sidekiq::Worker + sidekiq_options :queue => :working, :retry => 1, :backtrace => 10 + sidekiq_options :unique => false + + sidekiq_retries_exhausted do |msg| + Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + end + + def perform(param) + puts param + end +end \ No newline at end of file diff --git a/spec/support/sidekiq_meta.rb b/spec/support/sidekiq_meta.rb new file mode 100644 index 000000000..8d5ee5f8a --- /dev/null +++ b/spec/support/sidekiq_meta.rb @@ -0,0 +1,15 @@ +RSpec.configure do |config| + config.before(:each) do # |example| TODO: Add this for RSpec 3 + sidekiq = example.metadata[:sidekiq] + if sidekiq + sidekiq = :fake if sidekiq == true + Sidekiq::Testing.send("#{sidekiq}!") + end + end + + config.after(:each) do # |example| TODO: Add this for RSpec 3 + if sidekiq = example.metadata[:sidekiq] + Sidekiq::Testing.disable! + end + end +end \ No newline at end of file diff --git a/spec/support/unique_worker.rb b/spec/support/unique_worker.rb new file mode 100644 index 000000000..5e794d9df --- /dev/null +++ b/spec/support/unique_worker.rb @@ -0,0 +1,13 @@ +class UniqueWorker + include Sidekiq::Worker + sidekiq_options :queue => :working, :retry => 1, :backtrace => 10 + sidekiq_options :unique => true + + sidekiq_retries_exhausted do |msg| + Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}" + end + + def perform(param) + puts param + end +end \ No newline at end of file diff --git a/test/helper.rb b/test/helper.rb deleted file mode 100644 index 8ff6bfcce..000000000 --- a/test/helper.rb +++ /dev/null @@ -1,16 +0,0 @@ -ENV['RACK_ENV'] = ENV['RAILS_ENV'] = 'test' -if ENV.has_key?("SIMPLECOV") - require 'simplecov' - SimpleCov.start -end -require 'minitest/unit' -require 'minitest/pride' -require 'minitest/autorun' - -require 'sidekiq-unique-jobs' -require "sidekiq" -require 'sidekiq/util' -Sidekiq.logger.level = Logger::ERROR - -require 'sidekiq/redis_connection' -REDIS = Sidekiq::RedisConnection.create(:url => "redis://localhost/15", :namespace => 'testy') \ No newline at end of file diff --git a/test/lib/sidekiq/another_test_client.rb b/test/lib/sidekiq/another_test_client.rb deleted file mode 100644 index 3bb4dd742..000000000 --- a/test/lib/sidekiq/another_test_client.rb +++ /dev/null @@ -1,28 +0,0 @@ -require 'helper' -require 'sidekiq/worker' -require "sidekiq-unique-jobs" -require 'sidekiq/scheduled' -require 'sidekiq-unique-jobs/middleware/server/unique_jobs' - -class AnotherTestClient < MiniTest::Unit::TestCase - describe 'with real redis' do - before do - Sidekiq.redis = REDIS - Sidekiq.redis {|c| c.flushdb } - QueueWorker.sidekiq_options :unique => nil, :unique_job_expiration => nil - end - - class QueueWorker - include Sidekiq::Worker - sidekiq_options :queue => 'customqueue' - def perform(x) - end - end - - it 'does not push duplicate messages when configured for unique only' do - QueueWorker.sidekiq_options :unique => true - 10.times { Sidekiq::Client.push('class' => TestClient::QueueWorker, 'queue' => 'customqueue', 'args' => [1, 2]) } - assert_equal 1, Sidekiq.redis {|c| c.llen("queue:customqueue") } - end - end -end