diff --git a/changelog/5975.deprecation.rst b/changelog/5975.deprecation.rst new file mode 100644 index 00000000000..6e5dbc2acef --- /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/doc/en/deprecations.rst b/doc/en/deprecations.rst index 748d3ac65a4..88112b12a98 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -19,6 +19,16 @@ Below is a complete list of all pytest features which are considered deprecated. :class:`_pytest.warning_types.PytestWarning` or subclasses, which can be filtered using :ref:`standard warning filters `. + +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 93d8285bfa7..d30ab3841dc 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(path, parent) + return YamlFile.from_parent(parent, fspath=path) 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(name, self, spec) + yield YamlItem.from_parent(self, name=name, spec=spec) class YamlItem(pytest.Item): diff --git a/doc/en/example/py2py3/conftest.py b/doc/en/example/py2py3/conftest.py index 844510a25a3..0291b37b46b 100644 --- a/doc/en/example/py2py3/conftest.py +++ b/doc/en/example/py2py3/conftest.py @@ -13,4 +13,4 @@ def collect(self): def pytest_pycollect_makemodule(path, parent): bn = path.basename if "py3" in bn and not py3 or ("py2" in bn and py3): - return DummyCollector(path, parent=parent) + return DummyCollector.from_parent(parent, fspath=path) diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 5a7066041d4..1fdc37c047f 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -9,6 +9,7 @@ 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 @@ "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 f7d96257e93..66fbf8396d9 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 @@ def __init__(self, name, parent, runner=None, dtest=None): 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 @@ def collect(self): 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 @@ def _find(self, tests, obj, name, module, source_lines, globs, seen): 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 b4261c188dd..e46f54d9c00 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 @@ def __init__(self, config): 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 33067334c11..3eaafa91d5a 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -18,6 +18,7 @@ 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 @@ def __init__( 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 @@ def __init__( 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 97ad953a3a1..a1acf747e45 100644 --- a/src/_pytest/pytester.py +++ b/src/_pytest/pytester.py @@ -744,7 +744,7 @@ def getnode(self, config, arg): :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 @@ def getpathnode(self, path): """ 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 4e3a68867f1..95bae5a23bc 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 @@ def _genfunctions(self, name, funcobj): 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 @@ def _genfunctions(self, name, funcobj): 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 @@ def _genfunctions(self, name, funcobj): 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 @@ def collect(self): 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 @@ def collect(self): 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 @@ def __init__( #: .. 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 11dc77cc4ff..1ddb9c86799 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 @@ def collect(self): 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 @@ 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: - 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 @@ def stopTest(self, testcase): 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 5390d038d8b..59cb69a0034 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_foo(): 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 e036cb7d964..9ac1c9d311c 100644 --- a/testing/python/collect.py +++ b/testing/python/collect.py @@ -281,10 +281,10 @@ def make_function(testdir, **kwargs): 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 @@ def reportinfo(self): 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 73419eef424..35e86e6b96c 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(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 @@ def test_autouse_fixture(self, testdir): # rough jstests usage 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 9a1e1f96841..9b6471cdc50 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -30,7 +30,7 @@ class DefinitionMock(python.FunctionDefinition): 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 b791ac6f9be..624e9dd4e76 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -75,7 +75,7 @@ class CustomFile(pytest.File): 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 @@ def test_parsearg(self, testdir): 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 @@ def test_collect_topdir(self, testdir): # 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 0e44220259c..33276b63ca1 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -962,7 +962,11 @@ class TestBarClass(BaseTests): 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 836b458c6e4..56d5a762550 100644 --- a/testing/test_pluginmanager.py +++ b/testing/test_pluginmanager.py @@ -122,7 +122,7 @@ def pytest_plugin_registered(self): 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")