WIP: Revert "Node from parent (pytest-dev#5975)"

Keeps `Node.from_parent` for backward compatibility.

This reverts commit a3c8246, reversing
changes made to 886b8d2.

Ref: pytest-dev#5975 (comment)

Author: blueyed

changelog/5975.deprecation.rst

@@ -33,16 +33,6 @@ provide feedback.
 display captured output when tests fail: ``no``, ``stdout``, ``stderr``, ``log`` or ``all`` (the default).
 
 
-
-Node Construction changed to ``Node.from_parent``
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-
-.. deprecated:: 5.3
-
-The construction of nodes new should use the named constructor ``from_parent``.
-This limitation in api surface intends to enable better/simpler refactoring of the collection tree.
-
-
 ``junit_family`` default value change to "xunit2"
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

doc/en/deprecations.rst

@@ -4,7 +4,7 @@
 
 def pytest_collect_file(parent, path):
     if path.ext == ".yaml" and path.basename.startswith("test"):
-        return YamlFile.from_parent(parent, fspath=path)
+        return YamlFile(path, parent)
 
 
 class YamlFile(pytest.File):
@@ -13,7 +13,7 @@ def collect(self):
 
         raw = yaml.safe_load(self.fspath.open())
         for name, spec in sorted(raw.items()):
-            yield YamlItem.from_parent(self, name=name, spec=spec)
+            yield YamlItem(name, self, spec)
 
 
 class YamlItem(pytest.Item):

doc/en/example/nonpython/conftest.py

@@ -40,11 +40,6 @@
     "as a keyword argument instead."
 )
 
-NODE_USE_FROM_PARENT = UnformattedWarning(
-    PytestDeprecationWarning,
-    "direct construction of {name} has been deprecated, please use {name}.from_parent",
-)
-
 JUNIT_XML_DEFAULT_FAMILY = PytestDeprecationWarning(
     "The 'junit_family' default value will change to 'xunit2' in pytest 6.0.\n"
     "Add 'junit_family=xunit1' to your pytest.ini file to keep the current format "

src/_pytest/deprecated.py

@@ -112,9 +112,9 @@ def pytest_collect_file(path, parent):
     config = parent.config
     if path.ext == ".py":
         if config.option.doctestmodules and not _is_setup_py(config, path, parent):
-            return DoctestModule.from_parent(parent, fspath=path)
+            return DoctestModule(path, parent)
     elif _is_doctest(config, path, parent):
-        return DoctestTextfile.from_parent(parent, fspath=path)
+        return DoctestTextfile(path, parent)
 
 
 def _is_setup_py(config, path, parent):
@@ -221,16 +221,6 @@ def __init__(self, name, parent, runner=None, dtest=None):
         self.obj = None
         self.fixture_request = None
 
-    @classmethod
-    def from_parent(  # type: ignore
-        cls, parent: "Union[DoctestTextfile, DoctestModule]", *, name, runner, dtest
-    ):
-        # incompatible signature due to to imposed limits on sublcass
-        """
-        the public named constructor
-        """
-        return super().from_parent(name=name, parent=parent, runner=runner, dtest=dtest)
-
     def setup(self):
         if self.dtest is not None:
             self.fixture_request = _setup_fixtures(self)
@@ -389,9 +379,7 @@ def collect(self):
         parser = doctest.DocTestParser()
         test = parser.get_doctest(text, globs, name, filename, 0)
         if test.examples:
-            yield DoctestItem.from_parent(
-                self, name=test.name, runner=runner, dtest=test
-            )
+            yield DoctestItem(test.name, self, runner, test)
 
 
 def _check_all_skipped(test):
@@ -500,9 +488,7 @@ def _find(
 
         for test in finder.find(module, module.__name__):
             if test.examples:  # skip empty doctests
-                yield DoctestItem.from_parent(
-                    self, name=test.name, runner=runner, dtest=test
-                )
+                yield DoctestItem(test.name, self, runner, test)
 
 
 def _setup_fixtures(doctest_item):

src/_pytest/doctest.py

@@ -183,7 +183,7 @@ def wrap_session(
     config: Config, doit: Callable[[Config, "Session"], Optional[Union[int, ExitCode]]]
 ) -> Union[int, ExitCode]:
     """Skeleton command line program"""
-    session = Session.from_config(config)
+    session = Session(config)
     session.exitstatus = ExitCode.OK
     initstate = 0
     try:
@@ -387,10 +387,6 @@ def __init__(self, config: Config) -> None:
 
         self._deselected = []  # type: List[nodes.Item]
 
-    @classmethod
-    def from_config(cls, config):
-        return cls._create(config)
-
     def __repr__(self):
         return "<%s %s exitstatus=%r testsfailed=%d testscollected=%d>" % (
             self.__class__.__name__,

src/_pytest/main.py

@@ -20,7 +20,6 @@
 from _pytest.compat import TYPE_CHECKING
 from _pytest.config import Config
 from _pytest.config import PytestPluginManager
-from _pytest.deprecated import NODE_USE_FROM_PARENT
 from _pytest.fixtures import FixtureDef
 from _pytest.fixtures import FixtureLookupError
 from _pytest.fixtures import FixtureLookupErrorRepr
@@ -31,10 +30,13 @@
 from _pytest.outcomes import Failed
 
 if TYPE_CHECKING:
-    # Imported here due to circular import.
-    from _pytest.main import Session  # noqa: F401
+    from typing import Type
+    from typing import TypeVar
 
-    from _pytest._code.code import _TracebackStyle  # noqa: F401
+    from _pytest._code.code import _TracebackStyle
+    from _pytest.main import Session  # circular import
+
+    _N = TypeVar("_N", bound="Node")
 
 SEP = "/"
 
@@ -79,16 +81,7 @@ def ischildnode(baseid, nodeid):
     return node_parts[: len(base_parts)] == base_parts
 
 
-class NodeMeta(type):
-    def __call__(self, *k, **kw):
-        warnings.warn(NODE_USE_FROM_PARENT.format(name=self.__name__), stacklevel=2)
-        return super().__call__(*k, **kw)
-
-    def _create(self, *k, **kw):
-        return super().__call__(*k, **kw)
-
-
-class Node(metaclass=NodeMeta):
+class Node:
     """ base class for Collector and Item the test collection tree.
     Collector subclasses have children, Items are terminal nodes."""
 
@@ -151,22 +144,13 @@ def __init__(
         self._chain = self.listchain()
 
     @classmethod
-    def from_parent(cls, parent: "Node", **kw):
-        """
-        Public Constructor for Nodes
-
-        This indirection got introduced in order to enable removing
-        the fragile logic from the node constructors.
-
-        Subclasses can use ``super().from_parent(...)`` when overriding the construction
-
-        :param parent: the parent node of this test Node
-        """
+    def from_parent(cls: "Type[_N]", parent: "Node", **kw) -> "_N":
+        """Public constructor for Nodes (for compatibility with pytest only)."""
         if "config" in kw:
             raise TypeError("config is not a valid argument for from_parent")
         if "session" in kw:
             raise TypeError("session is not a valid argument for from_parent")
-        return cls._create(parent=parent, **kw)
+        return cls(parent=parent, **kw)
 
     @property
     def ihook(self):
@@ -471,13 +455,6 @@ def __init__(
 
         self._norecursepatterns = self.config.getini("norecursedirs")
 
-    @classmethod
-    def from_parent(cls, parent, *, fspath):
-        """
-        The public constructor
-        """
-        return super().from_parent(parent=parent, fspath=fspath)
-
     def _gethookproxy(self, fspath: py.path.local):
         # check if we have the common case of running
         # hooks with all conftest.py files

src/_pytest/nodes.py

@@ -906,7 +906,7 @@ def getnode(self, config, arg):
         :param arg: a :py:class:`py.path.local` instance of the file
 
         """
-        session = Session.from_config(config)
+        session = Session(config)
         assert "::" not in str(arg)
         p = py.path.local(arg)
         config.hook.pytest_sessionstart(session=session)
@@ -924,7 +924,7 @@ def getpathnode(self, path):
 
         """
         config = self.parseconfigure(path)
-        session = Session.from_config(config)
+        session = Session(config)
         x = session.fspath.bestrelpath(path)
         config.hook.pytest_sessionstart(session=session)
         res = session.perform_collect([x], genitems=False)[0]

src/_pytest/pytester/__init__.py

@@ -213,8 +213,8 @@ def path_matches_patterns(path, patterns):
 
 def pytest_pycollect_makemodule(path, parent):
     if path.basename == "__init__.py":
-        return Package.from_parent(parent, fspath=path)
-    return Module.from_parent(parent, fspath=path)
+        return Package(path, parent)
+    return Module(path, parent)
 
 
 @hookimpl(hookwrapper=True)
@@ -226,7 +226,7 @@ def pytest_pycollect_makeitem(collector, name, obj):
     # nothing was collected elsewhere, let's do it here
     if safe_isclass(obj):
         if collector.istestclass(obj, name):
-            outcome.force_result(Class.from_parent(collector, name=name, obj=obj))
+            outcome.force_result(Class(name, parent=collector))
     elif collector.istestfunction(obj, name):
         # mock seems to store unbound methods (issue473), normalize it
         obj = getattr(obj, "__func__", obj)
@@ -245,7 +245,7 @@ def pytest_pycollect_makeitem(collector, name, obj):
             )
         elif getattr(obj, "__test__", True):
             if is_generator(obj):
-                res = Function.from_parent(collector, name=name)
+                res = Function(name, parent=collector)
                 reason = "yield tests were removed in pytest 4.0 - {name} will be ignored".format(
                     name=name
                 )
@@ -415,7 +415,7 @@ def _genfunctions(self, name, funcobj):
         cls = clscol and clscol.obj or None
         fm = self.session._fixturemanager
 
-        definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj)
+        definition = FunctionDefinition(name=name, parent=self, callobj=funcobj)
         fixtureinfo = definition._fixtureinfo
 
         metafunc = Metafunc(
@@ -430,7 +430,7 @@ def _genfunctions(self, name, funcobj):
         self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc))
 
         if not metafunc._calls:
-            yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo)
+            yield Function(name, parent=self, fixtureinfo=fixtureinfo)
         else:
             # add funcargs() as fixturedefs to fixtureinfo.arg2fixturedefs
             fixtures.add_funcarg_pseudo_fixture_def(self, metafunc, fm)
@@ -442,9 +442,9 @@ def _genfunctions(self, name, funcobj):
 
             for callspec in metafunc._calls:
                 subname = "{}[{}]".format(name, callspec.id)
-                yield Function.from_parent(
-                    self,
+                yield Function(
                     name=subname,
+                    parent=self,
                     callspec=callspec,
                     callobj=funcobj,
                     fixtureinfo=fixtureinfo,
@@ -615,7 +615,7 @@ def collect(self):
         if init_module.check(file=1) and path_matches_patterns(
             init_module, self.config.getini("python_files")
         ):
-            yield Module.from_parent(self, fspath=init_module)
+            yield Module(init_module, self)
         pkg_prefixes = set()
         for path in this_path.visit(rec=self._recurse, bf=True, sort=True):
             # We will visit our own __init__.py file, in which case we skip it.
@@ -666,13 +666,6 @@ def _get_first_non_fixture_func(obj, names):
 class Class(PyCollector):
     """ Collector for test methods. """
 
-    @classmethod
-    def from_parent(cls, parent, *, name, obj=None):
-        """
-        The public constructor
-        """
-        return super().from_parent(name=name, parent=parent)
-
     def collect(self):
         if not safe_getattr(self.obj, "__test__", True):
             return []
@@ -698,7 +691,7 @@ def collect(self):
         self._inject_setup_class_fixture()
         self._inject_setup_method_fixture()
 
-        return [Instance.from_parent(self, name="()")]
+        return [Instance(name="()", parent=self)]
 
     def _inject_setup_class_fixture(self):
         """Injects a hidden autouse, class scoped fixture into the collected class object
@@ -1456,13 +1449,6 @@ def __init__(
         #: .. versionadded:: 3.0
         self.originalname = originalname
 
-    @classmethod
-    def from_parent(cls, parent, **kw):  # todo: determine sound type limitations
-        """
-        The public  constructor
-        """
-        return super().from_parent(parent=parent, **kw)
-
     def _initrequest(self):
         self.funcargs = {}
         self._request = fixtures.FixtureRequest(self)

src/_pytest/python.py

@@ -24,7 +24,7 @@ def pytest_pycollect_makeitem(collector, name, obj):
     except Exception:
         return
     # yes, so let's collect it
-    return UnitTestCase.from_parent(collector, name=name, obj=obj)
+    return UnitTestCase(name, parent=collector)
 
 
 class UnitTestCase(Class):
@@ -52,16 +52,15 @@ def collect(self):
             if not getattr(x, "__test__", True):
                 continue
             funcobj = getimfunc(x)
-            yield TestCaseFunction.from_parent(self, name=name, callobj=funcobj)
+            yield TestCaseFunction(name, parent=self, callobj=funcobj)
             foundsomething = True
 
         if not foundsomething:
             runtest = getattr(self.obj, "runTest", None)
             if runtest is not None:
                 ut = sys.modules.get("twisted.trial.unittest", None)
                 if ut is None or runtest != ut.TestCase.runTest:
-                    # TODO: callobj consistency
-                    yield TestCaseFunction.from_parent(self, name="runTest")
+                    yield TestCaseFunction("runTest", parent=self)
 
     def _inject_setup_teardown_fixtures(self, cls):
         """Injects a hidden auto-use fixture to invoke setUpClass/setup_method and corresponding

src/_pytest/unittest.py

@@ -2,7 +2,6 @@
 
 import pytest
 from _pytest import deprecated
-from _pytest import nodes
 
 
 @pytest.mark.parametrize(
@@ -103,20 +102,6 @@ def test_foo():
         result.stdout.fnmatch_lines([warning_msg])
 
 
-def test_node_direct_ctor_warning():
-    class MockConfig:
-        pass
-
-    ms = MockConfig()
-    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")
-    assert w[0].lineno == inspect.currentframe().f_lineno - 1
-    assert w[0].filename == __file__
-
-
 def assert_no_print_logs(testdir, args):
     result = testdir.runpytest(*args)
     result.stdout.fnmatch_lines(