Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix pytest.raises handling of unicode exceptions in Python 2 #5479

Merged
merged 5 commits into from Jul 4, 2019
Merged

Fix pytest.raises handling of unicode exceptions in Python 2 #5479

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
013d0e6
use safe_str to serialize Exceptions Fixes #5478
graingert Jun 25, 2019
86a4eb6
Update changelog/5478.bugfix.rst
graingert Jun 27, 2019
09dee29
Use unicode message if regex is also unicode in ExceptionInfo.match
nicoddemus Jun 30, 2019
a886015
Test various bytes <=> unicode cases as requested in review
nicoddemus Jul 1, 2019
34b4e21
Include two more cases for non-ascii encoded bytes
nicoddemus Jul 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/5478.bugfix.rst
View file
Open in desktop
@@ -0,0 +1 @@
Use safe_str to serialize Exceptions in pytest.raises
graingert marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 3 additions & 2 deletions src/_pytest/_code/code.py
View file
Open in desktop
Expand Up @@ -572,8 +572,9 @@ def match(self, regexp):
raised.
"""
__tracebackhide__ = True
if not re.search(regexp, str(self.value)):
assert 0, "Pattern '{!s}' not found in '{!s}'".format(regexp, self.value)
value = safe_str(self.value)
if not re.search(regexp, value):
assert 0, "Pattern '{!s}' not found in '{!s}'".format(regexp, value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

option or followup: we might want to use raise AssertionError(...) in that case

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Out of curiosity, is there any reason other than style to prefer one over the other?

return True


Expand Down
4 changes: 4 additions & 0 deletions testing/python/raises.py
View file
Open in desktop
Expand Up @@ -278,3 +278,7 @@ def __class__(self):
with pytest.raises(CrappyClass()):
pass
assert "via __class__" in excinfo.value.args[0]

def test_u(self):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

______________________________ TestRaises.test_u ______________________________

self = <raises.TestRaises object at 0x0000000007B297B8>

    def test_u(self):
        with pytest.raises(AssertionError, match=u"\u2603"):
>           assert False, u"\u2603"
E           UnicodeEncodeError: 'ascii' codec can't encode character u'\u2603' in position 0: ordinal not in range(128)

seems to be failing on windows CI

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've applied this patch:

diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py
index e621f3ee0..407155373 100644
--- a/src/_pytest/_code/code.py
+++ b/src/_pytest/_code/code.py
@@ -574,7 +574,7 @@ class ExceptionInfo(object):
         __tracebackhide__ = True
         value = safe_str(self.value)
         if not re.search(regexp, value):
-            assert 0, "Pattern '{!s}' not found in '{!s}'".format(regexp, value)
+            assert 0, u"Pattern {!r} not found in {!r}".format(regexp, value)
         return True

The problem is that even so the check still fails, when it should pass:

______________________________ test_u _______________________________

    def test_u():
        with pytest.raises(AssertionError, match=u"\u2603"):
>           assert False, u"\u2603"
E           AssertionError: Pattern u'\u2603' not found in '\xe2\x98\x83\nassert False'

.tmp\test_u.py:6: AssertionError

This will be true for any system which can't handle that unicode string using the default locale.

This solution works for me and the rest of the suite:

diff --git a/src/_pytest/_code/code.py b/src/_pytest/_code/code.py
index e621f3ee0..e04bbc915 100644
--- a/src/_pytest/_code/code.py
+++ b/src/_pytest/_code/code.py
@@ -14,6 +14,7 @@ from weakref import ref
 import attr
 import pluggy
 import py
+import six
 from six import text_type

 import _pytest
@@ -572,9 +573,9 @@ class ExceptionInfo(object):
         raised.
         """
         __tracebackhide__ = True
-        value = safe_str(self.value)
+        value = text_type(self.value) if isinstance(regexp, text_type) else str(self.value)
         if not re.search(regexp, value):
-            assert 0, "Pattern '{!s}' not found in '{!s}'".format(regexp, value)
+            assert 0, u"Pattern '{!s}' not found in '{!s}'".format(regexp, value)
         return True

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

though this probably triggers the same error I posted in the issue when using --tb=native since now we're raising a (n incorrect) unicode message in python 2 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, but actually it works:

 λ pytest .tmp\test_u.py  -q
.
1 passed in 0.02 seconds

λ pytest .tmp\test_u.py  -q --tb=native
.
1 passed in 0.02 seconds

so I think we can go with that solution. 👍

with pytest.raises(AssertionError, match=u"\u2603"):
assert False, u"\u2603"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you also test a failure of unicode<=>unicode as well?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but what do you mean by unicode<=>unicode? 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh yeah that is super unclear, what the heck did I mean 🤔

there's 6(?) combinations I think and maybe need more tests for them:

  • bytes exception bytes regex success
  • bytes exception bytes regex failure
  • unicode exception unicode regex success
  • unicode exception unicode regex failure (I suspect that this currently causes an undisplayable exception in python2 because pytest would be raising a unicode exception)
  • ascii unicode exception ascii unicode bytes success (only in py2 due to implicit py2 conversion)
  • ascii unicode exception ascii unicode bytes failure (only in py2 due to implicit py2 conversion)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks. Done.

Surprisingly on Python 3 we don't support raising exceptions using bytes (raise RuntimeError(b"foo")), which I find reasonable actually.