-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Improve reference and path/fspath docs #9341
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
70b989d
2c981ee
05cf2fd
aa3c8a1
69659a9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
(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). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Various methods commonly used for :ref:`non-python tests` are now correctly documented in the reference docs. They were undocumented previously. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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__ = ( | ||
|
@@ -188,26 +195,26 @@ def __init__( | |
#: The parent collector node. | ||
self.parent = parent | ||
|
||
#: The pytest config object. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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) | ||
|
@@ -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: | ||
|
@@ -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 | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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, butpath
in hooks is old. But if people prefer, I'm fine with removing those notes.There was a problem hiding this comment.
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. 👍
There was a problem hiding this comment.
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.