From f5ae825f85ea621d4feec555e67145b8bc28bc7e Mon Sep 17 00:00:00 2001 From: Dominik Sander Date: Mon, 7 Sep 2026 13:03:42 +0200 Subject: [PATCH 1/3] Update test setup and test related gems to run on ruby 3.4 This makes the tests work on ruby 3.4. No runtime gems have been updated yet. Removed usage of internal Sidekiq API in the test to hopefully make updates simpler. --- .github/workflows/test.yml | 24 ++++++++++++++ .gitignore | 1 + .travis.yml | 7 ---- README.md | 3 ++ Rakefile | 2 ++ sidekiq-repeat.gemspec | 15 +++++---- test/mini_ice_cube.rb | 5 ++- test/repeat.rb | 14 ++++---- test/test_helper.rb | 66 +++++++++++++++++++++++++------------- 9 files changed, 90 insertions(+), 47 deletions(-) create mode 100644 .github/workflows/test.yml delete mode 100644 .travis.yml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..949b86c --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,24 @@ +name: Tests + +on: [push, pull_request] + +jobs: + test: + runs-on: ubuntu-latest + services: + redis: + image: redis:8-alpine + ports: + - 16379:6379 + options: >- + --health-cmd "redis-cli ping" + --health-interval 1s + --health-timeout 5s + --health-retries 10 + steps: + - uses: actions/checkout@v4 + - uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.4' + bundler-cache: true + - run: bundle exec rake diff --git a/.gitignore b/.gitignore index 9b87472..8c6efc7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ Gemfile.lock *.gem gemfiles/ +coverage/ diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index 8b4f32b..0000000 --- a/.travis.yml +++ /dev/null @@ -1,7 +0,0 @@ -language: ruby -services: - - redis-server -rvm: - - 3.0.1 -script: - - bundle exec rake test diff --git a/README.md b/README.md index a7e414f..37663ee 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,9 @@ Check [the code](lib/sidekiq/repeat/mini_ice_cube.rb) for documentation. # setup bundle install +# Start Redis in another terminal (or use an existing instance) +docker run --rm -p 127.0.0.1:16379:6379 redis:8-alpine + # Run the tests bundle exec rake test diff --git a/Rakefile b/Rakefile index db43587..4288f83 100644 --- a/Rakefile +++ b/Rakefile @@ -4,3 +4,5 @@ Rake::TestTask.new(:test) do |test| test.libs << 'test' test.pattern = 'test/**/*.rb' end + +task default: :test diff --git a/sidekiq-repeat.gemspec b/sidekiq-repeat.gemspec index 5c0ce1d..dc6a7fd 100644 --- a/sidekiq-repeat.gemspec +++ b/sidekiq-repeat.gemspec @@ -14,11 +14,14 @@ Gem::Specification.new do |spec| spec.files = Dir['lib/**/*rb'] spec.require_paths = ['lib'] - spec.add_dependency 'sidekiq', '>= 6', '< 7.0' - spec.add_dependency 'parse-cron', '~> 0.1' - spec.add_dependency 'redlock', '~> 1' + spec.add_dependency 'sidekiq', '~> 6.5', '>= 6.5.12' + spec.add_dependency 'parse-cron', '~> 0.1.4' + spec.add_dependency 'redlock', '~> 1.3', '>= 1.3.2' + # Sidekiq 6.5 requires these libraries without declaring them as gems. + spec.add_dependency 'base64', '~> 0.3' + spec.add_dependency 'logger', '~> 1.7' - spec.add_development_dependency 'minitest', '~> 3' - spec.add_development_dependency 'rake', '>= 12.3.3' - spec.add_development_dependency 'redis-namespace', '~> 1.3' + spec.add_development_dependency 'minitest', '~> 5.25' + spec.add_development_dependency 'rake', '~> 13.3' + spec.add_development_dependency 'simplecov', '~> 1.0' end diff --git a/test/mini_ice_cube.rb b/test/mini_ice_cube.rb index 5bdddad..de1039e 100644 --- a/test/mini_ice_cube.rb +++ b/test/mini_ice_cube.rb @@ -1,7 +1,6 @@ -require 'minitest/autorun' -require 'sidekiq/repeat/mini_ice_cube' +require_relative 'test_helper' -class TestMiniIceCube < MiniTest::Unit::TestCase +class TestMiniIceCube < Minitest::Test def setup @dsl = Sidekiq::Repeat::MiniIceCube::MainDsl.new end diff --git a/test/repeat.rb b/test/repeat.rb index 64dbf26..ee13bfd 100644 --- a/test/repeat.rb +++ b/test/repeat.rb @@ -1,8 +1,6 @@ -require 'minitest/autorun' +require_relative 'test_helper' -require_relative './test_helper.rb' - -class TestRescheduling < MiniTest::Unit::TestCase +class TestRescheduling < Minitest::Test include TestHelper.assertions('SidekiqRepeatTestJob') include TestHelper.application_setup @@ -53,7 +51,7 @@ def test_reschedules_job_if_in_the_future end end -class TestArguments < MiniTest::Unit::TestCase +class TestArguments < Minitest::Test include TestHelper.assertions('SidekiqRepeatArgumentsTestJob', true) include TestHelper.application_setup @@ -64,7 +62,7 @@ def test_perform_called_with_parameters end end -class TestRedlockDefaultConfiguration < MiniTest::Unit::TestCase +class TestRedlockDefaultConfiguration < Minitest::Test include TestHelper.assertions('SidekiqRepeatTestJob') include TestHelper.application_setup(false) @@ -73,7 +71,7 @@ def test_startup_scheduling_is_locked end end -class TestRedlockDisabled < MiniTest::Unit::TestCase +class TestRedlockDisabled < Minitest::Test include TestHelper.assertions('SidekiqRepeatTestJob') include TestHelper.application_setup(false) @@ -86,7 +84,7 @@ def test_startup_scheduling_is_not_locked end end -class TestRedlockMultipleRedisInstances < MiniTest::Unit::TestCase +class TestRedlockMultipleRedisInstances < Minitest::Test include TestHelper.assertions('SidekiqRepeatTestJob') include TestHelper.application_setup(false) diff --git a/test/test_helper.rb b/test/test_helper.rb index 95b4217..5bfb612 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,15 +1,29 @@ # This test setup was taken from sidekiq-middleware: # https://github.com/krasnoukhov/sidekiq-middleware/blob/v0.3.0/test/test_unique_jobs.rb +require 'simplecov' +SimpleCov.start do + cover 'lib/**/*.rb' +end + +require 'minitest/autorun' require 'sidekiq' require 'sidekiq/cli' -require 'sidekiq/processor' -require 'sidekiq/redis_connection' +require 'sidekiq/testing' +require 'minitest/mock' + +Sidekiq::Testing.disable! Sidekiq.logger.level = Logger::ERROR -Sidekiq.redis = Sidekiq::RedisConnection.create(:namespace => 'sidekiq-repeat-test') +Sidekiq.redis = { + url: ENV.fetch('TEST_REDIS_URL', 'redis://127.0.0.1:16379') +} require 'sidekiq-repeat' +Sidekiq::Testing.server_middleware do |chain| + chain.add Sidekiq::Repeat::Middleware +end + class SidekiqRepeatTestJob include Sidekiq::Worker include Sidekiq::Repeat::Repeatable @@ -33,12 +47,6 @@ def perform(last, current) end end -UnitOfWork = Struct.new(:queue, :job) do - def acknowledge; end - def queue_name; end - def requeue; end -end - module TestHelper def self.assertions(klass, perform_with_arguments = false) Module.new do @@ -80,22 +88,21 @@ def delete_scheduled! scheduled_jobs.map(&:delete) end + # Enqueues a job in memory with +fake!+, then executes it with +perform_one+, the purpose is to only use public + # Sidekiq API. During execution, testing is disabled, so the middleware schedules the next occurrence in Redis. def perform_scheduled! - msg = Sidekiq.dump_json('class' => klass_name, 'queue' => 'default', 'args' => perform_args) - work = UnitOfWork.new('default', msg) - actor = MiniTest::Mock.new - actor.expect(:processor_done, nil, [@processor]) - 2.times { @boss.expect(:async, actor, []) } - @processor.send(:process, work) + worker = Object.const_get(klass_name) + Sidekiq::Testing.fake! { worker.perform_async(*perform_args) } + worker.perform_one end def expect_redlock!(redis_instances = nil) redis_instances ||= Sidekiq::Repeat::Configuration.instance.redlock_redis_instances - @redlock_client_instance = MiniTest::Mock.new + @redlock_client_instance = Minitest::Mock.new @redlock_client_instance.expect(:lock, nil, ['sidekiq-repeat-reschedule-all', 500]) - @redlock_client_new_method = MiniTest::Mock.new + @redlock_client_new_method = Minitest::Mock.new @redlock_client_new_method.expect(:call, @redlock_client_instance, [redis_instances]) Redlock::Client.stub(:new, @redlock_client_new_method) do @@ -115,29 +122,42 @@ def expect_no_redlock! end module ApplicationSetup + def run + Time.stub(:now, Time.local(2030, 1, 2, 12, 10, 30)) { super } + end + def configure(config) # To be overwritten in test class. end def setup + clear_test_redis + Sidekiq::Repeat::Repeatable.repeatables.each do |klass| + klass.repeat { hourly } + klass.instance_variable_set(:@cronline, nil) + klass.instance_variable_set(:@ss, nil) + end + if startup_sidekiq + Sidekiq::Repeat::Configuration.instance.redlock_redis_instances = [Sidekiq.redis_pool] + end # Allow the test to configure Sidekiq::Repeat. Sidekiq::Repeat.configure { |config| configure(config) } - @boss = MiniTest::Mock.new - 2.times { @boss.expect(:options, {:queues => ['default'] }, []) } - @processor = Sidekiq::Processor.new(@boss, queues: ['default']) startup_sidekiq! if startup_sidekiq end def teardown + clear_test_redis # Reset to defaults for next test case. Sidekiq::Repeat::Configuration.instance.reset_to_default! end def startup_sidekiq! - events = Sidekiq.options[:lifecycle_events][:startup].dup - @processor.fire_event(:startup) - Sidekiq.options[:lifecycle_events][:startup] = events + Sidekiq[:lifecycle_events][:startup].each(&:call) + end + + def clear_test_redis + Sidekiq.redis(&:flushdb) end end end From e91352d84321ab15c89e9314ce7b60ab58bb45d3 Mon Sep 17 00:00:00 2001 From: Dominik Sander Date: Mon, 7 Sep 2026 16:14:44 +0200 Subject: [PATCH 2/3] Fix redlock usage, remove mocks related to redlock `.lock` yields the lock/lock information, if it is false the lock was not acquired. Moving the mocks removes the reliance on internal API of redlock, updating it should surface breakage if they changed the API or behaviour. --- lib/sidekiq/repeat/configuration.rb | 4 +-- test/repeat.rb | 16 +++++++---- test/test_helper.rb | 42 +++++++++++++---------------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/lib/sidekiq/repeat/configuration.rb b/lib/sidekiq/repeat/configuration.rb index 0031e12..1e186e9 100644 --- a/lib/sidekiq/repeat/configuration.rb +++ b/lib/sidekiq/repeat/configuration.rb @@ -19,8 +19,8 @@ def reset_to_default! def self.with_lock if instance.redlock_enabled - Redlock::Client.new(instance.redlock_redis_instances).lock('sidekiq-repeat-reschedule-all', 500) do - yield + Redlock::Client.new(instance.redlock_redis_instances).lock('sidekiq-repeat-reschedule-all', 500) do |lock_info| + yield if lock_info end else yield diff --git a/test/repeat.rb b/test/repeat.rb index ee13bfd..24a2b2d 100644 --- a/test/repeat.rb +++ b/test/repeat.rb @@ -67,7 +67,11 @@ class TestRedlockDefaultConfiguration < Minitest::Test include TestHelper.application_setup(false) def test_startup_scheduling_is_locked - expect_redlock! { startup_sidekiq! } + with_redlock_held { startup_sidekiq! } + assert_not_scheduled + + startup_sidekiq! + assert_scheduled end end @@ -80,7 +84,8 @@ def configure(config) end def test_startup_scheduling_is_not_locked - expect_no_redlock! { startup_sidekiq! } + with_redlock_held { startup_sidekiq! } + assert_scheduled end end @@ -89,10 +94,11 @@ class TestRedlockMultipleRedisInstances < Minitest::Test include TestHelper.application_setup(false) def configure(config) - config.redlock_redis_instances = ['redis://1.2.3.4/', 'redis://5.6.7.8/'] + config.redlock_redis_instances = [Sidekiq.redis_pool, TestHelper.second_redis_pool] end - def test_startup_scheduling_is_not_locked - expect_redlock!(['redis://1.2.3.4/', 'redis://5.6.7.8/']) { startup_sidekiq! } + def test_startup_scheduling_is_locked + with_redlock_held(TestHelper.second_redis_pool) { startup_sidekiq! } + assert_not_scheduled end end diff --git a/test/test_helper.rb b/test/test_helper.rb index 5bfb612..47749bc 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -48,6 +48,15 @@ def perform(last, current) end module TestHelper + LOCK_KEY = 'sidekiq-repeat-reschedule-all' + + def self.second_redis_pool + @second_redis_pool ||= ConnectionPool.new(size: 1) do + primary_db = Sidekiq.redis { |redis| redis.connection[:db] } + Redis.new(url: ENV.fetch('TEST_REDIS_URL', 'redis://127.0.0.1:16379'), db: primary_db == 1 ? 0 : 1) + end + end + def self.assertions(klass, perform_with_arguments = false) Module.new do # NOTE: For some reason, we need to use define_method here, as otherwise `klass` @@ -96,28 +105,14 @@ def perform_scheduled! worker.perform_one end - def expect_redlock!(redis_instances = nil) - redis_instances ||= Sidekiq::Repeat::Configuration.instance.redlock_redis_instances - - @redlock_client_instance = Minitest::Mock.new - @redlock_client_instance.expect(:lock, nil, ['sidekiq-repeat-reschedule-all', 500]) - - @redlock_client_new_method = Minitest::Mock.new - @redlock_client_new_method.expect(:call, @redlock_client_instance, [redis_instances]) + def with_redlock_held(redis = Sidekiq.redis_pool) + client = Redlock::Client.new([redis], retry_count: 0) + lock = client.lock(TestHelper::LOCK_KEY, 30_000) + raise 'Could not acquire test lock' unless lock - Redlock::Client.stub(:new, @redlock_client_new_method) do - yield # to test case. - end - - @redlock_client_new_method.verify - @redlock_client_instance.verify - end - - def expect_no_redlock! - @redlock_client_new_method = Proc.new { flunk 'Redlock::Client::new should not be called' } - Redlock::Client.stub(:new, @redlock_client_new_method) do - yield - end + yield + ensure + client.unlock(lock) if lock end end @@ -137,9 +132,7 @@ def setup klass.instance_variable_set(:@cronline, nil) klass.instance_variable_set(:@ss, nil) end - if startup_sidekiq - Sidekiq::Repeat::Configuration.instance.redlock_redis_instances = [Sidekiq.redis_pool] - end + Sidekiq::Repeat::Configuration.instance.redlock_redis_instances = [Sidekiq.redis_pool] # Allow the test to configure Sidekiq::Repeat. Sidekiq::Repeat.configure { |config| configure(config) } @@ -158,6 +151,7 @@ def startup_sidekiq! def clear_test_redis Sidekiq.redis(&:flushdb) + TestHelper.second_redis_pool.with { |redis| redis.del(TestHelper::LOCK_KEY) } end end end From 56b469f8e5a76284dd222bdfa7fc6446832fcb5a Mon Sep 17 00:00:00 2001 From: Dominik Sander Date: Tue, 8 Sep 2026 10:54:39 +0200 Subject: [PATCH 3/3] Update to Sidekiq 8 and redlock 2 Update was relatively smooth, both gems switched from `redis-rb` to `redis-client`. Apart from that a few Sidekiq APIs changed. --- sidekiq-repeat.gemspec | 4 ++-- test/test_helper.rb | 24 ++++++++++++++---------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/sidekiq-repeat.gemspec b/sidekiq-repeat.gemspec index dc6a7fd..9a2d04a 100644 --- a/sidekiq-repeat.gemspec +++ b/sidekiq-repeat.gemspec @@ -14,9 +14,9 @@ Gem::Specification.new do |spec| spec.files = Dir['lib/**/*rb'] spec.require_paths = ['lib'] - spec.add_dependency 'sidekiq', '~> 6.5', '>= 6.5.12' + spec.add_dependency 'sidekiq', '~> 8', '< 9' spec.add_dependency 'parse-cron', '~> 0.1.4' - spec.add_dependency 'redlock', '~> 1.3', '>= 1.3.2' + spec.add_dependency 'redlock', '~> 2' # Sidekiq 6.5 requires these libraries without declaring them as gems. spec.add_dependency 'base64', '~> 0.3' spec.add_dependency 'logger', '~> 1.7' diff --git a/test/test_helper.rb b/test/test_helper.rb index 47749bc..3ab91e5 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -9,14 +9,17 @@ require 'minitest/autorun' require 'sidekiq' require 'sidekiq/cli' -require 'sidekiq/testing' require 'minitest/mock' -Sidekiq::Testing.disable! +Sidekiq.testing!(:disable) Sidekiq.logger.level = Logger::ERROR -Sidekiq.redis = { - url: ENV.fetch('TEST_REDIS_URL', 'redis://127.0.0.1:16379') -} +configure_sidekiq = proc do |config| + config.redis = { + url: ENV.fetch('TEST_REDIS_URL', 'redis://127.0.0.1:16379') + } +end +Sidekiq.configure_client(&configure_sidekiq) +Sidekiq.configure_server(&configure_sidekiq) require 'sidekiq-repeat' @@ -51,9 +54,10 @@ module TestHelper LOCK_KEY = 'sidekiq-repeat-reschedule-all' def self.second_redis_pool - @second_redis_pool ||= ConnectionPool.new(size: 1) do - primary_db = Sidekiq.redis { |redis| redis.connection[:db] } - Redis.new(url: ENV.fetch('TEST_REDIS_URL', 'redis://127.0.0.1:16379'), db: primary_db == 1 ? 0 : 1) + @second_redis_pool ||= begin + primary_db = Sidekiq.redis { |redis| redis.config.db } + client = RedisClient.config(url: ENV.fetch('TEST_REDIS_URL', 'redis://127.0.0.1:16379'), db: primary_db == 1 ? 0 : 1) + client.new_pool(size: 1) end end @@ -146,12 +150,12 @@ def teardown end def startup_sidekiq! - Sidekiq[:lifecycle_events][:startup].each(&:call) + Sidekiq.default_configuration[:lifecycle_events][:startup].each(&:call) end def clear_test_redis Sidekiq.redis(&:flushdb) - TestHelper.second_redis_pool.with { |redis| redis.del(TestHelper::LOCK_KEY) } + TestHelper.second_redis_pool.with { |redis| redis.call('DEL', TestHelper::LOCK_KEY) } end end end