Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(ci): add test coverage from bropoplpush #828

Merged
merged 3 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .rubocop.yml
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Loading
Loading