From 0225be53a2e9f8f69536ff91241b30a8b1d313d7 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 14 Jul 2019 21:42:53 +0300 Subject: [PATCH 1/4] saferepr: Remove dead SafeRepr.repr_unicode This function is not called anywhere directly, and cannot be called by the dynamic `repr_()` dispatch mechanism because unicode is no longer a type in Python 3. --- src/_pytest/_io/saferepr.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/_pytest/_io/saferepr.py b/src/_pytest/_io/saferepr.py index 74f75124f..b5387d205 100644 --- a/src/_pytest/_io/saferepr.py +++ b/src/_pytest/_io/saferepr.py @@ -25,24 +25,6 @@ class SafeRepr(reprlib.Repr): def repr(self, x): return self._callhelper(reprlib.Repr.repr, self, x) - def repr_unicode(self, x, level): - # Strictly speaking wrong on narrow builds - def repr(u): - if "'" not in u: - return "'%s'" % u - elif '"' not in u: - return '"%s"' % u - else: - return "'%s'" % u.replace("'", r"\'") - - s = repr(x[: self.maxstring]) - if len(s) > self.maxstring: - i = max(0, (self.maxstring - 3) // 2) - j = max(0, self.maxstring - 3 - i) - s = repr(x[:i] + x[len(x) - j :]) - s = s[:i] + "..." + s[len(s) - j :] - return s - def repr_instance(self, x, level): return self._callhelper(repr, x) From 0394ebffee0b711c78fb81e4e057f6190a111250 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 14 Jul 2019 21:50:59 +0300 Subject: [PATCH 2/4] saferepr: Use an __init__ instead of setting attributes after construction This will be easier to type-check, and also somewhat clearer. --- src/_pytest/_io/saferepr.py | 13 +++++++------ testing/io/test_saferepr.py | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/_pytest/_io/saferepr.py b/src/_pytest/_io/saferepr.py index b5387d205..5a4c76b4b 100644 --- a/src/_pytest/_io/saferepr.py +++ b/src/_pytest/_io/saferepr.py @@ -22,6 +22,12 @@ class SafeRepr(reprlib.Repr): and includes information on exceptions raised during the call. """ + def __init__(self, maxsize): + super().__init__() + self.maxstring = maxsize + self.maxsize = maxsize + self.maxother = 160 + def repr(self, x): return self._callhelper(reprlib.Repr.repr, self, x) @@ -52,9 +58,4 @@ def saferepr(obj, maxsize=240): care to never raise exceptions itself. This function is a wrapper around the Repr/reprlib functionality of the standard 2.6 lib. """ - # review exception handling - srepr = SafeRepr() - srepr.maxstring = maxsize - srepr.maxsize = maxsize - srepr.maxother = 160 - return srepr.repr(obj) + return SafeRepr(maxsize).repr(obj) diff --git a/testing/io/test_saferepr.py b/testing/io/test_saferepr.py index f6abfe322..f005f0cc4 100644 --- a/testing/io/test_saferepr.py +++ b/testing/io/test_saferepr.py @@ -48,7 +48,7 @@ def test_exceptions(): def test_big_repr(): from _pytest._io.saferepr import SafeRepr - assert len(saferepr(range(1000))) <= len("[" + SafeRepr().maxlist * "1000" + "]") + assert len(saferepr(range(1000))) <= len("[" + SafeRepr(0).maxlist * "1000" + "]") def test_repr_on_newstyle(): From c7aacc96bbeae1590342f6b10f2364751f187c0c Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 14 Jul 2019 22:21:15 +0300 Subject: [PATCH 3/4] saferepr: Remove unused setting of max_other max_other is used by the superclass repr_instance, but we override it and use maxsize instead. --- src/_pytest/_io/saferepr.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/_pytest/_io/saferepr.py b/src/_pytest/_io/saferepr.py index 5a4c76b4b..a97411876 100644 --- a/src/_pytest/_io/saferepr.py +++ b/src/_pytest/_io/saferepr.py @@ -26,7 +26,6 @@ class SafeRepr(reprlib.Repr): super().__init__() self.maxstring = maxsize self.maxsize = maxsize - self.maxother = 160 def repr(self, x): return self._callhelper(reprlib.Repr.repr, self, x) From 129600d698622633a9e8036f89e88b50aa101abd Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 14 Jul 2019 22:24:12 +0300 Subject: [PATCH 4/4] saferepr: Avoid indirect function calls The DRY savings they provide are rather small, while they make it harder to type-check, and IMO harder to understand. --- changelog/5603.trivial.rst | 1 + src/_pytest/_io/saferepr.py | 53 +++++++++++++++++++++---------------- testing/io/test_saferepr.py | 11 ++++++++ 3 files changed, 42 insertions(+), 23 deletions(-) create mode 100644 changelog/5603.trivial.rst diff --git a/changelog/5603.trivial.rst b/changelog/5603.trivial.rst new file mode 100644 index 000000000..310e88562 --- /dev/null +++ b/changelog/5603.trivial.rst @@ -0,0 +1 @@ +Simplified internal ``SafeRepr`` class and removed some dead code. diff --git a/src/_pytest/_io/saferepr.py b/src/_pytest/_io/saferepr.py index a97411876..7704421a2 100644 --- a/src/_pytest/_io/saferepr.py +++ b/src/_pytest/_io/saferepr.py @@ -2,19 +2,23 @@ import pprint import reprlib -def _call_and_format_exception(call, x, *args): +def _format_repr_exception(exc, obj): + exc_name = type(exc).__name__ 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 '<[{}("{}") raised in repr()] {} object at 0x{:x}>'.format( - exc_name, exc_info, x.__class__.__name__, id(x) - ) + exc_info = str(exc) + except Exception: + exc_info = "unknown" + return '<[{}("{}") raised in repr()] {} object at 0x{:x}>'.format( + exc_name, exc_info, obj.__class__.__name__, id(obj) + ) + + +def _ellipsize(s, maxsize): + if len(s) > maxsize: + i = max(0, (maxsize - 3) // 2) + j = max(0, maxsize - 3 - i) + return s[:i] + "..." + s[len(s) - j :] + return s class SafeRepr(reprlib.Repr): @@ -28,18 +32,18 @@ class SafeRepr(reprlib.Repr): self.maxsize = maxsize def repr(self, x): - return self._callhelper(reprlib.Repr.repr, self, x) + try: + s = super().repr(x) + except Exception as exc: + s = _format_repr_exception(exc, x) + return _ellipsize(s, self.maxsize) def repr_instance(self, x, level): - return self._callhelper(repr, x) - - def _callhelper(self, call, x, *args): - 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 + try: + s = repr(x) + except Exception as exc: + s = _format_repr_exception(exc, x) + return _ellipsize(s, self.maxsize) def safeformat(obj): @@ -47,7 +51,10 @@ def safeformat(obj): Failing __repr__ functions of user instances will be represented with a short exception info. """ - return _call_and_format_exception(pprint.pformat, obj) + try: + return pprint.pformat(obj) + except Exception as exc: + return _format_repr_exception(exc, obj) def saferepr(obj, maxsize=240): diff --git a/testing/io/test_saferepr.py b/testing/io/test_saferepr.py index f005f0cc4..86897b57c 100644 --- a/testing/io/test_saferepr.py +++ b/testing/io/test_saferepr.py @@ -45,6 +45,17 @@ def test_exceptions(): assert "unknown" in s2 +def test_buggy_builtin_repr(): + # Simulate a case where a repr for a builtin raises. + # reprlib dispatches by type name, so use "int". + + class int: + def __repr__(self): + raise ValueError("Buggy repr!") + + assert "Buggy" in saferepr(int()) + + def test_big_repr(): from _pytest._io.saferepr import SafeRepr