Skip to content

Commit

Permalink
[RFC] python: remove the Instance collector node
Browse files Browse the repository at this point in the history
  • Loading branch information
bluetech committed Nov 12, 2021
1 parent f87df9c commit 9a57106
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 66 deletions.
4 changes: 0 additions & 4 deletions src/_pytest/junitxml.py
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
67 changes: 31 additions & 36 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,20 +272,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 +301,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 +399,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 +481,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 +765,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 +795,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 +868,12 @@ 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.

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()

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
# 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 could probably
# be removed at some point.
class Instance:
pass


def hasinit(obj: object) -> bool:
Expand Down Expand Up @@ -1674,9 +1658,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 +1686,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
Original file line number Diff line number Diff line change
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
1 change: 0 additions & 1 deletion src/pytest/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"Collector",
"Module",
"Function",
"Instance",
"Session",
"Item",
"Class",
Expand Down
19 changes: 13 additions & 6 deletions testing/python/collect.py
Original file line number Diff line number Diff line change
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
26 changes: 26 additions & 0 deletions testing/python/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from _pytest._code import getfslineno
from _pytest.fixtures import getfixturemarker
from _pytest.pytester import Pytester
from _pytest.python import Function


class TestOEJSKITSpecials:
Expand Down Expand Up @@ -475,3 +476,28 @@ def test_params(a, b):
)
res = pytester.runpytest("--collect-only")
res.stdout.fnmatch_lines(["*spam-2*", "*ham-2*"])


def test_function_instance(pytester: Pytester) -> None:
items = pytester.getitems(
"""
def test_func(): pass
class TestIt:
def test_method(self): pass
@classmethod
def test_class(cls): pass
@staticmethod
def test_static(): pass
"""
)
assert len(items) == 3
assert isinstance(items[0], Function)
assert items[0].name == "test_func"
assert items[0].instance is None
assert isinstance(items[1], Function)
assert items[1].name == "test_method"
assert items[1].instance is not None
assert items[1].instance.__class__.__name__ == "TestIt"
assert isinstance(items[2], Function)
assert items[2].name == "test_static"
assert items[2].instance is None
2 changes: 1 addition & 1 deletion testing/test_cacheprovider.py
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ def test_other(): assert 0
"",
"<Module pkg1/test_1.py>",
" <Class TestFoo>",
" <Function test_fail>",
" <Function test_fail>",
" <Function test_other>",
"",
"*= 2/3 tests collected (1 deselected) in *",
Expand Down
4 changes: 1 addition & 3 deletions testing/test_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ def test_foo(self):
)
cls = pytester.collect_by_name(modcol, "TestClass")
assert isinstance(cls, pytest.Class)
instance = pytester.collect_by_name(cls, "()")
assert isinstance(instance, pytest.Instance)
fn = pytester.collect_by_name(instance, "test_foo")
fn = pytester.collect_by_name(cls, "test_foo")
assert isinstance(fn, pytest.Function)

module_parent = fn.getparent(pytest.Module)
Expand Down
2 changes: 1 addition & 1 deletion testing/test_junitxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -936,7 +936,7 @@ def test_function(arg):
def test_mangle_test_address() -> None:
from _pytest.junitxml import mangle_test_address

address = "::".join(["a/my.py.thing.py", "Class", "()", "method", "[a-1-::]"])
address = "::".join(["a/my.py.thing.py", "Class", "method", "[a-1-::]"])
newnames = mangle_test_address(address)
assert newnames == ["a.my.py.thing", "Class", "method", "[a-1-::]"]

Expand Down
4 changes: 3 additions & 1 deletion testing/test_nose.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,8 +272,10 @@ def setup_module(mod):
class TestClass(object):
def setup(self):
assert visited
self.visited_cls = True
def test_first(self):
pass
assert visited
assert self.visited_cls
"""
)
result = pytester.runpytest()
Expand Down
2 changes: 1 addition & 1 deletion testing/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ class TestY(TestX):
started = reprec.getcalls("pytest_collectstart")
finished = reprec.getreports("pytest_collectreport")
assert len(started) == len(finished)
assert len(started) == 8
assert len(started) == 6
colfail = [x for x in finished if x.failed]
assert len(colfail) == 1

Expand Down

0 comments on commit 9a57106

Please sign in to comment.