Merge pull request #5133 from kondratyev-nv/fix-handle-repr-error-with-showlocals

Fix handle repr error with showlocals and verbose output

Author: blueyed

AUTHORS

@@ -180,6 +180,7 @@ Nicholas Devenish
 Nicholas Murphy
 Niclas Olofsson
 Nicolas Delaby
+Nikolay Kondratyev
 Oleg Pidsadnyi
 Oleg Sushchenko
 Oliver Bestwalter

changelog/5089.bugfix.rst

@@ -0,0 +1 @@
+Fix crash caused by error in ``__repr__`` function with both ``showlocals`` and verbose output enabled.

src/_pytest/_code/code.py

@@ -3,7 +3,6 @@
 from __future__ import print_function
 
 import inspect
-import pprint
 import re
 import sys
 import traceback
@@ -18,6 +17,7 @@
 from six import text_type
 
 import _pytest
+from _pytest._io.saferepr import safeformat
 from _pytest._io.saferepr import saferepr
 from _pytest.compat import _PY2
 from _pytest.compat import _PY3
@@ -614,14 +614,11 @@ def _getentrysource(self, entry):
             source = source.deindent()
         return source
 
-    def _saferepr(self, obj):
-        return saferepr(obj)
-
     def repr_args(self, entry):
         if self.funcargs:
             args = []
             for argname, argvalue in entry.frame.getargs(var=True):
-                args.append((argname, self._saferepr(argvalue)))
+                args.append((argname, saferepr(argvalue)))
             return ReprFuncArgs(args)
 
     def get_source(self, source, line_index=-1, excinfo=None, short=False):
@@ -674,9 +671,9 @@ def repr_locals(self, locals):
                     # _repr() function, which is only reprlib.Repr in
                     # disguise, so is very configurable.
                     if self.truncate_locals:
-                        str_repr = self._saferepr(value)
+                        str_repr = saferepr(value)
                     else:
-                        str_repr = pprint.pformat(value)
+                        str_repr = safeformat(value)
                     # if len(str_repr) < 70 or not isinstance(value,
                     #                            (list, tuple, dict)):
                     lines.append("%-10s = %s" % (name, str_repr))

src/_pytest/_io/saferepr.py

@@ -1,8 +1,26 @@
-import sys
+import pprint
 
 from six.moves import reprlib
 
 
+def _call_and_format_exception(call, x, *args):
+    try:
+        # Try the vanilla repr and make sure that the result is a string
+        return call(x, *args)
+    except Exception as exc:
+        exc_name = type(exc).__name__
+        try:
+            exc_info = str(exc)
+        except Exception:
+            exc_info = "unknown"
+        return '<[%s("%s") raised in repr()] %s object at 0x%x>' % (
+            exc_name,
+            exc_info,
+            x.__class__.__name__,
+            id(x),
+        )
+
+
 class SafeRepr(reprlib.Repr):
     """subclass of repr.Repr that limits the resulting size of repr()
     and includes information on exceptions raised during the call.
@@ -33,28 +51,20 @@ def repr_instance(self, x, level):
         return self._callhelper(repr, x)
 
     def _callhelper(self, call, x, *args):
-        try:
-            # Try the vanilla repr and make sure that the result is a string
-            s = call(x, *args)
-        except Exception:
-            cls, e, tb = sys.exc_info()
-            exc_name = getattr(cls, "__name__", "unknown")
-            try:
-                exc_info = str(e)
-            except Exception:
-                exc_info = "unknown"
-            return '<[%s("%s") raised in repr()] %s object at 0x%x>' % (
-                exc_name,
-                exc_info,
-                x.__class__.__name__,
-                id(x),
-            )
-        else:
-            if len(s) > self.maxsize:
-                i = max(0, (self.maxsize - 3) // 2)
-                j = max(0, self.maxsize - 3 - i)
-                s = s[:i] + "..." + s[len(s) - j :]
-            return s
+        s = _call_and_format_exception(call, x, *args)
+        if len(s) > self.maxsize:
+            i = max(0, (self.maxsize - 3) // 2)
+            j = max(0, self.maxsize - 3 - i)
+            s = s[:i] + "..." + s[len(s) - j :]
+        return s
+
+
+def safeformat(obj):
+    """return a pretty printed string for the given object.
+    Failing __repr__ functions of user instances will be represented
+    with a short exception info.
+    """
+    return _call_and_format_exception(pprint.pformat, obj)
 
 
 def saferepr(obj, maxsize=240):

testing/code/test_excinfo.py

@@ -598,6 +598,35 @@ def test_repr_local(self):
         assert reprlocals.lines[2] == "y          = 5"
         assert reprlocals.lines[3] == "z          = 7"
 
+    def test_repr_local_with_error(self):
+        class ObjWithErrorInRepr:
+            def __repr__(self):
+                raise NotImplementedError
+
+        p = FormattedExcinfo(showlocals=True, truncate_locals=False)
+        loc = {"x": ObjWithErrorInRepr(), "__builtins__": {}}
+        reprlocals = p.repr_locals(loc)
+        assert reprlocals.lines
+        assert reprlocals.lines[0] == "__builtins__ = <builtins>"
+        assert '[NotImplementedError("") raised in repr()]' in reprlocals.lines[1]
+
+    def test_repr_local_with_exception_in_class_property(self):
+        class ExceptionWithBrokenClass(Exception):
+            @property
+            def __class__(self):
+                raise TypeError("boom!")
+
+        class ObjWithErrorInRepr:
+            def __repr__(self):
+                raise ExceptionWithBrokenClass()
+
+        p = FormattedExcinfo(showlocals=True, truncate_locals=False)
+        loc = {"x": ObjWithErrorInRepr(), "__builtins__": {}}
+        reprlocals = p.repr_locals(loc)
+        assert reprlocals.lines
+        assert reprlocals.lines[0] == "__builtins__ = <builtins>"
+        assert '[ExceptionWithBrokenClass("") raised in repr()]' in reprlocals.lines[1]
+
     def test_repr_local_truncated(self):
         loc = {"l": [i for i in range(10)]}
         p = FormattedExcinfo(showlocals=True)

testing/test_session.py

@@ -134,6 +134,30 @@ def test_implicit_bad_repr1(self):
             != -1
         )
 
+    def test_broken_repr_with_showlocals_verbose(self, testdir):
+        p = testdir.makepyfile(
+            """
+            class ObjWithErrorInRepr:
+                def __repr__(self):
+                    raise NotImplementedError
+
+            def test_repr_error():
+                x = ObjWithErrorInRepr()
+                assert x == "value"
+        """
+        )
+        reprec = testdir.inline_run("--showlocals", "-vv", p)
+        passed, skipped, failed = reprec.listoutcomes()
+        assert (len(passed), len(skipped), len(failed)) == (0, 0, 1)
+        entries = failed[0].longrepr.reprtraceback.reprentries
+        assert len(entries) == 1
+        repr_locals = entries[0].reprlocals
+        assert repr_locals.lines
+        assert len(repr_locals.lines) == 1
+        assert repr_locals.lines[0].startswith(
+            'x          = <[NotImplementedError("") raised in repr()] ObjWithErrorInRepr'
+        )
+
     def test_skip_file_by_conftest(self, testdir):
         testdir.makepyfile(
             conftest="""