-
Notifications
You must be signed in to change notification settings - Fork 26
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
feat: Promise#rescue
can now take a list of error class filters.
#32
base: master
Are you sure you want to change the base?
feat: Promise#rescue
can now take a list of error class filters.
#32
Conversation
83a77f6
to
138bdd8
Compare
expect(p).to be_fulfilled | ||
|
||
result = nil | ||
p = subject.rescue(RuntimeError) do |reason| |
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.
This feels like second test of three that got grouped together in a single test.
}.to raise_error(ArgumentError, 'no block given') | ||
end | ||
|
||
it 'can be called with with a block not taking an error argument' 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.
Isn't the ability to specify a block or proc without all the arguments just a property of ruby? E.g.
irb> proc { :foo }.call(1)
=> :foo
It looks like this is an unnecessary test.
expect(result).to eq(reason) | ||
expect(subject.reason).to eq(reason) | ||
expect(p).to be_fulfilled | ||
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.
I don't think these tests for catch
are necessary, since it is just an alias for rescue
Example: