From c99c7d0f95170a5e308ed6c37e63b2d90794a4f9 Mon Sep 17 00:00:00 2001 From: Ronny Pfannschmidt Date: Wed, 16 Oct 2019 21:52:04 +0200 Subject: [PATCH] deprecate direct node construction and introduce Node.from_parent --- changelog/5975.deprecation.rst | 6 ++++++ src/_pytest/deprecated.py | 6 ++++++ src/_pytest/doctest.py | 16 ++++++++++++---- src/_pytest/main.py | 6 +++++- src/_pytest/nodes.py | 20 +++++++++++++++++++- src/_pytest/pytester.py | 4 ++-- src/_pytest/python.py | 28 ++++++++++++++++++---------- src/_pytest/unittest.py | 14 ++++++++------ testing/deprecated_test.py | 17 +++++++++++++++++ testing/python/collect.py | 6 +++--- testing/python/integration.py | 4 ++-- testing/python/metafunc.py | 2 +- testing/test_collection.py | 6 +++--- testing/test_mark.py | 6 +++++- testing/test_pluginmanager.py | 2 +- 15 files changed, 108 insertions(+), 35 deletions(-) create mode 100644 changelog/5975.deprecation.rst diff --git a/changelog/5975.deprecation.rst b/changelog/5975.deprecation.rst new file mode 100644 index 000000000..6e5dbc2ac --- /dev/null +++ b/changelog/5975.deprecation.rst @@ -0,0 +1,6 @@ +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. diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 5a7066041..1fdc37c04 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -9,6 +9,7 @@ All constants defined in this module should be either PytestWarning instances or in case of warnings which need to format their messages. """ from _pytest.warning_types import PytestDeprecationWarning +from _pytest.warning_types import UnformattedWarning # set of plugins which have been integrated into the core; we use this list to ignore # them during registration to avoid conflicts @@ -35,6 +36,11 @@ FIXTURE_POSITIONAL_ARGUMENTS = PytestDeprecationWarning( "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=legacy' to your pytest.ini file to silence this warning and make your suite compatible." diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index f7d96257e..66fbf8396 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -108,9 +108,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(path, parent) + return DoctestModule.from_parent(parent, fspath=path) elif _is_doctest(config, path, parent): - return DoctestTextfile(path, parent) + return DoctestTextfile.from_parent(parent, fspath=path) def _is_setup_py(config, path, parent): @@ -215,6 +215,10 @@ class DoctestItem(pytest.Item): self.obj = None self.fixture_request = None + @classmethod + def from_parent(cls, parent, *, name, runner, dtest): + return cls._create(name=name, parent=parent, runner=runner, dtest=dtest) + def setup(self): if self.dtest is not None: self.fixture_request = _setup_fixtures(self) @@ -370,7 +374,9 @@ class DoctestTextfile(pytest.Module): parser = doctest.DocTestParser() test = parser.get_doctest(text, globs, name, filename, 0) if test.examples: - yield DoctestItem(test.name, self, runner, test) + yield DoctestItem.from_parent( + self, name=test.name, runner=runner, dtest=test + ) def _check_all_skipped(test): @@ -467,7 +473,9 @@ class DoctestModule(pytest.Module): for test in finder.find(module, module.__name__): if test.examples: # skip empty doctests - yield DoctestItem(test.name, self, runner, test) + yield DoctestItem.from_parent( + self, name=test.name, runner=runner, dtest=test + ) def _setup_fixtures(doctest_item): diff --git a/src/_pytest/main.py b/src/_pytest/main.py index b4261c188..e46f54d9c 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -184,7 +184,7 @@ def pytest_addoption(parser): def wrap_session(config, doit): """Skeleton command line program""" - session = Session(config) + session = Session.from_config(config) session.exitstatus = ExitCode.OK initstate = 0 try: @@ -395,6 +395,10 @@ class Session(nodes.FSCollector): self.config.pluginmanager.register(self, name="session") + @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 33067334c..3eaafa91d 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -18,6 +18,7 @@ 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 from _pytest.fixtures import FixtureLookupErrorRepr @@ -73,7 +74,16 @@ def ischildnode(baseid, nodeid): return node_parts[: len(base_parts)] == base_parts -class Node: +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): """ base class for Collector and Item the test collection tree. Collector subclasses have children, Items are terminal nodes.""" @@ -133,6 +143,10 @@ class Node: if self.name != "()": self._nodeid += "::" + self.name + @classmethod + def from_parent(cls, parent, *, name): + return cls._create(parent=parent, name=name) + @property def ihook(self): """ fspath sensitive hook proxy used to call pytest hooks""" @@ -418,6 +432,10 @@ class FSCollector(Collector): super().__init__(name, parent, config, session, nodeid=nodeid, fspath=fspath) + @classmethod + def from_parent(cls, parent, *, fspath): + return cls._create(parent=parent, fspath=fspath) + class File(FSCollector): """ base class for collecting tests from a file. """ diff --git a/src/_pytest/pytester.py b/src/_pytest/pytester.py index 97ad953a3..a1acf747e 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -744,7 +744,7 @@ class Testdir: :param arg: a :py:class:`py.path.local` instance of the file """ - session = Session(config) + session = Session.from_config(config) assert "::" not in str(arg) p = py.path.local(arg) config.hook.pytest_sessionstart(session=session) @@ -762,7 +762,7 @@ class Testdir: """ config = self.parseconfigure(path) - session = Session(config) + session = Session.from_config(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 4e3a68867..95bae5a23 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -190,8 +190,8 @@ def path_matches_patterns(path, patterns): def pytest_pycollect_makemodule(path, parent): if path.basename == "__init__.py": - return Package(path, parent) - return Module(path, parent) + return Package.from_parent(parent, fspath=path) + return Module.from_parent(parent, fspath=path) @hookimpl(hookwrapper=True) @@ -203,7 +203,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(name, parent=collector)) + outcome.force_result(Class.from_parent(collector, name=name, obj=obj)) elif collector.istestfunction(obj, name): # mock seems to store unbound methods (issue473), normalize it obj = getattr(obj, "__func__", obj) @@ -222,7 +222,7 @@ def pytest_pycollect_makeitem(collector, name, obj): ) elif getattr(obj, "__test__", True): if is_generator(obj): - res = Function(name, parent=collector) + res = Function.from_parent(collector, name=name) reason = "yield tests were removed in pytest 4.0 - {name} will be ignored".format( name=name ) @@ -381,7 +381,7 @@ class PyCollector(PyobjMixin, nodes.Collector): cls = clscol and clscol.obj or None fm = self.session._fixturemanager - definition = FunctionDefinition(name=name, parent=self, callobj=funcobj) + definition = FunctionDefinition.from_parent(self, name=name, callobj=funcobj) fixtureinfo = fm.getfixtureinfo(definition, funcobj, cls) metafunc = Metafunc( @@ -396,7 +396,7 @@ class PyCollector(PyobjMixin, nodes.Collector): self.ihook.pytest_generate_tests.call_extra(methods, dict(metafunc=metafunc)) if not metafunc._calls: - yield Function(name, parent=self, fixtureinfo=fixtureinfo) + yield Function.from_parent(self, name=name, fixtureinfo=fixtureinfo) else: # add funcargs() as fixturedefs to fixtureinfo.arg2fixturedefs fixtures.add_funcarg_pseudo_fixture_def(self, metafunc, fm) @@ -408,9 +408,9 @@ class PyCollector(PyobjMixin, nodes.Collector): for callspec in metafunc._calls: subname = "{}[{}]".format(name, callspec.id) - yield Function( + yield Function.from_parent( + self, name=subname, - parent=self, callspec=callspec, callobj=funcobj, fixtureinfo=fixtureinfo, @@ -626,7 +626,7 @@ class Package(Module): if init_module.check(file=1) and path_matches_patterns( init_module, self.config.getini("python_files") ): - yield Module(init_module, self) + yield Module.from_parent(self, fspath=init_module) 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. @@ -677,6 +677,10 @@ def _get_first_non_fixture_func(obj, names): class Class(PyCollector): """ Collector for test methods. """ + @classmethod + def from_parent(cls, parent, *, name, obj=None): + return cls._create(name=name, parent=parent) + def collect(self): if not safe_getattr(self.obj, "__test__", True): return [] @@ -702,7 +706,7 @@ class Class(PyCollector): self._inject_setup_class_fixture() self._inject_setup_method_fixture() - return [Instance(name="()", parent=self)] + return [Instance.from_parent(self, name="()")] def _inject_setup_class_fixture(self): """Injects a hidden autouse, class scoped fixture into the collected class object @@ -1454,6 +1458,10 @@ class Function(FunctionMixin, nodes.Item): #: .. versionadded:: 3.0 self.originalname = originalname + @classmethod + def from_parent(cls, parent, **kw): + return cls._create(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 11dc77cc4..1ddb9c867 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -23,7 +23,7 @@ def pytest_pycollect_makeitem(collector, name, obj): except Exception: return # yes, so let's collect it - return UnitTestCase(name, parent=collector) + return UnitTestCase.from_parent(collector, name=name, obj=obj) class UnitTestCase(Class): @@ -51,7 +51,7 @@ class UnitTestCase(Class): if not getattr(x, "__test__", True): continue funcobj = getimfunc(x) - yield TestCaseFunction(name, parent=self, callobj=funcobj) + yield TestCaseFunction.from_parent(self, name=name, callobj=funcobj) foundsomething = True if not foundsomething: @@ -59,7 +59,8 @@ class UnitTestCase(Class): if runtest is not None: ut = sys.modules.get("twisted.trial.unittest", None) if ut is None or runtest != ut.TestCase.runTest: - yield TestCaseFunction("runTest", parent=self) + # TODO: callobj consistency + yield TestCaseFunction.from_parent(self, name="runTest") def _inject_setup_teardown_fixtures(self, cls): """Injects a hidden auto-use fixture to invoke setUpClass/setup_method and corresponding @@ -190,20 +191,21 @@ class TestCaseFunction(Function): def _handle_skip(self): # implements the skipping machinery (see #2137) # analog to pythons Lib/unittest/case.py:run - testMethod = getattr(self._testcase, self._testcase._testMethodName) + test_method = getattr(self._testcase, self._testcase._testMethodName) if getattr(self._testcase.__class__, "__unittest_skip__", False) or getattr( - testMethod, "__unittest_skip__", False + test_method, "__unittest_skip__", False ): # If the class or method was skipped. skip_why = getattr( self._testcase.__class__, "__unittest_skip_why__", "" - ) or getattr(testMethod, "__unittest_skip_why__", "") + ) or getattr(test_method, "__unittest_skip_why__", "") self._testcase._addSkip(self, self._testcase, skip_why) return True return False def runtest(self): if self.config.pluginmanager.get_plugin("pdbinvoke") is None: + # TODO: move testcase reporter into separate class, this shouldnt be on item self._testcase(result=self) else: # disables tearDown and cleanups for post mortem debugging (see #1890) diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 5390d038d..59cb69a00 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -1,5 +1,8 @@ +import inspect + import pytest from _pytest import deprecated +from _pytest import nodes @pytest.mark.filterwarnings("default") @@ -73,3 +76,17 @@ def test_warn_about_imminent_junit_family_default_change(testdir, junit_family): result.stdout.no_fnmatch_line(warning_msg) else: 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__ diff --git a/testing/python/collect.py b/testing/python/collect.py index e036cb7d9..9ac1c9d31 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -281,10 +281,10 @@ class TestFunction: from _pytest.fixtures import FixtureManager config = testdir.parseconfigure() - session = testdir.Session(config) + session = testdir.Session.from_config(config) session._fixturemanager = FixtureManager(session) - return pytest.Function(config=config, parent=session, **kwargs) + return pytest.Function.from_parent(config=config, parent=session, **kwargs) def test_function_equality(self, testdir, tmpdir): def func1(): @@ -1024,7 +1024,7 @@ class TestReportInfo: return "ABCDE", 42, "custom" def pytest_pycollect_makeitem(collector, name, obj): if name == "test_func": - return MyFunction(name, parent=collector) + return MyFunction.from_parent(name=name, parent=collector) """ ) item = testdir.getitem("def test_func(): pass") diff --git a/testing/python/integration.py b/testing/python/integration.py index 73419eef4..35e86e6b9 100644 --- a/testing/python/integration.py +++ b/testing/python/integration.py @@ -10,7 +10,7 @@ class TestOEJSKITSpecials: import pytest def pytest_pycollect_makeitem(collector, name, obj): if name == "MyClass": - return MyCollector(name, parent=collector) + return MyCollector.from_parent(collector, name=name) class MyCollector(pytest.Collector): def reportinfo(self): return self.fspath, 3, "xyz" @@ -40,7 +40,7 @@ class TestOEJSKITSpecials: import pytest def pytest_pycollect_makeitem(collector, name, obj): if name == "MyClass": - return MyCollector(name, parent=collector) + return MyCollector.from_parent(collector, name=name) 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 9a1e1f968..9b6471cdc 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -30,7 +30,7 @@ class TestMetafunc: names = fixtures.getfuncargnames(func) fixtureinfo = FixtureInfo(names) - definition = DefinitionMock(func) + definition = DefinitionMock._create(func) return python.Metafunc(definition, fixtureinfo, config) def test_no_funcargs(self, testdir): diff --git a/testing/test_collection.py b/testing/test_collection.py index b791ac6f9..624e9dd4e 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -75,7 +75,7 @@ class TestCollector: pass def pytest_collect_file(path, parent): if path.ext == ".xxx": - return CustomFile(path, parent=parent) + return CustomFile.from_parent(fspath=path, parent=parent) """ ) node = testdir.getpathnode(hello) @@ -446,7 +446,7 @@ class TestSession: p.move(target) subdir.chdir() config = testdir.parseconfig(p.basename) - rcol = Session(config=config) + rcol = Session.from_config(config) assert rcol.fspath == subdir parts = rcol._parsearg(p.basename) @@ -463,7 +463,7 @@ class TestSession: # XXX migrate to collectonly? (see below) config = testdir.parseconfig(id) topdir = testdir.tmpdir - rcol = Session(config) + rcol = Session.from_config(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 0e4422025..33276b63c 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -962,7 +962,11 @@ def test_mark_expressions_no_smear(testdir): def test_addmarker_order(): - node = Node("Test", config=mock.Mock(), session=mock.Mock(), nodeid="Test") + session = mock.Mock() + session.own_markers = [] + session.parent = None + session.nodeid = "" + node = Node.from_parent(session, name="Test") 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 836b458c6..56d5a7625 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -122,7 +122,7 @@ class TestPytestPluginInteractions: def test_hook_proxy(self, testdir): """Test the gethookproxy function(#2016)""" config = testdir.parseconfig() - session = Session(config) + session = Session.from_config(config) testdir.makepyfile(**{"tests/conftest.py": "", "tests/subdir/conftest.py": ""}) conftest1 = testdir.tmpdir.join("tests/conftest.py")