diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..5be63fc --- /dev/null +++ b/.rspec @@ -0,0 +1,2 @@ +--require spec_helper +--format documentation diff --git a/.rubocop.yml b/.rubocop.yml index 03fa5df..4b695e3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,16 +7,13 @@ Style/Documentation: Style/TrailingCommaInArguments: EnforcedStyleForMultiline: comma -Style/TrailingCommaInLiteral: - EnforcedStyleForMultiline: comma - -Style/InheritException: +Lint/InheritException: Enabled: false -Style/IndentArray: +Layout/IndentArray: Enabled: false -Style/IndentHash: +Layout/IndentHash: Enabled: false Style/NegatedIf: diff --git a/.travis.yml b/.travis.yml index fa35468..e3d1a2b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,20 +6,21 @@ language: ruby rvm: - 2.0.0 - 2.1.10 - - 2.2.8 - - 2.3.5 - - 2.4.2 + - 2.2.10 + - 2.3.7 + - 2.4.4 + - 2.5.1 - ruby-head - jruby-9.0.5.0 - jruby-9.1.15.0 - jruby-head - - rbx-3.86 + - rbx-3.99 script: - bundle exec rspec matrix: allow_failures: - - rvm: rbx-3.86 + - rvm: rbx-3.99 before_install: - gem update --system diff --git a/CHANGELOG.md b/CHANGELOG.md index 78b68a1..3c62958 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ ## HEAD +* Replace `minitest` gem with `rspec` * Fancier README * Remove unnecessary short circuit in `randomize` method diff --git a/Gemfile b/Gemfile index b07b7ec..47930f4 100644 --- a/Gemfile +++ b/Gemfile @@ -4,7 +4,6 @@ gemspec group :test do gem "codeclimate-test-reporter", require: false - gem "minitest-focus" gem "rspec" gem "simplecov", require: false end diff --git a/Guardfile b/Guardfile deleted file mode 100644 index ea459dd..0000000 --- a/Guardfile +++ /dev/null @@ -1,8 +0,0 @@ -# A sample Guardfile -# More info at https://github.com/guard/guard#readme - -guard :minitest do - watch(%r{^spec/(.*)_spec\.rb}) - watch(%r{^lib/retriable/(.+)\.rb}) { |m| "spec/#{m[1]}_spec.rb" } - watch(%r{^spec/spec_helper\.rb}) { "spec" } -end diff --git a/README.md b/README.md index 8f48285..9d869e3 100644 --- a/README.md +++ b/README.md @@ -315,3 +315,9 @@ RetriableProxy.for_object(api_endpoint, on: Net::TimeoutError) ## Credits The randomized exponential backoff implementation was inspired by the one used in Google's [google-http-java-client](https://code.google.com/p/google-http-java-client/wiki/ExponentialBackoff) project. + +## Development +### Running Specs +```bash +bundle exec rspec +``` diff --git a/Rakefile b/Rakefile deleted file mode 100644 index 66d3762..0000000 --- a/Rakefile +++ /dev/null @@ -1,12 +0,0 @@ -# encoding: utf-8 - -require "bundler" -Bundler::GemHelper.install_tasks - -require "rake/testtask" -task default: :test - -Rake::TestTask.new do |t| - t.pattern = "spec/**/*_spec.rb" - t.verbose = true -end diff --git a/retriable.gemspec b/retriable.gemspec index b9579b9..cf73fcd 100644 --- a/retriable.gemspec +++ b/retriable.gemspec @@ -21,11 +21,7 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 2.0.0" spec.add_development_dependency "bundler" - spec.add_development_dependency "rake", "~> 12.0" - - spec.add_development_dependency "minitest", "~> 5.10" - spec.add_development_dependency "guard" - spec.add_development_dependency "guard-minitest" + spec.add_development_dependency "rspec", "~> 3" if RUBY_VERSION < "2.3" spec.add_development_dependency "ruby_dep", "~> 1.3.1" diff --git a/spec/config_spec.rb b/spec/config_spec.rb index b75742e..53fcb48 100644 --- a/spec/config_spec.rb +++ b/spec/config_spec.rb @@ -1,55 +1,53 @@ -require_relative "spec_helper" - describe Retriable::Config do - let(:config) { described_class.new } + let(:default_config) { described_class.new } context "defaults" do it "sleep defaults to enabled" do - expect(config.sleep_disabled).to be_falsey + expect(default_config.sleep_disabled).to be_falsey end it "tries defaults to 3" do - expect(config.tries).to eq(3) + expect(default_config.tries).to eq(3) end it "max interval defaults to 60" do - expect(config.max_interval).to eq(60) + expect(default_config.max_interval).to eq(60) end it "randomization factor defaults to 0.5" do - expect(config.base_interval).to eq(0.5) + expect(default_config.base_interval).to eq(0.5) end it "multiplier defaults to 1.5" do - expect(config.multiplier).to eq(1.5) + expect(default_config.multiplier).to eq(1.5) end it "max elapsed time defaults to 900" do - expect(config.max_elapsed_time).to eq(900) + expect(default_config.max_elapsed_time).to eq(900) end it "intervals defaults to nil" do - expect(config.intervals).to be_nil + expect(default_config.intervals).to be_nil end it "timeout defaults to nil" do - expect(config.timeout).to be_nil + expect(default_config.timeout).to be_nil end it "on defaults to [StandardError]" do - expect(config.on).to eq([StandardError]) + expect(default_config.on).to eq([StandardError]) end - it "on retry handler defaults to nil" do - expect(config.on_retry).to be_nil + it "on_retry handler defaults to nil" do + expect(default_config.on_retry).to be_nil end it "contexts defaults to {}" do - expect(config.contexts).to eq({}) + expect(default_config.contexts).to eq({}) end end it "raises errors on invalid configuration" do - expect { described_class.new(does_not_exist: 123) }.to raise_error(ArgumentError) + expect { described_class.new(does_not_exist: 123) }.to raise_error(ArgumentError, /not a valid option/) end end diff --git a/spec/exponential_backoff_spec.rb b/spec/exponential_backoff_spec.rb index 13d6345..a0c0d8b 100644 --- a/spec/exponential_backoff_spec.rb +++ b/spec/exponential_backoff_spec.rb @@ -1,5 +1,3 @@ -require_relative "spec_helper" - describe Retriable::ExponentialBackoff do context "defaults" do let(:backoff_config) { described_class.new } @@ -56,11 +54,7 @@ end it "generates intervals with a defined max interval" do - expect(described_class.new(max_interval: 1.0, rand_factor: 0.0).intervals).to eq([ - 0.5, - 0.75, - 1.0, - ]) + expect(described_class.new(max_interval: 1.0, rand_factor: 0.0).intervals).to eq([0.5, 0.75, 1.0]) end it "generates intervals with a defined rand_factor" do @@ -72,17 +66,7 @@ end it "generates 10 non-randomized intervals" do - expect(described_class.new(tries: 10, rand_factor: 0.0).intervals).to eq([ - 0.5, - 0.75, - 1.125, - 1.6875, - 2.53125, - 3.796875, - 5.6953125, - 8.54296875, - 12.814453125, - 19.2216796875, - ]) + non_random_intervals = 9.times.inject([0.5]) { |memo, _i| memo + [memo.last * 1.5] } + expect(described_class.new(tries: 10, rand_factor: 0.0).intervals).to eq(non_random_intervals) end end diff --git a/spec/retriable_spec.rb b/spec/retriable_spec.rb index 766c826..5084e2f 100644 --- a/spec/retriable_spec.rb +++ b/spec/retriable_spec.rb @@ -1,384 +1,265 @@ -require_relative "spec_helper" - describe Retriable do - before do + let(:time_table_handler) do + ->(_exception, try, _elapsed_time, next_interval) { @next_interval_table[try] = next_interval } + end + + before(:each) do + described_class.configure { |c| c.sleep_disabled = true } @tries = 0 + @next_interval_table = {} end - describe "with sleep disabled" do - before do - Retriable.configure do |c| - c.sleep_disabled = true - end + def increment_tries + @tries += 1 + end + + def increment_tries_with_exception(exception_class = nil) + exception_class ||= StandardError + increment_tries + raise exception_class, "#{exception_class} occurred" + end + + context "global scope extension" do + it "cannot be called in the global scope without requiring the core_ext/kernel" do + expect { retriable { puts "should raise NoMethodError" } }.to raise_error(NoMethodError) end - it "stops at first try if the block does not raise an exception" do - described_class.retriable { @tries += 1 } - expect(@tries).to eq(1) + it "can be called once the kernel extension is required" do + require_relative "../lib/retriable/core_ext/kernel" + + expect { retriable { increment_tries_with_exception } }.to raise_error(StandardError) + expect(@tries).to eq(3) end + end - it "raises a LocalJumpError if #retriable is not given a block" do - expect { described_class.retriable on: StandardError }.to raise_error(LocalJumpError) - expect { described_class.retriable on: StandardError, timeout: 2 }.to raise_error(LocalJumpError) + context "#retriable" do + it "raises a LocalJumpError if not given a block" do + expect { described_class.retriable }.to raise_error(LocalJumpError) + expect { described_class.retriable(timeout: 2) }.to raise_error(LocalJumpError) end - it "makes 3 tries when retrying block of code raising StandardError with no arguments" do - expect do - described_class.retriable do - @tries += 1 - raise StandardError, "StandardError occurred" - end - end.to raise_error(StandardError) + it "stops at first try if the block does not raise an exception" do + described_class.retriable { increment_tries } + expect(@tries).to eq(1) + end + it "makes 3 tries when retrying block of code raising StandardError with no arguments" do + expect { described_class.retriable { increment_tries_with_exception } }.to raise_error(StandardError) expect(@tries).to eq(3) end - it "makes only 1 try when exception raised is not ancestor of StandardError" do + it "makes only 1 try when exception raised is not descendent of StandardError" do expect do - described_class.retriable do - @tries += 1 - raise TestError, "TestError occurred" - end - end.to raise_error(TestError) + described_class.retriable { increment_tries_with_exception(NonStandardError) } + end.to raise_error(NonStandardError) expect(@tries).to eq(1) end - it "#retriable with custom exception tries 3 times and re-raises the exception" do + it "with custom exception tries 3 times and re-raises the exception" do expect do - described_class.retriable(on: TestError) do - @tries += 1 - raise TestError.new, "TestError occurred" - end - end.to raise_error(TestError) + described_class.retriable(on: NonStandardError) { increment_tries_with_exception(NonStandardError) } + end.to raise_error(NonStandardError) expect(@tries).to eq(3) end - it "#retriable tries 10 times" do - expect do - described_class.retriable(tries: 10) do - @tries += 1 - raise StandardError, "StandardError occurred" - end - end.to raise_error(StandardError) - + it "tries 10 times when specified" do + expect { described_class.retriable(tries: 10) { increment_tries_with_exception } }.to raise_error(StandardError) expect(@tries).to eq(10) end - it "#retriable will timeout after 1 second" do - expect do - described_class.retriable(timeout: 1) do - sleep(1.1) - end - end.to raise_error(Timeout::Error) + it "will timeout after 1 second" do + expect { described_class.retriable(timeout: 1) { sleep(1.1) } }.to raise_error(Timeout::Error) end it "applies a randomized exponential backoff to each try" do - time_table = [] - - handler = lambda do |exception, _try, _elapsed_time, next_interval| - expect(exception.class).to eq(ArgumentError) - time_table << next_interval - end - expect do - Retriable.retriable(on: [EOFError, ArgumentError], on_retry: handler, tries: 10) do - @tries += 1 - raise ArgumentError.new, "ArgumentError occurred" - end - end.to raise_error(ArgumentError) - - expect(time_table).to eq([ - 0.5244067512211441, - 0.9113920238761231, - 1.2406087918999114, - 1.7632403621664823, - 2.338001204738311, - 4.350816718580626, - 5.339852157217869, - 11.889873261212443, - 18.756037881636484, - nil, - ]) + described_class.retriable(on_retry: time_table_handler, tries: 10) { increment_tries_with_exception } + end.to raise_error(StandardError) + + expect(@next_interval_table).to eq( + 1 => 0.5244067512211441, + 2 => 0.9113920238761231, + 3 => 1.2406087918999114, + 4 => 1.7632403621664823, + 5 => 2.338001204738311, + 6 => 4.350816718580626, + 7 => 5.339852157217869, + 8 => 11.889873261212443, + 9 => 18.756037881636484, + 10 => nil, + ) expect(@tries).to eq(10) end - describe "retries with an on_#retriable handler, 6 max retries, and a 0.0 rand_factor" do - before do - tries = 6 - @try_count = 0 - @time_table = {} + context "with rand_factor 0.0 and an on_retry handler" do + let(:tries) { 6 } + let(:no_rand_timetable) { { 1 => 0.5, 2 => 0.75, 3 => 1.125 } } + let(:args) { { on_retry: time_table_handler, rand_factor: 0.0, tries: tries } } - handler = lambda do |exception, try, _elapsed_time, next_interval| - expect(exception.class).to eq(ArgumentError) - @time_table[try] = next_interval + it "applies a non-randomized exponential backoff to each try" do + described_class.retriable(args) do + increment_tries + raise StandardError if @tries < tries end - Retriable.retriable( - on: [EOFError, ArgumentError], - on_retry: handler, - rand_factor: 0.0, - tries: tries, - ) do - @try_count += 1 - raise ArgumentError.new, "ArgumentError occurred" if @try_count < tries - end + expect(@tries).to eq(tries) + expect(@next_interval_table).to eq(no_rand_timetable.merge(4 => 1.6875, 5 => 2.53125)) end - it "makes 6 tries" do - expect(@try_count).to eq(6) - end + it "obeys a max interval of 1.5 seconds" do + expect do + described_class.retriable(args.merge(max_interval: 1.5)) { increment_tries_with_exception } + end.to raise_error(StandardError) - it "applies a non-randomized exponential backoff to each try" do - expect(@time_table).to eq( - 1 => 0.5, - 2 => 0.75, - 3 => 1.125, - 4 => 1.6875, - 5 => 2.53125, - ) + expect(@next_interval_table).to eq(no_rand_timetable.merge(4 => 1.5, 5 => 1.5, 6 => nil)) end - end - - it "#retriable has a max interval of 1.5 seconds" do - time_table = {} - handler = lambda do |_exception, try, _elapsed_time, next_interval| - time_table[try] = next_interval - end + it "obeys custom defined intervals" do + interval_hash = no_rand_timetable.merge(4 => 1.5, 5 => 1.5, 6 => nil) + intervals = interval_hash.values.compact.sort - expect do - described_class.retriable( - on: StandardError, - on_retry: handler, - rand_factor: 0.0, - tries: 5, - max_interval: 1.5, - ) do - @tries += 1 - raise StandardError - end - end.to raise_error(StandardError) + expect do + described_class.retriable(on_retry: time_table_handler, intervals: intervals) do + increment_tries_with_exception + end + end.to raise_error(StandardError) - expect(time_table).to eq( - 1 => 0.5, - 2 => 0.75, - 3 => 1.125, - 4 => 1.5, - 5 => nil, - ) + expect(@next_interval_table).to eq(interval_hash) + expect(@tries).to eq(intervals.size + 1) + end end - it "#retriable with custom defined intervals" do - intervals = [ - 0.5, - 0.75, - 1.125, - 1.5, - 1.5, - ] - time_table = {} - - handler = lambda do |_exception, try, _elapsed_time, next_interval| - time_table[try] = next_interval - end + context "with an array :on parameter" do + it "handles both kinds of exceptions" do + described_class.retriable(on: [StandardError, NonStandardError]) do + increment_tries - expect do - described_class.retriable( - on_retry: handler, - intervals: intervals, - ) do - @tries += 1 - raise StandardError + raise StandardError if @tries == 1 + raise NonStandardError if @tries == 2 end - end.to raise_error(StandardError) - - expect(time_table).to eq( - 1 => 0.5, - 2 => 0.75, - 3 => 1.125, - 4 => 1.5, - 5 => 1.5, - 6 => nil, - ) - expect(@tries).to eq(6) + expect(@tries).to eq(3) + end end - context "hash exception list" do - let(:error_message) { "something went wrong" } - let(:hash_argument) { { TestError => /#{error_message}/ } } + context "with a hash :on parameter" do + let(:on_hash) { { NonStandardError => /NonStandardError occurred/ } } - it "#retriable with a hash exception where the value is an exception message pattern" do + it "where the value is an exception message pattern" do expect do - described_class.retriable(on: hash_argument) { raise TestError, error_message } - end.to raise_error(TestError, /#{error_message}/) - end + described_class.retriable(on: on_hash) { increment_tries_with_exception(NonStandardError) } + end.to raise_error(NonStandardError, /NonStandardError occurred/) - it "#retriable with a hash exception list matches exception subclasses" do - on_hash = hash_argument.merge( - DifferentTestError => /should never happen/, - DifferentTestError => /also should never happen/ - ) + expect(@tries).to eq(3) + end + it "matches exception subclasses when message matches pattern" do expect do - described_class.retriable(tries: 4, on: on_hash, tries: 4) do - @tries += 1 - raise SecondTestError, error_message + described_class.retriable(on: on_hash.merge(DifferentError => [/shouldn't happen/, /also not/])) do + increment_tries_with_exception(SecondNonStandardError) end - end.to raise_error(SecondTestError, /something went wrong/) + end.to raise_error(SecondNonStandardError, /SecondNonStandardError occurred/) - expect(@tries).to eq(4) + expect(@tries).to eq(3) end - it "#retriable with a hash exception list does not retry matching exception subclass but not message" do + it "does not retry matching exception subclass but not message" do expect do - described_class.retriable(on: hash_argument, tries: 4) do - @tries += 1 - raise SecondTestError, "not a match" + described_class.retriable(on: on_hash) do + increment_tries + raise SecondNonStandardError, "not a match" end - end.to raise_error(SecondTestError, /not a match/) + end.to raise_error(SecondNonStandardError, /not a match/) expect(@tries).to eq(1) end - end - + it "successfully retries when the values are arrays of exception message patterns" do + exceptions = [] + handler = ->(exception, try, _elapsed_time, _next_interval) { exceptions[try] = exception } + on_hash = { StandardError => nil, NonStandardError => [/foo/, /bar/] } - it "#retriable with a hash exception list where the values are exception message patterns" do - exceptions = [] - handler = lambda do |exception, try, _elapsed_time, _next_interval| - exceptions[try] = exception - end - - expect do - described_class.retriable(tries: 4, on: { StandardError => nil, TestError => [/foo/, /bar/] }, on_retry: handler) do - @tries += 1 - - case @tries - when 1 - raise TestError, "foo" - when 2 - raise TestError, "bar" - when 3 - raise StandardError - else - raise TestError, "crash" + expect do + described_class.retriable(tries: 4, on: on_hash, on_retry: handler) do + increment_tries + + case @tries + when 1 + raise NonStandardError, "foo" + when 2 + raise NonStandardError, "bar" + when 3 + raise StandardError + else + raise NonStandardError, "crash" + end end - end - end.to raise_error(TestError, /crash/) + end.to raise_error(NonStandardError, /crash/) - expect(exceptions[1].class).to eq(TestError) - expect(exceptions[1].message).to eq("foo") - expect(exceptions[2].class).to eq(TestError) - expect(exceptions[2].message).to eq("bar") - expect(exceptions[3].class).to eq(StandardError) + expect(exceptions[1]).to be_a(NonStandardError) + expect(exceptions[1].message).to eq("foo") + expect(exceptions[2]).to be_a(NonStandardError) + expect(exceptions[2].message).to eq("bar") + expect(exceptions[3]).to be_a(StandardError) + end end - it "#retriable cannot be called in the global scope without requiring the core_ext/kernel" do - expect do - retriable do - puts "should raise NoMethodError" - end - end.to raise_error(NoMethodError) - - require_relative "../lib/retriable/core_ext/kernel" + it "runs for a max elapsed time of 2 seconds" do + described_class.configure { |c| c.sleep_disabled = false } expect do - retriable do - @tries += 1 - raise StandardError + described_class.retriable(base_interval: 1.0, multiplier: 1.0, rand_factor: 0.0, max_elapsed_time: 2.0) do + increment_tries_with_exception end end.to raise_error(StandardError) - expect(@tries).to eq(3) - end - end - - it "#retriable runs for a max elapsed time of 2 seconds" do - described_class.configure do |c| - c.sleep_disabled = false + expect(@tries).to eq(2) end - expect(described_class.config.sleep_disabled).to be_falsey - - time_table = {} - - handler = lambda do |_exception, try, elapsed_time, _next_interval| - time_table[try] = elapsed_time + it "raises ArgumentError on invalid options" do + expect { described_class.retriable(does_not_exist: 123) { increment_tries } }.to raise_error(ArgumentError) end - - expect do - described_class.retriable( - base_interval: 1.0, - multiplier: 1.0, - rand_factor: 0.0, - max_elapsed_time: 2.0, - on_retry: handler, - ) do - @tries += 1 - raise EOFError - end - end.to raise_error(EOFError) - - expect(@tries).to eq(2) end - it "raises NoMethodError on invalid configuration" do - expect { Retriable.configure { |c| c.does_not_exist = 123 } }.to raise_error(NoMethodError) + context "#configure" do + it "raises NoMethodError on invalid configuration" do + expect { described_class.configure { |c| c.does_not_exist = 123 } }.to raise_error(NoMethodError) + end end - it "raises ArgumentError on invalid option on #retriable" do - expect { Retriable.retriable(does_not_exist: 123) }.to raise_error(ArgumentError) - end + context "#with_context" do + let(:api_tries) { 4 } - describe "#with_context" do before do - Retriable.configure do |c| - c.sleep_disabled = true + described_class.configure do |c| c.contexts[:sql] = { tries: 1 } - c.contexts[:api] = { tries: 3 } + c.contexts[:api] = { tries: api_tries } end end - it "sql context stops at first try if the block does not raise an exception" do - described_class.with_context(:sql) do - @tries += 1 - end - + it "stops at first try if the block does not raise an exception" do + described_class.with_context(:sql) { increment_tries } expect(@tries).to eq(1) end - it "with_context respects the context options" do - expect do - described_class.with_context(:api) do - @tries += 1 - raise StandardError, "StandardError occurred" - end - end.to raise_error(StandardError) - - expect(@tries).to eq(3) + it "respects the context options" do + expect { described_class.with_context(:api) { increment_tries_with_exception } }.to raise_error(StandardError) + expect(@tries).to eq(api_tries) end - it "with_context allows override options" do + it "allows override options" do expect do - described_class.with_context(:sql, tries: 5) do - @tries += 1 - raise StandardError, "StandardError occurred" - end + described_class.with_context(:sql, tries: 5) { increment_tries_with_exception } end.to raise_error(StandardError) expect(@tries).to eq(5) end it "raises an ArgumentError when the context isn't found" do - expect do - described_class.with_context(:wtf) do - @tries += 1 - end - end.to raise_error(ArgumentError, /wtf not found/) + expect { described_class.with_context(:wtf) { increment_tries } }.to raise_error(ArgumentError, /wtf not found/) end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 6ba1175..d201ddd 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,21 +7,10 @@ SimpleCov.start -require "minitest/autorun" -require "minitest/focus" require "pry" - require_relative "../lib/retriable" require_relative "support/exceptions.rb" -class TryRecorder - attr_accessor :tries - - def initialize - @tries = 0 - end -end - RSpec.configure do |config| config.before(:each) do srand(0) diff --git a/spec/support/exceptions.rb b/spec/support/exceptions.rb index 1119004..17e896c 100644 --- a/spec/support/exceptions.rb +++ b/spec/support/exceptions.rb @@ -1,3 +1,3 @@ -class TestError < Exception; end -class SecondTestError < TestError; end -class DifferentTestError < Exception; end +class NonStandardError < Exception; end +class SecondNonStandardError < NonStandardError; end +class DifferentError < Exception; end