-
Notifications
You must be signed in to change notification settings - Fork 47
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
Remove minitest gem altogether, make specs more rspec-y, add a spec for the array form of :on #62
Conversation
Merge master
Merge master
Minor: Better ruby style & remove unnecessary short circuit check (kamui#48)
Merge master
spec/retriable_spec.rb
Outdated
expect do | ||
described_class.retriable( | ||
base_interval: 1.0, | ||
multiplier: 1.0, | ||
rand_factor: 0.0, | ||
max_elapsed_time: 2.0, | ||
on_retry: handler, | ||
max_elapsed_time: 2.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put a comma after the last parameter of a multiline method call.
spec/retriable_spec.rb
Outdated
puts "should raise NoMethodError" | ||
end | ||
end.to raise_error(NoMethodError) | ||
context 'global scope extension' do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
spec/retriable_spec.rb
Outdated
described_class.retriable(tries: 4, on: { StandardError => nil, TestError => [/foo/, /bar/] }, on_retry: handler) do | ||
@tries += 1 | ||
expect do | ||
described_class.retriable(tries: 4, on: { StandardError => nil, TestError => [/foo/, /bar/] }, on_retry: handler) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [126/120]
spec/retriable_spec.rb
Outdated
describe "with sleep disabled" do | ||
def increment_tries_with_exception(exception_class) | ||
increment_tries | ||
raise exception_class, "#{exception_class.to_s} occurred" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant use of Object#to_s in interpolation.
spec/retriable_spec.rb
Outdated
exceptions[try] = exception | ||
end | ||
expect do | ||
described_class.retriable(tries: 4, on: { StandardError => nil, TestError => [/foo/, /bar/] }, on_retry: handler) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line is too long. [126/120]
spec/retriable_spec.rb
Outdated
4 => 1.6875, | ||
5 => 2.53125, | ||
) | ||
3 => 1.125 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put a comma after the last item of a multiline hash.
…t methods for raising errors in specs
spec/retriable_spec.rb
Outdated
end | ||
it "successfully retries when the values are arrays of exception message patterns" do | ||
exceptions = [] | ||
handler = lambda { |exception, try, _elapsed_time, _next_interval| exceptions[try] = exception } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the -> { ... } lambda literal syntax for single line lambdas.
spec/exponential_backoff_spec.rb
Outdated
12.814453125, | ||
19.2216796875, | ||
]) | ||
non_random_intervals = 9.times.inject([0.5]) { |memo, i| memo + [memo.last * 1.5] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused block argument - i. If it's necessary, use _ or _i as an argument name to indicate that it won't be used.
spec/exponential_backoff_spec.rb
Outdated
12.814453125, | ||
19.2216796875, | ||
]) | ||
non_random_intervals = 9.times.inject([0.5]) { |memo, i| memo + [memo.last * 1.5] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused block argument - i. If it's necessary, use _ or _i as an argument name to indicate that it won't be used.
@kamui any questions/thoughts on this PR, style or otherwise? it only touches the specs and just makes them "rspec style". |
3 month ping |
This is supernice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some files could not be reviewed due to errors:
.rubocop.yml: Style/InheritException has the wrong namespace - should be Lint
.rubocop.yml: Style/InheritException has the wrong namespace - should be Lint .rubocop.yml: Style/IndentArray has the wrong namespace - should be Layout .rubocop.yml: Style/IndentHash has the wrong namespace - should be Layout Error: The `Style/TrailingCommaInLiteral` cop no longer exists. Please use `Style/TrailingCommaInArrayLiteral` and/or `Style/TrailingCommaInHashLiteral` instead. (obsolete configuration found in .rubocop.yml, please update it)
class TestError < Exception; end | ||
class SecondTestError < TestError; end | ||
class DifferentTestError < Exception; end | ||
class NonStandardError < Exception; end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style/FrozenStringLiteralComment: Missing magic comment # frozen_string_literal: true.
Thanks for the PR! |
Sorry; didn't realize I forgot to remove
minitest
on the previous PR. PR also includes a reorganization of specs to make them more in the rspec style.