From 7f989203ed58119bf63e026cbb99df274c7700d6 Mon Sep 17 00:00:00 2001
From: Ran Benita <ran@unusedvar.com>
Date: Thu, 14 Jan 2021 11:58:59 +0200
Subject: Improve way in which skip location is fixed up when skipped by mark

When `pytest.skip()` is called inside a test function, the skip location
should be reported as the line that made the call, however when
`pytest.skip()` is called by the `pytest.mark.skip` and similar
mechanisms, the location should be reported at the item's location,
because the exact location is some irrelevant internal code.

Currently the item-location case is implemented by the caller setting a
boolean key on the item's store and the `skipping` plugin checking it
and fixing up the location if needed. This is really roundabout IMO and
breaks encapsulation.

Instead, allow the caller to specify directly on the skip exception
whether to use the item's location or not. For now, this is entirely
private.
---
 src/_pytest/outcomes.py |  5 +++++
 src/_pytest/reports.py  |  7 ++++++-
 src/_pytest/skipping.py | 18 +-----------------
 src/_pytest/unittest.py |  6 ++----
 4 files changed, 14 insertions(+), 22 deletions(-)

(limited to 'src')

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 6a90188ca..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(
-- 
cgit v1.2.3