Skip to content

WIP POC RFC: remove config/session as possible parameters for node #6293

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -374,9 +374,11 @@ class Session(nodes.FSCollector):
_setupstate = None # type: SetupState

def __init__(self, config):
nodes.FSCollector.__init__(
self, config.rootdir, parent=None, config=config, session=self, nodeid=""
)
self.config = config
self.fspath = config.rootdir
self.session = self
super().__init__(fspath=config.rootdir, parent=self, nodeid="")
self.parent = None
self.testsfailed = 0
self.testscollected = 0
self.shouldstop = False
Expand Down
45 changes: 17 additions & 28 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import warnings
from functools import lru_cache
from typing import Any
from typing import cast
from typing import Dict
from typing import List
from typing import Optional
Expand All @@ -17,7 +18,6 @@
from _pytest._code.code import ReprExceptionInfo
from _pytest.compat import cached_property
from _pytest.compat import getfslineno
from _pytest.config import Config
from _pytest.deprecated import NODE_USE_FROM_PARENT
from _pytest.fixtures import FixtureDef
from _pytest.fixtures import FixtureLookupError
Expand All @@ -30,6 +30,7 @@
if False: # TYPE_CHECKING
# Imported here due to circular import.
from _pytest.main import Session # noqa: F401
from _pytest.config import Config # noqa: F401

SEP = "/"

Expand Down Expand Up @@ -90,37 +91,27 @@ class Node(metaclass=NodeMeta):
def __init__(
self,
name,
parent: Optional["Node"] = None,
config: Optional[Config] = None,
session: Optional["Session"] = None,
parent: Union["Node", "Session"],
fspath: Optional[py.path.local] = None,
nodeid: Optional[str] = None,
Copy link
Member

Choose a reason for hiding this comment

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

Should we still allow to customize the entire node id?

I know there are items which are derived from other items, for example flake8 items which are derived from normal Python items, but those can be customized by passing a different name already.

Copy link
Member Author

Choose a reason for hiding this comment

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

actualy an oversight, we should also disallow it quickly for from_parent

Copy link
Member Author

Choose a reason for hiding this comment

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

my main intend with this pr was the structural details, so nodeid was not instrumental to the data structure

Copy link
Member

Choose a reason for hiding this comment

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

I see.

So the main intent is to remove, in 6.1, config, session and nodeid from the Node constructor, as I understand they are the cause of structural problems, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup

) -> None:
#: a unique name within the scope of the parent node
self.name = name

#: the parent collector node.
self.parent = parent

#: the pytest config object
if config:
self.config = config
if parent is self:
self.parent: Optional["Node"] = None # temporary hack for session
self.session = cast("Session", parent)
else:
if not parent:
raise TypeError("config or parent must be provided")
self.config = parent.config

#: the session this node is part of
if session:
self.session = session
else:
if not parent:
raise TypeError("session or parent must be provided")
self.parent = parent
#: the session this node is part of
self.session = parent.session

#: filesystem path where this node was collected from (can be None)
self.fspath = fspath or getattr(parent, "fspath", None)
#: the pytest config object
self.config: "Config" = parent.config

#: filesystem path where this node was collected from (can be None)
self.fspath: py.path.local = fspath or parent.fspath
#: keywords/markers collected from all scopes
self.keywords = NodeKeywords(self)

Expand Down Expand Up @@ -409,9 +400,7 @@ def _check_initialpaths_for_relpath(session, fspath):


class FSCollector(Collector):
def __init__(
self, fspath: py.path.local, parent=None, config=None, session=None, nodeid=None
) -> None:
def __init__(self, fspath: py.path.local, parent, nodeid=None) -> None:
name = fspath.basename
if parent is not None:
rel = fspath.relto(parent.fspath)
Expand All @@ -420,7 +409,7 @@ def __init__(
name = name.replace(os.sep, SEP)
self.fspath = fspath

session = session or parent.session
session = parent.session

if nodeid is None:
nodeid = self.fspath.relto(session.config.rootdir)
Expand All @@ -430,7 +419,7 @@ def __init__(
if nodeid and os.sep != SEP:
nodeid = nodeid.replace(os.sep, SEP)

super().__init__(name, parent, config, session, nodeid=nodeid, fspath=fspath)
super().__init__(name=name, parent=parent, nodeid=nodeid, fspath=fspath)

@classmethod
def from_parent(cls, parent, *, fspath):
Expand All @@ -448,8 +437,8 @@ class Item(Node):

nextitem = None

def __init__(self, name, parent=None, config=None, session=None, nodeid=None):
super().__init__(name, parent, config, session, nodeid=nodeid)
def __init__(self, name, parent=None, nodeid=None):
super().__init__(name=name, parent=parent, nodeid=nodeid)
self._report_sections = [] # type: List[Tuple[str, str, str]]

#: user properties is a list of tuples (name, value) that holds user
Expand Down
10 changes: 3 additions & 7 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -539,11 +539,9 @@ def _importtestmodule(self):


class Package(Module):
def __init__(self, fspath, parent=None, config=None, session=None, nodeid=None):
def __init__(self, fspath, parent=None, nodeid=None):
session = parent.session
nodes.FSCollector.__init__(
self, fspath, parent=parent, config=config, session=session, nodeid=nodeid
)
nodes.FSCollector.__init__(self, fspath, parent=parent, nodeid=nodeid)
self.name = fspath.dirname
self.trace = session.trace
self._norecursepatterns = session._norecursepatterns
Expand Down Expand Up @@ -1405,15 +1403,13 @@ def __init__(
name,
parent,
args=None,
config=None,
callspec=None,
callobj=NOTSET,
keywords=None,
session=None,
fixtureinfo=None,
originalname=None,
):
super().__init__(name, parent, config=config, session=session)
super().__init__(name=name, parent=parent)
self._args = args
if callobj is not NOTSET:
self.obj = callobj
Expand Down
20 changes: 16 additions & 4 deletions testing/deprecated_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,26 @@ def test_foo():


def test_node_direct_ctor_warning():
class MockConfig:
pass
class MockAll:
@property
def parent(self):
return None

@property
def config(self):
return self

@property
def session(self):
return self

fspath = None

ms = MockConfig()
ms = MockAll()
with pytest.warns(
DeprecationWarning,
match="direct construction of .* has been deprecated, please use .*.from_parent",
) as w:
nodes.Node(name="test", config=ms, session=ms, nodeid="None")
nodes.Node(name="test", parent=ms, nodeid="None")
assert w[0].lineno == inspect.currentframe().f_lineno - 1
assert w[0].filename == __file__
4 changes: 2 additions & 2 deletions testing/example_scripts/fixtures/custom_item/conftest.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import pytest


class CustomItem(pytest.Item, pytest.File):
class CustomItem(pytest.Item):
def runtest(self):
pass


def pytest_collect_file(path, parent):
return CustomItem(path, parent)
return CustomItem.from_parent(parent, fspath=path)
2 changes: 1 addition & 1 deletion testing/python/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ def make_function(testdir, **kwargs):
session = testdir.Session.from_config(config)
session._fixturemanager = FixtureManager(session)

return pytest.Function.from_parent(config=config, parent=session, **kwargs)
return pytest.Function.from_parent(parent=session, **kwargs)

def test_function_equality(self, testdir, tmpdir):
def func1():
Expand Down
1 change: 1 addition & 0 deletions testing/python/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,7 @@ def test_package(one):
reprec = testdir.inline_run()
reprec.assertoutcome(passed=2)

@pytest.mark.xfail(reason="file item dulity mess")
def test_collect_custom_items(self, testdir):
testdir.copy_example("fixtures/custom_item")
result = testdir.runpytest("foo")
Expand Down
9 changes: 5 additions & 4 deletions testing/test_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,7 @@ def test_y(self):
assert ids == ["MyTestSuite.x_test", "TestCase.test_y"]


@pytest.mark.xfail(reason="item<>file missmatch, a mess we should break")
def test_matchnodes_two_collections_same_file(testdir):
testdir.makeconftest(
"""
Expand All @@ -756,18 +757,18 @@ def pytest_configure(config):
class Plugin2(object):
def pytest_collect_file(self, path, parent):
if path.ext == ".abc":
return MyFile2(path, parent)
return MyFile2.from_parent(parent, fspath=path)

def pytest_collect_file(path, parent):
if path.ext == ".abc":
return MyFile1(path, parent)
return MyFile1.from_parent(parent, fspath=path)

class MyFile1(pytest.Item, pytest.File):
class MyFile1(pytest.Item):
def runtest(self):
pass
class MyFile2(pytest.File):
def collect(self):
return [Item2("hello", parent=self)]
return [Item2.from_parent(name="hello", parent=self)]

class Item2(pytest.Item):
def runtest(self):
Expand Down