-
-
Notifications
You must be signed in to change notification settings - Fork 503
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
Fix the issue where the exception was not caught when the purchase was distributed. #4894
Fix the issue where the exception was not caught when the purchase was distributed. #4894
Conversation
PurchaseDestroyService.call(purchase) | ||
begin | ||
PurchaseDestroyService.call(purchase) | ||
rescue InventoryError => e |
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.
Is there a reason you can't catch any StandardError
here?
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.
No. I will change it.
end | ||
end | ||
|
||
context "When the purchase has been distributed" 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.
Really all we're testing here is the ability to catch errors. So you can ensure that the delete service throws an error:
allow(PurchaseDestroyService).to receive(:call).and_raise('an error')
and you don't need all the setup here.
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.
Oh, that's really easy. I will change it. Thanks for your review.
7a88ad3
to
ca9e75d
Compare
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.
Looks good to me! @cielf did you want to take a look?
A definite improvment! |
Resolves #4886
Description
I am not sure whether this test is practical. If you have any ideas to improve it, please let me know.
Type of change
How Has This Been Tested?
bundle exec rspec spec/system/purchase_system_spec.rb:296
Screenshots