Merge pull request #10816 from pytest-dev/backport-10772-to-7.2.x

[7.2.x] Correctly handle tracebackhide for chained exceptions

.pre-commit-config.yaml

@@ -2,17 +2,17 @@ default_language_version:
     python: "3.10"
 repos:
 -   repo: https://github.com/psf/black
-    rev: 22.12.0
+    rev: 23.1.0
     hooks:
     -   id: black
         args: [--safe, --quiet]
 -   repo: https://github.com/asottile/blacken-docs
-    rev: v1.12.1
+    rev: 1.13.0
     hooks:
     -   id: blacken-docs
-        additional_dependencies: [black==20.8b1]
+        additional_dependencies: [black==23.1.0]
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.3.0
+    rev: v4.4.0
     hooks:
     -   id: trailing-whitespace
     -   id: end-of-file-fixer
@@ -23,7 +23,7 @@ repos:
         exclude: _pytest/(debugging|hookspec).py
         language_version: python3
 -   repo: https://github.com/PyCQA/autoflake
-    rev: v1.7.6
+    rev: v2.0.2
     hooks:
     -   id: autoflake
         name: autoflake
@@ -31,7 +31,7 @@ repos:
         language: python
         files: \.py$
 -   repo: https://github.com/PyCQA/flake8
-    rev: 5.0.4
+    rev: 6.0.0
     hooks:
     -   id: flake8
         language_version: python3
@@ -39,7 +39,7 @@ repos:
           - flake8-typing-imports==1.12.0
           - flake8-docstrings==1.5.0
 -   repo: https://github.com/asottile/reorder_python_imports
-    rev: v3.8.5
+    rev: v3.9.0
     hooks:
     -   id: reorder-python-imports
         args: ['--application-directories=.:src', --py37-plus]
@@ -49,16 +49,16 @@ repos:
     -   id: pyupgrade
         args: [--py37-plus]
 -   repo: https://github.com/asottile/setup-cfg-fmt
-    rev: v2.1.0
+    rev: v2.2.0
     hooks:
     -   id: setup-cfg-fmt
         args: ["--max-py-version=3.11", "--include-version-classifiers"]
 -   repo: https://github.com/pre-commit/pygrep-hooks
-    rev: v1.9.0
+    rev: v1.10.0
     hooks:
     -   id: python-use-type-annotations
 -   repo: https://github.com/pre-commit/mirrors-mypy
-    rev: v0.982
+    rev: v1.1.1
     hooks:
     -   id: mypy
         files: ^(src/|testing/)

AUTHORS

@@ -126,6 +126,7 @@ Erik M. Bray
 Evan Kepner
 Fabien Zarifian
 Fabio Zadrozny
+Felix Hofstätter
 Felix Nieuwenhuizen
 Feng Ma
 Florian Bruhin

changelog/1904.bugfix.rst

@@ -0,0 +1 @@
+Correctly handle ``__tracebackhide__`` for chained exceptions.

doc/en/deprecations.rst

@@ -1052,7 +1052,7 @@ that are then turned into proper test methods. Example:
 .. code-block:: python
 
     def check(x, y):
-        assert x ** x == y
+        assert x**x == y
 
 
     def test_squared():
@@ -1067,7 +1067,7 @@ This form of test function doesn't support fixtures properly, and users should s
 
     @pytest.mark.parametrize("x, y", [(2, 4), (3, 9)])
     def test_squared(x, y):
-        assert x ** x == y
+        assert x**x == y
 
 .. _internal classes accessed through node deprecated:
 

doc/en/how-to/fixtures.rst

@@ -1237,7 +1237,6 @@ If the data created by the factory requires managing, the fixture can take care
 
     @pytest.fixture
     def make_customer_record():
-
         created_records = []
 
         def _make_customer_record(name):

doc/en/how-to/monkeypatch.rst

@@ -135,10 +135,10 @@ This can be done in our test file by defining a class to represent ``r``.
     # this is the previous code block example
     import app
 
+
     # custom class to be the mock return value
     # will override the requests.Response returned from requests.get
     class MockResponse:
-
         # mock json() method always returns a specific testing dictionary
         @staticmethod
         def json():
@@ -146,7 +146,6 @@ This can be done in our test file by defining a class to represent ``r``.
 
 
     def test_get_json(monkeypatch):
-
         # Any arguments may be passed and mock_get() will always return our
         # mocked object, which only has the .json() method.
         def mock_get(*args, **kwargs):
@@ -181,6 +180,7 @@ This mock can be shared across tests using a ``fixture``:
     # app.py that includes the get_json() function
     import app
 
+
     # custom class to be the mock return value of requests.get()
     class MockResponse:
         @staticmethod
@@ -358,7 +358,6 @@ For testing purposes we can patch the ``DEFAULT_CONFIG`` dictionary to specific
 
 
     def test_connection(monkeypatch):
-
         # Patch the values of DEFAULT_CONFIG to specific
         # testing values only for this test.
         monkeypatch.setitem(app.DEFAULT_CONFIG, "user", "test_user")
@@ -383,7 +382,6 @@ You can use the :py:meth:`monkeypatch.delitem <MonkeyPatch.delitem>` to remove v
 
 
     def test_missing_user(monkeypatch):
-
         # patch the DEFAULT_CONFIG t be missing the 'user' key
         monkeypatch.delitem(app.DEFAULT_CONFIG, "user", raising=False)
 
@@ -404,6 +402,7 @@ separate fixtures for each potential mock and reference them in the needed tests
     # app.py with the connection string function
     import app
 
+
     # all of the mocks are moved into separated fixtures
     @pytest.fixture
     def mock_test_user(monkeypatch):
@@ -425,7 +424,6 @@ separate fixtures for each potential mock and reference them in the needed tests
 
     # tests reference only the fixture mocks that are needed
     def test_connection(mock_test_user, mock_test_database):
-
         expected = "User Id=test_user; Location=test_db;"
 
         result = app.create_connection_string()
@@ -433,7 +431,6 @@ separate fixtures for each potential mock and reference them in the needed tests
 
 
     def test_missing_user(mock_missing_default_user):
-
         with pytest.raises(KeyError):
             _ = app.create_connection_string()
 

doc/en/how-to/writing_hook_functions.rst

@@ -249,6 +249,7 @@ and use pytest_addoption as follows:
 
    # contents of hooks.py
 
+
    # Use firstresult=True because we only want one plugin to define this
    # default value
    @hookspec(firstresult=True)

src/_pytest/_code/code.py

@@ -411,13 +411,13 @@ class Traceback(List[TracebackEntry]):
         """
         return Traceback(filter(fn, self), self._excinfo)
 
-    def getcrashentry(self) -> TracebackEntry:
+    def getcrashentry(self) -> Optional[TracebackEntry]:
         """Return last non-hidden traceback entry that lead to the exception of a traceback."""
         for i in range(-1, -len(self) - 1, -1):
             entry = self[i]
             if not entry.ishidden():
                 return entry
-        return self[-1]
+        return None
 
     def recursionindex(self) -> Optional[int]:
         """Return the index of the frame/TracebackEntry where recursion originates if
@@ -602,11 +602,13 @@ class ExceptionInfo(Generic[E]):
         """
         return isinstance(self.value, exc)
 
-    def _getreprcrash(self) -> "ReprFileLocation":
+    def _getreprcrash(self) -> Optional["ReprFileLocation"]:
         exconly = self.exconly(tryshort=True)
         entry = self.traceback.getcrashentry()
+        if entry:
             path, lineno = entry.frame.code.raw.co_filename, entry.lineno
             return ReprFileLocation(path, lineno + 1, exconly)
+        return None
 
     def getrepr(
         self,
@@ -942,9 +944,14 @@ class FormattedExcinfo:
                     )
                 else:
                     reprtraceback = self.repr_traceback(excinfo_)
-                reprcrash: Optional[ReprFileLocation] = (
-                    excinfo_._getreprcrash() if self.style != "value" else None
-                )
+
+                # will be None if all traceback entries are hidden
+                reprcrash: Optional[ReprFileLocation] = excinfo_._getreprcrash()
+                if reprcrash:
+                    if self.style == "value":
+                        repr_chain += [(reprtraceback, None, descr)]
+                    else:
+                        repr_chain += [(reprtraceback, reprcrash, descr)]
             else:
                 # Fallback to native repr if the exception doesn't have a traceback:
                 # ExceptionInfo objects require a full traceback to work.
@@ -952,8 +959,8 @@ class FormattedExcinfo:
                     traceback.format_exception(type(e), e, None)
                 )
                 reprcrash = None
-
                 repr_chain += [(reprtraceback, reprcrash, descr)]
+
             if e.__cause__ is not None and self.chain:
                 e = e.__cause__
                 excinfo_ = (
@@ -1037,7 +1044,7 @@ class ExceptionChainRepr(ExceptionRepr):
 @attr.s(eq=False, auto_attribs=True)
 class ReprExceptionInfo(ExceptionRepr):
     reprtraceback: "ReprTraceback"
-    reprcrash: "ReprFileLocation"
+    reprcrash: Optional["ReprFileLocation"]
 
     def toterminal(self, tw: TerminalWriter) -> None:
         self.reprtraceback.toterminal(tw)

src/_pytest/_py/path.py

@@ -24,6 +24,7 @@ from stat import S_ISLNK
 from stat import S_ISREG
 from typing import Any
 from typing import Callable
+from typing import cast
 from typing import overload
 from typing import TYPE_CHECKING
 
@@ -146,7 +147,7 @@ class Visitor:
         self.fil = fil
         self.ignore = ignore
         self.breadthfirst = bf
-        self.optsort = sort and sorted or (lambda x: x)
+        self.optsort = cast(Callable[[Any], Any], sorted) if sort else (lambda x: x)
 
     def gen(self, path):
         try:
@@ -224,7 +225,7 @@ class Stat:
             raise NotImplementedError("XXX win32")
         import pwd
 
-        entry = error.checked_call(pwd.getpwuid, self.uid)
+        entry = error.checked_call(pwd.getpwuid, self.uid)  # type:ignore[attr-defined]
         return entry[0]
 
     @property
@@ -234,7 +235,7 @@ class Stat:
             raise NotImplementedError("XXX win32")
         import grp
 
-        entry = error.checked_call(grp.getgrgid, self.gid)
+        entry = error.checked_call(grp.getgrgid, self.gid)  # type:ignore[attr-defined]
         return entry[0]
 
     def isdir(self):
@@ -252,7 +253,7 @@ def getuserid(user):
     import pwd
 
     if not isinstance(user, int):
-        user = pwd.getpwnam(user)[2]
+        user = pwd.getpwnam(user)[2]  # type:ignore[attr-defined]
     return user
 
 
@@ -260,7 +261,7 @@ def getgroupid(group):
     import grp
 
     if not isinstance(group, int):
-        group = grp.getgrnam(group)[2]
+        group = grp.getgrnam(group)[2]  # type:ignore[attr-defined]
     return group
 
 

src/_pytest/assertion/rewrite.py

@@ -278,7 +278,6 @@ class AssertionRewritingHook(importlib.abc.MetaPathFinder, importlib.abc.Loader)
             return f.read()
 
     if sys.version_info >= (3, 10):
-
         if sys.version_info >= (3, 12):
             from importlib.resources.abc import TraversableResources
         else:

src/_pytest/capture.py

@@ -253,7 +253,6 @@ class NoCapture:
 
 
 class SysCaptureBinary:
-
     EMPTY_BUFFER = b""
 
     def __init__(self, fd: int, tmpfile=None, *, tee: bool = False) -> None:

src/_pytest/config/__init__.py

@@ -62,7 +62,6 @@ from _pytest.warning_types import PytestConfigWarning
 from _pytest.warning_types import warn_explicit_for
 
 if TYPE_CHECKING:
-
     from _pytest._code.code import _TracebackStyle
     from _pytest.terminal import TerminalReporter
     from .argparsing import Argument
@@ -1059,7 +1058,6 @@ class Config:
         try:
             self.parse(args)
         except UsageError:
-
             # Handle --version and --help here in a minimal fashion.
             # This gets done via helpconfig normally, but its
             # pytest_cmdline_main is not called in case of errors.

src/_pytest/config/compat.py

@@ -43,7 +43,6 @@ class PathAwareHookProxy:
 
             @_wraps(hook)
             def fixed_hook(**kw):
-
                 path_value: Optional[Path] = kw.pop(path_var, None)
                 fspath_value: Optional[LEGACY_PATH] = kw.pop(fspath_var, None)
                 if fspath_value is not None:

src/_pytest/doctest.py

@@ -531,7 +531,6 @@ class DoctestModule(Module):
                 if _is_mocked(obj):
                     return
                 with _patch_unwrap_mock_aware():
-
                     # Type ignored because this is a private function.
                     super()._find(  # type:ignore[misc]
                         tests, obj, name, module, source_lines, globs, seen

src/_pytest/hookspec.py

@@ -738,7 +738,7 @@ def pytest_assertion_pass(item: "Item", lineno: int, orig: str, expl: str) -> No
 # -------------------------------------------------------------------------
 
 
-def pytest_report_header(
+def pytest_report_header(  # type:ignore[empty-body]
     config: "Config", start_path: Path, startdir: "LEGACY_PATH"
 ) -> Union[str, List[str]]:
     """Return a string or list of strings to be displayed as header info for terminal reporting.
@@ -767,7 +767,7 @@ def pytest_report_header(
     """
 
 
-def pytest_report_collectionfinish(
+def pytest_report_collectionfinish(  # type:ignore[empty-body]
     config: "Config",
     start_path: Path,
     startdir: "LEGACY_PATH",
@@ -800,7 +800,7 @@ def pytest_report_collectionfinish(
 
 
 @hookspec(firstresult=True)
-def pytest_report_teststatus(
+def pytest_report_teststatus(  # type:ignore[empty-body]
     report: Union["CollectReport", "TestReport"], config: "Config"
 ) -> Tuple[str, str, Union[str, Mapping[str, bool]]]:
     """Return result-category, shortletter and verbose word for status
@@ -880,7 +880,9 @@ def pytest_warning_recorded(
 # -------------------------------------------------------------------------
 
 
-def pytest_markeval_namespace(config: "Config") -> Dict[str, Any]:
+def pytest_markeval_namespace(  # type:ignore[empty-body]
+    config: "Config",
+) -> Dict[str, Any]:
     """Called when constructing the globals dictionary used for
     evaluating string conditions in xfail/skipif markers.
 

src/_pytest/outcomes.py

@@ -223,7 +223,6 @@ def _resolve_msg_to_reason(
     """
     __tracebackhide__ = True
     if msg is not None:
-
         if reason:
             from pytest import UsageError
 

src/_pytest/python.py

@@ -790,7 +790,8 @@ def _call_with_optional_argument(func, arg) -> None:
 
 def _get_first_non_fixture_func(obj: object, names: Iterable[str]) -> Optional[object]:
     """Return the attribute from the given object to be used as a setup/teardown
-    xunit-style function, but only if not marked as a fixture to avoid calling it twice."""
+    xunit-style function, but only if not marked as a fixture to avoid calling it twice.
+    """
     for name in names:
         meth: Optional[object] = getattr(obj, name, None)
         if meth is not None and fixtures.getfixturemarker(meth) is None:

src/_pytest/reports.py

@@ -337,6 +337,10 @@ class TestReport(BaseReport):
             elif isinstance(excinfo.value, skip.Exception):
                 outcome = "skipped"
                 r = excinfo._getreprcrash()
+                if r is None:
+                    raise ValueError(
+                        "There should always be a traceback entry for skipping a test."
+                    )
                 if excinfo.value._use_item_location:
                     path, line = item.reportinfo()[:2]
                     assert line is not None
@@ -573,7 +577,6 @@ def _report_kwargs_from_json(reportdict: Dict[str, Any]) -> Dict[str, Any]:
         and "reprcrash" in reportdict["longrepr"]
         and "reprtraceback" in reportdict["longrepr"]
     ):
-
         reprtraceback = deserialize_repr_traceback(
             reportdict["longrepr"]["reprtraceback"]
         )

testing/_py/test_local.py

@@ -803,7 +803,7 @@ class TestLocalPath(CommonFSTests):
         # depending on how the paths are used), but > 4096 (which is the
         # Linux' limitation) - the behaviour of paths with names > 4096 chars
         # is undetermined
-        newfilename = "/test" * 60
+        newfilename = "/test" * 60  # type:ignore[unreachable]
         l1 = tmpdir.join(newfilename)
         l1.ensure(file=True)
         l1.write("foo")
@@ -1344,8 +1344,8 @@ class TestPOSIXLocalPath:
         assert realpath.basename == "file"
 
     def test_owner(self, path1, tmpdir):
-        from pwd import getpwuid
-        from grp import getgrgid
+        from pwd import getpwuid  # type:ignore[attr-defined]
+        from grp import getgrgid  # type:ignore[attr-defined]
 
         stat = path1.stat()
         assert stat.path == path1

testing/acceptance_test.py

@@ -879,7 +879,6 @@ class TestDurations:
         )
 
     def test_calls_show_2(self, pytester: Pytester, mock_timing) -> None:
-
         pytester.makepyfile(self.source)
         result = pytester.runpytest_inprocess("--durations=2")
         assert result.ret == 0

testing/code/test_excinfo.py

@@ -294,6 +294,7 @@ class TestTraceback_f_g_h:
         excinfo = pytest.raises(ValueError, f)
         tb = excinfo.traceback
         entry = tb.getcrashentry()
+        assert entry is not None
         co = _pytest._code.Code.from_function(h)
         assert entry.frame.code.path == co.path
         assert entry.lineno == co.firstlineno + 1
@@ -311,10 +312,7 @@ class TestTraceback_f_g_h:
         excinfo = pytest.raises(ValueError, f)
         tb = excinfo.traceback
         entry = tb.getcrashentry()
-        co = _pytest._code.Code.from_function(g)
-        assert entry.frame.code.path == co.path
-        assert entry.lineno == co.firstlineno + 2
-        assert entry.frame.code.name == "g"
+        assert entry is None
 
 
 def test_excinfo_exconly():

testing/conftest.py

@@ -196,7 +196,6 @@ def mock_timing(monkeypatch: MonkeyPatch):
 
     @attr.s
     class MockTiming:
-
         _current_time = attr.ib(default=1590150050.0)
 
         def sleep(self, seconds):

testing/test_cacheprovider.py

@@ -494,7 +494,6 @@ class TestLastFailed:
     def test_lastfailed_collectfailure(
         self, pytester: Pytester, monkeypatch: MonkeyPatch
     ) -> None:
-
         pytester.makepyfile(
             test_maybe="""
             import os

testing/test_doctest.py

@@ -1236,7 +1236,6 @@ class TestDoctestSkips:
 
 
 class TestDoctestAutoUseFixtures:
-
     SCOPES = ["module", "session", "class", "function"]
 
     def test_doctest_module_session_fixture(self, pytester: Pytester):
@@ -1379,7 +1378,6 @@ class TestDoctestAutoUseFixtures:
 
 
 class TestDoctestNamespaceFixture:
-
     SCOPES = ["module", "session", "class", "function"]
 
     @pytest.mark.parametrize("scope", SCOPES)

testing/test_junitxml.py

@@ -253,7 +253,6 @@ class TestPython:
         duration_report: str,
         run_and_parse: RunAndParse,
     ) -> None:
-
         # mock LogXML.node_reporter so it always sets a known duration to each test report object
         original_node_reporter = LogXML.node_reporter
 
@@ -603,7 +602,6 @@ class TestPython:
         node.assert_attr(failures=3, tests=3)
 
         for index, char in enumerate("<&'"):
-
             tnode = node.find_nth_by_tag("testcase", index)
             tnode.assert_attr(
                 classname="test_failure_escape", name="test_func[%s]" % char

testing/test_monkeypatch.py

@@ -92,7 +92,7 @@ class TestSetattrWithImportPath:
             mp.delattr("os.path.abspath")
             assert not hasattr(os.path, "abspath")
             mp.undo()
-            assert os.path.abspath
+            assert os.path.abspath  # type:ignore[truthy-function]
 
 
 def test_delattr() -> None:

testing/test_runner.py

@@ -880,6 +880,7 @@ def test_makereport_getsource_dynamic_code(
 def test_store_except_info_on_error() -> None:
     """Test that upon test failure, the exception info is stored on
     sys.last_traceback and friends."""
+
     # Simulate item that might raise a specific exception, depending on `raise_error` class var
     class ItemMightRaise:
         nodeid = "item_that_raises"

testing/test_tracebackhide.py

@@ -0,0 +1,25 @@
+def test_tbh_chained(testdir):
+    """Ensure chained exceptions whose frames contain "__tracebackhide__" are not shown (#1904)."""
+    p = testdir.makepyfile(
+        """
+        import pytest
+
+        def f1():
+            __tracebackhide__ = True
+            try:
+                return f1.meh
+            except AttributeError:
+                pytest.fail("fail")
+
+        @pytest.fixture
+        def fix():
+            f1()
+
+
+        def test(fix):
+            pass
+        """
+    )
+    result = testdir.runpytest(str(p))
+    assert "'function' object has no attribute 'meh'" not in result.stdout.str()
+    assert result.ret == 1