Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve reference and path/fspath docs #9341

Merged
merged 5 commits into from Dec 6, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions changelog/7259.deprecation.rst
@@ -1,3 +1,12 @@
``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.

.. note::
The name of the :class:`~_pytest.nodes.Node` arguments and attributes (the
new attribute being ``path``) is **the opposite** of the situation for hooks
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this still applies, as we don't have the same situation anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, kind of - we still have the situation that Node.path is new, but path in hooks is old. But if people prefer, I'm fine with removing those notes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh you are correct. Nevermind, I misunderstood the text. 馃憤

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have a suggestion for a rewording to make things clearer, let me know! Probably going to be Monday or Tuesday until I can finish the (rc) release anyways.

(the old argument being ``path``).

This is an unfortunate artifact due to historical reasons, which should be
resolved in future versions as we slowly get rid of the :pypi:`py`
dependency (see :issue:`9283` for a longer discussion).
9 changes: 9 additions & 0 deletions changelog/8144.feature.rst
Expand Up @@ -5,3 +5,12 @@ The following hooks now receive an additional ``pathlib.Path`` argument, equival
- :func:`pytest_pycollect_makemodule <_pytest.hookspec.pytest_pycollect_makemodule>` - The ``module_path`` parameter (equivalent to existing ``path`` parameter).
- :func:`pytest_report_header <_pytest.hookspec.pytest_report_header>` - The ``start_path`` parameter (equivalent to existing ``startdir`` parameter).
- :func:`pytest_report_collectionfinish <_pytest.hookspec.pytest_report_collectionfinish>` - The ``start_path`` parameter (equivalent to existing ``startdir`` parameter).

.. note::
The name of the :class:`~_pytest.nodes.Node` arguments and attributes (the
new attribute being ``path``) is **the opposite** of the situation for hooks
(the old argument being ``path``).

This is an unfortunate artifact due to historical reasons, which should be
resolved in future versions as we slowly get rid of the :pypi:`py`
dependency (see :issue:`9283` for a longer discussion).
12 changes: 11 additions & 1 deletion changelog/8251.feature.rst
@@ -1 +1,11 @@
Implement ``Node.path`` as a ``pathlib.Path``.
Implement ``Node.path`` as a ``pathlib.Path``. Both the old ``fspath`` and this new attribute gets set no matter whether ``path`` or ``fspath`` (deprecated) is passed to the constructor. It is a replacement for the ``fspath`` attribute (which represents the same path as ``py.path.local``). While ``fspath`` is not deprecated yet
due to the ongoing migration of methods like :meth:`~_pytest.Item.reportinfo`, we expect to deprecate it in a future release.

.. note::
The name of the :class:`~_pytest.nodes.Node` arguments and attributes (the
new attribute being ``path``) is **the opposite** of the situation for hooks
(the old argument being ``path``).

This is an unfortunate artifact due to historical reasons, which should be
resolved in future versions as we slowly get rid of the :pypi:`py`
dependency (see :issue:`9283` for a longer discussion).
1 change: 1 addition & 0 deletions changelog/9341.doc.rst
@@ -0,0 +1 @@
Various methods commonly used for :ref:`non-python tests` are now correctly documented in the reference docs. They were undocumented previously.
29 changes: 27 additions & 2 deletions doc/en/deprecations.rst
Expand Up @@ -53,9 +53,24 @@ 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.
``fspath`` parameters (``py.path.local``) with ``path`` parameters
(``pathlib.Path``), and drop any other usage of the ``py`` library if possible.

.. note::
The name of the :class:`~_pytest.nodes.Node` arguments and attributes (the
new attribute being ``path``) is **the opposite** of the situation for
hooks, :ref:`outlined below <legacy-path-hooks-deprecated>` (the old
argument being ``path``).

This is an unfortunate artifact due to historical reasons, which should be
resolved in future versions as we slowly get rid of the :pypi:`py`
dependency (see :issue:`9283` for a longer discussion).

Due to the ongoing migration of methods like :meth:`~_pytest.Item.reportinfo`
which still is expected to return a ``py.path.local`` object, nodes still have
both ``fspath`` (``py.path.local``) and ``path`` (``pathlib.Path``) attributes,
no matter what argument was used in the constructor. We expect to deprecate the
``fspath`` attribute in a future release.

.. _legacy-path-hooks-deprecated:

Expand All @@ -74,6 +89,16 @@ In order to support the transition from ``py.path.local`` to :mod:`pathlib`, the

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.

.. note::
The name of the :class:`~_pytest.nodes.Node` arguments and attributes,
:ref:`outlined above <node-ctor-fspath-deprecation>` (the new attribute
being ``path``) is **the opposite** of the situation for hooks (the old
argument being ``path``).

This is an unfortunate artifact due to historical reasons, which should be
resolved in future versions as we slowly get rid of the :pypi:`py`
dependency (see :issue:`9283` for a longer discussion).

Directly constructing internal classes
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
33 changes: 29 additions & 4 deletions src/_pytest/nodes.py
Expand Up @@ -159,6 +159,13 @@ class Node(metaclass=NodeMeta):
Collector subclasses have children; Items are leaf nodes.
"""

# Implemented in the legacypath plugin.
#: A ``LEGACY_PATH`` copy of the :attr:`path` attribute. Intended for usage
#: for methods not migrated to ``pathlib.Path`` yet, such as
#: :meth:`Item.reportinfo`. Will be deprecated in a future release, prefer
#: using :attr:`path` instead.
fspath: LEGACY_PATH

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bluetech What's your take on this? From what I understand, the aim of #9208 was to decouple legacy stuff like fspath, so this might seem out of place. However, I'd really prefer to keep this attribute documented (even more so because it's not deprecated yet!), and having this here is the only way I found to make Sphinx pick it up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two aspects to this - docs and typing. For the docs it might work to add it in reference.rst as regular sphinx attr instead of autodoc? As for typing, without this PR it would show up as a missing attribute error, but maybe it's a good thing?

Anyway, I'm fine with whatever you think is best (including this approach).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like having this as a "missing" attribute would be wrong - especially given that we explicitly un-deprecated it. Until we deprecate (or, ideally, remove) it, I feel like typing should know that it's there too.

# Use __slots__ to make attribute access faster.
# Note that __dict__ is still available.
__slots__ = (
Expand Down Expand Up @@ -188,26 +195,26 @@ def __init__(
#: The parent collector node.
self.parent = parent

#: The pytest config object.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like Sphinx silently ignores those comments (and hides the attribute as undocumented) if this comment isn't next to the line actually setting the attribute.

if config:
#: The pytest config object.
self.config: Config = config
else:
if not parent:
raise TypeError("config or parent must be provided")
self.config = parent.config

#: The pytest session this node is part of.
if session:
#: The pytest session this node is part of.
self.session = session
else:
if not parent:
raise TypeError("session or parent must be provided")
self.session = parent.session

#: Filesystem path where this node was collected from (can be None).
if path is None and fspath is None:
path = getattr(parent, "path", None)
self.path = _imply_path(type(self), path, fspath=fspath)
#: Filesystem path where this node was collected from (can be None).
self.path: Path = _imply_path(type(self), path, fspath=fspath)

# The explicit annotation is to avoid publicly exposing NodeKeywords.
#: Keywords/markers collected from all scopes.
Expand Down Expand Up @@ -478,6 +485,8 @@ def repr_failure(
) -> Union[str, TerminalRepr]:
"""Return a representation of a collection or test failure.

.. seealso:: :ref:`non-python tests`

:param excinfo: Exception information for the failure.
"""
return self._repr_failure_py(excinfo, style)
Expand Down Expand Up @@ -686,6 +695,12 @@ def __init__(
self.user_properties: List[Tuple[str, object]] = []

def runtest(self) -> None:
"""Run the test case for this item.

Must be implemented by subclasses.

.. seealso:: :ref:`non-python tests`
"""
raise NotImplementedError("runtest must be implemented by Item subclass")

def add_report_section(self, when: str, key: str, content: str) -> None:
Expand All @@ -706,6 +721,16 @@ def add_report_section(self, when: str, key: str, content: str) -> None:
self._report_sections.append((when, key, content))

def reportinfo(self) -> Tuple[Union["os.PathLike[str]", str], Optional[int], str]:
"""Get location information for this item for test reports.

Returns a tuple with three elements:

- The path of the test (default ``self.path``)
- The line number of the test (default ``None``)
- A name of the test to be shown (default ``""``)

.. seealso:: :ref:`non-python tests`
"""
return self.path, None, ""

@cached_property
Expand Down
18 changes: 9 additions & 9 deletions src/_pytest/python.py
Expand Up @@ -1578,26 +1578,26 @@ def write_docstring(tw: TerminalWriter, doc: str, indent: str = " ") -> None:
class Function(PyobjMixin, nodes.Item):
"""An Item responsible for setting up and executing a Python test function.

param name:
:param name:
The full function name, including any decorations like those
added by parametrization (``my_func[my_param]``).
param parent:
:param parent:
The parent Node.
param config:
:param config:
The pytest Config object.
param callspec:
:param callspec:
If given, this is function has been parametrized and the callspec contains
meta information about the parametrization.
param callobj:
:param callobj:
If given, the object which will be called when the Function is invoked,
otherwise the callobj will be obtained from ``parent`` using ``originalname``.
param keywords:
:param keywords:
Keywords bound to the function object for "-k" matching.
param session:
:param session:
The pytest Session object.
param fixtureinfo:
:param fixtureinfo:
Fixture information already resolved at this fixture node..
param originalname:
:param originalname:
The attribute name to use for accessing the underlying function object.
Defaults to ``name``. Set this if name is different from the original name,
for example when it contains decorations like those added by parametrization
Expand Down