pytest-dev · nicoddemus · Oct 19, 2018 · Oct 18, 2018 · Oct 19, 2018 · Zac-HD
diff --git a/changelog/3691.bugfix.rst b/changelog/3691.bugfix.rst
@@ -0,0 +1,2 @@
+Python 2: safely format warning message about passing unicode strings to ``warnings.warn``, which may cause
+surprising ``MemoryError`` exception when monkey patching ``warnings.warn`` itself.
diff --git a/src/_pytest/warnings.py b/src/_pytest/warnings.py
@@ -123,7 +123,7 @@ def warning_record_to_str(warning_message):
     if unicode_warning:
         warnings.warn(
             "Warning is using unicode non convertible to ascii, "
-            "converting to a safe representation:\n  %s" % msg,
+            "converting to a safe representation:\n  {!r}".format(compat.safe_str(msg)),
             UnicodeWarning,
         )
     return msg

diff --git a/testing/test_warnings.py b/testing/test_warnings.py
@@ -3,6 +3,8 @@
 
 import sys
 
+import six
+
 import pytest
 
 
@@ -562,3 +564,30 @@ def test_hidden_by_system(self, testdir, monkeypatch):
         monkeypatch.setenv(str("PYTHONWARNINGS"), str("once::UserWarning"))
         result = testdir.runpytest_subprocess()
         assert WARNINGS_SUMMARY_HEADER not in result.stdout.str()
+
+
+@pytest.mark.skipif(six.PY3, reason="Python 2 only issue")
+def test_infinite_loop_warning_against_unicode_usage_py2(testdir):
+    """
+    We need to be careful when raising the warning about unicode usage with "warnings.warn"
+    because it might be overwritten by users and this itself causes another warning (#3691).
+    """
+    testdir.makepyfile(
+        """
+        # -*- coding: utf8 -*-
+        from __future__ import unicode_literals
+        import warnings
+        import pytest
+
+        def _custom_showwarning(message, *a, **b):
+            return "WARNING: {}".format(message)
+
+        warnings.formatwarning = _custom_showwarning
+
+        @pytest.mark.filterwarnings("default")
+        def test_custom_warning_formatter():
+            warnings.warn("¥")
+    """
+    )
+    result = testdir.runpytest_subprocess()
+    result.stdout.fnmatch_lines(["*1 passed, * warnings in*"])
diff --git a/tox.ini b/tox.ini
@@ -18,7 +18,7 @@ envlist =
 
 [testenv]
 commands =
-    {env:_PYTEST_TOX_COVERAGE_RUN:} pytest --lsof
+    {env:_PYTEST_TOX_COVERAGE_RUN:} pytest --lsof {posargs}
     coverage: coverage combine
     coverage: coverage report
 passenv = USER USERNAME COVERAGE_* TRAVIS
@@ -41,7 +41,7 @@ deps =
     py27: mock
     nose
 commands =
-    pytest -n auto --runpytest=subprocess
+    pytest -n auto --runpytest=subprocess {posargs}
 
 
 [testenv:linting]
@@ -58,7 +58,7 @@ deps =
     hypothesis>=3.56
     {env:_PYTEST_TOX_EXTRA_DEP:}
 commands =
-    {env:_PYTEST_TOX_COVERAGE_RUN:} pytest -n auto
+    {env:_PYTEST_TOX_COVERAGE_RUN:} pytest -n auto {posargs}
 
 [testenv:py36-xdist]
 # NOTE: copied from above due to https://github.com/tox-dev/tox/issues/706.
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Python 2: safely format warning message about passing unicode strings to ``warnings.warn``, which may cause
		surprising ``MemoryError`` exception when monkey patching ``warnings.warn`` itself.
Copy link Member Zac-HD Oct 19, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. (comment for my own education about pytest's documentation) In Hypothesis we have the `intersphinx_mapping` configured so that we can write these as: surprising :class:`python:MemoryError` exception when monkey patching :func:`python:warnings.warn` itself. It allows for somewhat nicer formatting, but more importantly adds a hyperlink - and can check that the link target exists, which caught dozens of typos in our docs. Has pytest decided not to do this, or just not considered it? It can be fiddly at times but also pretty useful. Copy link Member RonnyPfannschmidt Oct 19, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. i don't recall use taking a deeper look into this so it may be worth trying it - ideally someone with experience setting it up Zac-HD reacted with thumbs up emoji Copy link Member Zac-HD Oct 19, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. I'll write up an issue then 😄 Copy link Member Author nicoddemus Oct 19, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. @Zac-HD thanks a lot for the pointer, actually or `intersphinx_mapping` is already configured. I tested locally your changes and they actually do work. 😁 We should remember to use those sphinx references more often. 👍 Zac-HD reacted with hooray emoji Copy link Member Author nicoddemus Oct 19, 2018 Choose a reason for hiding this comment The reason will be displayed to describe this comment to others. Learn more. Hmm actually I spoke too soon. While it generates the proper docs, our linter complains: `ERROR changelog/3691.bugfix.rst:1 Unknown interpreted text role "func". ERROR changelog/3691.bugfix.rst:1 Unknown interpreted text role "class". ERROR changelog/3691.bugfix.rst:1 Unknown interpreted text role "func". ERROR changelog/4192.bugfix.rst:1 Unknown interpreted text role "func". ERROR changelog/4192.bugfix.rst:1 Unknown interpreted text role "func".`