Merge pull request #5162 from blueyed/mm

blueyed · web-flow · commit 76c6ed8b05d7 · 2019-04-27T01:42:42.000+02:00
Merge master into features
diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md
@@ -1,8 +1,10 @@
+<!--
 Thanks for submitting an issue!
 
-Here's a quick checklist in what to include:
+Here's a quick checklist for what to provide:
+-->
 
-- [ ] Include a detailed description of the bug or suggestion
-- [ ] `pip list` of the virtual environment you are using
+- [ ] a detailed description of the bug or suggestion
+- [ ] output of `pip list` from the virtual environment you are using
 - [ ] pytest and operating system versions
-- [ ] Minimal example if possible
+- [ ] minimal example if possible
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -1,7 +1,9 @@
+<!--
 Thanks for submitting a PR, your contribution is really appreciated!
 
-Here's a quick checklist that should be present in PRs (you can delete this text from the final description, this is
-just a guideline):
+Here's a quick checklist that should be present in PRs.
+(please delete this text from the final description, this is just a guideline)
+-->
 
 - [ ] Create a new changelog file in the `changelog` folder, with a name like `<ISSUE NUMBER>.<TYPE>.rst`. See [changelog/README.rst](https://github.com/pytest-dev/pytest/blob/master/changelog/README.rst) for details.
 - [ ] Target the `master` branch for bug fixes, documentation updates and trivial changes.
diff --git a/AUTHORS b/AUTHORS
@@ -105,6 +105,7 @@ Hugo van Kemenade
 Hui Wang (coldnight)
 Ian Bicking
 Ian Lesperance
+Ilya Konstantinov
 Ionuț Turturică
 Iwan Briquemont
 Jaap Broekhuizen
@@ -179,6 +180,7 @@ Nicholas Devenish
 Nicholas Murphy
 Niclas Olofsson
 Nicolas Delaby
+Nikolay Kondratyev
 Oleg Pidsadnyi
 Oleg Sushchenko
 Oliver Bestwalter
diff --git a/changelog/5089.bugfix.rst b/changelog/5089.bugfix.rst
@@ -0,0 +1 @@
+Fix crash caused by error in ``__repr__`` function with both ``showlocals`` and verbose output enabled.
diff --git a/changelog/5139.bugfix.rst b/changelog/5139.bugfix.rst
@@ -0,0 +1 @@
+Eliminate core dependency on 'terminal' plugin.
diff --git a/doc/en/skipping.rst b/doc/en/skipping.rst
@@ -208,7 +208,7 @@ Here's a quick guide on how to skip tests in a module in different situations:
 
   .. code-block:: python
 
-        pytestmark = pytest.mark.skipif(sys.platform == "win32", "tests for linux only")
+        pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="tests for linux only")
 
 3. Skip all tests in a module if some import is missing:
 
diff --git a/src/_pytest/_code/code.py b/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))
diff --git a/src/_pytest/_io/saferepr.py b/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):
diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py
@@ -704,7 +704,7 @@ def pytest_cmdline_parse(self, pluginmanager, args):
         return self
 
     def notify_exception(self, excinfo, option=None):
-        if option and option.fulltrace:
+        if option and getattr(option, "fulltrace", False):
             style = "long"
         else:
             style = "native"
diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py
@@ -248,7 +248,7 @@ def _repr_failure_py(self, excinfo, style=None):
         if excinfo.errisinstance(fm.FixtureLookupError):
             return excinfo.value.formatrepr()
         tbfilter = True
-        if self.config.option.fulltrace:
+        if self.config.getoption("fulltrace", False):
             style = "long"
         else:
             tb = _pytest._code.Traceback([excinfo.traceback[-1]])
@@ -260,12 +260,12 @@ def _repr_failure_py(self, excinfo, style=None):
                 style = "long"
         # XXX should excinfo.getrepr record all data and toterminal() process it?
         if style is None:
-            if self.config.option.tbstyle == "short":
+            if self.config.getoption("tbstyle", "auto") == "short":
                 style = "short"
             else:
                 style = "long"
 
-        if self.config.option.verbose > 1:
+        if self.config.getoption("verbose", 0) > 1:
             truncate_locals = False
         else:
             truncate_locals = True
@@ -279,7 +279,7 @@ def _repr_failure_py(self, excinfo, style=None):
         return excinfo.getrepr(
             funcargs=True,
             abspath=abspath,
-            showlocals=self.config.option.showlocals,
+            showlocals=self.config.getoption("showlocals", False),
             style=style,
             tbfilter=tbfilter,
             truncate_locals=truncate_locals,
diff --git a/src/_pytest/python.py b/src/_pytest/python.py
@@ -820,7 +820,7 @@ def setup(self):
             self.obj = self._getobj()
 
     def _prunetraceback(self, excinfo):
-        if hasattr(self, "_obj") and not self.config.option.fulltrace:
+        if hasattr(self, "_obj") and not self.config.getoption("fulltrace", False):
             code = _pytest._code.Code(get_real_func(self.obj))
             path, firstlineno = code.path, code.firstlineno
             traceback = excinfo.traceback
@@ -835,14 +835,14 @@ def _prunetraceback(self, excinfo):
             excinfo.traceback = ntraceback.filter()
             # issue364: mark all but first and last frames to
             # only show a single-line message for each frame
-            if self.config.option.tbstyle == "auto":
+            if self.config.getoption("tbstyle", "auto") == "auto":
                 if len(excinfo.traceback) > 2:
                     for entry in excinfo.traceback[1:-1]:
                         entry.set_repr_style("short")
 
     def repr_failure(self, excinfo, outerr=None):
         assert outerr is None, "XXX outerr usage is deprecated"
-        style = self.config.option.tbstyle
+        style = self.config.getoption("tbstyle", "auto")
         if style == "auto":
             style = "long"
         return self._repr_failure_py(excinfo, style=style)
diff --git a/src/_pytest/reports.py b/src/_pytest/reports.py
@@ -368,7 +368,7 @@ def from_item_and_call(cls, item, call):
                     longrepr = item.repr_failure(excinfo)
                 else:  # exception in setup or teardown
                     longrepr = item._repr_failure_py(
-                        excinfo, style=item.config.option.tbstyle
+                        excinfo, style=item.config.getoption("tbstyle", "auto")
                     )
         for rwhen, key, content in item._report_sections:
             sections.append(("Captured %s %s" % (key, rwhen), content))
diff --git a/testing/code/test_excinfo.py b/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)
diff --git a/testing/test_config.py b/testing/test_config.py
@@ -770,7 +770,7 @@ def test_notify_exception(testdir, capfd):
     config = testdir.parseconfig()
     with pytest.raises(ValueError) as excinfo:
         raise ValueError(1)
-    config.notify_exception(excinfo)
+    config.notify_exception(excinfo, config.option)
     out, err = capfd.readouterr()
     assert "ValueError" in err
 
@@ -779,10 +779,17 @@ def pytest_internalerror(self, excrepr):
             return True
 
     config.pluginmanager.register(A())
-    config.notify_exception(excinfo)
+    config.notify_exception(excinfo, config.option)
     out, err = capfd.readouterr()
     assert not err
 
+    config = testdir.parseconfig("-p", "no:terminal")
+    with pytest.raises(ValueError) as excinfo:
+        raise ValueError(1)
+    config.notify_exception(excinfo, config.option)
+    out, err = capfd.readouterr()
+    assert "ValueError" in err
+
 
 def test_load_initial_conftest_last_ordering(testdir, _config_for_test):
     pm = _config_for_test.pluginmanager
diff --git a/testing/test_session.py b/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="""
diff --git a/tox.ini b/tox.ini
@@ -73,7 +73,8 @@ commands = pre-commit run --all-files --show-diff-on-failure
 
 [testenv:docs]
 basepython = python3
-usedevelop = True
+# broken due to pip 19.1 (#5167)
+# usedevelop = True
 changedir = doc/en
 deps = -r{toxinidir}/doc/en/requirements.txt
 
@@ -127,7 +128,8 @@ commands =
 [testenv:release]
 decription = do a release, required posarg of the version number
 basepython = python3.6
-usedevelop = True
+# broken due to pip 19.1 (#5167)
+# usedevelop = True
 passenv = *
 deps =
     colorama

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Fix crash caused by error in ``__repr__`` function with both ``showlocals`` and verbose output enabled.
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Eliminate core dependency on 'terminal' plugin.`