Skip to content

Commit

Permalink
chore(ci): add test coverage from bropoplpush (#828)
Browse files Browse the repository at this point in the history
* chore(ci): add test coverage from bropoplpush

* chore(lint): lint'em real good

* fix(encoding): prevent rubocop from crashing
  • Loading branch information
mhenrixon committed Feb 5, 2024
1 parent ec3afd9 commit ce1e57e
Show file tree
Hide file tree
Showing 23 changed files with 503 additions and 27 deletions.
13 changes: 11 additions & 2 deletions .rubocop.yml
Expand Up @@ -12,12 +12,21 @@ AllCops:
TargetRubyVersion: 2.7
Include:
- "examples/**/*.rb"
- "*.rb"
Exclude:
- "**/*.erb"
- "**/.DS_Store"
- "**/*.lua"
- "**/vendor/**/*"
- "assets/**/*"
- "bin/**/*"
- "doc/**/*"
- "docs/**/*"
- "lib/sidekiq_unique_jobs/lua/**/*"
- "lib/sidekiq_unique_jobs/web/views/**/*"
- "lib/tasks/**/*"
- "myapp/**/*"
- "bin/bench"
- "Sidekiq/**/*"
- "vendor/**/*"

Layout/EndAlignment:
EnforcedStyleAlignWith: variable
Expand Down
8 changes: 4 additions & 4 deletions CHANGELOG.md
Expand Up @@ -938,7 +938,7 @@
**Fixed bugs:**

- V7 - `on_conflict:` no longer accepts a Hash [\#495](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/495)
- Brpoplpush::RedisScript::LuaError: WRONGTYPE Operation against a key holding the wrong kind of value [\#491](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/491)
- SidekiqUniqueJobs::Script::LuaError: WRONGTYPE Operation against a key holding the wrong kind of value [\#491](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/491)
- Lua script bug [\#489](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/489)
- Reaper will delete locks for running jobs [\#488](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/488)
- Fix access to hash members [\#496](https://github.com/mhenrixon/sidekiq-unique-jobs/pull/496) ([mhenrixon](https://github.com/mhenrixon))
Expand Down Expand Up @@ -1691,7 +1691,7 @@
- deprecation warnings with redis-namespace 2.0 [\#212](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/212)
- Unclear docs / examples for unique\_args [\#211](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/211)
- Jobs Console fails to launch [\#208](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/208)
- Util.del Redis::CommandError: ERR syntax error [\#207](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/207)
- Util.del RedisClient::CommandError: ERR syntax error [\#207](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/207)
- version 4.0.19 [\#206](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/206)
- Job.delete does not remove lock in all circumstances [\#205](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/205)
- disappearing jobs - known issue in conjunction with other extensions? [\#202](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/202)
Expand Down Expand Up @@ -1881,7 +1881,7 @@

- Rails + Sidekiq will go bezerk after sidekiq-unique-jobs testing check. [\#128](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/128)
- NoMethodError: undefined method `to\_sym' for true:TrueClass [\#125](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/125)
- Redis::CommandError: ERR unknown command 'eval' [\#124](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/124)
- RedisClient::CommandError: ERR unknown command 'eval' [\#124](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/124)

**Merged pull requests:**

Expand Down Expand Up @@ -1944,7 +1944,7 @@

**Closed issues:**

- 3.0.14 Error: ERR wrong number of arguments for 'set' command \(Redis::CommandError\) [\#104](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/104)
- 3.0.14 Error: ERR wrong number of arguments for 'set' command \(RedisClient::CommandError\) [\#104](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/104)
- Testing [\#103](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/103)
- Active Job [\#102](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/102)
- Why is SidekiqUnique behaviour applied to regular Workers? [\#100](https://github.com/mhenrixon/sidekiq-unique-jobs/issues/100)
Expand Down
2 changes: 1 addition & 1 deletion README.md
Expand Up @@ -814,7 +814,7 @@ In my benchmarks deleting 1000 orphaned locks with lua performs around 65% faste

On the other hand if I increase it to 10 000 orphaned locks per cleanup (`reaper_count: 10_0000`) then redis starts throwing:

> BUSY Redis is busy running a script. You can only call SCRIPT KILL or SHUTDOWN NOSAVE. (Redis::CommandError)
> BUSY Redis is busy running a script. You can only call SCRIPT KILL or SHUTDOWN NOSAVE. (RedisClient::CommandError)
If you want to disable the reaper set it to `:none`, `nil` or `false`. Actually, any value that isn't `:ruby` or `:lua` will disable the reaping.

Expand Down
4 changes: 2 additions & 2 deletions doc/SidekiqUniqueJobs/Script.html
Expand Up @@ -73,7 +73,7 @@

<dl>
<dt>Includes:</dt>
<dd>Brpoplpush::RedisScript::DSL</dd>
<dd>SidekiqUniqueJobs::Script::DSL</dd>
</dl>


Expand Down Expand Up @@ -145,4 +145,4 @@ <h2>Overview</h2><div class="docstring">

</div>
</body>
</html>
</html>
10 changes: 5 additions & 5 deletions doc/file.CHANGELOG.html
Expand Up @@ -694,7 +694,7 @@ <h2 id="v7-0-0-beta15-2020-04-10"><a href="https://github.com/mhenrixon/sidekiq-

<ul>
<li>V7 - <code>on_conflict:</code> no longer accepts a Hash <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/495">#495</a></li>
<li>Brpoplpush::RedisScript::LuaError: WRONGTYPE Operation against a key holding the wrong kind of value <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/491">#491</a></li>
<li>SidekiqUniqueJobs::Script::LuaError: WRONGTYPE Operation against a key holding the wrong kind of value <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/491">#491</a></li>
<li>Lua script bug <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/489">#489</a></li>
<li>Reaper will delete locks for running jobs <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/488">#488</a></li>
<li>Fix access to hash members <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/pull/496">#496</a> (<a href="https://github.com/mhenrixon">mhenrixon</a>)</li>
Expand Down Expand Up @@ -1641,7 +1641,7 @@ <h2 id="v5-0-1-2017-04-16"><a href="https://github.com/mhenrixon/sidekiq-unique-
<li>deprecation warnings with redis-namespace 2.0 <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/212">#212</a></li>
<li>Unclear docs / examples for unique_args <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/211">#211</a></li>
<li>Jobs Console fails to launch <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/208">#208</a></li>
<li>Util.del Redis::CommandError: ERR syntax error <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/207">#207</a></li>
<li>Util.del RedisClient::CommandError: ERR syntax error <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/207">#207</a></li>
<li>version 4.0.19 <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/206">#206</a></li>
<li>Job.delete does not remove lock in all circumstances <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/205">#205</a></li>
<li>disappearing jobs - known issue in conjunction with other extensions? <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/202">#202</a></li>
Expand Down Expand Up @@ -1871,7 +1871,7 @@ <h2 id="v4-0-5-2015-10-13"><a href="https://github.com/mhenrixon/sidekiq-unique-
<ul>
<li> Rails + Sidekiq will go bezerk after sidekiq-unique-jobs testing check. <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/128">#128</a></li>
<li> NoMethodError: undefined method `to_sym&#39; for true:TrueClass <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/125">#125</a></li>
<li>Redis::CommandError: ERR unknown command &#39;eval&#39; <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/124">#124</a></li>
<li>RedisClient::CommandError: ERR unknown command &#39;eval&#39; <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/124">#124</a></li>
</ul>

<p><strong>Merged pull requests:</strong></p>
Expand Down Expand Up @@ -1948,7 +1948,7 @@ <h2 id="v4-0-0-2015-10-05"><a href="https://github.com/mhenrixon/sidekiq-unique-
<p><strong>Closed issues:</strong></p>

<ul>
<li>3.0.14 Error: ERR wrong number of arguments for &#39;set&#39; command (Redis::CommandError) <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/104">#104</a></li>
<li>3.0.14 Error: ERR wrong number of arguments for &#39;set&#39; command (RedisClient::CommandError) <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/104">#104</a></li>
<li>Testing <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/103">#103</a></li>
<li>Active Job <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/102">#102</a></li>
<li>Why is SidekiqUnique behaviour applied to regular Workers? <a href="https://github.com/mhenrixon/sidekiq-unique-jobs/issues/100">#100</a></li>
Expand Down Expand Up @@ -2178,4 +2178,4 @@ <h2 id="v2-1-0-2012-08-07"><a href="https://github.com/mhenrixon/sidekiq-unique-

</div>
</body>
</html>
</html>
4 changes: 2 additions & 2 deletions doc/file.README.html
Expand Up @@ -861,7 +861,7 @@ <h4 id="reaper">reaper</h4>
<p>On the other hand if I increase it to 10 000 orphaned locks per cleanup (<code>reaper_count: 10_0000</code>) then redis starts throwing:</p>

<blockquote>
<p>BUSY Redis is busy running a script. You can only call SCRIPT KILL or SHUTDOWN NOSAVE. (Redis::CommandError)</p>
<p>BUSY Redis is busy running a script. You can only call SCRIPT KILL or SHUTDOWN NOSAVE. (RedisClient::CommandError)</p>
</blockquote>

<p>If you want to disable the reaper set it to <code>:none</code>, <code>nil</code> or <code>false</code>. Actually, any value that isn&#39;t <code>:ruby</code> or <code>:lua</code> will disable the reaping.</p>
Expand Down Expand Up @@ -1088,4 +1088,4 @@ <h2 id="contributors">Contributors</h2>

</div>
</body>
</html>
</html>
4 changes: 2 additions & 2 deletions doc/index.html
Expand Up @@ -861,7 +861,7 @@ <h4 id="reaper">reaper</h4>
<p>On the other hand if I increase it to 10 000 orphaned locks per cleanup (<code>reaper_count: 10_0000</code>) then redis starts throwing:</p>

<blockquote>
<p>BUSY Redis is busy running a script. You can only call SCRIPT KILL or SHUTDOWN NOSAVE. (Redis::CommandError)</p>
<p>BUSY Redis is busy running a script. You can only call SCRIPT KILL or SHUTDOWN NOSAVE. (RedisClient::CommandError)</p>
</blockquote>

<p>If you want to disable the reaper set it to <code>:none</code>, <code>nil</code> or <code>false</code>. Actually, any value that isn&#39;t <code>:ruby</code> or <code>:lua</code> will disable the reaping.</p>
Expand Down Expand Up @@ -1088,4 +1088,4 @@ <h2 id="contributors">Contributors</h2>

</div>
</body>
</html>
</html>
7 changes: 1 addition & 6 deletions lib/sidekiq_unique_jobs/script/script.rb
Expand Up @@ -66,12 +66,7 @@ def compiled_sha
end

def load(conn)
@sha =
if conn.respond_to?(:namespace)
conn.redis.script(:load, source)
else
conn.script(:load, source)
end
@sha = conn.script(:load, source)

self
end
Expand Down
2 changes: 1 addition & 1 deletion myapp/config/initializers/sidekiq.rb
Expand Up @@ -3,7 +3,7 @@
require "sidekiq"
require "sidekiq-unique-jobs"

REDIS = Redis.new(url: ENV.fetch("REDIS_URL", nil))
REDIS = RedisClient.new(url: ENV.fetch("REDIS_URL", nil))

Sidekiq.default_job_options = {
backtrace: true,
Expand Down
2 changes: 1 addition & 1 deletion spec/sidekiq_unique_jobs/script/caller_spec.rb
Expand Up @@ -14,7 +14,7 @@
let(:keys) { [unique_key] }
let(:argv) { [jid, max_lock_time] }
let(:script_arguments) { [keys, argv, redis] }
let(:redis) { Redis.new }
let(:redis) { RedisClient.new }
let(:scriptsha) { "abcdefab" }
let(:script_name) { :acquire_lock }
let(:error_message) { "Some interesting error" }
Expand Down
130 changes: 130 additions & 0 deletions spec/sidekiq_unique_jobs/script/client/execute_spec.rb
@@ -0,0 +1,130 @@
# frozen_string_literal: true

RSpec.describe SidekiqUniqueJobs::Script::Client, "#execute" do
subject(:execute) { client.execute(script_name, redis, **arguments) }

include_context "with test config"

let(:client) { described_class.new(config) }
let(:script_name) { :test }
let(:redis) { instance_spy(RedisClient) }
let(:arguments) { { keys: keys, argv: argv } }
let(:keys) { %w[key_one key_two key_tre key_for key_fav] }
let(:argv) { %w[arg_one arg_two arg_tre arg_for arg_fav] }
let(:exception) { nil }
let(:redis_error_message) { nil }
let(:script) { SidekiqUniqueJobs::Script::Script.new(name: script_name, root_path: SCRIPTS_PATH) }
let(:scripts) { instance_spy(SidekiqUniqueJobs::Script::Scripts) }

let(:script_source) do
<<~LUA
local key_one = KEYS[1]
local key_two = KEYS[2]
local key_tre = KEYS[3]
local key_for = KEYS[4]
local key_fiv = KEYS[5]
local arg_one = ARGV[1]
local arg_two = ARGV[2]
local arg_tre = ARGV[3]
local arg_for = ARGV[4]
local arg_fiv = ARGV[5]
LUA
end

context "when error is raised" do
before do
allow(SidekiqUniqueJobs::Script::Scripts).to receive(:fetch).with(config.scripts_path).and_return(scripts)
allow(scripts).to receive_messages(delete: true, kill: true, load: true, fetch: script)

allow(client.logger).to receive_messages(warn: true, debug: true)

call_count = 0
allow(scripts).to receive(:execute) do
call_count += 1
call_count.odd? ? raise(RedisClient::CommandError, redis_error_message) : "bogus"
end

exception
end

context "when message starts with ERR" do
let(:redis_error_message) do
<<~ERR
ERR Error running script (execute to f_178d75adaa46af3d8237cfd067c9fdff7b9d504f): [string "func definition"]:7: attempt to compare nil with number
ERR
end

let(:error_message) do
<<~ERR_MSG
attempt to compare nil with number
5: local key_fiv = KEYS[5]
6: local arg_one = ARGV[1]
=> 7: local arg_two = ARGV[2]
8: local arg_tre = ARGV[3]
9: local arg_for = ARGV[4]
ERR_MSG
end

let(:exception) do
execute
rescue SidekiqUniqueJobs::Script::LuaError => ex
ex
end

specify do
expect(exception.message).to eq(error_message)
expect(exception.backtrace.first).to match(%r{spec/support/lua/test.lua:7})
expect(exception.backtrace[1]).to match(/client.rb/)

expect(scripts).not_to have_received(:delete)
expect(scripts).to have_received(:execute).with(script_name, redis, keys: keys, argv: argv).once
end
end

context "when message starts with BUSY" do
let(:redis_error_message) do
"BUSY Redis is busy running a script. " \
"You can only execute SCRIPT KILL or SHUTDOWN NOSAVE."
end

context "when .script(:kill) raises CommandError" do
before do
allow(scripts).to receive(:kill).and_raise(RedisClient::CommandError, "NOT BUSY")
allow(client.logger).to receive(:warn)
end

specify do
expect { execute }.not_to raise_error
expect(client.logger).to have_received(:warn).with(kind_of(RedisClient::CommandError))
expect(scripts).to have_received(:execute).with(script_name, redis, keys: keys, argv: argv).twice
end
end

context "when .script(:kill) is successful" do
before do
allow(scripts).to receive(:kill).and_return(true)
end

specify do
expect { execute }.not_to raise_error

expect(scripts).to have_received(:kill).with(redis)
expect(scripts).to have_received(:execute).with(script_name, redis, keys: keys, argv: argv).twice
end
end
end

context "when message starts with NOSCRIPT" do
let(:redis_error_message) { "NOSCRIPT No matching script. Please use EVAL." }

specify do
expect { execute }.not_to raise_error

expect(scripts).to have_received(:delete).with(script_name)
expect(scripts).to have_received(:execute).with(script_name, redis, keys: keys, argv: argv).twice
end
end
end
end
24 changes: 24 additions & 0 deletions spec/sidekiq_unique_jobs/script/client_spec.rb
@@ -0,0 +1,24 @@
# frozen_string_literal: true

RSpec.describe SidekiqUniqueJobs::Script::Client do
let(:client) { described_class.new(config) }

include_context "with test config"

describe ".execute" do
subject(:execute) { client.execute(script_name, redis, **arguments) }

let(:keys) { %w[key_one key_two key_tre key_for key_fav] }
let(:argv) { %w[arg_one arg_two arg_tre arg_for arg_fav] }
let(:redis) { Sidekiq.redis { |conn| return conn } }
let(:scriptsha) { "abcdefab" }
let(:arguments) { { keys: keys, argv: argv } }
let(:script_name) { :test }

before do
allow(client.logger).to receive(:debug).and_return(true)
end

it { is_expected.to eq("arg_for") }
end
end

0 comments on commit ce1e57e

Please sign in to comment.