-
Notifications
You must be signed in to change notification settings - Fork 11
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 error handling issues #95
Conversation
@blairlunceford Could we add a test case that exhibits the broken behavior and passes with the changes? |
@@ -82,6 +82,27 @@ def test_bad_request_exceptions_include_expected_request_data( | |||
# This'll fail for sure here but... just using the nice error that'd come up | |||
assert ex.__class__ == BadRequestException | |||
|
|||
def test_bad_request_exceptions_exclude_expected_request_data( |
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 this test gives us the proper coverage.
I tried running this test locally without the fix and it still passes.
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.
Yeah I've been looking at all of these tests, and I don't think any of them are doing the right things. Trying to figure this out - hoping my Ruby translates to Python 😆
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.
With the latest changes, I was able to run locally without the fix and see the error AttributeError: 'BadRequestException' object has no attribute 'error'
and when I added the fix, the tests pass.
Codecov Report
@@ Coverage Diff @@
## master #95 +/- ##
==========================================
+ Coverage 93.01% 95.51% +2.49%
==========================================
Files 17 17
Lines 401 401
==========================================
+ Hits 373 383 +10
+ Misses 28 18 -10
Continue to review full report at Codecov.
|
Fix bug reported in:
#94
#93