Better tests (#382)

* Update ruby versions on travis * Beautify documentation * Bump rubocop versions and config * More documentation layout improvements * Attempt some type of benchmarks * Initiative to easy testing various lock * Add a couple of more test workers * Update README * Run rubocop in example project * Keep working away at lock simulation * Adding more real world worker code/gems * Delete runtime lock when worker raises error * Mandatory rubocop commit * Update appraisals * Move redcarpet into Gemfile to avoid jruby * Rubocop * Rubocop
mhenrixon · Apr 14, 2019 · f6c9b24 · f6c9b24
1 parent 331cffa
commit f6c9b24
Show file tree

Hide file tree

Showing 49 changed files with 362 additions and 92 deletions.
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -1,4 +1,5 @@
 require:
+  - rubocop-performance
   - rubocop-rspec
 
 inherit_mode:
@@ -64,6 +65,9 @@ Naming/FileName:
   Exclude:
     - lib/sidekiq-unique-jobs.rb
 
+Naming/RescuedExceptionsVariableName:
+  PreferredName: ex
+
 Naming/UncommunicativeMethodParamName:
   AllowedNames:
     - ex

diff --git a/.travis.yml b/.travis.yml
@@ -25,22 +25,21 @@ script:
 
 after_script:
   - if [[ "${COV}" = "true" ]]; then ./cc-test-reporter after-build --exit-code $TRAVIS_TEST_RESULT; fi;
-
 rvm:
-  - 2.6.1
+  - 2.6.2
   - 2.4.5
   - 2.3.8
-  - jruby-9.2.5.0
+  - jruby-9.2.7.0
 
 matrix:
   fast_finish: true
   include:
-    - rvm: 2.5.3
+    - rvm: 2.5.5
       gemfile: gemfiles/sidekiq_develop.gemfile
       env: COV=true
-    - rvm: 2.6.0
+    - rvm: 2.6.2
       gemfile: gemfiles/sidekiq_6.0.gemfile
-    - rvm: 2.5.3
+    - rvm: 2.5.5
       gemfile: gemfiles/sidekiq_6.0.gemfile
 
 gemfile:

diff --git a/Gemfile b/Gemfile
@@ -19,8 +19,10 @@ platforms :mri_25 do
   gem "guard-rubocop"
   gem "memory_profiler"
   gem "pry"
+  gem "redcarpet", "~> 3.4"
   gem "reek", ">= 5.3"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/README.md b/README.md
@@ -5,6 +5,8 @@
 - [Introduction](#introduction)
 - [Documentation](#documentation)
 - [Requirements](#requirements)
+  - [ActiveJob](#activejob)
+  - [redis-namespace](#redis-namespace)
 - [Installation](#installation)
 - [Support Me](#support-me)
 - [General Information](#general-information)
@@ -56,9 +58,17 @@ Below are links to the latest major versions (4 & 5):
 
 ## Requirements
 
-See [Sidekiq requirements][] for what is required. Starting from 5.0.0 only sidekiq >= 4 is supported and support for MRI <= 2.1 is dropped. ActiveJob is not supported
+See [Sidekiq requirements][] for what is required. Starting from 5.0.0 only sidekiq >= 4 and MRI >= 2.2. ActiveJob is not supported
 
-Version 6 requires Redis >= 3 and pure Sidekiq, no ActiveJob supported anymore. See [About ActiveJob](https://github.com/mhenrixon/sidekiq-unique-jobs/wiki/About-ActiveJob) for why.
+### ActiveJob
+
+Version 6 requires Redis >= 3 and pure Sidekiq, no ActiveJob supported anymore. See [About ActiveJob](https://github.com/mhenrixon/sidekiq-unique-jobs/wiki/About-ActiveJob) for why. It simply is too complex and generates more issues than I can handle given how little timer I have to spend on this project.
+
+### redis-namespace
+
+Will not be officially supported anymore. Since Mike [won't support redis-namespace](https://github.com/mperham/sidekiq/issues/3366#issuecomment-284270120) neither will I.
+
+[Read this](http://www.mikeperham.com/2017/04/10/migrating-from-redis-namespace/) for how to migrate away from namespacing.
 
 ## Installation
 

diff --git a/bin/bench b/bin/bench
@@ -10,11 +10,27 @@ require "benchmark/ips"
 require "sidekiq-unique-jobs"
 
 ITERATIONS ||= 10_000
+LOCK_TYPES = %w[
+  until_executed
+  until_expired
+  until_and_while_executing
+  until_executing
+  while_executing
+].freeze
 
 Benchmark.ips do |x|
-  x.config(time: 5, warmup: 2)
-  x.report("new_shit") do |_times|
-    SidekiqUniqueJobs::Scripts::AcquireLock.execute(nil, SecureRandom.hex, SecureRandom.hex)
+  x.config(time: 180, warmup: 2)
+  x.report("locksmith lock and unlock") do |_times|
+    item = {
+      "ttl" => rand(10),
+      "jid" => SecureRandom.hex,
+      "unique_digest" => SecureRandom.hex,
+      "lock" => LOCK_TYPES.sample,
+    }
+    locksmith = SidekiqUniqueJobs::Locksmith.new(item)
+    locksmith.lock
+    locksmith.unlock
   end
+
   x.compare!
 end
diff --git a/gemfiles/sidekiq_4.0.gemfile b/gemfiles/sidekiq_4.0.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/gemfiles/sidekiq_4.1.gemfile b/gemfiles/sidekiq_4.1.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/gemfiles/sidekiq_4.2.gemfile b/gemfiles/sidekiq_4.2.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/gemfiles/sidekiq_5.0.gemfile b/gemfiles/sidekiq_5.0.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/gemfiles/sidekiq_5.1.gemfile b/gemfiles/sidekiq_5.1.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/gemfiles/sidekiq_5.2.gemfile b/gemfiles/sidekiq_5.2.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/gemfiles/sidekiq_6.0.gemfile b/gemfiles/sidekiq_6.0.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/gemfiles/sidekiq_develop.gemfile b/gemfiles/sidekiq_develop.gemfile
@@ -19,7 +19,9 @@ platforms :mri_25 do
   gem "memory_profiler"
   gem "pry"
   gem "reek", ">= 5.3"
+  gem "redcarpet", "~> 3.4"
   gem "rubocop"
+  gem "rubocop-performance"
   gem "rubocop-rspec"
   gem "simplecov-json"
   gem "travis"

diff --git a/lib/sidekiq_unique_jobs/lock/until_and_while_executing.rb b/lib/sidekiq_unique_jobs/lock/until_and_while_executing.rb
@@ -19,7 +19,6 @@ def execute
         return unless locked?
 
         unlock
-
         runtime_lock.execute { yield }
       end
 

diff --git a/lib/sidekiq_unique_jobs/lock/while_executing.rb b/lib/sidekiq_unique_jobs/lock/while_executing.rb
@@ -16,6 +16,7 @@ class WhileExecuting < BaseLock
       # @param [Hash] item the Sidekiq job hash
       # @param [Proc] callback callback to call after unlock
       # @param [Sidekiq::RedisConnection, ConnectionPool] redis_pool the redis connection
+      #
       def initialize(item, callback, redis_pool = nil)
         super(item, callback, redis_pool)
         append_unique_key_suffix
@@ -34,7 +35,12 @@ def lock
       def execute
         return strategy.call unless locksmith.lock(item[LOCK_TIMEOUT_KEY])
 
-        with_cleanup { yield }
+        yield
+      rescue Exception # rubocop:disable Lint/RescueException
+        delete!
+        raise
+      else
+        unlock_with_callback
       end
 
       private

diff --git a/lib/sidekiq_unique_jobs/lock/while_executing_reject.rb b/lib/sidekiq_unique_jobs/lock/while_executing_reject.rb
@@ -11,14 +11,6 @@ class Lock
     #
     # @author Mikael Henriksson <mikael@zoolutions.se>
     class WhileExecutingReject < WhileExecuting
-      # Executes in the Sidekiq server process
-      # @yield to the worker class perform method
-      def execute
-        return strategy.call unless locksmith.lock(item[LOCK_TIMEOUT_KEY])
-
-        with_cleanup { yield }
-      end
-
       # Overridden with a forced {OnConflict::Reject} strategy
       # @return [OnConflict::Reject] a reject strategy
       def strategy

diff --git a/lib/sidekiq_unique_jobs/locksmith.rb b/lib/sidekiq_unique_jobs/locksmith.rb
@@ -23,7 +23,10 @@ def initialize(item, redis_pool = nil)
       @redis_pool    = redis_pool
     end
 
+    #
     # Deletes the lock unless it has a ttl set
+    #
+    #
     def delete
       return if ttl
 
@@ -39,11 +42,14 @@ def delete!
       )
     end
 
+    #
     # Create a lock for the item
+    #
     # @param [Integer] timeout the number of seconds to wait for a lock.
-    #   nil means wait indefinitely
-    # @yield the block to execute if a lock is successful
-    # @return the Sidekiq job_id (jid)
+    #
+    # @return [String] the Sidekiq job_id (jid)
+    #
+    #
     def lock(timeout = nil, &block)
       Scripts.call(:lock, redis_pool,
                    keys: [exists_key, grabbed_key, available_key, UNIQUE_SET, unique_digest],
@@ -56,19 +62,27 @@ def lock(timeout = nil, &block)
     end
     alias wait lock
 
+    #
     # Removes the lock keys from Redis if locked by the provided jid/token
+    #
     # @return [false] unless locked?
     # @return [String] Sidekiq job_id (jid) if successful
+    #
     def unlock(token = nil)
       token ||= jid
       return false unless locked?(token)
 
       unlock!(token)
     end
 
+    #
     # Removes the lock keys from Redis
+    #
+    # @param [String] token the token to unlock (defaults to jid)
+    #
     # @return [false] unless locked?
     # @return [String] Sidekiq job_id (jid) if successful
+    #
     def unlock!(token = nil)
       token ||= jid
 
@@ -80,10 +94,17 @@ def unlock!(token = nil)
       )
     end
 
-    # Checks if this instance is considered locked
+    #
     # @param [String] token the unique token to check for a lock.
     #   nil will default to the jid provided in the initializer
     # @return [true, false]
+    #
+    # Checks if this instance is considered locked
+    #
+    # @param [<type>] token <description>
+    #
+    # @return [<type>] <description>
+    #
     def locked?(token = nil)
       token ||= jid