Merge pull request #9232 from bluetech/deprecate-node-fspath-args

Deprecate Node constuctor fspath argument, and other small changes/fixes

Author: bluetech

changelog/7259.deprecation.rst

@@ -0,0 +1,3 @@
+``py.path.local`` arguments for hooks have been deprecated. See :ref:`the deprecation note <legacy-path-hooks-deprecated>` for full details.
+
+``py.path.local`` arguments to Node constructors have been deprecated. See :ref:`the deprecation note <node-ctor-fspath-deprecation>` for full details.

doc/en/deprecations.rst

@@ -18,17 +18,40 @@ Deprecated Features
 Below is a complete list of all pytest features which are considered deprecated. Using those features will issue
 :class:`PytestWarning` or subclasses, which can be filtered using :ref:`standard warning filters <warnings>`.
 
+.. _node-ctor-fspath-deprecation:
+
+``fspath`` argument for Node constructors replaced with ``pathlib.Path``
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. deprecated:: 7.0
+
+In order to support the transition from ``py.path.local`` to :mod:`pathlib`,
+the ``fspath`` argument to :class:`~_pytest.nodes.Node` constructors like
+:func:`pytest.Function.from_parent()` and :func:`pytest.Class.from_parent()`
+is now deprecated.
+
+Plugins which construct nodes should pass the ``path`` argument, of type
+:class:`pathlib.Path`, instead of the ``fspath`` argument.
+
+Plugins which implement custom items and collectors are encouraged to replace
+``py.path.local`` ``fspath`` parameters with ``pathlib.Path`` parameters, and
+drop any other usage of the ``py`` library if possible.
+
+
+.. _legacy-path-hooks-deprecated:
 
 ``py.path.local`` arguments for hooks replaced with ``pathlib.Path``
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-In order to support the transition to :mod:`pathlib`, the following hooks now receive additional arguments:
+.. deprecated:: 7.0
+
+In order to support the transition from ``py.path.local`` to :mod:`pathlib`, the following hooks now receive additional arguments:
 
-*  :func:`pytest_ignore_collect(fspath: pathlib.Path) <_pytest.hookspec.pytest_ignore_collect>`
-*  :func:`pytest_collect_file(fspath: pathlib.Path) <_pytest.hookspec.pytest_collect_file>`
-*  :func:`pytest_pycollect_makemodule(fspath: pathlib.Path) <_pytest.hookspec.pytest_pycollect_makemodule>`
-*  :func:`pytest_report_header(startpath: pathlib.Path) <_pytest.hookspec.pytest_report_header>`
-*  :func:`pytest_report_collectionfinish(startpath: pathlib.Path) <_pytest.hookspec.pytest_report_collectionfinish>`
+*  :func:`pytest_ignore_collect(fspath: pathlib.Path) <_pytest.hookspec.pytest_ignore_collect>` instead of ``path``
+*  :func:`pytest_collect_file(fspath: pathlib.Path) <_pytest.hookspec.pytest_collect_file>` instead of ``path``
+*  :func:`pytest_pycollect_makemodule(fspath: pathlib.Path) <_pytest.hookspec.pytest_pycollect_makemodule>` instead of ``path``
+*  :func:`pytest_report_header(startpath: pathlib.Path) <_pytest.hookspec.pytest_report_header>` instead of ``startdir``
+*  :func:`pytest_report_collectionfinish(startpath: pathlib.Path) <_pytest.hookspec.pytest_report_collectionfinish>` instead of ``startdir``
 
 The accompanying ``py.path.local`` based paths have been deprecated: plugins which manually invoke those hooks should only pass the new ``pathlib.Path`` arguments, and users should change their hook implementations to use the new ``pathlib.Path`` arguments.
 
@@ -59,7 +82,7 @@ Implement the :func:`pytest_load_initial_conftests <_pytest.hookspec.pytest_load
 Diamond inheritance between :class:`pytest.File` and :class:`pytest.Item`
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-.. deprecated:: 6.3
+.. deprecated:: 7.0
 
 Inheriting from both Item and file at once has never been supported officially,
 however some plugins providing linting/code analysis have been using this as a hack.
@@ -86,7 +109,7 @@ scheduled for removal in pytest 7 (deprecated since pytest 2.4.0):
 Raising ``unittest.SkipTest`` during collection
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-.. deprecated:: 6.3
+.. deprecated:: 7.0
 
 Raising :class:`unittest.SkipTest` to skip collection of tests during the
 pytest collection phase is deprecated. Use :func:`pytest.skip` instead.

doc/en/how-to/capture-warnings.rst

@@ -268,7 +268,7 @@ argument ``match`` to assert that the exception matches a text or regex::
     ...     warnings.warn("this is not here", UserWarning)
     Traceback (most recent call last):
       ...
-    Failed: DID NOT WARN. No warnings of type ...UserWarning... was emitted...
+    Failed: DID NOT WARN. No warnings of type ...UserWarning... were emitted...
 
 You can also call :func:`pytest.warns` on a function or code string:
 

src/_pytest/cacheprovider.py

@@ -128,7 +128,7 @@ def mkdir(self, name: str) -> Path:
         it to manage files to e.g. store/retrieve database dumps across test
         sessions.
 
-        .. versionadded:: 6.3
+        .. versionadded:: 7.0
 
         :param name:
             Must be a string not containing a ``/`` separator.

src/_pytest/config/argparsing.py

@@ -185,7 +185,7 @@ def addini(
                 * ``paths``: a list of :class:`pathlib.Path`, separated as in a shell
                 * ``pathlist``: a list of ``py.path``, separated as in a shell
 
-            .. versionadded:: 6.3
+            .. versionadded:: 7.0
                 The ``paths`` variable type.
 
             Defaults to ``string`` if ``None`` or not passed.

src/_pytest/config/compat.py

@@ -4,8 +4,9 @@
 from typing import Optional
 
 from ..compat import LEGACY_PATH
+from ..compat import legacy_path
 from ..deprecated import HOOK_LEGACY_PATH_ARG
-from _pytest.nodes import _imply_path
+from _pytest.nodes import _check_path
 
 # hookname: (Path, LEGACY_PATH)
 imply_paths_hooks = {
@@ -52,7 +53,15 @@ def fixed_hook(**kw):
                         ),
                         stacklevel=2,
                     )
-                path_value, fspath_value = _imply_path(path_value, fspath_value)
+                if path_value is not None:
+                    if fspath_value is not None:
+                        _check_path(path_value, fspath_value)
+                    else:
+                        fspath_value = legacy_path(path_value)
+                else:
+                    assert fspath_value is not None
+                    path_value = Path(fspath_value)
+
                 kw[path_var] = path_value
                 kw[fspath_var] = fspath_value
                 return hook(**kw)

src/_pytest/deprecated.py

@@ -101,6 +101,14 @@
     "#py-path-local-arguments-for-hooks-replaced-with-pathlib-path",
 )
 
+NODE_CTOR_FSPATH_ARG = UnformattedWarning(
+    PytestDeprecationWarning,
+    "The (fspath: py.path.local) argument to {node_type_name} is deprecated. "
+    "Please use the (path: pathlib.Path) argument instead.\n"
+    "See https://docs.pytest.org/en/latest/deprecations.html"
+    "#fspath-argument-for-node-constructors-replaced-with-pathlib-path",
+)
+
 WARNS_NONE_ARG = PytestDeprecationWarning(
     "Passing None to catch any warning has been deprecated, pass no arguments instead:\n"
     " Replace pytest.warns(None) by simply pytest.warns()."

src/_pytest/hookspec.py

@@ -272,12 +272,13 @@ def pytest_ignore_collect(
     Stops at first non-None result, see :ref:`firstresult`.
 
     :param pathlib.Path fspath: The path to analyze.
-    :param LEGACY_PATH path: The path to analyze.
+    :param LEGACY_PATH path: The path to analyze (deprecated).
     :param pytest.Config config: The pytest config object.
 
-    .. versionchanged:: 6.3.0
+    .. versionchanged:: 7.0.0
         The ``fspath`` parameter was added as a :class:`pathlib.Path`
-        equivalent of the ``path`` parameter.
+        equivalent of the ``path`` parameter. The ``path`` parameter
+        has been deprecated.
     """
 
 
@@ -289,11 +290,12 @@ def pytest_collect_file(
     The new node needs to have the specified ``parent`` as a parent.
 
     :param pathlib.Path fspath: The path to analyze.
-    :param LEGACY_PATH path: The path to collect.
+    :param LEGACY_PATH path: The path to collect (deprecated).
 
-    .. versionchanged:: 6.3.0
+    .. versionchanged:: 7.0.0
         The ``fspath`` parameter was added as a :class:`pathlib.Path`
-        equivalent of the ``path`` parameter.
+        equivalent of the ``path`` parameter. The ``path`` parameter
+        has been deprecated.
     """
 
 
@@ -345,11 +347,13 @@ def pytest_pycollect_makemodule(
     Stops at first non-None result, see :ref:`firstresult`.
 
     :param pathlib.Path fspath: The path of the module to collect.
-    :param legacy_path path: The path of the module to collect.
+    :param LEGACY_PATH path: The path of the module to collect (deprecated).
 
-    .. versionchanged:: 6.3.0
+    .. versionchanged:: 7.0.0
         The ``fspath`` parameter was added as a :class:`pathlib.Path`
         equivalent of the ``path`` parameter.
+
+        The ``path`` parameter has been deprecated in favor of ``fspath``.
     """
 
 
@@ -674,7 +678,7 @@ def pytest_report_header(
 
     :param pytest.Config config: The pytest config object.
     :param Path startpath: The starting dir.
-    :param LEGACY_PATH startdir: The starting dir.
+    :param LEGACY_PATH startdir: The starting dir (deprecated).
 
     .. note::
 
@@ -689,9 +693,10 @@ def pytest_report_header(
         files situated at the tests root directory due to how pytest
         :ref:`discovers plugins during startup <pluginorder>`.
 
-    .. versionchanged:: 6.3.0
+    .. versionchanged:: 7.0.0
         The ``startpath`` parameter was added as a :class:`pathlib.Path`
-        equivalent of the ``startdir`` parameter.
+        equivalent of the ``startdir`` parameter. The ``startdir`` parameter
+        has been deprecated.
     """
 
 
@@ -709,8 +714,8 @@ def pytest_report_collectionfinish(
     .. versionadded:: 3.2
 
     :param pytest.Config config: The pytest config object.
-    :param Path startpath: The starting path.
-    :param LEGACY_PATH startdir: The starting dir.
+    :param Path startpath: The starting dir.
+    :param LEGACY_PATH startdir: The starting dir (deprecated).
     :param items: List of pytest items that are going to be executed; this list should not be modified.
 
     .. note::
@@ -720,9 +725,10 @@ def pytest_report_collectionfinish(
         If you want to have your line(s) displayed first, use
         :ref:`trylast=True <plugin-hookorder>`.
 
-    .. versionchanged:: 6.3.0
+    .. versionchanged:: 7.0.0
         The ``startpath`` parameter was added as a :class:`pathlib.Path`
-        equivalent of the ``startdir`` parameter.
+        equivalent of the ``startdir`` parameter. The ``startdir`` parameter
+        has been deprecated.
     """
 
 

src/_pytest/main.py

@@ -500,7 +500,7 @@ def __repr__(self) -> str:
     def startpath(self) -> Path:
         """The path from which pytest was invoked.
 
-        .. versionadded:: 6.3.0
+        .. versionadded:: 7.0.0
         """
         return self.config.invocation_params.dir
 

src/_pytest/nodes.py

@@ -28,6 +28,7 @@
 from _pytest.config import Config
 from _pytest.config import ConftestImportFailure
 from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH
+from _pytest.deprecated import NODE_CTOR_FSPATH_ARG
 from _pytest.mark.structures import Mark
 from _pytest.mark.structures import MarkDecorator
 from _pytest.mark.structures import NodeKeywords
@@ -102,22 +103,17 @@ def _check_path(path: Path, fspath: LEGACY_PATH) -> None:
 
 
 def _imply_path(
-    path: Optional[Path], fspath: Optional[LEGACY_PATH]
-) -> Tuple[Path, LEGACY_PATH]:
-    if path is not None:
-        if fspath is not None:
-            _check_path(path, fspath)
-        else:
-            fspath = legacy_path(path)
-        return path, fspath
-    else:
-        assert fspath is not None
-        return Path(fspath), fspath
-
-
-# Optimization: use _imply_path_only over _imply_path when only need Path.
-# This is to avoid `legacy_path(path)` which is surprisingly heavy.
-def _imply_path_only(path: Optional[Path], fspath: Optional[LEGACY_PATH]) -> Path:
+    node_type: Type["Node"],
+    path: Optional[Path],
+    fspath: Optional[LEGACY_PATH],
+) -> Path:
+    if fspath is not None:
+        warnings.warn(
+            NODE_CTOR_FSPATH_ARG.format(
+                node_type_name=node_type.__name__,
+            ),
+            stacklevel=3,
+        )
     if path is not None:
         if fspath is not None:
             _check_path(path, fspath)
@@ -210,9 +206,9 @@ def __init__(
             self.session = parent.session
 
         #: Filesystem path where this node was collected from (can be None).
-        self.path = _imply_path_only(
-            path or getattr(parent, "path", None), fspath=fspath
-        )
+        if path is None and fspath is None:
+            path = getattr(parent, "path", None)
+        self.path = _imply_path(type(self), path, fspath=fspath)
 
         # The explicit annotation is to avoid publicly exposing NodeKeywords.
         #: Keywords/markers collected from all scopes.
@@ -589,7 +585,7 @@ def __init__(
                 assert path is None
                 path = path_or_parent
 
-        path = _imply_path_only(path, fspath=fspath)
+        path = _imply_path(type(self), path, fspath=fspath)
         if name is None:
             name = path.name
             if parent is not None and parent.path != path:
@@ -634,7 +630,6 @@ def from_parent(
         **kw,
     ):
         """The public constructor."""
-        path, fspath = _imply_path(path, fspath=fspath)
         return super().from_parent(parent=parent, fspath=fspath, path=path, **kw)
 
     def gethookproxy(self, fspath: "os.PathLike[str]"):