[7.0] Change Node.reportinfo() return value from py.path to str|os.PathLike[str]
#9184
+28
−27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hLike[str]` `reportinfo()` is the last remaining py.path-only code path in pytest, i.e. the last piece holding back py.path deprecation. The problem with it is that plugins/users use it from both sides -- implementing it (returning the value) and using it (using the return value). Dealing with implementers is easy enough -- allow to return `os.PathLike[str]`. But for callers who expect strictly `py.path` this will break and there's not really a good way to provide backward compat for this. From analyzing a corpus of 680 pytest plugins, the vast majority of `reportinfo` appearances are implementations, and the few callers don't actually access the path part of the return tuple. As for test suites that might access `reportinfo` (e.g. using `request.node.reportinfo()` or other ways), that is much harder to survey, but from the ones I searched, I only found case (`pytest_teamcity`, but even then it uses `str(fspath)` so is unlikely to be affected in practice). They are better served with using `node.location` or `node.path` directly. Therefore, just break it and change the return type to `str|os.PathLike[str]`. Refs pytest-dev#7259.
bluetech
force-pushed
the
reportinfo-pathlike
branch
from
1584e24 to
7eee5c1
Compare
nicoddemus
approved these changes
Oct 9, 2021
There was a problem hiding this comment.
Sounds reasonable, thanks for doing the research and going ahead with this! 👍
Refs #7259. Should only be merged for the 7.0 (i.e. after 6.3.x is branched).
I think we don't plan to have 6.3.X releases in the future, we will go straight for 7.0 for the next major release: main is in a state where a 6.3.X would not be possible anyway, as it already contains breaking changes.
Oh good to know. So according to the plan set out in #7259, we should merge this now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refs #7259. Should only be merged for the 7.0 (i.e. after 6.3.x is branched).
reportinfo()is the last remaining py.path-only code path in pytest,i.e. the last piece holding back py.path deprecation. The problem with
it is that plugins/users use it from both sides -- implementing it
(returning the value) and using it (using the return value). Dealing
with implementers is easy enough -- allow to return
os.PathLike[str].But for callers who expect strictly
py.paththis will break andthere's not really a good way to provide backward compat for this.
From analyzing a corpus of 680 pytest plugins, the vast majority of
reportinfoappearances are implementations, and the few callers don'tactually access the path part of the return tuple.
As for test suites that might access
reportinfo(e.g. usingrequest.node.reportinfo()or other ways), that is much harder tosurvey, but from the ones I searched, I only found case
(
pytest_teamcity, but even then it usesstr(fspath)so is unlikelyto be affected in practice). They are better served with using
node.locationornode.pathdirectly.Therefore, just break it and change the return type to
str|os.PathLike[str].