Merge pull request #8241 from bluetech/skip-improvements

Minor code improvements in nose, unittest, skipping

6 files changed, 46 insertions, 53 deletions

diff --git a/src/_pytest/fixtures.py b/src/_pytest/fixtures.py
index 5bdee3096..43a40a864 100644
--- a/src/_pytest/fixtures.py
+++ b/src/_pytest/fixtures.py
@@ -543,10 +543,8 @@ class FixtureRequest:
         self._addfinalizer(finalizer, scope=self.scope)
 
     def _addfinalizer(self, finalizer: Callable[[], object], scope) -> None:
-        colitem = self._getscopeitem(scope)
-        self._pyfuncitem.session._setupstate.addfinalizer(
-            finalizer=finalizer, colitem=colitem
-        )
+        item = self._getscopeitem(scope)
+        item.addfinalizer(finalizer)
 
     def applymarker(self, marker: Union[str, MarkDecorator]) -> None:
         """Apply a marker to a single test function invocation.
@@ -694,9 +692,7 @@ class FixtureRequest:
         self, fixturedef: "FixtureDef[object]", subrequest: "SubRequest"
     ) -> None:
         # If fixture function failed it might have registered finalizers.
-        self.session._setupstate.addfinalizer(
-            functools.partial(fixturedef.finish, request=subrequest), subrequest.node
-        )
+        subrequest.node.addfinalizer(lambda: fixturedef.finish(request=subrequest))
 
     def _check_scope(
         self,
diff --git a/src/_pytest/nose.py b/src/_pytest/nose.py
index de91af85a..16d5224e9 100644
--- a/src/_pytest/nose.py
+++ b/src/_pytest/nose.py
@@ -1,33 +1,35 @@
 """Run testsuites written for nose."""
-from _pytest import python
-from _pytest import unittest
 from _pytest.config import hookimpl
 from _pytest.fixtures import getfixturemarker
 from _pytest.nodes import Item
+from _pytest.python import Function
+from _pytest.unittest import TestCaseFunction
 
 
 @hookimpl(trylast=True)
-def pytest_runtest_setup(item) -> None:
-    if is_potential_nosetest(item):
-        if not call_optional(item.obj, "setup"):
-            # Call module level setup if there is no object level one.
-            call_optional(item.parent.obj, "setup")
-        # XXX This implies we only call teardown when setup worked.
-        item.session._setupstate.addfinalizer((lambda: teardown_nose(item)), item)
-
-
-def teardown_nose(item) -> None:
-    if is_potential_nosetest(item):
-        if not call_optional(item.obj, "teardown"):
-            call_optional(item.parent.obj, "teardown")
-
-
-def is_potential_nosetest(item: Item) -> bool:
-    # Extra check needed since we do not do nose style setup/teardown
-    # on direct unittest style classes.
-    return isinstance(item, python.Function) and not isinstance(
-        item, unittest.TestCaseFunction
-    )
+def pytest_runtest_setup(item: Item) -> None:
+    if not isinstance(item, Function):
+        return
+    # Don't do nose style setup/teardown on direct unittest style classes.
+    if isinstance(item, TestCaseFunction):
+        return
+
+    # Capture the narrowed type of item for the teardown closure,
+    # see https://github.com/python/mypy/issues/2608
+    func = item
+
+    if not call_optional(func.obj, "setup"):
+        # Call module level setup if there is no object level one.
+        assert func.parent is not None
+        call_optional(func.parent.obj, "setup")  # type: ignore[attr-defined]
+
+    def teardown_nose() -> None:
+        if not call_optional(func.obj, "teardown"):
+            assert func.parent is not None
+            call_optional(func.parent.obj, "teardown")  # type: ignore[attr-defined]
+
+    # XXX This implies we only call teardown when setup worked.
+    func.addfinalizer(teardown_nose)
 
 
 def call_optional(obj: object, name: str) -> bool:
diff --git a/src/_pytest/outcomes.py b/src/_pytest/outcomes.py
index 8f6203fd7..756b4098b 100644
--- a/src/_pytest/outcomes.py
+++ b/src/_pytest/outcomes.py
@@ -58,9 +58,14 @@ class Skipped(OutcomeException):
         msg: Optional[str] = None,
         pytrace: bool = True,
         allow_module_level: bool = False,
+        *,
+        _use_item_location: bool = False,
     ) -> None:
         OutcomeException.__init__(self, msg=msg, pytrace=pytrace)
         self.allow_module_level = allow_module_level
+        # If true, the skip location is reported as the item's location,
+        # instead of the place that raises the exception/calls skip().
+        self._use_item_location = _use_item_location
 
 
 class Failed(OutcomeException):
diff --git a/src/_pytest/reports.py b/src/_pytest/reports.py
index d2d7115b2..303f731dd 100644
--- a/src/_pytest/reports.py
+++ b/src/_pytest/reports.py
@@ -324,7 +324,12 @@ class TestReport(BaseReport):
             elif isinstance(excinfo.value, skip.Exception):
                 outcome = "skipped"
                 r = excinfo._getreprcrash()
-                longrepr = (str(r.path), r.lineno, r.message)
+                if excinfo.value._use_item_location:
+                    filename, line = item.reportinfo()[:2]
+                    assert line is not None
+                    longrepr = str(filename), line + 1, r.message
+                else:
+                    longrepr = (str(r.path), r.lineno, r.message)
             else:
                 outcome = "failed"
                 if call.when == "call":
diff --git a/src/_pytest/skipping.py b/src/_pytest/skipping.py
index c7afef5db..1ad312919 100644
--- a/src/_pytest/skipping.py
+++ b/src/_pytest/skipping.py
@@ -230,8 +230,6 @@ def evaluate_xfail_marks(item: Item) -> Optional[Xfail]:
     return None
 
 
-# Whether skipped due to skip or skipif marks.
-skipped_by_mark_key = StoreKey[bool]()
 # Saves the xfail mark evaluation. Can be refreshed during call if None.
 xfailed_key = StoreKey[Optional[Xfail]]()
 
@@ -239,9 +237,8 @@ xfailed_key = StoreKey[Optional[Xfail]]()
 @hookimpl(tryfirst=True)
 def pytest_runtest_setup(item: Item) -> None:
     skipped = evaluate_skip_marks(item)
-    item._store[skipped_by_mark_key] = skipped is not None
     if skipped:
-        skip(skipped.reason)
+        raise skip.Exception(skipped.reason, _use_item_location=True)
 
     item._store[xfailed_key] = xfailed = evaluate_xfail_marks(item)
     if xfailed and not item.config.option.runxfail and not xfailed.run:
@@ -292,19 +289,6 @@ def pytest_runtest_makereport(item: Item, call: CallInfo[None]):
                 rep.outcome = "passed"
                 rep.wasxfail = xfailed.reason
 
-    if (
-        item._store.get(skipped_by_mark_key, True)
-        and rep.skipped
-        and type(rep.longrepr) is tuple
-    ):
-        # Skipped by mark.skipif; change the location of the failure
-        # to point to the item definition, otherwise it will display
-        # the location of where the skip exception was raised within pytest.
-        _, _, reason = rep.longrepr
-        filename, line = item.reportinfo()[:2]
-        assert line is not None
-        rep.longrepr = str(filename), line + 1, reason
-
 
 def pytest_report_teststatus(report: BaseReport) -> Optional[Tuple[str, str, str]]:
     if hasattr(report, "wasxfail"):
diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py
index cc616578b..719eb4e88 100644
--- a/src/_pytest/unittest.py
+++ b/src/_pytest/unittest.py
@@ -29,7 +29,6 @@ from _pytest.python import Class
 from _pytest.python import Function
 from _pytest.python import PyCollector
 from _pytest.runner import CallInfo
-from _pytest.skipping import skipped_by_mark_key
 
 if TYPE_CHECKING:
     import unittest
@@ -150,7 +149,7 @@ def _make_xunit_fixture(
     def fixture(self, request: FixtureRequest) -> Generator[None, None, None]:
         if _is_skipped(self):
             reason = self.__unittest_skip_why__
-            pytest.skip(reason)
+            raise pytest.skip.Exception(reason, _use_item_location=True)
         if setup is not None:
             try:
                 if pass_self:
@@ -256,9 +255,8 @@ class TestCaseFunction(Function):
 
     def addSkip(self, testcase: "unittest.TestCase", reason: str) -> None:
         try:
-            skip(reason)
+            raise pytest.skip.Exception(reason, _use_item_location=True)
         except skip.Exception:
-            self._store[skipped_by_mark_key] = True
             self._addexcinfo(sys.exc_info())
 
     def addExpectedFailure(
@@ -343,6 +341,10 @@ def pytest_runtest_makereport(item: Item, call: CallInfo[None]) -> None:
             except AttributeError:
                 pass
 
+    # Convert unittest.SkipTest to pytest.skip.
+    # This is actually only needed for nose, which reuses unittest.SkipTest for
+    # its own nose.SkipTest. For unittest TestCases, SkipTest is already
+    # handled internally, and doesn't reach here.
     unittest = sys.modules.get("unittest")
     if (
         unittest
@@ -350,7 +352,6 @@ def pytest_runtest_makereport(item: Item, call: CallInfo[None]) -> None:
         and isinstance(call.excinfo.value, unittest.SkipTest)  # type: ignore[attr-defined]
     ):
         excinfo = call.excinfo
-        # Let's substitute the excinfo with a pytest.skip one.
         call2 = CallInfo[None].from_call(
             lambda: pytest.skip(str(excinfo.value)), call.when
         )