From 0d9a4ea43df845083e476ac5aea5f7537ed05118 Mon Sep 17 00:00:00 2001 From: Mikael Henriksson Date: Fri, 14 Jul 2023 21:29:43 +0200 Subject: [PATCH] Fix active worker detection by using correct keys (#756) (#799) * Fix active worker detection by using correct keys (#756) Rename keys from :workers to :work since that's correct name set in Redis * chore(lint): fix linter issues * fix(unlock): ensure callback and unlock (#771) * chore(deps): update gems (solargraph is awesome) * fix(unlock): ensure unlock and callback runs * chore(lint): lint'em real good # Conflicts: # .github/workflows/rspec.yml # myapp/.tool-versions * fix: backport the fix for the return value of #deep_transform_keys (#750) Backport fix the return value of #deep_transform_keys * Hide lock info debug suggestion on lock page if it's already enabled. (#763) only show lock info suggestion if value is not already on * chore(v7): backport fixes from v8 * chore(ci): backport ci changes from v8 --------- Co-authored-by: Dominik Szromik Co-authored-by: Egor Romanov Co-authored-by: Jeremiah --- .github/workflows/lint.yml | 6 +- .github/workflows/rspec.yml | 25 +++++--- .gitignore | 3 + .tool-versions | 3 + Gemfile | 2 - bin/_guard-core | 2 +- bin/appraisal | 2 +- bin/bundle | 2 + bin/code_climate_reek | 2 +- bin/rake | 2 +- bin/reek | 2 +- bin/rspec | 2 +- bin/rubocop | 2 +- gemfiles/sidekiq_5.0.gemfile | 2 +- lib/sidekiq_unique_jobs.rb | 1 + lib/sidekiq_unique_jobs/batch_delete.rb | 2 +- lib/sidekiq_unique_jobs/core_ext.rb | 2 +- lib/sidekiq_unique_jobs/job.rb | 5 ++ .../lock/until_executed.rb | 1 + .../lock/while_executing.rb | 3 +- lib/sidekiq_unique_jobs/lock_type.rb | 37 ++++++++++++ lib/sidekiq_unique_jobs/locksmith.rb | 16 ++++- .../shared/_find_digest_in_process_set.lua | 2 +- .../options_with_fallback.rb | 2 +- lib/sidekiq_unique_jobs/orphans/manager.rb | 4 +- .../orphans/ruby_reaper.rb | 2 +- lib/sidekiq_unique_jobs/script/caller.rb | 14 ++--- .../sidekiq_unique_jobs.rb | 6 +- lib/sidekiq_unique_jobs/web/views/lock.erb | 8 ++- myapp/.tool-versions | 4 ++ myapp/bin/bundle | 2 + myapp/bin/puma | 2 +- myapp/bin/pumactl | 2 +- myapp/bin/rake | 2 +- myapp/bin/rspec | 2 +- myapp/bin/rubocop | 2 +- myapp/bin/sidekiq | 2 +- myapp/bin/sidekiqmon | 2 +- myapp/cable.ru | 2 +- myapp/config.ru | 2 +- myapp/config/environments/production.rb | 2 +- spec/sidekiq/api_spec.rb | 32 +++++----- spec/sidekiq_unique_jobs/batch_delete_spec.rb | 4 +- spec/sidekiq_unique_jobs/changelog_spec.rb | 8 +-- spec/sidekiq_unique_jobs/core_ext_spec.rb | 14 +++++ spec/sidekiq_unique_jobs/digests_spec.rb | 2 +- spec/sidekiq_unique_jobs/job_spec.rb | 1 + .../lock/while_executing_spec.rb | 10 +++- spec/sidekiq_unique_jobs/lock_spec.rb | 10 ++-- spec/sidekiq_unique_jobs/locksmith_spec.rb | 12 ++-- spec/sidekiq_unique_jobs/lua/lock_spec.rb | 12 ++-- spec/sidekiq_unique_jobs/lua/queue_spec.rb | 8 +-- .../lua/reap_orphans_spec.rb | 18 +++--- spec/sidekiq_unique_jobs/lua/unlock_spec.rb | 6 +- .../orphans/reaper_spec.rb | 26 ++++---- .../orphans/ruby_reaper_spec.rb | 10 ++-- spec/sidekiq_unique_jobs/redis/hash_spec.rb | 2 +- spec/sidekiq_unique_jobs/redis/set_spec.rb | 2 +- .../redis/sorted_set_spec.rb | 2 +- .../shared_examples/a_lockable_lock.rb | 58 ------------------ .../an_executing_lock_implementation.rb | 59 +++++++++++++++++++ .../custom_queue_job_with_filter_proc.rb | 2 +- spec/workers/bad_worker_spec.rb | 4 +- 63 files changed, 295 insertions(+), 195 deletions(-) create mode 100644 .tool-versions create mode 100644 lib/sidekiq_unique_jobs/lock_type.rb create mode 100644 myapp/.tool-versions create mode 100644 spec/support/shared_examples/an_executing_lock_implementation.rb diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index b0876b90b..ee28fe1f8 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1,4 +1,4 @@ -name: Lint +name: ci on: pull_request: push: @@ -14,7 +14,7 @@ jobs: - uses: ruby/setup-ruby@v1 with: ruby-version: 3.1 - bundler: 2.3.19 + bundler: 2.4.12 bundler-cache: true - run: bin/bundle --jobs=$(nproc) --retry=$(nproc) - run: bin/rubocop -P @@ -29,7 +29,7 @@ jobs: - uses: ruby/setup-ruby@v1 with: ruby-version: 3.1 - bundler: 2.3.19 + bundler: 2.4.12 bundler-cache: true - run: bin/bundle --jobs=$(nproc) --retry=$(nproc) - run: bin/reek . diff --git a/.github/workflows/rspec.yml b/.github/workflows/rspec.yml index 3b7b4426c..12fe08437 100644 --- a/.github/workflows/rspec.yml +++ b/.github/workflows/rspec.yml @@ -1,4 +1,4 @@ -name: RSpec +name: ci on: pull_request: push: @@ -22,8 +22,8 @@ jobs: - uses: actions/checkout@v3 - uses: ruby/setup-ruby@v1 with: - ruby-version: 3.1 - bundler: 2.3.19 + ruby-version: 3.2 + bundler: 2.4.12 bundler-cache: true - name: Install Code Climate reporter @@ -42,7 +42,7 @@ jobs: COV=true bin/rspec --require spec_helper --tag ~perf ./cc-test-reporter after-build --coverage-input-type simplecov --exit-code $? - tests: + rspec: services: toxiproxy: image: ghcr.io/shopify/toxiproxy @@ -59,14 +59,23 @@ jobs: strategy: fail-fast: true matrix: - ruby: [2.5, 2.6, 2.7, '3.0', 3.1] + ruby: [2.7, '3.0', 3.1, 3.2] + gemfile: + - sidekiq_5.0 + - sidekiq_5.1 + - sidekiq_5.2 + - sidekiq_6.0 + - sidekiq_6.1 + - sidekiq_6.2 + - sidekiq_6.3 + - sidekiq_6.4 + - sidekiq_6.5 steps: - uses: actions/checkout@v3 - uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby }} - bundler: 2.3.19 + bundler: 2.4.12 bundler-cache: true - - run: bin/appraisal install --jobs=$(nproc) --retry=$(nproc) - - run: bin/appraisal rspec --require spec_helper --tag ~perf + - run: bin/rspec --require spec_helper --tag ~perf diff --git a/.gitignore b/.gitignore index a8cc8801e..ad79a6ded 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,6 @@ tmp/ /gemfiles/vendor/ /vendor/ /myapp/vendor/bundle/ +/.bundle/ +/myapp/node_modules/ +/myapp/yarn-error.log diff --git a/.tool-versions b/.tool-versions new file mode 100644 index 000000000..c308cfa14 --- /dev/null +++ b/.tool-versions @@ -0,0 +1,3 @@ +nodejs 20.4.0 +yarn 1.22.19 +direnv 2.32.2 diff --git a/Gemfile b/Gemfile index a577a4ecd..ebe6e4780 100644 --- a/Gemfile +++ b/Gemfile @@ -32,12 +32,10 @@ end if respond_to?(:install_if) install_if -> { RUBY_PLATFORM.include?("darwin") } do - gem "fasterer" gem "fuubar" gem "github_changelog_generator" gem "pry" gem "redcarpet", "~> 3.4" - gem "rspec-nc" gem "ruby-prof", ">= 0.17.0", require: false gem "stackprof", ">= 0.2.9", require: false gem "test-prof" diff --git a/bin/_guard-core b/bin/_guard-core index 006dd5cc2..a322c181e 100755 --- a/bin/_guard-core +++ b/bin/_guard-core @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/bin/appraisal b/bin/appraisal index 86df4b2cd..42960e179 100755 --- a/bin/appraisal +++ b/bin/appraisal @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/bin/bundle b/bin/bundle index 4e95f2537..41c575af3 100755 --- a/bin/bundle +++ b/bin/bundle @@ -63,9 +63,11 @@ m = Module.new do end def bundler_version + # rubocop:disable ThreadSafety/InstanceVariableInClassMethod @bundler_version ||= env_var_version || cli_arg_version || lockfile_version + # rubocop:enable ThreadSafety/InstanceVariableInClassMethod end def bundler_requirement diff --git a/bin/code_climate_reek b/bin/code_climate_reek index c02239088..e67e917d7 100755 --- a/bin/code_climate_reek +++ b/bin/code_climate_reek @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/bin/rake b/bin/rake index 849244445..48fa796f6 100755 --- a/bin/rake +++ b/bin/rake @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/bin/reek b/bin/reek index ab0b35c30..a1f4fd3bb 100755 --- a/bin/reek +++ b/bin/reek @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/bin/rspec b/bin/rspec index c9fea7c29..18a257fd8 100755 --- a/bin/rspec +++ b/bin/rspec @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/bin/rubocop b/bin/rubocop index ba009623e..f7e8a3892 100755 --- a/bin/rubocop +++ b/bin/rubocop @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/gemfiles/sidekiq_5.0.gemfile b/gemfiles/sidekiq_5.0.gemfile index af8ebc994..6a200175b 100644 --- a/gemfiles/sidekiq_5.0.gemfile +++ b/gemfiles/sidekiq_5.0.gemfile @@ -3,11 +3,11 @@ source "https://rubygems.org" gem "appraisal" +gem "faraday-retry" gem "gem-release" gem "github-markup" gem "rack-test" gem "rake", "13.0.3" -gem "redis-namespace" gem "reek", ">= 5.3" gem "rspec" gem "rspec-benchmark" diff --git a/lib/sidekiq_unique_jobs.rb b/lib/sidekiq_unique_jobs.rb index 249551b7b..32cc66ee1 100644 --- a/lib/sidekiq_unique_jobs.rb +++ b/lib/sidekiq_unique_jobs.rb @@ -27,6 +27,7 @@ require "sidekiq_unique_jobs/logging/middleware_context" require "sidekiq_unique_jobs/timing" require "sidekiq_unique_jobs/sidekiq_worker_methods" +require "sidekiq_unique_jobs/lock_type" require "sidekiq_unique_jobs/connection" require "sidekiq_unique_jobs/exceptions" require "sidekiq_unique_jobs/script" diff --git a/lib/sidekiq_unique_jobs/batch_delete.rb b/lib/sidekiq_unique_jobs/batch_delete.rb index f4796e596..69c1edaff 100644 --- a/lib/sidekiq_unique_jobs/batch_delete.rb +++ b/lib/sidekiq_unique_jobs/batch_delete.rb @@ -112,7 +112,7 @@ def del_digest(pipeline, digest) def keys_for_digest(digest) [digest, "#{digest}:RUN"].each_with_object([]) do |key, digest_keys| - digest_keys.concat([key]) + digest_keys.push(key) digest_keys.concat(SUFFIXES.map { |suffix| "#{key}:#{suffix}" }) end end diff --git a/lib/sidekiq_unique_jobs/core_ext.rb b/lib/sidekiq_unique_jobs/core_ext.rb index ca0e3d6e3..3ecc171d9 100644 --- a/lib/sidekiq_unique_jobs/core_ext.rb +++ b/lib/sidekiq_unique_jobs/core_ext.rb @@ -98,7 +98,7 @@ def slice!(*keys) def _deep_transform_keys_in_object(object, &block) case object when Hash - object.each_with_object({}) do |(key, value), result| + object.each_with_object(self.class.new) do |(key, value), result| result[yield(key)] = _deep_transform_keys_in_object(value, &block) end when Array diff --git a/lib/sidekiq_unique_jobs/job.rb b/lib/sidekiq_unique_jobs/job.rb index 46c158e6e..7d0a7eae4 100644 --- a/lib/sidekiq_unique_jobs/job.rb +++ b/lib/sidekiq_unique_jobs/job.rb @@ -11,6 +11,7 @@ module Job # @return [Hash] the job hash def prepare(item) stringify_on_conflict_hash(item) + add_lock_type(item) add_lock_timeout(item) add_lock_ttl(item) add_digest(item) @@ -54,5 +55,9 @@ def add_lock_digest(item) def add_lock_prefix(item) item[LOCK_PREFIX] ||= SidekiqUniqueJobs.config.lock_prefix end + + def add_lock_type(item) + item[LOCK] ||= SidekiqUniqueJobs::LockType.call(item) + end end end diff --git a/lib/sidekiq_unique_jobs/lock/until_executed.rb b/lib/sidekiq_unique_jobs/lock/until_executed.rb index 7571f1948..cc7c62afa 100644 --- a/lib/sidekiq_unique_jobs/lock/until_executed.rb +++ b/lib/sidekiq_unique_jobs/lock/until_executed.rb @@ -35,6 +35,7 @@ def lock(&block) def execute executed = locksmith.execute do yield + ensure unlock_and_callback end diff --git a/lib/sidekiq_unique_jobs/lock/while_executing.rb b/lib/sidekiq_unique_jobs/lock/while_executing.rb index 51baf02b2..a7affb689 100644 --- a/lib/sidekiq_unique_jobs/lock/while_executing.rb +++ b/lib/sidekiq_unique_jobs/lock/while_executing.rb @@ -42,9 +42,8 @@ def execute(&block) with_logging_context do executed = locksmith.execute do yield - callback_safely if locksmith.unlock ensure - locksmith.unlock + unlock_and_callback end unless executed diff --git a/lib/sidekiq_unique_jobs/lock_type.rb b/lib/sidekiq_unique_jobs/lock_type.rb new file mode 100644 index 000000000..df5790582 --- /dev/null +++ b/lib/sidekiq_unique_jobs/lock_type.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module SidekiqUniqueJobs + # Calculates the lock type + # + class LockType + # includes "SidekiqUniqueJobs::SidekiqWorkerMethods" + # @!parse include SidekiqUniqueJobs::SidekiqWorkerMethods + include SidekiqUniqueJobs::SidekiqWorkerMethods + + # + # Computes lock type from job arguments, sidekiq_options. + # + # @return [Symbol] the lock type + # @return [NilClass] if no lock type is found. + # + def self.call(item) + new(item).call + end + + # @!attribute [r] item + # @return [Hash] the Sidekiq job hash + attr_reader :item + + # @param [Hash] item the Sidekiq job hash + # @option item [Symbol, nil] :lock the type of lock to use. + # @option item [String] :class the class of the sidekiq worker + def initialize(item) + @item = item + self.job_class = item[CLASS] + end + + def call + item[LOCK] || job_options[LOCK] || default_job_options[LOCK] + end + end +end diff --git a/lib/sidekiq_unique_jobs/locksmith.rb b/lib/sidekiq_unique_jobs/locksmith.rb index 5d93a7d53..c2bf3a9f9 100644 --- a/lib/sidekiq_unique_jobs/locksmith.rb +++ b/lib/sidekiq_unique_jobs/locksmith.rb @@ -127,7 +127,10 @@ def unlock(conn = nil) # def unlock!(conn = nil) call_script(:unlock, key.to_a, argv, conn) do |unlocked_jid| - reflect(:debug, :unlocked, item, unlocked_jid) if unlocked_jid == job_id + if unlocked_jid == job_id + reflect(:debug, :unlocked, item, unlocked_jid) + reflect(:unlocked, item) + end unlocked_jid end @@ -248,8 +251,12 @@ def primed_async(conn, wait = nil, &block) concurrent_timeout = add_drift(timeout) reflect(:debug, :timeouts, item, - timeouts: { brpoplpush_timeout: brpoplpush_timeout, concurrent_timeout: concurrent_timeout }) + timeouts: { + brpoplpush_timeout: brpoplpush_timeout, + concurrent_timeout: concurrent_timeout, + }) + # NOTE: When debugging, change .value to .value! primed_jid = Concurrent::Promises .future(conn) { |red_con| pop_queued(red_con, timeout) } .value @@ -300,7 +307,10 @@ def pop_queued(conn, wait = 1) def brpoplpush(conn, wait) # passing timeout 0 to brpoplpush causes it to block indefinitely raise InvalidArgument, "wait must be an integer" unless wait.is_a?(Integer) - return conn.brpoplpush(key.queued, key.primed, wait) if conn.class.to_s == "Redis::Namespace" + + if defined?(::Redis::Namespace) && conn.instance_of?(::Redis::Namespace) + return conn.brpoplpush(key.queued, key.primed, wait) + end if VersionCheck.satisfied?(redis_version, ">= 6.2.0") && conn.respond_to?(:blmove) conn.blmove(key.queued, key.primed, "RIGHT", "LEFT", timeout: wait) diff --git a/lib/sidekiq_unique_jobs/lua/shared/_find_digest_in_process_set.lua b/lib/sidekiq_unique_jobs/lua/shared/_find_digest_in_process_set.lua index d38a8c032..876e38a1e 100644 --- a/lib/sidekiq_unique_jobs/lua/shared/_find_digest_in_process_set.lua +++ b/lib/sidekiq_unique_jobs/lua/shared/_find_digest_in_process_set.lua @@ -15,7 +15,7 @@ local function find_digest_in_process_set(digest, threshold) log_debug("Found number of processes:", #processes, "next cursor:", next_process_cursor) for _, process in ipairs(processes) do - local workers_key = process .. ":workers" + local workers_key = process .. ":work" log_debug("searching in process set:", process, "for digest:", digest, "cursor:", process_cursor) diff --git a/lib/sidekiq_unique_jobs/options_with_fallback.rb b/lib/sidekiq_unique_jobs/options_with_fallback.rb index dbeecdc22..962f51a74 100644 --- a/lib/sidekiq_unique_jobs/options_with_fallback.rb +++ b/lib/sidekiq_unique_jobs/options_with_fallback.rb @@ -55,7 +55,7 @@ def lock_class # The type of lock for this worker # # - # @return [Symbol] + # @return [Symbol, NilClass] # def lock_type @lock_type ||= options[LOCK] || item[LOCK] diff --git a/lib/sidekiq_unique_jobs/orphans/manager.rb b/lib/sidekiq_unique_jobs/orphans/manager.rb index b42cefc88..96540b41c 100644 --- a/lib/sidekiq_unique_jobs/orphans/manager.rb +++ b/lib/sidekiq_unique_jobs/orphans/manager.rb @@ -72,7 +72,7 @@ def stop # @return [] # def task - @task ||= default_task + @task ||= default_task # rubocop:disable ThreadSafety/InstanceVariableInClassMethod end # @@ -100,7 +100,7 @@ def default_task # @return [void] # def task=(task) - @task = task + @task = task # rubocop:disable ThreadSafety/InstanceVariableInClassMethod end # diff --git a/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb b/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb index 95c139530..ad57c0f08 100644 --- a/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb +++ b/lib/sidekiq_unique_jobs/orphans/ruby_reaper.rb @@ -195,7 +195,7 @@ def active?(digest) # rubocop:disable Metrics/MethodLength, Metrics/CyclomaticCo else pipeline.exists(key) end - pipeline.hgetall("#{key}:workers") + pipeline.hgetall("#{key}:work") end next unless valid diff --git a/lib/sidekiq_unique_jobs/script/caller.rb b/lib/sidekiq_unique_jobs/script/caller.rb index 834de8fbb..4f757e06c 100644 --- a/lib/sidekiq_unique_jobs/script/caller.rb +++ b/lib/sidekiq_unique_jobs/script/caller.rb @@ -54,13 +54,13 @@ def call_script(file_name, *args) # Only used to reduce a little bit of duplication # @see call_script def do_call(file_name, conn, keys, argv) - argv = argv.dup.concat([ - now_f, - debug_lua, - max_history, - file_name, - redis_version, - ]) + argv = argv.dup.push( + now_f, + debug_lua, + max_history, + file_name, + redis_version, + ) Script.execute(file_name, conn, keys: keys, argv: argv) end diff --git a/lib/sidekiq_unique_jobs/sidekiq_unique_jobs.rb b/lib/sidekiq_unique_jobs/sidekiq_unique_jobs.rb index 68af5c105..a931966d5 100644 --- a/lib/sidekiq_unique_jobs/sidekiq_unique_jobs.rb +++ b/lib/sidekiq_unique_jobs/sidekiq_unique_jobs.rb @@ -17,7 +17,7 @@ module SidekiqUniqueJobs # rubocop:disable Metrics/ModuleLength # @return [SidekiqUniqueJobs::Config] the gem configuration # def config - @config ||= reset! + @config ||= reset! # rubocop:disable ThreadSafety/InstanceVariableInClassMethod end # @@ -108,7 +108,7 @@ def use_config(tmp_config = {}) # @return [SidekiqUniqueJobs::Config] a default gem configuration # def reset! - @config = SidekiqUniqueJobs::Config.default + @config = SidekiqUniqueJobs::Config.default # rubocop:disable ThreadSafety/InstanceVariableInClassMethod end # @@ -288,7 +288,7 @@ def safe_constantize(str) # @return [Reflections] # def reflections - @reflections ||= Reflections.new + @reflections ||= Reflections.new # rubocop:disable ThreadSafety/InstanceVariableInClassMethod end # diff --git a/lib/sidekiq_unique_jobs/web/views/lock.erb b/lib/sidekiq_unique_jobs/web/views/lock.erb index fe8dd7aea..f8beba3b4 100644 --- a/lib/sidekiq_unique_jobs/web/views/lock.erb +++ b/lib/sidekiq_unique_jobs/web/views/lock.erb @@ -7,9 +7,11 @@
<% if @lock.info.none? %>

No Lock Information Available

-

To use it turn the following setting on: - SidekiqUniqueJobs.config.lock_info = true -

+ <% unless SidekiqUniqueJobs.config.lock_info %> +

To use it turn the following setting on: + SidekiqUniqueJobs.config.lock_info = true +

+ <% end %> <% else %> diff --git a/myapp/.tool-versions b/myapp/.tool-versions new file mode 100644 index 000000000..d13bd602d --- /dev/null +++ b/myapp/.tool-versions @@ -0,0 +1,4 @@ +ruby 3.2.2 +nodejs 19.1.0 +yarn 1.22.19 +direnv 2.32.2 diff --git a/myapp/bin/bundle b/myapp/bin/bundle index 4e95f2537..41c575af3 100755 --- a/myapp/bin/bundle +++ b/myapp/bin/bundle @@ -63,9 +63,11 @@ m = Module.new do end def bundler_version + # rubocop:disable ThreadSafety/InstanceVariableInClassMethod @bundler_version ||= env_var_version || cli_arg_version || lockfile_version + # rubocop:enable ThreadSafety/InstanceVariableInClassMethod end def bundler_requirement diff --git a/myapp/bin/puma b/myapp/bin/puma index 779bb4319..3070ef780 100755 --- a/myapp/bin/puma +++ b/myapp/bin/puma @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/myapp/bin/pumactl b/myapp/bin/pumactl index eebcc56c4..c211ba19f 100755 --- a/myapp/bin/pumactl +++ b/myapp/bin/pumactl @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/myapp/bin/rake b/myapp/bin/rake index 849244445..48fa796f6 100755 --- a/myapp/bin/rake +++ b/myapp/bin/rake @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/myapp/bin/rspec b/myapp/bin/rspec index c9fea7c29..18a257fd8 100755 --- a/myapp/bin/rspec +++ b/myapp/bin/rspec @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/myapp/bin/rubocop b/myapp/bin/rubocop index ba009623e..f7e8a3892 100755 --- a/myapp/bin/rubocop +++ b/myapp/bin/rubocop @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/myapp/bin/sidekiq b/myapp/bin/sidekiq index 546e77814..7e2c82318 100755 --- a/myapp/bin/sidekiq +++ b/myapp/bin/sidekiq @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/myapp/bin/sidekiqmon b/myapp/bin/sidekiqmon index 768f6ab77..8408de9cd 100755 --- a/myapp/bin/sidekiqmon +++ b/myapp/bin/sidekiqmon @@ -15,7 +15,7 @@ ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../../Gemfile", bundle_binstub = File.expand_path("bundle", __dir__) if File.file?(bundle_binstub) - if /This file was generated by Bundler/.match?(File.read(bundle_binstub, 300)) + if File.read(bundle_binstub, 300).include?("This file was generated by Bundler") load(bundle_binstub) else abort("Your `bin/bundle` was not generated by Bundler, so this binstub cannot run. diff --git a/myapp/cable.ru b/myapp/cable.ru index 9235ca943..19c52fb40 100644 --- a/myapp/cable.ru +++ b/myapp/cable.ru @@ -2,7 +2,7 @@ # This file is used by Rack-based servers to start the application. -require ::File.expand_path("config/environment", __dir__) +require File.expand_path("config/environment", __dir__) Rails.application.eager_load! run ActionCable.server diff --git a/myapp/config.ru b/myapp/config.ru index 90f60786d..e7d005c78 100644 --- a/myapp/config.ru +++ b/myapp/config.ru @@ -2,6 +2,6 @@ # This file is used by Rack-based servers to start the application. -require ::File.expand_path("config/environment", __dir__) +require File.expand_path("config/environment", __dir__) run Rails.application diff --git a/myapp/config/environments/production.rb b/myapp/config/environments/production.rb index 73b33d3ba..5912b48e4 100644 --- a/myapp/config/environments/production.rb +++ b/myapp/config/environments/production.rb @@ -76,7 +76,7 @@ config.active_support.deprecation = :notify # Use default logging formatter so that PID and timestamp are not suppressed. - config.log_formatter = ::Logger::Formatter.new + config.log_formatter = Logger::Formatter.new # Use a different logger for distributed setups. # require 'syslog/logger' diff --git a/spec/sidekiq/api_spec.rb b/spec/sidekiq/api_spec.rb index 8217e5293..40cce8325 100644 --- a/spec/sidekiq/api_spec.rb +++ b/spec/sidekiq/api_spec.rb @@ -12,17 +12,17 @@ describe Sidekiq::SortedEntry::UniqueExtension do it "deletes uniqueness lock on delete" do expect(JustAWorker.perform_in(60 * 60 * 3, foo: "bar")).to be_truthy - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty Sidekiq::ScheduledSet.new.each(&:delete) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty expect(JustAWorker.perform_in(60 * 60 * 3, boo: "far")).to be_truthy end it "deletes uniqueness lock on remove_job" do expect(JustAWorker.perform_in(60 * 60 * 3, foo: "bar")).to be_truthy - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty Sidekiq::ScheduledSet.new.each do |entry| entry.send(:remove_job) do |message| @@ -44,7 +44,7 @@ ) end end - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty expect(JustAWorker.perform_in(60 * 60 * 3, boo: "far")).to be_truthy end end @@ -53,18 +53,18 @@ describe Sidekiq::JobRecord::UniqueExtension do it "deletes uniqueness lock on delete" do jid = JustAWorker.perform_async(roo: "baf") - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty Sidekiq::Queue.new("testqueue").find_job(jid).delete - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end else describe Sidekiq::Job::UniqueExtension do it "deletes uniqueness lock on delete" do jid = JustAWorker.perform_async(roo: "baf") - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty Sidekiq::Queue.new("testqueue").find_job(jid).delete - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end end @@ -72,38 +72,38 @@ describe Sidekiq::Queue::UniqueExtension do it "deletes uniqueness locks on clear" do JustAWorker.perform_async(oob: "far") - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty Sidekiq::Queue.new("testqueue").clear - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end describe Sidekiq::JobSet::UniqueExtension do it "deletes uniqueness locks on clear" do JustAWorker.perform_in(60 * 60 * 3, roo: "fab") - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty Sidekiq::JobSet.new("schedule").clear - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end describe Sidekiq::ScheduledSet::UniqueExtension do it "deletes uniqueness locks on clear" do JustAWorker.perform_in(60 * 60 * 3, roo: "fab") - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty Sidekiq::ScheduledSet.new.clear - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end it "deletes uniqueness locks on delete_by_score" do JustAWorker.perform_in(60 * 60 * 3, roo: "fab") - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty scheduled_set = Sidekiq::ScheduledSet.new scheduled_set.each do |job| scheduled_set.delete(job.score, job.jid) end - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end end diff --git a/spec/sidekiq_unique_jobs/batch_delete_spec.rb b/spec/sidekiq_unique_jobs/batch_delete_spec.rb index 6ace830e2..98265da22 100644 --- a/spec/sidekiq_unique_jobs/batch_delete_spec.rb +++ b/spec/sidekiq_unique_jobs/batch_delete_spec.rb @@ -26,8 +26,8 @@ call locks.all? do |lock| - expect(lock.all_jids).to match_array([]) - expect(unique_keys).to match_array([]) + expect(lock.all_jids).to be_empty + expect(unique_keys).to be_empty end end end diff --git a/spec/sidekiq_unique_jobs/changelog_spec.rb b/spec/sidekiq_unique_jobs/changelog_spec.rb index 1554d6ff1..c6e57f811 100644 --- a/spec/sidekiq_unique_jobs/changelog_spec.rb +++ b/spec/sidekiq_unique_jobs/changelog_spec.rb @@ -174,13 +174,13 @@ } end - it { is_expected.to match_array([locked_entry, queued_entry]) } + it { is_expected.to contain_exactly(locked_entry, queued_entry) } context "when given count 1" do let(:count) { 1 } # count only is considered per iteration, this would have iterated twice - it { is_expected.to match_array([locked_entry, queued_entry]) } + it { is_expected.to contain_exactly(locked_entry, queued_entry) } end end end @@ -193,7 +193,7 @@ let(:page_size) { 1 } context "when no entries exist" do - it { is_expected.to match_array([0, 0, []]) } + it { is_expected.to contain_exactly(0, 0, []) } end context "when entries exist" do @@ -221,7 +221,7 @@ } end - it { is_expected.to match_array([2, anything, a_collection_including(kind_of(Hash))]) } + it { is_expected.to contain_exactly(2, anything, a_collection_including(kind_of(Hash))) } end end end diff --git a/spec/sidekiq_unique_jobs/core_ext_spec.rb b/spec/sidekiq_unique_jobs/core_ext_spec.rb index 6d29bc099..8964ee8e7 100644 --- a/spec/sidekiq_unique_jobs/core_ext_spec.rb +++ b/spec/sidekiq_unique_jobs/core_ext_spec.rb @@ -25,6 +25,20 @@ end end + describe "ExtendedHash" do + before do + stub_const("ExtendedHash", Hash) + end + + let(:hash) { ExtendedHash[:test, :me, :not, :me] } + + describe "#deep_transform_keys" do + subject(:deep_transform_keys) { hash.deep_transform_keys(&:to_s) } + + it { is_expected.to be_a(ExtendedHash) } + end + end + describe Array do let(:array) { [1, 2, nil, last_argument] } let(:last_argument) { Object.new } diff --git a/spec/sidekiq_unique_jobs/digests_spec.rb b/spec/sidekiq_unique_jobs/digests_spec.rb index 1e5368413..fbbb26586 100644 --- a/spec/sidekiq_unique_jobs/digests_spec.rb +++ b/spec/sidekiq_unique_jobs/digests_spec.rb @@ -129,7 +129,7 @@ it "deletes all matching digests" do expect(delete_by_pattern).to be_a(Integer) - expect(digests.entries).to match_array([]) + expect(digests.entries).to be_empty end it "logs performance info" do diff --git a/spec/sidekiq_unique_jobs/job_spec.rb b/spec/sidekiq_unique_jobs/job_spec.rb index 7f27c2898..32129aee2 100644 --- a/spec/sidekiq_unique_jobs/job_spec.rb +++ b/spec/sidekiq_unique_jobs/job_spec.rb @@ -21,6 +21,7 @@ "class" => job_class, "queue" => queue, "args" => [1, 2], + "lock" => :until_executed, "lock_timeout" => MyUniqueJob.get_sidekiq_options["lock_timeout"].to_i, "lock_ttl" => MyUniqueJob.get_sidekiq_options["lock_ttl"], "lock_args" => args, diff --git a/spec/sidekiq_unique_jobs/lock/while_executing_spec.rb b/spec/sidekiq_unique_jobs/lock/while_executing_spec.rb index c11bd96e7..ba5448127 100644 --- a/spec/sidekiq_unique_jobs/lock/while_executing_spec.rb +++ b/spec/sidekiq_unique_jobs/lock/while_executing_spec.rb @@ -77,7 +77,7 @@ context "when no callback is defined" do let(:job_class) { WhileExecutingRescheduleJob } let(:callback_one) { -> { true } } - let(:callback_two) { nil } + let(:callback_two) { -> { true } } let(:strategy_one) { process_one.send(:server_strategy) } let(:strategy_two) { process_two.send(:server_strategy) } @@ -85,19 +85,25 @@ before do allow(strategy_one).to receive(:call).and_call_original allow(strategy_two).to receive(:call).and_call_original + allow(process_one).to receive(:reflect).and_call_original allow(process_two).to receive(:reflect).and_call_original end it "reflects execution_failed" do process_one.execute do process_two.execute { puts "BOGUS!" } + # NOTE: Below looks weird but tests that + # the result from process_two (which is nil) isn't considered. + jid_one end + expect(process_one).not_to have_received(:reflect).with(:execution_failed, item_one) expect(callback_one).to have_received(:call).once expect(strategy_one).not_to have_received(:call) - expect(strategy_two).to have_received(:call).once expect(process_two).to have_received(:reflect).with(:execution_failed, item_two) + expect(callback_two).not_to have_received(:call) + expect(strategy_two).to have_received(:call).once end end diff --git a/spec/sidekiq_unique_jobs/lock_spec.rb b/spec/sidekiq_unique_jobs/lock_spec.rb index 89b991ad1..ea1503d07 100644 --- a/spec/sidekiq_unique_jobs/lock_spec.rb +++ b/spec/sidekiq_unique_jobs/lock_spec.rb @@ -45,7 +45,7 @@ it "creates all expected keys in redis" do create - expect(keys).to match_array([key.digest, key.locked, key.info, key.changelog, key.digests]) + expect(keys).to contain_exactly(key.digest, key.locked, key.info, key.changelog, key.digests) expect(create.locked_jids).to include(job_id) end end @@ -60,7 +60,7 @@ context "when locks exists" do before { simulate_lock(key, job_id) } - it { is_expected.to match_array([job_id]) } + it { is_expected.to contain_exactly(job_id) } end end @@ -70,7 +70,7 @@ it "creates keys and adds job_id to locked hash" do expect { lock }.to change { entity.locked_jids }.to([job_id]) - expect(keys).to match_array([key.digest, key.locked, key.info, key.changelog, key.digests]) + expect(keys).to contain_exactly(key.digest, key.locked, key.info, key.changelog, key.digests) end end @@ -82,7 +82,7 @@ it "creates keys and adds job_id to locked hash" do expect { lock }.to change { entity.locked_jids }.to([job_id]) del - expect(keys).not_to match_array([key.digest, key.locked, key.info, key.changelog, key.digests]) + expect(keys).not_to contain_exactly(key.digest, key.locked, key.info, key.changelog, key.digests) end end @@ -115,7 +115,7 @@ } end - it { is_expected.to match_array([locked_entry, queued_entry]) } + it { is_expected.to contain_exactly(locked_entry, queued_entry) } end end end diff --git a/spec/sidekiq_unique_jobs/locksmith_spec.rb b/spec/sidekiq_unique_jobs/locksmith_spec.rb index 5f0477cb2..b7b050662 100644 --- a/spec/sidekiq_unique_jobs/locksmith_spec.rb +++ b/spec/sidekiq_unique_jobs/locksmith_spec.rb @@ -338,10 +338,10 @@ # expect(locksmith_two).not_to have_received(:reflect).with(:locked, item_two) # end - # it "reflects on unlocked" do - # locksmith_one.lock - # allow(locksmith_one).to receive(:reflect) - # locksmith_one.unlock - # expect(locksmith_one).to have_received(:reflect).with(:unlocked, item_one) - # end + it "reflects on unlocked" do + locksmith_one.lock + allow(locksmith_one).to receive(:reflect) + locksmith_one.unlock + expect(locksmith_one).to have_received(:reflect).with(:unlocked, item_one) + end end diff --git a/spec/sidekiq_unique_jobs/lua/lock_spec.rb b/spec/sidekiq_unique_jobs/lua/lock_spec.rb index ed0dddb6d..98bb92de0 100644 --- a/spec/sidekiq_unique_jobs/lua/lock_spec.rb +++ b/spec/sidekiq_unique_jobs/lua/lock_spec.rb @@ -157,7 +157,7 @@ expect(primed.count).to eq(0) expect(locked.count).to eq(1) - expect(locked.entries).to match_array([job_id_one]) + expect(locked.entries).to contain_exactly(job_id_one) expect(locked[job_id_one].to_f).to be_within(0.5).of(now_f) end end @@ -176,7 +176,7 @@ expect(primed.count).to eq(0) expect(locked.count).to eq(1) - expect(locked.entries).to match_array([job_id_one]) + expect(locked.entries).to contain_exactly(job_id_one) expect(locked[job_id_one].to_f).to be_within(0.5).of(now_f) end end @@ -197,7 +197,7 @@ expect(primed.count).to eq(0) expect(locked.count).to eq(1) - expect(locked.entries).to match_array([job_id_one]) + expect(locked.entries).to contain_exactly(job_id_one) expect(locked[job_id_one].to_f).to be_within(0.5).of(now_f) end end @@ -220,7 +220,7 @@ expect(primed.count).to eq(0) expect(locked.count).to eq(1) - expect(locked.entries).to match_array([job_id_two]) + expect(locked.entries).to contain_exactly(job_id_two) expect(locked[job_id_two].to_f).to be_within(0.5).of(now_f) end end @@ -247,7 +247,7 @@ expect(primed.count).to eq(0) expect(locked.count).to eq(2) - expect(locked.entries).to match_array([job_id_two, job_id_one]) + expect(locked.entries).to contain_exactly(job_id_two, job_id_one) expect(locked[job_id_two].to_f).to be_within(0.5).of(now_f) expect(locked[job_id_one].to_f).to be_within(0.5).of(now_f) end @@ -272,7 +272,7 @@ expect(primed.count).to eq(0) expect(locked.count).to eq(1) - expect(locked.entries).to match_array([job_id_one]) + expect(locked.entries).to contain_exactly(job_id_one) expect(locked[job_id_one].to_f).to be_within(0.5).of(now_f) end end diff --git a/spec/sidekiq_unique_jobs/lua/queue_spec.rb b/spec/sidekiq_unique_jobs/lua/queue_spec.rb index ab13e090b..f4c2268e5 100644 --- a/spec/sidekiq_unique_jobs/lua/queue_spec.rb +++ b/spec/sidekiq_unique_jobs/lua/queue_spec.rb @@ -67,7 +67,7 @@ expect(get(key.digest)).to eq(job_id_two) expect(pttl(key.digest)).to eq(-1) # key exists without pttl expect(llen(key.queued)).to eq(1) - expect(lrange(key.queued, 0, -1)).to match_array([job_id_two]) + expect(lrange(key.queued, 0, -1)).to contain_exactly(job_id_two) expect(rpop(key.queued)).to eq(job_id_two) expect(exists(key.primed)).to be(false) expect(exists(key.locked)).to be(false) @@ -84,7 +84,7 @@ expect(get(key.digest)).to eq(job_id_one) expect(pttl(key.digest)).to eq(-1) # key exists without pttl expect(llen(key.queued)).to eq(2) - expect(lrange(key.queued, 0, -1)).to match_array([job_id_two, job_id_one]) + expect(lrange(key.queued, 0, -1)).to contain_exactly(job_id_two, job_id_one) expect(rpop(key.queued)).to eq(job_id_two) expect(exists(key.primed)).to be(false) expect(exists(key.locked)).to be(false) @@ -104,7 +104,7 @@ expect(get(key.digest)).to eq(job_id_one) expect(pttl(key.digest)).to eq(-1) # key exists without pttl expect(llen(key.queued)).to eq(1) - expect(lrange(key.queued, 0, -1)).to match_array([job_id_one]) + expect(lrange(key.queued, 0, -1)).to contain_exactly(job_id_one) expect(rpop(key.queued)).to eq(job_id_one) expect(exists(key.primed)).to be(false) expect(exists(key.locked)).to be(false) @@ -141,7 +141,7 @@ expect(queue).to eq(job_id_one) expect(get(key.digest)).to eq(job_id_one) expect(llen(key.queued)).to eq(1) # There should be no keys available to be locked - expect(lrange(key.queued, 0, -1)).to match_array([job_id_one]) + expect(lrange(key.queued, 0, -1)).to contain_exactly(job_id_one) expect(llen(key.primed)).to eq(0) expect(exists(key.locked)).to be(true) expect(hexists(key.locked, job_id_two)).to be(true) diff --git a/spec/sidekiq_unique_jobs/lua/reap_orphans_spec.rb b/spec/sidekiq_unique_jobs/lua/reap_orphans_spec.rb index 0d0e0a265..d62029266 100644 --- a/spec/sidekiq_unique_jobs/lua/reap_orphans_spec.rb +++ b/spec/sidekiq_unique_jobs/lua/reap_orphans_spec.rb @@ -61,7 +61,7 @@ context "without scheduled job" do it "keeps the digest" do expect { reap_orphans }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end @@ -70,7 +70,7 @@ it "keeps the digest" do expect { reap_orphans }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -81,7 +81,7 @@ context "without job in retry" do it "keeps the digest" do expect { reap_orphans }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end @@ -90,7 +90,7 @@ it "keeps the digest" do expect { reap_orphans }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -99,7 +99,7 @@ context "without enqueued job" do it "keeps the digest" do expect { reap_orphans }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end @@ -108,7 +108,7 @@ it "keeps the digest" do expect { reap_orphans }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -117,14 +117,14 @@ context "without job" do it "keeps the digest" do expect { reap_orphans }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end context "with job" do let(:process_key) { "process-id" } let(:thread_id) { "thread-id" } - let(:worker_key) { "#{process_key}:workers" } + let(:worker_key) { "#{process_key}:work" } before do SidekiqUniqueJobs.redis do |conn| @@ -143,7 +143,7 @@ it "keeps the digest" do expect { reap_orphans }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end diff --git a/spec/sidekiq_unique_jobs/lua/unlock_spec.rb b/spec/sidekiq_unique_jobs/lua/unlock_spec.rb index 86ffacf05..3319d427d 100644 --- a/spec/sidekiq_unique_jobs/lua/unlock_spec.rb +++ b/spec/sidekiq_unique_jobs/lua/unlock_spec.rb @@ -176,7 +176,7 @@ expect(primed.count).to be == 0 expect(locked.count).to be == 1 - expect(locked.entries).to match_array([job_id_two]) + expect(locked.entries).to contain_exactly(job_id_two) expect(locked[job_id_two].to_f).to be_within(0.5).of(now_f) end end @@ -187,12 +187,12 @@ .and change { digests.count }.by(-1) expect(queued.count).to eq(1) - expect(queued.entries).to match_array(["1"]) + expect(queued.entries).to contain_exactly("1") expect(primed.count).to be == 0 expect(locked.count).to be == 0 - expect(locked.entries).to match_array([]) + expect(locked.entries).to be_empty expect(locked[job_id_one]).to be_nil end end diff --git a/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb b/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb index 30440a713..dce70a1a1 100644 --- a/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb +++ b/spec/sidekiq_unique_jobs/orphans/reaper_spec.rb @@ -68,7 +68,7 @@ context "without scheduled job" do it "deletes the digest" do expect { call }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end @@ -77,7 +77,7 @@ it "keeps the digest" do expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -88,7 +88,7 @@ context "without job in retry" do it "deletes the digest" do expect { call }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end @@ -97,7 +97,7 @@ it "keeps the digest" do expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -106,7 +106,7 @@ context "without enqueued job" do it "deletes the digest" do expect { call }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end @@ -115,7 +115,7 @@ it "keeps the digest" do expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -124,14 +124,14 @@ context "without job in process" do it "deletes the digest" do expect { call }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end context "with job in process" do let(:process_key) { "process-id" } let(:thread_id) { "thread-id" } - let(:worker_key) { "#{process_key}:workers" } + let(:worker_key) { "#{process_key}:work" } let(:created_at) { (Time.now - reaper_timeout).to_f } let(:reaper_timeout) { SidekiqUniqueJobs.config.reaper_timeout } @@ -159,7 +159,7 @@ it "keeps the digest" do expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -170,7 +170,7 @@ it "keeps the digest" do expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end @@ -179,7 +179,7 @@ it "keeps the digest" do expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end @@ -192,7 +192,7 @@ it "deletes the digest" do expect { call }.to change { digests.count }.by(-1) - expect(unique_keys).to match_array([]) + expect(unique_keys).to be_empty end end @@ -201,7 +201,7 @@ it "keeps the digest" do expect { call }.not_to change { digests.count }.from(1) - expect(unique_keys).not_to match_array([]) + expect(unique_keys).not_to be_empty end end end diff --git a/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb b/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb index be2b74949..76d222832 100644 --- a/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb +++ b/spec/sidekiq_unique_jobs/orphans/ruby_reaper_spec.rb @@ -52,7 +52,7 @@ end it "does not check for orphans" do - expect(orphans).to match_array([]) + expect(orphans).to be_empty expect(service).not_to have_received(:belongs_to_job?) end end @@ -75,7 +75,7 @@ SidekiqUniqueJobs.use_config(reaper_count: 1) do expect(orphans.size).to eq(1) # This is the first one to be created and should therefor be the only match - expect(orphans).to match_array([digest]) + expect(orphans).to contain_exactly(digest) end end end @@ -84,7 +84,7 @@ let(:item) { raw_item.merge("at" => Time.now.to_f + 3_600) } context "without scheduled job" do - it { is_expected.to match_array([digest]) } + it { is_expected.to contain_exactly(digest) } end context "with scheduled job" do @@ -98,7 +98,7 @@ let(:item) { raw_item.merge("retry_count" => 2, "failed_at" => now_f) } context "without job in retry" do - it { is_expected.to match_array([digest]) } + it { is_expected.to contain_exactly(digest) } end context "with job in retry" do @@ -110,7 +110,7 @@ context "when digest exists in a queue" do context "without enqueued job" do - it { is_expected.to match_array([digest]) } + it { is_expected.to contain_exactly(digest) } end context "with enqueued job" do diff --git a/spec/sidekiq_unique_jobs/redis/hash_spec.rb b/spec/sidekiq_unique_jobs/redis/hash_spec.rb index e1351c09a..c8ac4fa17 100644 --- a/spec/sidekiq_unique_jobs/redis/hash_spec.rb +++ b/spec/sidekiq_unique_jobs/redis/hash_spec.rb @@ -23,7 +23,7 @@ context "when with_values: false" do let(:with_values) { false } - it { is_expected.to match_array([job_id]) } + it { is_expected.to contain_exactly(job_id) } end context "when with_values: true" do diff --git a/spec/sidekiq_unique_jobs/redis/set_spec.rb b/spec/sidekiq_unique_jobs/redis/set_spec.rb index 3f24bb275..decdd270d 100644 --- a/spec/sidekiq_unique_jobs/redis/set_spec.rb +++ b/spec/sidekiq_unique_jobs/redis/set_spec.rb @@ -16,7 +16,7 @@ context "with entries" do before { sadd(digest, job_id) } - it { is_expected.to match_array([job_id]) } + it { is_expected.to contain_exactly(job_id) } end end diff --git a/spec/sidekiq_unique_jobs/redis/sorted_set_spec.rb b/spec/sidekiq_unique_jobs/redis/sorted_set_spec.rb index 95a974b6a..eb79ed6f4 100644 --- a/spec/sidekiq_unique_jobs/redis/sorted_set_spec.rb +++ b/spec/sidekiq_unique_jobs/redis/sorted_set_spec.rb @@ -19,7 +19,7 @@ context "when given with_scores: false" do let(:with_scores) { false } - it { is_expected.to match_array([job_id]) } + it { is_expected.to contain_exactly(job_id) } end context "when given with_scores: true" do diff --git a/spec/support/shared_examples/a_lockable_lock.rb b/spec/support/shared_examples/a_lockable_lock.rb index 220f4b515..a3e175add 100644 --- a/spec/support/shared_examples/a_lockable_lock.rb +++ b/spec/support/shared_examples/a_lockable_lock.rb @@ -35,61 +35,3 @@ end end end - -RSpec.shared_examples "an executing lock implementation" do - context "when job can't be locked" do - before do - allow(process_one.locksmith).to receive(:execute).and_return(nil) - end - - it "does not execute" do - unset = true - process_one.execute { unset = false } - expect(unset).to be(true) - end - end - - context "when process_one executes the job" do - before { process_one.lock } - - it "keeps being locked while executing" do - process_one.execute do - expect(process_one).to be_locked - end - end - - it "keeps being locked when an error is raised" do - allow(process_one.locksmith).to receive(:execute).and_raise(RuntimeError, "Hell") - - expect { process_one.execute { "hey ho" } }.to raise_error("Hell") - - expect(process_one).to be_locked - end - - it "prevents process_two from locking" do - process_one.execute do - expect(process_two.lock).to be_nil - expect(process_two).not_to be_locked - end - end - - it "prevents process_two from executing" do - expect { process_two.execute { raise "Hell" } }.not_to raise_error - end - - it "reflects execution_failed on failure" do - allow(process_two).to receive(:reflect).and_call_original - process_two.execute { puts "Failed to execute" } - - expect(process_two).to have_received(:reflect).with(:execution_failed, item_two) - end - - it "yields without arguments" do - process_one.lock - process_one.execute {} - blk = -> {} - - expect { process_one.execute(&blk) }.not_to raise_error - end - end -end diff --git a/spec/support/shared_examples/an_executing_lock_implementation.rb b/spec/support/shared_examples/an_executing_lock_implementation.rb new file mode 100644 index 000000000..90f23eed8 --- /dev/null +++ b/spec/support/shared_examples/an_executing_lock_implementation.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +RSpec.shared_examples "an executing lock implementation" do + context "when job can't be locked" do + before do + allow(process_one.locksmith).to receive(:execute).and_return(nil) + end + + it "does not execute" do + unset = true + process_one.execute { unset = false } + expect(unset).to be(true) + end + end + + context "when process_one executes the job" do + before { process_one.lock } + + it "keeps being locked while executing" do + process_one.execute do + expect(process_one).to be_locked + end + end + + it "keeps being locked when an error is raised" do + allow(process_one.locksmith).to receive(:execute).and_raise(RuntimeError, "Hell") + + expect { process_one.execute { "hey ho" } }.to raise_error("Hell") + + expect(process_one).to be_locked + end + + it "prevents process_two from locking" do + process_one.execute do + expect(process_two.lock).to be_nil + expect(process_two).not_to be_locked + end + end + + it "prevents process_two from executing" do + expect { process_two.execute { raise "Hell" } }.not_to raise_error + end + + it "reflects execution_failed on failure" do + allow(process_two).to receive(:reflect).and_call_original + process_two.execute { puts "Failed to execute" } + + expect(process_two).to have_received(:reflect).with(:execution_failed, item_two) + end + + it "yields without arguments" do + process_one.lock + process_one.execute {} + blk = -> {} + + expect { process_one.execute(&blk) }.not_to raise_error + end + end +end diff --git a/spec/support/workers/custom_queue_job_with_filter_proc.rb b/spec/support/workers/custom_queue_job_with_filter_proc.rb index 4bd203136..75dd90b84 100644 --- a/spec/support/workers/custom_queue_job_with_filter_proc.rb +++ b/spec/support/workers/custom_queue_job_with_filter_proc.rb @@ -2,7 +2,7 @@ # :nocov: -require_relative "./custom_queue_job" +require_relative "custom_queue_job" class CustomQueueJobWithFilterProc < CustomQueueJob # slightly contrived example of munging args to the diff --git a/spec/workers/bad_worker_spec.rb b/spec/workers/bad_worker_spec.rb index 5752a60ce..7b225198b 100644 --- a/spec/workers/bad_worker_spec.rb +++ b/spec/workers/bad_worker_spec.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true RSpec.describe BadWorker do - xit { expect(described_class).to have_valid_sidekiq_options } + it do + expect(described_class).not_to have_valid_sidekiq_options + end end
Information about lock