Correct warnings that contains Unicode message #2437
Conversation
There was a problem hiding this comment.
good work
when seeing the actual impact in code, its a nice to have feature, but not without warning the user, that his code is broken
a test runner that hides breakages with a workaround does a disservice after all
it might be best to trigger a pytest warning while explicitly passing the file/lineno
_pytest/warnings.py
Outdated
| @@ -62,7 +64,7 @@ def catch_warnings_for_item(item): | |||
|
|
|||
| for warning in log: | |||
| msg = warnings.formatwarning( | |||
| warning.message, warning.category, | |||
| compat.safe_str(warning.message), warning.category, | |||
There was a problem hiding this comment.
its required to warn the user here that his warning message is broken as it is not a str instance (after all this is a stdlib problem wrokaround)
_pytest/warnings.py
Outdated
| @@ -62,7 +64,7 @@ def catch_warnings_for_item(item): | |||
|
|
|||
| for warning in log: | |||
| msg = warnings.formatwarning( | |||
| warning.message, warning.category, | |||
| compat.safe_str(warning.message), warning.category, | |||
testing/test_warnings.py
Outdated
|
|
||
|
|
||
| def test_unicode(testdir, pyfile_with_warnings): | ||
| testdir.makepyfile(''' |
There was a problem hiding this comment.
Excellent work! Please add a CHANGELOG entry as well. 👍
There was a problem hiding this comment.
Sorry, I don't know which section should I add the entry to CHANGELOG, could you please tell me that?
There was a problem hiding this comment.
Sorry did not realize we did not prepare the CHANGELOG for new entries.
Please add something like this to the CHANGELOG:
3.1.1 (unreleased)
==================
* Fix encoding errors for unicode warnings in Python 2.
We are introducing towncrier to manage the CHANGELOG in #2431, but we will take care of moving the entry to that format later! 👍
|
Excellent work, thanks a ton! 👍 Btw, would you like to be added to the AUTHORS file? And if so by which name? |
|
@nicoddemus Sure, that's my honor. Just use |
|
@coldnight done, thanks again! 😁 |
This PR try to fix #2436.
master;Unless your change is trivial documentation fix (e.g., a typo or reword of a small section) please:
AUTHORS;CHANGELOG.rstCHANGELOG, so please add a thank note to yourself ("Thanks @user for the PR") and a link to your GitHub profile. It may sound weird thanking yourself, but otherwise a maintainer would have to do it manually before or after merging instead of just using GitHub's merge button. This makes it easier on the maintainers to merge PRs.