Unexpected keyword argument 'path' from plugins #8435
Comments
|
Hey @jaraco, I'm making a quick guess as I don't have time to delve deep into the code, but perhaps |
|
That's about it, I wonder if i can make this one a warning for the next releases |
Yes, perhaps. And also pytest-black and pytest-mypy and pytest-flake8 likely and maybe others. |
|
Sounds like I definitely should sort out the signature of the create call and issue a warning |
Zac-HD
added
the
type: regression
|
I tried applying the suggested workaround. diff --git a/pytest_checkdocs/__init__.py b/pytest_checkdocs/__init__.py
index 3162319..8469ebe 100644
--- a/pytest_checkdocs/__init__.py
+++ b/pytest_checkdocs/__init__.py
@@ -38,18 +38,18 @@ class Description(str):
class CheckdocsItem(pytest.Item, pytest.File):
- def __init__(self, fspath, parent):
+ def __init__(self, fspath, parent, **kw):
# ugly hack to add support for fspath parameter
# Ref pytest-dev/pytest#6928
- super().__init__(fspath, parent)
+ super().__init__(fspath, parent, **kw)
@classmethod
- def from_parent(cls, parent, fspath):
+ def from_parent(cls, parent, fspath, **kw):
"""
Compatibility shim to support
"""
try:
- return super().from_parent(parent, fspath=fspath)
+ return super().from_parent(parent, fspath=fspath, **kw)
except AttributeError:
# pytest < 5.4
return cls(fspath, parent)But that only pushed the error down: (I tried it with and without adding I don't understand what these hacks are trying to accomplish, so I'm out of my depth. If someone more familiar with the changes to the interfaces could suggest a fix, I'd be happy to test it and incorporate it. I'm also happy to drop support for older pytest versions (prior to 5.4) if that helps. |
|
@jaraco problem is that the hacks to make the switch from fspath to just pathlib paths where incomplete, and the backward compatibility handling is not yet aware of non keyword parameters If you pass everything as keywords it should work, I should however fix that way of invocation |
|
@jaraco the correct fix is to stop merging items and files, currently python has absolutely no sane support for that inheritance structure, it worked by sheer accident, we should actually just deprecate collecting items and collectors together i`m going to add a fitting deprecation warning |
RonnyPfannschmidt
added
the
status: critical
…ce (#8447) * issue a warning when Items and Collector form a diamond addresses #8435 * Apply suggestions from code review Co-authored-by: Ran Benita <ran@unusedvar.com> * Return support for the broken File/Item hybrids * adds deprecation * ads necessary support code in node construction * fix incorrect mypy based assertions * add docs for deprecation of Item/File inheritance * warn when a non-cooperative ctor is encountered * use getattr instead of cast to get the class __init__ for legacy ctors * update documentation references for node inheritance * clean up file+item inheritance test enhance docs move import upwards Co-authored-by: Ran Benita <ran@unusedvar.com>
|
The bugs reported here still exist in pytest-black and pytest-flake8, even though they've been fixed in pytest-checkdocs and pytest.mypy. |
|
@jaraco does it happen against pytest main, or the last release? |
|
Main. I only report the status here because I was too lazy to report it to the downstream projects. |
|
@bluetech Could this be related to the recent change in imply? |
|
I don't think it's a recent regression. I think the regression happened due to a refactoring in main. I believe the original analysis is still the same. I just came in to report that this incompatibility still exists. I've since filed issues with the downstream projects so they're aware of the impending, intended breakage. |
|
The assert |
|
TL;DR: @RonnyPfannschmidt should look at the diff below and say if we want to go ahead with it to keep this plugins working for a bit longer, or accept the breakage. My opinion is that we should apply it, so the plugins have a chance to fix their code, otherwise they go from working -> broken without deprecation in the middle. The source of the trouble in both pytest-black and pytest-flake8 is that they have their item type inherit from both Taking pytest-black as an example, it does class BlackItem(pytest.Item, pytest.File):
def __init__(self, fspath, parent):
super(BlackItem, self).__init__(fspath, parent)the MRO here is of interest are the def __init__(
self,
name,
parent=None,
config: Optional[Config] = None,
session: Optional["Session"] = None,
nodeid: Optional[str] = None,
**kw,
) -> None:
super().__init__(
name=name,
parent=parent,
config=config,
session=session,
nodeid=nodeid,
**kw,
)and the class FSCollector(Collector):
def __init__(
self,
fspath: Optional[LEGACY_PATH] = None,
path_or_parent: Optional[Union[Path, Node]] = None,
path: Optional[Path] = None,
name: Optional[str] = None,
parent: Optional[Node] = None,
config: Optional[Config] = None,
session: Optional["Session"] = None,
nodeid: Optional[str] = None,
) -> None:
if path_or_parent:
if isinstance(path_or_parent, Node):
assert parent is None
parent = cast(FSCollector, path_or_parent)
elif isinstance(path_or_parent, Path):
assert path is None
path = path_or_parent
path = _imply_path(type(self), path, fspath=fspath)You can see how the diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py
index 09bbda0a2..20a3f8739 100644
--- a/src/_pytest/nodes.py
+++ b/src/_pytest/nodes.py
@@ -670,8 +670,8 @@ class Item(Node):
**kw,
) -> None:
super().__init__(
- name=name,
- parent=parent,
+ name,
+ parent,
config=config,
session=session,
nodeid=nodeid, |
|
Let's do that and make things nice once the other plugins are fixed, thanks for the investigation |
I don't think we had, it's more likely to be plugins evolving from a common ancestor. In my case, I have this pattern in my pytest-mccabe plugin, which was based on pytest-flakes, which in turn was based on pytest-pep8, which finally seems to be based on pytest-codecheckers, the only one in the chain not using that pattern. Given that most affected plugins are linter integrations of some sorts, my guess would be they all were based off pytest-pep8 or something based on it. |
While troubleshooting #8332, I stumbled onto a new error, a
TypeErrorthat occurs when using pytest-black against the current main HEAD (32ad70d), easily reproducible with an empty test file and pip-run:Same problem happens with pytest-checkdocs:
The text was updated successfully, but these errors were encountered: