From c9909e7a9c86a52d17e9cd7cda9e9207768cbe49 Mon Sep 17 00:00:00 2001 From: Mikael Henriksson Date: Sat, 21 Mar 2020 20:11:46 +0100 Subject: [PATCH] Fix access to lock (#476) * Bump gems * Bump ruby version for coverage ## To make a commit, type your commit message and press SUPER-ENTER. ## To cancel the commit, close the window. To sign off on the commit, ## press SUPER-S. ## ## You may also reference or close a GitHub issue with this commit. ## To do so, type `#` followed by the `tab` key. You will be shown a ## list of issues related to the current repo. You may also type ## `owner/repo#` plus the `tab` key to reference an issue in a ## different GitHub repo. spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 15d8d26..6a7140b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,7 +2,7 @@ require "bundler/setup" -if RUBY_ENGINE == "ruby" && RUBY_VERSION >= "2.5" && RUBY_VERSION < "2.6" +if RUBY_ENGINE == "ruby" && RUBY_VERSION >= "2.6" && RUBY_VERSION < "2.7" require "simplecov" unless %w[false 0].include?(ENV["COV"]) begin * Fallback to UNIQUE_KEY * Fix console spec * Mandatory rubocop commit * Due to bug in rack https://github.com/rack/rack/pull/1610 * Fix broken spec * Da fork jRuby...? * Skip spec for jruby * sodsohdoasdhifa;sodb ;oqwuefg2398yr 2t89f ouwefqo --- .rubocop.yml | 16 ++++-- .simplecov | 7 +-- .travis.yml | 8 +-- Gemfile | 4 +- gemfiles/sidekiq_4.0.gemfile | 8 ++- gemfiles/sidekiq_4.1.gemfile | 8 ++- gemfiles/sidekiq_4.2.gemfile | 8 ++- gemfiles/sidekiq_5.0.gemfile | 8 ++- gemfiles/sidekiq_5.1.gemfile | 8 ++- gemfiles/sidekiq_5.2.gemfile | 8 ++- gemfiles/sidekiq_6.0.gemfile | 8 ++- gemfiles/sidekiq_develop.gemfile | 8 ++- lib/sidekiq_unique_jobs/cli.rb | 5 +- lib/sidekiq_unique_jobs/locksmith.rb | 2 +- lib/sidekiq_unique_jobs/version_check.rb | 2 +- lib/sidekiq_unique_jobs/web.rb | 4 +- .../client/middleware_spec.rb | 2 +- .../lock/while_executing_spec.rb | 4 +- .../sidekiq_unique_jobs/web_spec.rb | 1 - spec/spec_helper.rb | 6 +-- spec/unit/sidekiq_unique_jobs/cli_spec.rb | 53 ++++++++++--------- .../on_conflict/strategy_spec.rb | 6 --- .../sidekiq_worker_methods_spec.rb | 2 +- 23 files changed, 125 insertions(+), 61 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 1f3245092..82fa43cdd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -30,7 +30,7 @@ Lint/AmbiguousBlockAssociation: Exclude: - "spec/**/*" -Lint/HandleExceptions: +Lint/SuppressedException: Enabled: true Lint/UselessAssignment: @@ -42,7 +42,7 @@ Metrics/AbcSize: Metrics/CyclomaticComplexity: Max: 7 -Metrics/LineLength: +Layout/LineLength: Max: 120 Metrics/MethodLength: @@ -54,6 +54,7 @@ Metrics/BlockLength: - "**/spec/**/*.rb" - "**/*.rake" - "Rakefile" + - "Gemfile" - "*.gemspec" Metrics/PerceivedComplexity: @@ -73,7 +74,7 @@ Naming/FileName: Naming/RescuedExceptionsVariableName: PreferredName: ex -Naming/UncommunicativeMethodParamName: +Naming/MethodParameterName: AllowedNames: - ex @@ -134,6 +135,15 @@ Style/FrozenStringLiteralComment: Style/GlobalVars: Enabled: true +Style/HashEachMethods: + Enabled: false + +Style/HashTransformKeys: + Enabled: false + +Style/HashTransformValues: + Enabled: false + Style/ModuleFunction: Enabled: false diff --git a/.simplecov b/.simplecov index 359ab51b1..5698790eb 100644 --- a/.simplecov +++ b/.simplecov @@ -1,12 +1,13 @@ # frozen_string_literal: true -require "simplecov-json" +require "simplecov-oj" +require "simplecov-material" SimpleCov.command_name "RSpec" # SimpleCov.refuse_coverage_drop SimpleCov.formatters = [ - SimpleCov::Formatter::HTMLFormatter, - SimpleCov::Formatter::JSONFormatter, + SimpleCov::Formatter::MaterialFormatter, + SimpleCov::Formatter::OjFormatter, ] SimpleCov.start do diff --git a/.travis.yml b/.travis.yml index 41e079e57..fd88f5cee 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,21 +37,21 @@ after_script: - if [[ "${COV}" = "true" ]]; then ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT; fi; rvm: - - 2.4.8 + - 2.4.9 matrix: fast_finish: true exclude: - - rvm: 2.4.8 + - rvm: 2.4.9 gemfile: gemfiles/sidekiq_6.0.gemfile - - rvm: 2.4.8 + - rvm: 2.4.9 gemfile: gemfiles/sidekiq_develop.gemfile include: - rvm: 2.5.7 gemfile: gemfiles/sidekiq_6.0.gemfile - - rvm: jruby-9.2.8.0 + - rvm: jruby-9.2.11.0 gemfile: gemfiles/sidekiq_6.0.gemfile - rvm: 2.6.5 gemfile: gemfiles/sidekiq_develop.gemfile diff --git a/Gemfile b/Gemfile index 51503f14c..91a581963 100644 --- a/Gemfile +++ b/Gemfile @@ -32,7 +32,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_4.0.gemfile b/gemfiles/sidekiq_4.0.gemfile index 181443ac7..3ccac81ac 100644 --- a/gemfiles/sidekiq_4.0.gemfile +++ b/gemfiles/sidekiq_4.0.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", "~> 4.0.0" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_4.1.gemfile b/gemfiles/sidekiq_4.1.gemfile index 9cb5dce64..5aab36faf 100644 --- a/gemfiles/sidekiq_4.1.gemfile +++ b/gemfiles/sidekiq_4.1.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", "~> 4.1.0" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_4.2.gemfile b/gemfiles/sidekiq_4.2.gemfile index ec0e1a482..25f6ea032 100644 --- a/gemfiles/sidekiq_4.2.gemfile +++ b/gemfiles/sidekiq_4.2.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", "~> 4.2.0" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_5.0.gemfile b/gemfiles/sidekiq_5.0.gemfile index 715a9dd00..1e47d86c1 100644 --- a/gemfiles/sidekiq_5.0.gemfile +++ b/gemfiles/sidekiq_5.0.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", "~> 5.0.0" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_5.1.gemfile b/gemfiles/sidekiq_5.1.gemfile index b60e68cb1..b0000f8e5 100644 --- a/gemfiles/sidekiq_5.1.gemfile +++ b/gemfiles/sidekiq_5.1.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", "~> 5.1.0" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_5.2.gemfile b/gemfiles/sidekiq_5.2.gemfile index 2b5a9b48a..eee73e563 100644 --- a/gemfiles/sidekiq_5.2.gemfile +++ b/gemfiles/sidekiq_5.2.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", "~> 5.2.0" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_6.0.gemfile b/gemfiles/sidekiq_6.0.gemfile index c64a106f5..f35dc7d82 100644 --- a/gemfiles/sidekiq_6.0.gemfile +++ b/gemfiles/sidekiq_6.0.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", ">= 6.0.pre", "< 6.1" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/gemfiles/sidekiq_develop.gemfile b/gemfiles/sidekiq_develop.gemfile index 091e5a9d5..6752b5aff 100644 --- a/gemfiles/sidekiq_develop.gemfile +++ b/gemfiles/sidekiq_develop.gemfile @@ -7,6 +7,10 @@ gem "rspec-eventually", require: false gem "rspec-its", require: false gem "sidekiq", git: "https://github.com/mperham/sidekiq.git" +platforms :jruby do + gem "pry-debugger-jruby" +end + platforms :mri do gem "benchmark-ips" gem "fasterer" @@ -26,7 +30,9 @@ platforms :mri do gem "rubocop-performance" gem "rubocop-rspec" gem "ruby-prof" - gem "simplecov-json" + gem "simplecov", "< 0.18" + gem "simplecov-material" + gem "simplecov-oj" gem "stackprof" gem "terminal-notifier-guard" gem "test-prof" diff --git a/lib/sidekiq_unique_jobs/cli.rb b/lib/sidekiq_unique_jobs/cli.rb index a2f1c2a89..93f3f6e33 100644 --- a/lib/sidekiq_unique_jobs/cli.rb +++ b/lib/sidekiq_unique_jobs/cli.rb @@ -46,7 +46,10 @@ def console no_commands do def console_class - return IRB if RUBY_PLATFORM == JAVA + if RUBY_PLATFORM == "JAVA" + require "irb" + return IRB + end require "pry" Pry diff --git a/lib/sidekiq_unique_jobs/locksmith.rb b/lib/sidekiq_unique_jobs/locksmith.rb index 13407a33c..81fc31a97 100644 --- a/lib/sidekiq_unique_jobs/locksmith.rb +++ b/lib/sidekiq_unique_jobs/locksmith.rb @@ -18,7 +18,7 @@ def initialize(item, redis_pool = nil) @ttl = item[LOCK_EXPIRATION_KEY] @jid = item[JID_KEY] @unique_digest = item[UNIQUE_DIGEST_KEY] - @lock_type = item[LOCK_KEY] + @lock_type = item[LOCK_KEY] || item[UNIQUE_KEY] @lock_type &&= @lock_type.to_sym @redis_pool = redis_pool end diff --git a/lib/sidekiq_unique_jobs/version_check.rb b/lib/sidekiq_unique_jobs/version_check.rb index d552a23b5..e3a437c6f 100644 --- a/lib/sidekiq_unique_jobs/version_check.rb +++ b/lib/sidekiq_unique_jobs/version_check.rb @@ -7,7 +7,7 @@ module SidekiqUniqueJobs # @author Mikael Henriksson # class VersionCheck - PATTERN = /(?[<>=]+)?\s?(?(\d+.?)+)(\s+&&\s+)?(?[<>=]+)?\s?(?(\d+.?)+)?/m.freeze # rubocop:disable Metrics/LineLength + PATTERN = /(?[<>=]+)?\s?(?(\d+.?)+)(\s+&&\s+)?(?[<>=]+)?\s?(?(\d+.?)+)?/m.freeze # rubocop:disable Layout/LineLength # # Checks if a version is consrtaint is satisfied diff --git a/lib/sidekiq_unique_jobs/web.rb b/lib/sidekiq_unique_jobs/web.rb index dc74ef07f..751768b4f 100644 --- a/lib/sidekiq_unique_jobs/web.rb +++ b/lib/sidekiq_unique_jobs/web.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true begin + require "delegate" + require "rack" require "sidekiq/web" -rescue LoadError # rubocop:disable Lint/HandleExceptions +rescue LoadError # rubocop:disable Lint/SuppressedException # client-only usage end diff --git a/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb b/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb index 3d73d0b2a..b6f866ab6 100644 --- a/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb +++ b/spec/integration/sidekiq_unique_jobs/client/middleware_spec.rb @@ -48,7 +48,7 @@ def self.do_it(_one) ) [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20].each do |x| - ShitClass.delay_for(x, unique: :while_executing).do_it(1) + ShitClass.delay_for(x, queue: 'custom', unique: :while_executing).do_it(1) end expect(schedule_count).to eq(20) diff --git a/spec/integration/sidekiq_unique_jobs/lock/while_executing_spec.rb b/spec/integration/sidekiq_unique_jobs/lock/while_executing_spec.rb index 5876eac5c..cf876786e 100644 --- a/spec/integration/sidekiq_unique_jobs/lock/while_executing_spec.rb +++ b/spec/integration/sidekiq_unique_jobs/lock/while_executing_spec.rb @@ -4,8 +4,8 @@ RSpec.describe SidekiqUniqueJobs::Lock::WhileExecuting, redis: :redis do include SidekiqHelpers - let(:process_one) { described_class.new(item_one, callback_one) } - let(:process_two) { described_class.new(item_two, callback_two) } + let(:process_one) { described_class.new(item_one, callback_one) } + let(:process_two) { described_class.new(item_two, callback_two) } let(:strategy_one) { nil } let(:strategy_two) { nil } diff --git a/spec/integration/sidekiq_unique_jobs/web_spec.rb b/spec/integration/sidekiq_unique_jobs/web_spec.rb index 3736106c6..6f5577d11 100644 --- a/spec/integration/sidekiq_unique_jobs/web_spec.rb +++ b/spec/integration/sidekiq_unique_jobs/web_spec.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "sidekiq/web" require "sidekiq_unique_jobs/web" require "rack/test" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 15d8d2687..be851f512 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -2,7 +2,7 @@ require "bundler/setup" -if RUBY_ENGINE == "ruby" && RUBY_VERSION >= "2.5" && RUBY_VERSION < "2.6" +if RUBY_ENGINE == "ruby" && RUBY_VERSION >= "2.6" && RUBY_VERSION < "2.7" require "simplecov" unless %w[false 0].include?(ENV["COV"]) begin @@ -27,8 +27,8 @@ require "sidekiq/redis_connection" -Dir[File.join(File.dirname(__FILE__), "support", "**", "*.rb")].each { |f| require f } -Dir[File.join(File.dirname(__FILE__), "..", "examples", "**", "*.rb")].each { |f| require f } +Dir[File.join(File.dirname(__FILE__), "support", "**", "*.rb")].sort.each { |f| require f } +Dir[File.join(File.dirname(__FILE__), "..", "examples", "**", "*.rb")].sort.each { |f| require f } RSpec.configure do |config| config.define_derived_metadata do |meta| diff --git a/spec/unit/sidekiq_unique_jobs/cli_spec.rb b/spec/unit/sidekiq_unique_jobs/cli_spec.rb index 192bd1dd5..5071cba42 100644 --- a/spec/unit/sidekiq_unique_jobs/cli_spec.rb +++ b/spec/unit/sidekiq_unique_jobs/cli_spec.rb @@ -88,36 +88,41 @@ def exec(*cmds) end end - describe ".console", ruby_ver: ">= 2.6.5" do - subject(:console) { capture(:stdout) { exec(:console) } } - - let(:header) do - <<~HEADER - Use `keys '*', 1000 to display the first 1000 unique keys matching '*' - Use `del '*', 1000, true (default) to see how many keys would be deleted for the pattern '*' - Use `del '*', 1000, false to delete the first 1000 keys matching '*' - HEADER - end + unless RUBY_PLATFORM == "JAVA" + describe ".console" do + subject(:console) { capture(:stdout) { described_class.start(%w[console]) } } + + shared_examples "start console" do + specify do + allow(console_class).to receive(:start).and_return(true) + expect(console).to include <<~HEADER + Use `keys '*', 1000 to display the first 1000 unique keys matching '*' + Use `del '*', 1000, true (default) to see how many keys would be deleted for the pattern '*' + Use `del '*', 1000, false to delete the first 1000 keys matching '*' + HEADER + end + end - before do - allow(self).to receive(:require).with("pry").and_return(true) - allow(console_class).to receive(:start).and_return(true) - end + context "when Pry is available" do + let(:console_class) { defined?(Pry) ? Pry : IRB } - def stub_console(const) - stub_const(const, Class.new { def self.start; end }) - end + before do + begin + require "pry" + rescue NameError, LoadError, NoMethodError # rubocop:disable Lint/ShadowedException, Lint/SuppressedException + end + end - context "when Pry is available" do - let(:console_class) { stub_console("Pry") } + it_behaves_like "start console" + end - it { is_expected.to include(header) } - end + context "when Pry is unavailable" do + let(:console_class) { IRB } - context "when Pry is unavailable" do - let(:console_class) { stub_console("IRB") } + before { hide_const("Pry") } - it { is_expected.to include(header) } + it_behaves_like "start console" + end end end end diff --git a/spec/unit/sidekiq_unique_jobs/on_conflict/strategy_spec.rb b/spec/unit/sidekiq_unique_jobs/on_conflict/strategy_spec.rb index 4521d077f..bcf8096f2 100644 --- a/spec/unit/sidekiq_unique_jobs/on_conflict/strategy_spec.rb +++ b/spec/unit/sidekiq_unique_jobs/on_conflict/strategy_spec.rb @@ -21,10 +21,4 @@ expect { call }.to raise_error(NotImplementedError, "needs to be implemented in child class") end end - - describe "#replace?" do - subject { strategy.replace? } - - it { is_expected.to eq(false) } - end end diff --git a/spec/unit/sidekiq_unique_jobs/sidekiq_worker_methods_spec.rb b/spec/unit/sidekiq_unique_jobs/sidekiq_worker_methods_spec.rb index d8a84ff30..aac1cc58d 100644 --- a/spec/unit/sidekiq_unique_jobs/sidekiq_worker_methods_spec.rb +++ b/spec/unit/sidekiq_unique_jobs/sidekiq_worker_methods_spec.rb @@ -32,7 +32,7 @@ def initialize(worker_class) it { is_expected.to eq(MyUniqueJob) } end - context "when worker_class is MyUniqueJob" do + context "when worker_class is UntilExecutedJob" do let(:worker_class) { "UntilExecutedJob" } it { is_expected.to eq(UntilExecutedJob) }