Skip to content

Commit

Permalink
Add sidekiq 7.2 to test matrix (#22)
Browse files Browse the repository at this point in the history
Initial discovery on #21

Root cause: https://github.com/sidekiq/sidekiq/blob/main/Changes.md#720

> Adjust redis-client adapter to avoid method_missing [#6083] This can
result in app code breaking if your app's Redis API usage was depending
on Sidekiq's adapter to correct invalid redis-client API usage

7.2 changed their internal API regarding calling `redis-client`; Support
to 7.0 was added by _community_ in #18 and I have very vague memory of
it
  • Loading branch information
rwojsznis authored Feb 7, 2024
2 parents b998d3f + 475d175 commit 3db1099
Show file tree
Hide file tree
Showing 18 changed files with 100 additions and 77 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
matrix:
os: [ ubuntu-latest ]
ruby: [ '2.7', '3.0', '3.1' ]
gemfile: [ sidekiq_5, sidekiq_6, sidekiq_7 ]
gemfile: [ sidekiq_6_1, sidekiq_7_1, sidekiq_7_2 ]
redis: [ '7.0-alpine3.18', '6.2.12-alpine3.18' ]
env:
BUNDLE_GEMFILE: ${{ github.workspace }}/gemfiles/${{ matrix.gemfile }}.gemfile
Expand All @@ -24,7 +24,7 @@ jobs:
ports:
- 6379:6379
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ matrix.ruby }}
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,4 @@ tmp
*.gemfile.lock
.idea/
.DS_Store
.tool-versions
16 changes: 10 additions & 6 deletions Appraisals
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
appraise 'sidekiq-5' do
gem 'sidekiq', '~> 5.1', '>= 5.1'
appraise 'sidekiq-6_1' do
gem 'sidekiq', '~> 6.1', '>= 6.1.1', '< 6.2'
end

appraise 'sidekiq-6' do
gem 'sidekiq', '~> 6.1', '>= 6.1.1'
appraise 'sidekiq-6_2' do
gem 'sidekiq', '~> 6.2', '>= 6.2'
end

appraise 'sidekiq-7' do
gem 'sidekiq', '~> 7.1', '>= 7.1.0'
appraise 'sidekiq-7_1' do
gem 'sidekiq', '~> 7.1', '>= 7.1.0', '< 7.2'
end

appraise 'sidekiq-7_2' do
gem 'sidekiq', '~> 7.2', '>= 7.2.0'
end
9 changes: 8 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
@@ -1,2 +1,9 @@
source "https://rubygems.org"
gemspec

group :test do
gem "minitest"
gem "mocha", require: false
end

gem "appraisal"

7 changes: 0 additions & 7 deletions gemfiles/sidekiq_5.gemfile

This file was deleted.

7 changes: 0 additions & 7 deletions gemfiles/sidekiq_6.gemfile

This file was deleted.

11 changes: 11 additions & 0 deletions gemfiles/sidekiq_6_1.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "appraisal"
gem "sidekiq", "~> 6.1", ">= 6.1.1", "< 6.2"

group :test do
gem "minitest"
gem "mocha", require: false
end
11 changes: 11 additions & 0 deletions gemfiles/sidekiq_6_2.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "appraisal"
gem "sidekiq", "~> 6.2", ">= 6.2"

group :test do
gem "minitest"
gem "mocha", require: false
end
7 changes: 0 additions & 7 deletions gemfiles/sidekiq_7.gemfile

This file was deleted.

11 changes: 11 additions & 0 deletions gemfiles/sidekiq_7_1.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "appraisal"
gem "sidekiq", "~> 7.1", ">= 7.1.0", "< 7.2"

group :test do
gem "minitest"
gem "mocha", require: false
end
11 changes: 11 additions & 0 deletions gemfiles/sidekiq_7_2.gemfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# This file was generated by Appraisal

source "https://rubygems.org"

gem "appraisal"
gem "sidekiq", "~> 7.2", ">= 7.2.0"

group :test do
gem "minitest"
gem "mocha", require: false
end
6 changes: 5 additions & 1 deletion lib/sidekiq/lock/redis_lock.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ def acquired?
# this also requires redis-rb >= 3.0.5
def acquire!
@acquired ||= Sidekiq.redis do |r|
r.set(name, value, nx: true, px: timeout)
if Sidekiq::VERSION >= '7.2'
r.set(name, value, 'nx', 'px', timeout)
else
r.set(name, value, nx: true, px: timeout)
end
end
end

Expand Down
6 changes: 0 additions & 6 deletions sidekiq-lock.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,4 @@ Gem::Specification.new do |spec|

spec.add_dependency "sidekiq", ">= 2.14.0"
spec.add_dependency "redis", ">= 3.0.5"

spec.add_development_dependency "bundler"
spec.add_development_dependency "rake"
spec.add_development_dependency "mocha", "~> 0.14.0"
spec.add_development_dependency "minitest"
spec.add_development_dependency "appraisal"
end
9 changes: 5 additions & 4 deletions test/lib/lock_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,24 @@ module Sidekiq
skip 'Sidekiq 7+ does not print out middleware information' if Sidekiq::VERSION >= '7'

cmd = 'sidekiq -r ./test/test_workers.rb -v'
buffer = ''
buffer_out = ''

# very not fancy (https://78.media.tumblr.com/tumblr_lzkpw7DAl21qhy6c9o2_400.gif)
# solution, but should do the job
Open3.popen3(cmd) do |stdin, stdout, stderr, thread|
begin
Timeout.timeout(5) do
Timeout.timeout(3) do
until stdout.eof? do
buffer << stdout.read_nonblock(16)
buffer_out << stdout.read_nonblock(16)
end
end

rescue Timeout::Error
Process.kill('KILL', thread.pid)
end
end

assert_match(/\s?Middleware:.*Sidekiq::Lock::Middleware/i, buffer)
assert_match(/\s?Middleware:.*Sidekiq::Lock::Middleware/i, buffer_out)
end
end
end
5 changes: 3 additions & 2 deletions test/lib/middleware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ module Lock
reset_lock_variable!
end

let(:handler) { Sidekiq::Lock::Middleware.new }

it 'sets lock variable with provided static lock options' do
handler = Sidekiq::Lock::Middleware.new
handler.call(LockWorker.new, { 'class' => LockWorker, 'args' => [] }, 'default') do
true
end
Expand All @@ -26,6 +25,7 @@ module Lock
end

it 'sets lock variable with provided dynamic options' do
handler = Sidekiq::Lock::Middleware.new
handler.call(DynamicLockWorker.new, { 'class' => DynamicLockWorker, 'args' => [1234, 1000] }, 'default') do
true
end
Expand All @@ -35,6 +35,7 @@ module Lock
end

it 'sets nothing for workers without lock options' do
handler = Sidekiq::Lock::Middleware.new
handler.call(RegularWorker.new, { 'class' => RegularWorker, 'args' => [] }, 'default') do
true
end
Expand Down
51 changes: 20 additions & 31 deletions test/lib/worker_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
module Sidekiq
module Lock
describe Worker do
# after { }

class CustomContainer
def initialize
@lock = nil
Expand All @@ -19,47 +17,38 @@ def store(lock)
end
end

# it 'sets lock method that points to thread variable' do
# set_lock_variable! "test"
# assert_equal "test", LockWorker.new.lock
# end

it 'allows method name configuration' do
begin
Sidekiq.lock_method = :custom_lock_name
Sidekiq.lock_method = :custom_lock_name

class WorkerWithCustomLockName
include Sidekiq::Worker
include Sidekiq::Lock::Worker
end
class WorkerWithCustomLockName
include Sidekiq::Worker
include Sidekiq::Lock::Worker
end

set_lock_variable! "custom_name"
set_lock_variable! "custom_name"

assert_equal "custom_name", WorkerWithCustomLockName.new.custom_lock_name
assert_equal "custom_name", WorkerWithCustomLockName.new.custom_lock_name

reset_lock_variable!
ensure
reset_lock_variable!
ensure

Sidekiq.lock_method = Sidekiq::Lock::METHOD_NAME
end
Sidekiq.lock_method = Sidekiq::Lock::METHOD_NAME
end

it 'allows container configuration' do
begin
container = CustomContainer.new
Sidekiq.lock_container = container
container = CustomContainer.new
Sidekiq.lock_container = container

class WorkerWithCustomContainer
include Sidekiq::Worker
include Sidekiq::Lock::Worker
end
class WorkerWithCustomContainer
include Sidekiq::Worker
include Sidekiq::Lock::Worker
end

container.store "lock-variable"
container.store "lock-variable"

assert_equal "lock-variable", WorkerWithCustomContainer.new.lock
ensure
Sidekiq.lock_container = Sidekiq::Lock::Container.new
end
assert_equal "lock-variable", WorkerWithCustomContainer.new.lock
ensure
Sidekiq.lock_container = Sidekiq::Lock::Container.new
end
end
end
Expand Down
4 changes: 1 addition & 3 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
require 'minitest/autorun'
require 'minitest/pride'
require 'mocha/setup'

require 'mocha/minitest'
require 'sidekiq'
require 'test_workers'

Expand Down
1 change: 1 addition & 0 deletions test/test_workers.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
$LOAD_PATH.unshift(File.expand_path('../lib', __dir__))
require 'sidekiq-lock'

class LockWorker
Expand Down

0 comments on commit 3db1099

Please sign in to comment.