Fix UnicodeDecodeError in assertion with mixed non-ascii bytes repr + text #4001
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4001 +/- ##
==========================================
+ Coverage 94.34% 94.49% +0.15%
==========================================
Files 109 109
Lines 23766 23778 +12
Branches 2356 2357 +1
==========================================
+ Hits 22421 22470 +49
+ Misses 1026 1000 -26
+ Partials 319 308 -11
Continue to review full report at Codecov.
|
| @@ -608,6 +609,21 @@ def __repr__(self): | |||
|
|
|||
| assert r"where 1 = \n{ \n~ \n}.a" in util._format_lines([getmsg(f)])[0] | |||
|
|
|||
| def test_custom_repr_non_ascii(self): | |||
There was a problem hiding this comment.
Is this test something we want to keep around after dropping Python 2? If not, I suggest to leave a comment so we can remove the test itself when the time comes (the code itself already contains a "python2.x" so I think that's enough). 👍
There was a problem hiding this comment.
shouldn't have a need for this when python2.x is gone, should I update the test or do you think the 2.x comment in the code is enough? (I actually added the comment with the same thought because I knew we were eventually going to want to clean it up!)
There was a problem hiding this comment.
The comment in the code is enough to remove the code, but the test doesn't have any reference so I think it will mostly stay around. One alternative is to skip the test on python 3,this makes it clear it is python2 only.
There was a problem hiding this comment.
cool, yeah I added a comment to the test
| if isinstance(r, bytes): | ||
| # Represent unprintable bytes as `\x##` | ||
| r = u"".join( | ||
| u"\\x{:x}".format(ord(c)) if c not in string.printable else c.decode() |
There was a problem hiding this comment.
isn't there the hex-escape encoding we can use instead?
There was a problem hiding this comment.
string-escape, yes: but it also escapes quote characters
unicode-escape: almost, but it decodes escape sequences to their unicode character
There was a problem hiding this comment.
@asottile i see, thanks for researching the details
|
does codecov load for anyone else? I can't get the page to render for me :( |
|
Me neither. 😞 @blueyed do you know if codecov is facing some stability problems lately? |
|
Re @codecov: you can browse some commits etc, but PR pages appear to hang / time out often (and constantly when it happens). |
|
I.e. you can browse to commits via https://codecov.io/gh/pytest-dev/pytest/commits, e.g. https://codecov.io/gh/pytest-dev/pytest/commit/7122fa5613de8ef767eeb8106a5fee6dea0d9285 for this PR. |
|
Not sure if correct/relevant, but there is shows that the test is not covered: https://codecov.io/gh/pytest-dev/pytest/commit/7122fa5613de8ef767eeb8106a5fee6dea0d9285#D2-612 (sorry, have not read this PR/issue/details). |
|
yeah that has to do with the way that |
| a = A() | ||
| assert not a.name | ||
|
|
||
| msg = getmsg(f) |
There was a problem hiding this comment.
Should this call f maybe?
Resolves #3999