-
Notifications
You must be signed in to change notification settings - Fork 360
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
[MRG + 1] Add suppress_warnings flag #155
[MRG + 1] Add suppress_warnings flag #155
Conversation
resolves atlanhq#139
Codecov Report
@@ Coverage Diff @@
## master #155 +/- ##
==========================================
+ Coverage 84.47% 84.89% +0.42%
==========================================
Files 12 12
Lines 1198 1205 +7
Branches 288 289 +1
==========================================
+ Hits 1012 1023 +11
+ Misses 142 139 -3
+ Partials 44 43 -1
Continue to review full report at Codecov.
|
@jonathanlloyd Happy Hacktoberfest to you too! This looks good to me. Can you add the suppress_warnings option to the CLI too? |
Added a --quiet flag to the CLI 🙂 |
except Exception as e: | ||
assert type(e).__name__ == 'UserWarning' | ||
assert str(e) == 'No tables found on page-1' | ||
assert str(e.value) == 'No tables found on page-1' |
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.
@jonathanlloyd Should this be one indent level down?
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.
Its a bit weird but that's actually how it's documented: https://docs.pytest.org/en/latest/assert.html
It seems to work 🤷♂️
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. I looked at the tests for requests and they seem to do it this way too. https://github.com/requests/requests/blob/master/tests/test_requests.py#L817 If you ever find why pytest implemented it this way, do comment 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.
Very odd!
LGTM! Can you just address the one comment on the indent level? @jonathanlloyd |
@jonathanlloyd Thanks for the contribution! |
resolves #139
I've used the built in warning suppression here - let me know what you think.
Happy Hacktoberfest!