From 43213add579d1550031d5e4d4eff489c4096eb1d Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Mon, 15 Nov 2021 21:00:32 +0200 Subject: [PATCH 1/2] testing/test_session: add a regression test for an old bug Nothing tests this currently. Make sure it doesn't regress if/when the complex code in `Session.collect` is cleaned up. --- testing/test_session.py | 48 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/testing/test_session.py b/testing/test_session.py index 3ca6d3903..f73dc89ef 100644 --- a/testing/test_session.py +++ b/testing/test_session.py @@ -335,6 +335,54 @@ def test_sessionfinish_with_start(pytester: Pytester) -> None: assert res.ret == ExitCode.NO_TESTS_COLLECTED +def test_collection_args_do_not_duplicate_modules(pytester: Pytester) -> None: + """Test that when multiple collection args are specified on the command line + for the same module, only a single Module collector is created. + + Regression test for #723, #3358. + """ + pytester.makepyfile( + **{ + "d/test_it": """ + def test_1(): pass + def test_2(): pass + """ + } + ) + + result = pytester.runpytest( + "--collect-only", + "d/test_it.py::test_1", + "d/test_it.py::test_2", + ) + result.stdout.fnmatch_lines( + [ + "", + " ", + " ", + ], + consecutive=True, + ) + + # Different, but related case. + result = pytester.runpytest( + "--collect-only", + "--keep-duplicates", + "d", + "d", + ) + result.stdout.fnmatch_lines( + [ + "", + " ", + " ", + " ", + " ", + ], + consecutive=True, + ) + + @pytest.mark.parametrize("path", ["root", "{relative}/root", "{environment}/root"]) def test_rootdir_option_arg( pytester: Pytester, monkeypatch: MonkeyPatch, path: str From e05e696fdac097ebad4a77a47f878c81fca39cb2 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 4 Nov 2021 00:21:35 +0200 Subject: [PATCH 2/2] Make PyCollector an implementation detail - don't use in hook type annotation The `pytest_pycollector_makeitem` argument `collector` is currently annotated with type `PyCollector`. As part of #7469, that would have required us to expose it in the public API. But really it's an implementation detail, not something we want to expose. So replace the annotation with the concrete python collector types that are passed. Strictly speaking, `pytest_pycollector_makeitem` is called from `PyCollector.collect()`, so the new type annotation is incorrect if another type subclasses `PyCollector`. But the set of python collectors is closed (mapping to language constructs), and the type is private, so there shouldn't be any other deriving classes, and we can consider it effectively sealed (unfortunately Python does not provide a way to express this - yet?). --- src/_pytest/hookspec.py | 4 ++-- src/_pytest/python.py | 15 ++++++++++----- src/_pytest/unittest.py | 4 ++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index b65be9815..8aaa6e434 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -34,10 +34,10 @@ if TYPE_CHECKING: from _pytest.nodes import Collector from _pytest.nodes import Item from _pytest.outcomes import Exit + from _pytest.python import Class from _pytest.python import Function from _pytest.python import Metafunc from _pytest.python import Module - from _pytest.python import PyCollector from _pytest.reports import CollectReport from _pytest.reports import TestReport from _pytest.runner import CallInfo @@ -360,7 +360,7 @@ def pytest_pycollect_makemodule( @hookspec(firstresult=True) def pytest_pycollect_makeitem( - collector: "PyCollector", name: str, obj: object + collector: Union["Module", "Class"], name: str, obj: object ) -> Union[None, "Item", "Collector", List[Union["Item", "Collector"]]]: """Return a custom item/collector for a Python object in a module, or None. diff --git a/src/_pytest/python.py b/src/_pytest/python.py index b557cd8fa..b5e391fb2 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -224,11 +224,15 @@ def pytest_pycollect_makemodule(module_path: Path, parent) -> "Module": @hookimpl(trylast=True) -def pytest_pycollect_makeitem(collector: "PyCollector", name: str, obj: object): +def pytest_pycollect_makeitem( + collector: Union["Module", "Class"], name: str, obj: object +) -> Union[None, nodes.Item, nodes.Collector, List[Union[nodes.Item, nodes.Collector]]]: + assert isinstance(collector, (Class, Module)), type(collector) # Nothing was collected elsewhere, let's do it here. if safe_isclass(obj): if collector.istestclass(obj, name): - return Class.from_parent(collector, name=name, obj=obj) + klass: Class = Class.from_parent(collector, name=name, obj=obj) + return klass elif collector.istestfunction(obj, name): # mock seems to store unbound methods (issue473), normalize it. obj = getattr(obj, "__func__", obj) @@ -247,15 +251,16 @@ def pytest_pycollect_makeitem(collector: "PyCollector", name: str, obj: object): ) elif getattr(obj, "__test__", True): if is_generator(obj): - res = Function.from_parent(collector, name=name) + res: Function = Function.from_parent(collector, name=name) reason = "yield tests were removed in pytest 4.0 - {name} will be ignored".format( name=name ) res.add_marker(MARK_GEN.xfail(run=False, reason=reason)) res.warn(PytestCollectionWarning(reason)) + return res else: - res = list(collector._genfunctions(name, obj)) - return res + return list(collector._genfunctions(name, obj)) + return None class PyobjMixin(nodes.Node): diff --git a/src/_pytest/unittest.py b/src/_pytest/unittest.py index 108095bfc..a05c3b4bc 100644 --- a/src/_pytest/unittest.py +++ b/src/_pytest/unittest.py @@ -27,7 +27,7 @@ from _pytest.outcomes import skip from _pytest.outcomes import xfail from _pytest.python import Class from _pytest.python import Function -from _pytest.python import PyCollector +from _pytest.python import Module from _pytest.runner import CallInfo from _pytest.scope import Scope @@ -42,7 +42,7 @@ if TYPE_CHECKING: def pytest_pycollect_makeitem( - collector: PyCollector, name: str, obj: object + collector: Union[Module, Class], name: str, obj: object ) -> Optional["UnitTestCase"]: # Has unittest been imported and is obj a subclass of its TestCase? try: