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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] python: remove the Instance collector node #9277

Merged
merged 2 commits into from Nov 13, 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
3 changes: 3 additions & 0 deletions changelog/9277.breaking.rst
@@ -0,0 +1,3 @@
The ``pytest.Instance`` collector type has been removed.
Importing ``pytest.Instance`` or ``_pytest.python.Instance`` returns a dummy type and emits a deprecation warning.
Copy link

@CaselIT CaselIT Nov 27, 2021

Choose a reason for hiding this comment

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

Personally I think it's would be better to just remove this, since trying with sqlalchemy returned no warning whatsoever, just no collection.
If it was removed we would have at least an attribute error to go by.

See also #9343 (comment)

To reproduce:

git clone https://github.com/sqlalchemy/sqlalchemy.git --depth 100
pip install git+https://github.com/pytest-dev/pytest.git
pytest -Werror

will run and print no tests ran in 13.57s, so the warning seems to not trigger

See :ref:`instance-collector-deprecation` for details.
19 changes: 19 additions & 0 deletions doc/en/deprecations.rst
Expand Up @@ -18,6 +18,25 @@ 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>`.

.. _instance-collector-deprecation:

The ``pytest.Instance`` collector
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. versionremoved:: 7.0

The ``pytest.Instance`` collector type has been removed.

Previously, Python test methods were collected as :class:`~pytest.Class` -> ``Instance`` -> :class:`~pytest.Function`.
Now :class:`~pytest.Class` collects the test methods directly.

Most plugins which reference ``Instance`` do so in order to ignore or skip it,
using a check such as ``if isinstance(node, Instance): return``.
Such plugins should simply remove consideration of ``Instance`` on pytest>=7.
However, to keep such uses working, a dummy type has been instanted in ``pytest.Instance`` and ``_pytest.python.Instance``,
and importing it emits a deprecation warning. This will be removed in pytest 8.


.. _node-ctor-fspath-deprecation:

``fspath`` argument for Node constructors replaced with ``pathlib.Path``
Expand Down
5 changes: 5 additions & 0 deletions src/_pytest/deprecated.py
Expand Up @@ -119,6 +119,11 @@
"pytest.{func}(msg=...) is now deprecated, use pytest.{func}(reason=...) instead",
)

INSTANCE_COLLECTOR = PytestDeprecationWarning(
"The pytest.Instance collector type is deprecated and is no longer used. "
"See https://docs.pytest.org/en/latest/deprecations.html#the-pytest-instance-collector",
)

# You want to make some `__init__` or function "private".
#
# def my_private_function(some, args):
Expand Down
4 changes: 0 additions & 4 deletions src/_pytest/junitxml.py
Expand Up @@ -450,10 +450,6 @@ def pytest_unconfigure(config: Config) -> None:
def mangle_test_address(address: str) -> List[str]:
path, possible_open_bracket, params = address.partition("[")
names = path.split("::")
try:
names.remove("()")
except ValueError:
pass
# Convert file path to dotted path.
names[0] = names[0].replace(nodes.SEP, ".")
names[0] = re.sub(r"\.py$", "", names[0])
Expand Down
3 changes: 0 additions & 3 deletions src/_pytest/main.py
Expand Up @@ -781,9 +781,6 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]:
submatchnodes.append(r)
if submatchnodes:
work.append((submatchnodes, matchnames[1:]))
# XXX Accept IDs that don't have "()" for class instances.
elif len(rep.result) == 1 and rep.result[0].name == "()":
work.append((rep.result, matchnames))
else:
# Report collection failures here to avoid failing to run some test
# specified in the command line because the module could not be
Expand Down
2 changes: 1 addition & 1 deletion src/_pytest/mark/__init__.py
Expand Up @@ -158,7 +158,7 @@ def from_item(cls, item: "Item") -> "KeywordMatcher":
import pytest

for node in item.listchain():
if not isinstance(node, (pytest.Instance, pytest.Session)):
if not isinstance(node, pytest.Session):
mapped_names.add(node.name)

# Add the names added as extra keywords to current or parent items.
Expand Down
4 changes: 1 addition & 3 deletions src/_pytest/nodes.py
Expand Up @@ -225,9 +225,7 @@ def __init__(
else:
if not self.parent:
raise TypeError("nodeid or parent must be provided")
self._nodeid = self.parent.nodeid
if self.name != "()":
self._nodeid += "::" + self.name
self._nodeid = self.parent.nodeid + "::" + self.name

#: A place where plugins can store information on the node for their
#: own use.
Expand Down
72 changes: 39 additions & 33 deletions src/_pytest/python.py
Expand Up @@ -58,6 +58,7 @@
from _pytest.config.argparsing import Parser
from _pytest.deprecated import check_ispytest
from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH
from _pytest.deprecated import INSTANCE_COLLECTOR
from _pytest.fixtures import FuncFixtureInfo
from _pytest.main import Session
from _pytest.mark import MARK_GEN
Expand Down Expand Up @@ -272,20 +273,14 @@ def cls(self):
node = self.getparent(Class)
return node.obj if node is not None else None

@property
def instance(self):
"""Python instance object this node was collected from (can be None)."""
node = self.getparent(Instance)
return node.obj if node is not None else None

@property
def obj(self):
"""Underlying Python object."""
obj = getattr(self, "_obj", None)
if obj is None:
self._obj = obj = self._getobj()
# XXX evil hack
# used to avoid Instance collector marker duplication
# used to avoid Function marker duplication
if self._ALLOW_MARKERS:
self.own_markers.extend(get_unpacked_marks(self.obj))
return obj
Expand All @@ -307,8 +302,6 @@ def getmodpath(self, stopatmodule: bool = True, includemodule: bool = False) ->
chain.reverse()
parts = []
for node in chain:
if isinstance(node, Instance):
continue
name = node.name
if isinstance(node, Module):
name = os.path.splitext(name)[0]
Expand Down Expand Up @@ -407,8 +400,9 @@ def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]:

# Avoid random getattrs and peek in the __dict__ instead.
dicts = [getattr(self.obj, "__dict__", {})]
for basecls in self.obj.__class__.__mro__:
dicts.append(basecls.__dict__)
if isinstance(self.obj, type):
for basecls in self.obj.__mro__:
dicts.append(basecls.__dict__)

# In each class, nodes should be definition ordered. Since Python 3.6,
# __dict__ is definition ordered.
Expand Down Expand Up @@ -488,7 +482,6 @@ def _genfunctions(self, name: str, funcobj) -> Iterator["Function"]:
self,
name=subname,
callspec=callspec,
callobj=funcobj,
fixtureinfo=fixtureinfo,
keywords={callspec.id: True},
originalname=name,
Expand Down Expand Up @@ -773,6 +766,9 @@ def from_parent(cls, parent, *, name, obj=None, **kw):
"""The public constructor."""
return super().from_parent(name=name, parent=parent, **kw)

def newinstance(self):
return self.obj()

def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]:
if not safe_getattr(self.obj, "__test__", True):
return []
Expand Down Expand Up @@ -800,7 +796,9 @@ def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]:
self._inject_setup_class_fixture()
self._inject_setup_method_fixture()

return [Instance.from_parent(self, name="()")]
self.session._fixturemanager.parsefactories(self.newinstance(), self.nodeid)

return super().collect()

def _inject_setup_class_fixture(self) -> None:
"""Inject a hidden autouse, class scoped fixture into the collected class object
Expand Down Expand Up @@ -871,25 +869,22 @@ def xunit_setup_method_fixture(self, request) -> Generator[None, None, None]:
self.obj.__pytest_setup_method = xunit_setup_method_fixture


class Instance(PyCollector):
_ALLOW_MARKERS = False # hack, destroy later
# Instances share the object with their parents in a way
# that duplicates markers instances if not taken out
# can be removed at node structure reorganization time.
class InstanceDummy:
"""Instance used to be a node type between Class and Function. It has been
removed in pytest 7.0. Some plugins exist which reference `pytest.Instance`
only to ignore it; this dummy class keeps them working. This will be removed
in pytest 8."""

def _getobj(self):
# TODO: Improve the type of `parent` such that assert/ignore aren't needed.
assert self.parent is not None
obj = self.parent.obj # type: ignore[attr-defined]
return obj()
pass

def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]:
self.session._fixturemanager.parsefactories(self)
return super().collect()

def newinstance(self):
self.obj = self._getobj()
return self.obj
# Note: module __getattr__ only works on Python>=3.7. Unfortunately
# we can't provide this deprecation warning on Python 3.6.
def __getattr__(name: str) -> object:
if name == "Instance":
warnings.warn(INSTANCE_COLLECTOR, 2)
return InstanceDummy
raise AttributeError(f"module {__name__} has no attribute {name}")


def hasinit(obj: object) -> bool:
Expand Down Expand Up @@ -1674,9 +1669,23 @@ def function(self):
"""Underlying python 'function' object."""
return getimfunc(self.obj)

@property
def instance(self):
"""Python instance object the function is bound to.

Returns None if not a test method, e.g. for a standalone test function
or a staticmethod.
"""
return getattr(self.obj, "__self__", None)

def _getobj(self):
assert self.parent is not None
return getattr(self.parent.obj, self.originalname) # type: ignore[attr-defined]
if isinstance(self.parent, Class):
# Each Function gets a fresh class instance.
parent_obj = self.parent.newinstance()
else:
parent_obj = self.parent.obj # type: ignore[attr-defined]
return getattr(parent_obj, self.originalname)

@property
def _pyfuncitem(self):
Expand All @@ -1688,9 +1697,6 @@ def runtest(self) -> None:
self.ihook.pytest_pyfunc_call(pyfuncitem=self)

def setup(self) -> None:
if isinstance(self.parent, Instance):
self.parent.newinstance()
self.obj = self._getobj()
self._request._fillfixtures()

def _prunetraceback(self, excinfo: ExceptionInfo[BaseException]) -> None:
Expand Down
5 changes: 0 additions & 5 deletions src/_pytest/terminal.py
Expand Up @@ -756,9 +756,6 @@ def pytest_collection_finish(self, session: "Session") -> None:
rep.toterminal(self._tw)

def _printcollecteditems(self, items: Sequence[Item]) -> None:
# To print out items and their parent collectors
# we take care to leave out Instances aka ()
# because later versions are going to get rid of them anyway.
if self.config.option.verbose < 0:
if self.config.option.verbose < -1:
counts = Counter(item.nodeid.split("::", 1)[0] for item in items)
Expand All @@ -778,8 +775,6 @@ def _printcollecteditems(self, items: Sequence[Item]) -> None:
stack.pop()
for col in needed_collectors[len(stack) :]:
stack.append(col)
if col.name == "()": # Skip Instances.
continue
indent = (len(stack) - 1) * " "
self._tw.line(f"{indent}{col}")
if self.config.option.verbose >= 1:
Expand Down
12 changes: 10 additions & 2 deletions src/pytest/__init__.py
Expand Up @@ -48,7 +48,6 @@
from _pytest.pytester import RunResult
from _pytest.python import Class
from _pytest.python import Function
from _pytest.python import Instance
from _pytest.python import Metafunc
from _pytest.python import Module
from _pytest.python import Package
Expand Down Expand Up @@ -77,6 +76,7 @@

set_trace = __pytestPDB.set_trace


__all__ = [
"__version__",
"_fillfuncargs",
Expand Down Expand Up @@ -106,7 +106,6 @@
"HookRecorder",
"hookspec",
"importorskip",
"Instance",
"Item",
"LineMatcher",
"LogCaptureFixture",
Expand Down Expand Up @@ -153,3 +152,12 @@
"xfail",
"yield_fixture",
]


def __getattr__(name: str) -> object:
if name == "Instance":
# The import emits a deprecation warning.
from _pytest.python import Instance

return Instance
raise AttributeError(f"module {__name__} has no attribute {name}")
1 change: 0 additions & 1 deletion src/pytest/collect.py
Expand Up @@ -11,7 +11,6 @@
"Collector",
"Module",
"Function",
"Instance",
"Session",
"Item",
"Class",
Expand Down
18 changes: 18 additions & 0 deletions testing/deprecated_test.py
Expand Up @@ -286,3 +286,21 @@ def test_node_ctor_fspath_argument_is_deprecated(pytester: Pytester) -> None:
parent=mod.parent,
fspath=legacy_path("bla"),
)


@pytest.mark.skipif(
sys.version_info < (3, 7),
reason="This deprecation can only be emitted on python>=3.7",
)
def test_importing_instance_is_deprecated(pytester: Pytester) -> None:
with pytest.warns(
pytest.PytestDeprecationWarning,
match=re.escape("The pytest.Instance collector type is deprecated"),
):
pytest.Instance

with pytest.warns(
pytest.PytestDeprecationWarning,
match=re.escape("The pytest.Instance collector type is deprecated"),
):
from _pytest.python import Instance # noqa: F401
19 changes: 13 additions & 6 deletions testing/python/collect.py
Expand Up @@ -12,7 +12,7 @@
from _pytest.nodes import Collector
from _pytest.pytester import Pytester
from _pytest.python import Class
from _pytest.python import Instance
from _pytest.python import Function


class TestModule:
Expand Down Expand Up @@ -585,7 +585,7 @@ def test2(self, x, y):
pass
"""
)
colitems = modcol.collect()[0].collect()[0].collect()
colitems = modcol.collect()[0].collect()
assert colitems[0].name == "test1[a-c]"
assert colitems[1].name == "test1[b-c]"
assert colitems[2].name == "test2[a-c]"
Expand Down Expand Up @@ -1183,19 +1183,26 @@ def test_reportinfo_with_nasty_getattr(self, pytester: Pytester) -> None:
modcol = pytester.getmodulecol(
"""
# lineno 0
class TestClass(object):
class TestClass:
def __getattr__(self, name):
return "this is not an int"

def __class_getattr__(cls, name):
return "this is not an int"

def intest_foo(self):
pass

def test_bar(self):
pass
"""
)
classcol = pytester.collect_by_name(modcol, "TestClass")
assert isinstance(classcol, Class)
instance = list(classcol.collect())[0]
assert isinstance(instance, Instance)
path, lineno, msg = instance.reportinfo()
path, lineno, msg = classcol.reportinfo()
func = list(classcol.collect())[0]
assert isinstance(func, Function)
path, lineno, msg = func.reportinfo()


def test_customized_python_discovery(pytester: Pytester) -> None:
Expand Down