From e2c3d6a04e64236d2ac77b07ab34c6c12b411b24 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 1 Sep 2020 18:13:29 +0200 Subject: [PATCH] WIP: Revert "Node from parent (pytest-dev/pytest#5975)" Keeps `Node.from_parent` for backward compatibility. This reverts commit a3c8246b6053de15d9b95bfa7a425d59054c2693, reversing changes made to 886b8d27c669c6a65488192f75299a7cb9e1ab52. Ref: https://github.com/pytest-dev/pytest/pull/5975#discussion_r339523282 --- changelog/5975.deprecation.rst | 10 ------- doc/en/deprecations.rst | 10 ------- doc/en/example/nonpython/conftest.py | 4 +-- src/_pytest/deprecated.py | 5 ---- src/_pytest/doctest.py | 22 +++----------- src/_pytest/main.py | 6 +--- src/_pytest/nodes.py | 43 +++++++--------------------- src/_pytest/pytester/__init__.py | 4 +-- src/_pytest/python.py | 34 +++++++--------------- src/_pytest/unittest.py | 7 ++--- testing/deprecated_test.py | 15 ---------- testing/python/collect.py | 4 +-- testing/python/integration.py | 4 +-- testing/python/metafunc.py | 2 +- testing/test_collection.py | 6 ++-- testing/test_mark.py | 4 +-- testing/test_pluginmanager.py | 2 +- 17 files changed, 43 insertions(+), 139 deletions(-) delete mode 100644 changelog/5975.deprecation.rst diff --git a/changelog/5975.deprecation.rst b/changelog/5975.deprecation.rst deleted file mode 100644 index 257249efe01..00000000000 --- a/changelog/5975.deprecation.rst +++ /dev/null @@ -1,10 +0,0 @@ -Deprecate using direct constructors for ``Nodes``. - -Instead they are new constructed via ``Node.from_parent``. - -This transitional mechanism enables us to detangle the very intensely -entangled ``Node`` relationships by enforcing more controlled creation/configruation patterns. - -As part of that session/config are already disallowed parameters and as we work on the details we might need disallow a few more as well. - -Subclasses are expected to use `super().from_parent` if they intend to expand the creation of `Nodes`. diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index bb91b9a11ec..0e38f67ac8e 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.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" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/en/example/nonpython/conftest.py b/doc/en/example/nonpython/conftest.py index d30ab3841dc..93d8285bfa7 100644 --- a/doc/en/example/nonpython/conftest.py +++ b/doc/en/example/nonpython/conftest.py @@ -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): diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 1d8a8883c5d..2bd97657d5b 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.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 " diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index d2e632867eb..3cbdb540bb0 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.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): diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 1d4fea08b92..b943e7e59f3 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.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__, diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index c452e63c48e..3b94036ebe4 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.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): @@ -473,13 +457,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 diff --git a/src/_pytest/pytester/__init__.py b/src/_pytest/pytester/__init__.py index 5097665bc1a..23575a9687a 100644 --- a/src/_pytest/pytester/__init__.py +++ b/src/_pytest/pytester/__init__.py @@ -913,7 +913,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) @@ -931,7 +931,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] diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 4989d6b3984..31f8fb75285 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.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) diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index a5512e9443c..586de541518 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.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,7 +52,7 @@ 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: @@ -60,8 +60,7 @@ def collect(self): 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 diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 8ee1d3d7855..74a72add0de 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.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( diff --git a/testing/python/collect.py b/testing/python/collect.py index eab4732e44d..b003f16ab79 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -316,10 +316,10 @@ def make_function(testdir, **kwargs): from _pytest.fixtures import FixtureManager config = testdir.parseconfigure() - session = testdir.Session.from_config(config) + session = testdir.Session(config) session._fixturemanager = FixtureManager(session) - return pytest.Function.from_parent(parent=session, **kwargs) + return pytest.Function(config=config, parent=session, **kwargs) def test_function_equality(self, testdir): def func1(): diff --git a/testing/python/integration.py b/testing/python/integration.py index 35e86e6b96c..73419eef424 100644 --- a/testing/python/integration.py +++ b/testing/python/integration.py @@ -10,7 +10,7 @@ def test_funcarg_non_pycollectobj(self, testdir): # rough jstests usage import pytest def pytest_pycollect_makeitem(collector, name, obj): if name == "MyClass": - return MyCollector.from_parent(collector, name=name) + return MyCollector(name, parent=collector) class MyCollector(pytest.Collector): def reportinfo(self): return self.fspath, 3, "xyz" @@ -40,7 +40,7 @@ def test_autouse_fixture(self, testdir): # rough jstests usage import pytest def pytest_pycollect_makeitem(collector, name, obj): if name == "MyClass": - return MyCollector.from_parent(collector, name=name) + return MyCollector(name, parent=collector) class MyCollector(pytest.Collector): def reportinfo(self): return self.fspath, 3, "xyz" diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index c74b7770898..f47c44889c2 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -36,7 +36,7 @@ class DefinitionMock(python.FunctionDefinition): names = fixtures.getfuncargnames(func) fixtureinfo = FuncFixtureInfoMock(names) # type: Any - definition = DefinitionMock._create(func) # type: Any + definition = DefinitionMock(func) # type: Any return python.Metafunc(definition, fixtureinfo, config) def test_no_funcargs(self) -> None: diff --git a/testing/test_collection.py b/testing/test_collection.py index d6d93a73ef7..7d5a19b1575 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -79,7 +79,7 @@ class CustomFile(pytest.File): pass def pytest_collect_file(path, parent): if path.ext == ".xxx": - return CustomFile.from_parent(fspath=path, parent=parent) + return CustomFile(path, parent=parent) """ ) node = testdir.getpathnode(hello) @@ -475,7 +475,7 @@ def test_parsearg(self, testdir) -> None: p.move(target) subdir.chdir() config = testdir.parseconfig(p.basename) - rcol = Session.from_config(config) + rcol = Session(config=config) assert rcol.fspath == subdir fspath, parts = rcol._parsearg(p.basename) @@ -492,7 +492,7 @@ def test_collect_topdir(self, testdir): # XXX migrate to collectonly? (see below) config = testdir.parseconfig(id) topdir = testdir.tmpdir - rcol = Session.from_config(config) + rcol = Session(config) assert topdir == rcol.fspath # rootid = rcol.nodeid # root2 = rcol.perform_collect([rcol.nodeid], genitems=False)[0] diff --git a/testing/test_mark.py b/testing/test_mark.py index 09b1ab26421..8d34e49a968 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -1019,13 +1019,13 @@ def test_3(): assert reprec.countoutcomes() == [3, 0, 0] -def test_addmarker_order(): +def test_addmarker_order() -> None: session = mock.Mock() session.own_markers = [] session.parent = None session.nodeid = "" session.listchain = lambda: [session] - node = Node.from_parent(session, name="Test") + node = Node("Test", parent=session) node.add_marker("foo") node.add_marker("bar") node.add_marker("baz", append=False) diff --git a/testing/test_pluginmanager.py b/testing/test_pluginmanager.py index c6227229ee8..f02e66fab50 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -126,7 +126,7 @@ def pytest_plugin_registered(self): def test_hook_proxy(self, testdir: "Testdir") -> None: """Test the gethookproxy function(#2016)""" config = testdir.parseconfig() - session = Session.from_config(config) + session = Session(config) testdir.makepyfile(**{"tests/conftest.py": "", "tests/subdir/conftest.py": ""}) conftest1 = testdir.tmpdir.join("tests/conftest.py")