From 222457d7d9898681bc3e035e8a65133503d13e04 Mon Sep 17 00:00:00 2001 From: sven <42868792+FreerGit@users.noreply.github.com> Date: Thu, 12 Sep 2024 19:33:16 +0200 Subject: [PATCH 01/12] Add `discover_imports` in conf - Allows a user to define whether or not pytest should treat imported (but not in testpaths) as test classes. - Imagine a class is called TestFoo defined in src/ dir, when discover_imports is disabled, TestFoo is not treated as a test class. --- AUTHORS | 1 + changelog/12749.feature.rst | 3 ++ src/_pytest/main.py | 5 ++ src/_pytest/python.py | 6 +++ testing/test_discover_imports.py | 88 ++++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+) create mode 100644 changelog/12749.feature.rst create mode 100644 testing/test_discover_imports.py diff --git a/AUTHORS b/AUTHORS index 374e6ad9bcc..b1dd40dbd4c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -400,6 +400,7 @@ Stefanie Molin Stefano Taschini Steffen Allner Stephan Obermann +Sven Sven-Hendrik Haase Sviatoslav Sydorenko Sylvain Marié diff --git a/changelog/12749.feature.rst b/changelog/12749.feature.rst new file mode 100644 index 00000000000..138a5bc7914 --- /dev/null +++ b/changelog/12749.feature.rst @@ -0,0 +1,3 @@ +Add :confval:`discover_imports`, when disabled (default) will make sure to not consider classes which are imported by a test file and starts with Test. + +-- by :user:`FreerGit` \ No newline at end of file diff --git a/src/_pytest/main.py b/src/_pytest/main.py index e5534e98d69..4887d336b2d 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -78,6 +78,11 @@ def pytest_addoption(parser: Parser) -> None: type="args", default=[], ) + parser.addini( + "discover_imports", + "Whether to discover tests in imported modules outside `testpaths`", + default=False, + ) group = parser.getgroup("general", "Running and selection options") group._addoption( "-x", diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9c54dd20f80..9467b26fd02 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -741,6 +741,12 @@ def newinstance(self): return self.obj() def collect(self) -> Iterable[nodes.Item | nodes.Collector]: + if self.config.getini("discover_imports") == ("false" or False): + paths = self.config.getini("testpaths") + class_file = inspect.getfile(self.obj) + if not any(string in class_file for string in paths): + return [] + if not safe_getattr(self.obj, "__test__", True): return [] if hasinit(self.obj): diff --git a/testing/test_discover_imports.py b/testing/test_discover_imports.py new file mode 100644 index 00000000000..829b614ed46 --- /dev/null +++ b/testing/test_discover_imports.py @@ -0,0 +1,88 @@ +import pytest +import textwrap + +def test_discover_imports_enabled(pytester): + src_dir = pytester.mkdir("src") + tests_dir = pytester.mkdir("tests") + pytester.makeini(""" + [pytest] + testpaths = "tests" + discover_imports = true + """) + + src_file = src_dir / "foo.py" + + src_file.write_text(textwrap.dedent("""\ + class TestClass(object): + def __init__(self): + super().__init__() + + def test_foobar(self): + return true + """ + ), encoding="utf-8") + + test_file = tests_dir / "foo_test.py" + test_file.write_text(textwrap.dedent("""\ + import sys + import os + + current_file = os.path.abspath(__file__) + current_dir = os.path.dirname(current_file) + parent_dir = os.path.abspath(os.path.join(current_dir, '..')) + sys.path.append(parent_dir) + + from src.foo import TestClass + + class TestDomain: + def test_testament(self): + testament = TestClass() + pass + """), encoding="utf-8") + + result = pytester.runpytest() + result.assert_outcomes(errors=1) + +def test_discover_imports_disabled(pytester): + + src_dir = pytester.mkdir("src") + tests_dir = pytester.mkdir("tests") + pytester.makeini(""" + [pytest] + testpaths = "tests" + discover_imports = false + """) + + src_file = src_dir / "foo.py" + + src_file.write_text(textwrap.dedent("""\ + class Testament(object): + def __init__(self): + super().__init__() + self.collections = ["stamp", "coin"] + + def personal_property(self): + return [f"my {x} collection" for x in self.collections] + """ + ), encoding="utf-8") + + test_file = tests_dir / "foo_test.py" + test_file.write_text(textwrap.dedent("""\ + import sys + import os + + current_file = os.path.abspath(__file__) + current_dir = os.path.dirname(current_file) + parent_dir = os.path.abspath(os.path.join(current_dir, '..')) + sys.path.append(parent_dir) + + from src.foo import Testament + + class TestDomain: + def test_testament(self): + testament = Testament() + assert testament.personal_property() + """), encoding="utf-8") + + result = pytester.runpytest() + result.assert_outcomes(passed=1) \ No newline at end of file From fa3b6310c8202e820cd0195960d14d10a1f99303 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 12 Sep 2024 17:38:20 +0000 Subject: [PATCH 02/12] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- changelog/12749.feature.rst | 2 +- testing/test_discover_imports.py | 38 +++++++++++++++++++++----------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/changelog/12749.feature.rst b/changelog/12749.feature.rst index 138a5bc7914..a6fb8bf9e62 100644 --- a/changelog/12749.feature.rst +++ b/changelog/12749.feature.rst @@ -1,3 +1,3 @@ Add :confval:`discover_imports`, when disabled (default) will make sure to not consider classes which are imported by a test file and starts with Test. --- by :user:`FreerGit` \ No newline at end of file +-- by :user:`FreerGit` diff --git a/testing/test_discover_imports.py b/testing/test_discover_imports.py index 829b614ed46..cdb55916ba6 100644 --- a/testing/test_discover_imports.py +++ b/testing/test_discover_imports.py @@ -1,6 +1,8 @@ -import pytest +from __future__ import annotations + import textwrap + def test_discover_imports_enabled(pytester): src_dir = pytester.mkdir("src") tests_dir = pytester.mkdir("tests") @@ -12,18 +14,21 @@ def test_discover_imports_enabled(pytester): src_file = src_dir / "foo.py" - src_file.write_text(textwrap.dedent("""\ + src_file.write_text( + textwrap.dedent("""\ class TestClass(object): def __init__(self): super().__init__() def test_foobar(self): return true - """ - ), encoding="utf-8") + """), + encoding="utf-8", + ) test_file = tests_dir / "foo_test.py" - test_file.write_text(textwrap.dedent("""\ + test_file.write_text( + textwrap.dedent("""\ import sys import os @@ -38,13 +43,15 @@ class TestDomain: def test_testament(self): testament = TestClass() pass - """), encoding="utf-8") + """), + encoding="utf-8", + ) result = pytester.runpytest() result.assert_outcomes(errors=1) + def test_discover_imports_disabled(pytester): - src_dir = pytester.mkdir("src") tests_dir = pytester.mkdir("tests") pytester.makeini(""" @@ -55,7 +62,8 @@ def test_discover_imports_disabled(pytester): src_file = src_dir / "foo.py" - src_file.write_text(textwrap.dedent("""\ + src_file.write_text( + textwrap.dedent("""\ class Testament(object): def __init__(self): super().__init__() @@ -63,11 +71,13 @@ def __init__(self): def personal_property(self): return [f"my {x} collection" for x in self.collections] - """ - ), encoding="utf-8") + """), + encoding="utf-8", + ) test_file = tests_dir / "foo_test.py" - test_file.write_text(textwrap.dedent("""\ + test_file.write_text( + textwrap.dedent("""\ import sys import os @@ -82,7 +92,9 @@ class TestDomain: def test_testament(self): testament = Testament() assert testament.personal_property() - """), encoding="utf-8") + """), + encoding="utf-8", + ) result = pytester.runpytest() - result.assert_outcomes(passed=1) \ No newline at end of file + result.assert_outcomes(passed=1) From 68ac4a12e362fedfec7b963739571c7c326b61da Mon Sep 17 00:00:00 2001 From: sven <42868792+FreerGit@users.noreply.github.com> Date: Tue, 24 Sep 2024 14:28:10 +0200 Subject: [PATCH 03/12] `collect_imported_tests` option --- changelog/12749.feature.rst | 4 +- src/_pytest/main.py | 30 ++++++- src/_pytest/python.py | 6 -- testing/test_discover_imports.py | 144 ++++++++++++++++++++++--------- 4 files changed, 133 insertions(+), 51 deletions(-) diff --git a/changelog/12749.feature.rst b/changelog/12749.feature.rst index 138a5bc7914..798d02c6e49 100644 --- a/changelog/12749.feature.rst +++ b/changelog/12749.feature.rst @@ -1,3 +1,3 @@ -Add :confval:`discover_imports`, when disabled (default) will make sure to not consider classes which are imported by a test file and starts with Test. +Add :confval:`collect_imported_tests`, when enabled (default is disabled) will make sure to not consider classes/functions which are imported by a test file and contains Test/test_*/*_test. --- by :user:`FreerGit` \ No newline at end of file +-- by :user:`FreerGit` diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 4887d336b2d..611ce033ea9 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -79,8 +79,9 @@ def pytest_addoption(parser: Parser) -> None: default=[], ) parser.addini( - "discover_imports", - "Whether to discover tests in imported modules outside `testpaths`", + "collect_imported_tests", + "Whether to collect tests in imported modules outside `testpaths`", + type="bool", default=False, ) group = parser.getgroup("general", "Running and selection options") @@ -963,9 +964,22 @@ def collect(self) -> Iterator[nodes.Item | nodes.Collector]: self.trace.root.indent -= 1 def genitems(self, node: nodes.Item | nodes.Collector) -> Iterator[nodes.Item]: + import inspect + + from _pytest.python import Class + from _pytest.python import Function + from _pytest.python import Module + self.trace("genitems", node) if isinstance(node, nodes.Item): node.ihook.pytest_itemcollected(item=node) + if self.config.getini("collect_imported_tests"): + if isinstance(node.parent, Module) and isinstance(node, Function): + if inspect.isfunction(node._getobj()): + fn_defined_at = node._getobj().__module__ + in_module = node.parent._getobj().__name__ + if fn_defined_at != in_module: + return yield node else: assert isinstance(node, nodes.Collector) @@ -973,6 +987,18 @@ def genitems(self, node: nodes.Item | nodes.Collector) -> Iterator[nodes.Item]: # For backward compat, dedup only applies to files. handle_dupes = not (keepduplicates and isinstance(node, nodes.File)) rep, duplicate = self._collect_one_node(node, handle_dupes) + + if self.config.getini("collect_imported_tests"): + for subnode in rep.result: + if isinstance(subnode, Class) and isinstance( + subnode.parent, Module + ): + if inspect.isclass(subnode._getobj()): + class_defined_at = subnode._getobj().__module__ + in_module = subnode.parent._getobj().__name__ + if class_defined_at != in_module: + rep.result.remove(subnode) + if duplicate and not keepduplicates: return if rep.passed: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9467b26fd02..9c54dd20f80 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -741,12 +741,6 @@ def newinstance(self): return self.obj() def collect(self) -> Iterable[nodes.Item | nodes.Collector]: - if self.config.getini("discover_imports") == ("false" or False): - paths = self.config.getini("testpaths") - class_file = inspect.getfile(self.obj) - if not any(string in class_file for string in paths): - return [] - if not safe_getattr(self.obj, "__test__", True): return [] if hasinit(self.obj): diff --git a/testing/test_discover_imports.py b/testing/test_discover_imports.py index 829b614ed46..e328d16a182 100644 --- a/testing/test_discover_imports.py +++ b/testing/test_discover_imports.py @@ -1,29 +1,31 @@ -import pytest +from __future__ import annotations + import textwrap -def test_discover_imports_enabled(pytester): +from _pytest.pytester import Pytester + + +def run_import_class_test(pytester: Pytester, passed: int = 0, errors: int = 0) -> None: src_dir = pytester.mkdir("src") tests_dir = pytester.mkdir("tests") - pytester.makeini(""" - [pytest] - testpaths = "tests" - discover_imports = true - """) - src_file = src_dir / "foo.py" - src_file.write_text(textwrap.dedent("""\ - class TestClass(object): + src_file.write_text( + textwrap.dedent("""\ + class Testament(object): def __init__(self): super().__init__() + self.collections = ["stamp", "coin"] - def test_foobar(self): - return true - """ - ), encoding="utf-8") + def personal_property(self): + return [f"my {x} collection" for x in self.collections] + """), + encoding="utf-8", + ) test_file = tests_dir / "foo_test.py" - test_file.write_text(textwrap.dedent("""\ + test_file.write_text( + textwrap.dedent("""\ import sys import os @@ -32,42 +34,78 @@ def test_foobar(self): parent_dir = os.path.abspath(os.path.join(current_dir, '..')) sys.path.append(parent_dir) - from src.foo import TestClass + from src.foo import Testament class TestDomain: def test_testament(self): - testament = TestClass() - pass - """), encoding="utf-8") + testament = Testament() + assert testament.personal_property() + """), + encoding="utf-8", + ) result = pytester.runpytest() - result.assert_outcomes(errors=1) + result.assert_outcomes(passed=passed, errors=errors) -def test_discover_imports_disabled(pytester): - - src_dir = pytester.mkdir("src") - tests_dir = pytester.mkdir("tests") + +def test_collect_imports_disabled(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + testpaths = "tests" + collect_imported_tests = false + """) + + run_import_class_test(pytester, errors=1) + + +def test_collect_imports_default(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + testpaths = "tests" + """) + + run_import_class_test(pytester, errors=1) + + +def test_collect_imports_enabled(pytester: Pytester) -> None: pytester.makeini(""" [pytest] testpaths = "tests" - discover_imports = false + collect_imported_tests = true """) + run_import_class_test(pytester, passed=1) + + +def run_import_functions_test( + pytester: Pytester, passed: int, errors: int, failed: int +) -> None: + src_dir = pytester.mkdir("src") + tests_dir = pytester.mkdir("tests") + src_file = src_dir / "foo.py" - src_file.write_text(textwrap.dedent("""\ - class Testament(object): - def __init__(self): - super().__init__() - self.collections = ["stamp", "coin"] + # Note that these "tests" are should _not_ be treated as tests. + # They are normal functions that happens to have test_* or *_test in the name. + # Thus should _not_ be collected! + src_file.write_text( + textwrap.dedent("""\ + def test_function(): + some_random_computation = 5 + return some_random_computation - def personal_property(self): - return [f"my {x} collection" for x in self.collections] - """ - ), encoding="utf-8") + def test_bar(): + pass + """), + encoding="utf-8", + ) test_file = tests_dir / "foo_test.py" - test_file.write_text(textwrap.dedent("""\ + + # Inferred from the comment above, this means that there is _only_ one actual test + # which should result in only 1 passing test being ran. + test_file.write_text( + textwrap.dedent("""\ import sys import os @@ -76,13 +114,37 @@ def personal_property(self): parent_dir = os.path.abspath(os.path.join(current_dir, '..')) sys.path.append(parent_dir) - from src.foo import Testament + from src.foo import * class TestDomain: - def test_testament(self): - testament = Testament() - assert testament.personal_property() - """), encoding="utf-8") + def test_important(self): + res = test_function() + if res == 5: + pass + + """), + encoding="utf-8", + ) result = pytester.runpytest() - result.assert_outcomes(passed=1) \ No newline at end of file + result.assert_outcomes(passed=passed, errors=errors, failed=failed) + + +def test_collect_function_imports_enabled(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + testpaths = "tests" + collect_imported_tests = true + """) + + run_import_functions_test(pytester, passed=1, errors=0, failed=0) + + +def test_collect_function_imports_disabled(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + testpaths = "tests" + collect_imported_tests = false + """) + + run_import_functions_test(pytester, passed=2, errors=0, failed=1) From eb8592c526b3d7967573d816efc5b35fc88be2a8 Mon Sep 17 00:00:00 2001 From: sven <42868792+FreerGit@users.noreply.github.com> Date: Tue, 1 Oct 2024 18:41:30 +0200 Subject: [PATCH 04/12] update default and add docs --- changelog/12749.feature.rst | 4 +++- doc/en/reference/reference.rst | 10 +++++++++- src/_pytest/main.py | 6 +++--- testing/test_collect_imports.py | 26 ++++++++++++++++++++++---- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/changelog/12749.feature.rst b/changelog/12749.feature.rst index 798d02c6e49..cda0db6c930 100644 --- a/changelog/12749.feature.rst +++ b/changelog/12749.feature.rst @@ -1,3 +1,5 @@ -Add :confval:`collect_imported_tests`, when enabled (default is disabled) will make sure to not consider classes/functions which are imported by a test file and contains Test/test_*/*_test. +New :confval:`collect_imported_tests`: when enabled (the default) pytest will collect classes/functions in test modules even if they are imported from another file. + +Setting this to False will make pytest collect classes/functions from test files only if they are defined in that file (as opposed to imported there). -- by :user:`FreerGit` diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index f7dfb3ffa71..99b1a19c3a4 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -1839,9 +1839,17 @@ passed multiple times. The expected format is ``name=value``. For example:: pytest testing doc -.. confval:: tmp_path_retention_count +.. confval:: collect_imported_tests + + Setting this to `false` will make pytest collect classes/functions from test + files only if they are defined in that file (as opposed to imported there). + + .. code-block:: ini + [pytest] + collect_imported_tests = false +.. confval:: tmp_path_retention_count How many sessions should we keep the `tmp_path` directories, according to `tmp_path_retention_policy`. diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 611ce033ea9..e40f09a9acb 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -82,7 +82,7 @@ def pytest_addoption(parser: Parser) -> None: "collect_imported_tests", "Whether to collect tests in imported modules outside `testpaths`", type="bool", - default=False, + default=True, ) group = parser.getgroup("general", "Running and selection options") group._addoption( @@ -973,7 +973,7 @@ def genitems(self, node: nodes.Item | nodes.Collector) -> Iterator[nodes.Item]: self.trace("genitems", node) if isinstance(node, nodes.Item): node.ihook.pytest_itemcollected(item=node) - if self.config.getini("collect_imported_tests"): + if not self.config.getini("collect_imported_tests"): if isinstance(node.parent, Module) and isinstance(node, Function): if inspect.isfunction(node._getobj()): fn_defined_at = node._getobj().__module__ @@ -988,7 +988,7 @@ def genitems(self, node: nodes.Item | nodes.Collector) -> Iterator[nodes.Item]: handle_dupes = not (keepduplicates and isinstance(node, nodes.File)) rep, duplicate = self._collect_one_node(node, handle_dupes) - if self.config.getini("collect_imported_tests"): + if not self.config.getini("collect_imported_tests"): for subnode in rep.result: if isinstance(subnode, Class) and isinstance( subnode.parent, Module diff --git a/testing/test_collect_imports.py b/testing/test_collect_imports.py index e328d16a182..7922058721c 100644 --- a/testing/test_collect_imports.py +++ b/testing/test_collect_imports.py @@ -55,7 +55,7 @@ def test_collect_imports_disabled(pytester: Pytester) -> None: collect_imported_tests = false """) - run_import_class_test(pytester, errors=1) + run_import_class_test(pytester, passed=1) def test_collect_imports_default(pytester: Pytester) -> None: @@ -74,7 +74,7 @@ def test_collect_imports_enabled(pytester: Pytester) -> None: collect_imported_tests = true """) - run_import_class_test(pytester, passed=1) + run_import_class_test(pytester, errors=1) def run_import_functions_test( @@ -137,14 +137,32 @@ def test_collect_function_imports_enabled(pytester: Pytester) -> None: collect_imported_tests = true """) - run_import_functions_test(pytester, passed=1, errors=0, failed=0) + run_import_functions_test(pytester, passed=2, errors=0, failed=1) def test_collect_function_imports_disabled(pytester: Pytester) -> None: pytester.makeini(""" [pytest] - testpaths = "tests" + # testpaths = "tests" collect_imported_tests = false """) + run_import_functions_test(pytester, passed=1, errors=0, failed=0) + + +def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + collect_imported_tests = false + """) + + run_import_functions_test(pytester, passed=1, errors=0, failed=0) + + +def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + collect_imported_tests = true + """) + run_import_functions_test(pytester, passed=2, errors=0, failed=1) From 935c06de09df95c48e867fa70851b9a2acd9fcf7 Mon Sep 17 00:00:00 2001 From: sven <42868792+FreerGit@users.noreply.github.com> Date: Mon, 4 Nov 2024 02:02:18 +0100 Subject: [PATCH 05/12] WIP: don't collect instead of filtering out --- doc/en/reference/reference.rst | 25 +++++---- src/_pytest/main.py | 25 --------- src/_pytest/python.py | 19 +++++++ testing/test_collect_imports.py | 99 +++++++++++++++++++++++++++------ 4 files changed, 115 insertions(+), 53 deletions(-) diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index 99b1a19c3a4..53f9470f756 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -1301,6 +1301,20 @@ passed multiple times. The expected format is ``name=value``. For example:: variables, that will be expanded. For more information about cache plugin please refer to :ref:`cache_provider`. +.. confval:: collect_imported_tests + + .. versionadded:: 8.4 + + Setting this to ``false`` will make pytest collect classes/functions from test + files only if they are defined in that file (as opposed to imported there). + + .. code-block:: ini + + [pytest] + collect_imported_tests = false + + Default: ``true`` + .. confval:: consider_namespace_packages Controls if pytest should attempt to identify `namespace packages `__ @@ -1838,17 +1852,6 @@ passed multiple times. The expected format is ``name=value``. For example:: pytest testing doc - -.. confval:: collect_imported_tests - - Setting this to `false` will make pytest collect classes/functions from test - files only if they are defined in that file (as opposed to imported there). - - .. code-block:: ini - - [pytest] - collect_imported_tests = false - .. confval:: tmp_path_retention_count How many sessions should we keep the `tmp_path` directories, diff --git a/src/_pytest/main.py b/src/_pytest/main.py index e40f09a9acb..41063a9bc18 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -964,22 +964,9 @@ def collect(self) -> Iterator[nodes.Item | nodes.Collector]: self.trace.root.indent -= 1 def genitems(self, node: nodes.Item | nodes.Collector) -> Iterator[nodes.Item]: - import inspect - - from _pytest.python import Class - from _pytest.python import Function - from _pytest.python import Module - self.trace("genitems", node) if isinstance(node, nodes.Item): node.ihook.pytest_itemcollected(item=node) - if not self.config.getini("collect_imported_tests"): - if isinstance(node.parent, Module) and isinstance(node, Function): - if inspect.isfunction(node._getobj()): - fn_defined_at = node._getobj().__module__ - in_module = node.parent._getobj().__name__ - if fn_defined_at != in_module: - return yield node else: assert isinstance(node, nodes.Collector) @@ -987,18 +974,6 @@ def genitems(self, node: nodes.Item | nodes.Collector) -> Iterator[nodes.Item]: # For backward compat, dedup only applies to files. handle_dupes = not (keepduplicates and isinstance(node, nodes.File)) rep, duplicate = self._collect_one_node(node, handle_dupes) - - if not self.config.getini("collect_imported_tests"): - for subnode in rep.result: - if isinstance(subnode, Class) and isinstance( - subnode.parent, Module - ): - if inspect.isclass(subnode._getobj()): - class_defined_at = subnode._getobj().__module__ - in_module = subnode.parent._getobj().__name__ - if class_defined_at != in_module: - rep.result.remove(subnode) - if duplicate and not keepduplicates: return if rep.passed: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9c54dd20f80..9c00252c5f8 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -416,6 +416,15 @@ def collect(self) -> Iterable[nodes.Item | nodes.Collector]: if name in seen: continue seen.add(name) + + if not self.session.config.getini("collect_imported_tests"): + # Do not collect imported functions + if inspect.isfunction(obj) and isinstance(self, Module): + fn_defined_at = obj.__module__ + in_module = self._getobj().__name__ + if fn_defined_at != in_module: + continue + res = ihook.pytest_pycollect_makeitem( collector=self, name=name, obj=obj ) @@ -741,6 +750,16 @@ def newinstance(self): return self.obj() def collect(self) -> Iterable[nodes.Item | nodes.Collector]: + if not self.config.getini("collect_imported_tests"): + # This entire branch will discard (not collect) a class + # if it is imported (defined in a different module) + if isinstance(self, Class) and isinstance(self.parent, Module): + if inspect.isclass(self._getobj()): + class_defined_at = self._getobj().__module__ + in_module = self.parent._getobj().__name__ + if class_defined_at != in_module: + return [] + if not safe_getattr(self.obj, "__test__", True): return [] if hasinit(self.obj): diff --git a/testing/test_collect_imports.py b/testing/test_collect_imports.py index 7922058721c..fe1e6b3e863 100644 --- a/testing/test_collect_imports.py +++ b/testing/test_collect_imports.py @@ -5,6 +5,9 @@ from _pytest.pytester import Pytester +# Start of tests for classes + + def run_import_class_test(pytester: Pytester, passed: int = 0, errors: int = 0) -> None: src_dir = pytester.mkdir("src") tests_dir = pytester.mkdir("tests") @@ -57,26 +60,37 @@ def test_collect_imports_disabled(pytester: Pytester) -> None: run_import_class_test(pytester, passed=1) + # Verify that the state of hooks + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + assert len(items_collected) == 1 + for x in items_collected: + assert x.item._getobj().__name__ == "test_testament" -def test_collect_imports_default(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - testpaths = "tests" - """) +def test_collect_imports_default(pytester: Pytester) -> None: run_import_class_test(pytester, errors=1) + # TODO, hooks + def test_collect_imports_enabled(pytester: Pytester) -> None: pytester.makeini(""" [pytest] - testpaths = "tests" collect_imported_tests = true """) run_import_class_test(pytester, errors=1) +# # TODO, hooks + + +# End of tests for classes +################################# +# Start of tests for functions + + def run_import_functions_test( pytester: Pytester, passed: int, errors: int, failed: int ) -> None: @@ -85,8 +99,8 @@ def run_import_functions_test( src_file = src_dir / "foo.py" - # Note that these "tests" are should _not_ be treated as tests. - # They are normal functions that happens to have test_* or *_test in the name. + # Note that these "tests" should _not_ be treated as tests if `collect_imported_tests = false` + # They are normal functions in that case, that happens to have test_* or *_test in the name. # Thus should _not_ be collected! src_file.write_text( textwrap.dedent("""\ @@ -121,7 +135,6 @@ def test_important(self): res = test_function() if res == 5: pass - """), encoding="utf-8", ) @@ -138,31 +151,83 @@ def test_collect_function_imports_enabled(pytester: Pytester) -> None: """) run_import_functions_test(pytester, passed=2, errors=0, failed=1) + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + # Recall that the default is `collect_imported_tests = true`. + # Which means that the normal functions are now interpreted as + # valid tests and `test_function()` will fail. + assert len(items_collected) == 3 + for x in items_collected: + assert x.item._getobj().__name__ in [ + "test_important", + "test_bar", + "test_function", + ] -def test_collect_function_imports_disabled(pytester: Pytester) -> None: +def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: + # Make sure `collect_imported_tests` has no dependence on `testpaths` pytester.makeini(""" [pytest] - # testpaths = "tests" collect_imported_tests = false """) run_import_functions_test(pytester, passed=1, errors=0, failed=0) + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + assert len(items_collected) == 1 + for x in items_collected: + assert x.item._getobj().__name__ == "test_important" -def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: +def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: + # Make sure `collect_imported_tests` has no dependence on `testpaths` pytester.makeini(""" [pytest] - collect_imported_tests = false + collect_imported_tests = true """) - run_import_functions_test(pytester, passed=1, errors=0, failed=0) + run_import_functions_test(pytester, passed=2, errors=0, failed=1) + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + assert len(items_collected) == 3 -def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: +def test_hook_behaviour_when_collect_off(pytester: Pytester) -> None: pytester.makeini(""" [pytest] - collect_imported_tests = true + collect_imported_tests = false """) - run_import_functions_test(pytester, passed=2, errors=0, failed=1) + run_import_functions_test(pytester, passed=1, errors=0, failed=0) + reprec = pytester.inline_run() + + # reports = reprec.getreports("pytest_collectreport") + items_collected = reprec.getcalls("pytest_itemcollected") + modified = reprec.getcalls("pytest_collection_modifyitems") + + # print("Reports: ----------------") + # print(reports) + # for r in reports: + # print(r) + + # TODO this is want I want, I think.... + # + # + # + # + # + + # TODO + # assert(reports.outcome == "passed") + # assert(len(reports.result) == 1) + + # print("Items collected: ----------------") + # print(items_collected) + # print("Modified : ----------------") + + assert len(items_collected) == 1 + for x in items_collected: + assert x.item._getobj().__name__ == "test_important" + + assert len(modified) == 1 From 191456e00d89c2c814f8659070eb8863298996be Mon Sep 17 00:00:00 2001 From: sven <42868792+FreerGit@users.noreply.github.com> Date: Tue, 12 Nov 2024 20:00:47 +0100 Subject: [PATCH 06/12] WIP: modified items, reports and items collected - pytest_collectreport hook currently gets more reports (TODO) - `pytest_collection_modifyitems` and `pytest_itemcollected` is correct. --- testing/test_collect_imports.py | 477 +++++++++++++++++++------------- 1 file changed, 278 insertions(+), 199 deletions(-) diff --git a/testing/test_collect_imports.py b/testing/test_collect_imports.py index fe1e6b3e863..cf8ff9cb983 100644 --- a/testing/test_collect_imports.py +++ b/testing/test_collect_imports.py @@ -1,88 +1,89 @@ from __future__ import annotations import textwrap +from typing import Any +from _pytest.fixtures import FixtureRequest +from _pytest.main import Session from _pytest.pytester import Pytester +from _pytest.pytester import RecordedHookCall +from _pytest.pytester import RunResult +import pytest # Start of tests for classes +# def run_import_class_test(pytester: Pytester, passed: int = 0, errors: int = 0) -> None: +# src_dir = pytester.mkdir("src") +# tests_dir = pytester.mkdir("tests") +# src_file = src_dir / "foo.py" + +# src_file.write_text( +# textwrap.dedent("""\ +# class Testament(object): +# def __init__(self): +# super().__init__() +# self.collections = ["stamp", "coin"] + +# def personal_property(self): +# return [f"my {x} collection" for x in self.collections] +# """), +# encoding="utf-8", +# ) + +# test_file = tests_dir / "foo_test.py" +# test_file.write_text( +# textwrap.dedent("""\ +# import sys +# import os + +# current_file = os.path.abspath(__file__) +# current_dir = os.path.dirname(current_file) +# parent_dir = os.path.abspath(os.path.join(current_dir, '..')) +# sys.path.append(parent_dir) + +# from src.foo import Testament + +# class TestDomain: +# def test_testament(self): +# testament = Testament() +# assert testament.personal_property() +# """), +# encoding="utf-8", +# ) + +# result = pytester.runpytest() +# result.assert_outcomes(passed=passed, errors=errors) + +# def test_collect_imports_disabled(pytester: Pytester) -> None: +# pytester.makeini(""" +# [pytest] +# testpaths = "tests" +# collect_imported_tests = false +# """) + +# run_import_class_test(pytester, passed=1) + +# # Verify that the state of hooks +# reprec = pytester.inline_run() +# items_collected = reprec.getcalls("pytest_itemcollected") +# assert len(items_collected) == 1 +# for x in items_collected: +# assert x.item._getobj().__name__ == "test_testament" + +# def test_collect_imports_default(pytester: Pytester) -> None: +# run_import_class_test(pytester, errors=1) -def run_import_class_test(pytester: Pytester, passed: int = 0, errors: int = 0) -> None: - src_dir = pytester.mkdir("src") - tests_dir = pytester.mkdir("tests") - src_file = src_dir / "foo.py" - - src_file.write_text( - textwrap.dedent("""\ - class Testament(object): - def __init__(self): - super().__init__() - self.collections = ["stamp", "coin"] - - def personal_property(self): - return [f"my {x} collection" for x in self.collections] - """), - encoding="utf-8", - ) - - test_file = tests_dir / "foo_test.py" - test_file.write_text( - textwrap.dedent("""\ - import sys - import os - - current_file = os.path.abspath(__file__) - current_dir = os.path.dirname(current_file) - parent_dir = os.path.abspath(os.path.join(current_dir, '..')) - sys.path.append(parent_dir) - - from src.foo import Testament - - class TestDomain: - def test_testament(self): - testament = Testament() - assert testament.personal_property() - """), - encoding="utf-8", - ) - - result = pytester.runpytest() - result.assert_outcomes(passed=passed, errors=errors) - - -def test_collect_imports_disabled(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - testpaths = "tests" - collect_imported_tests = false - """) - - run_import_class_test(pytester, passed=1) - - # Verify that the state of hooks - reprec = pytester.inline_run() - items_collected = reprec.getcalls("pytest_itemcollected") - assert len(items_collected) == 1 - for x in items_collected: - assert x.item._getobj().__name__ == "test_testament" - - -def test_collect_imports_default(pytester: Pytester) -> None: - run_import_class_test(pytester, errors=1) - - # TODO, hooks - - -def test_collect_imports_enabled(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - collect_imported_tests = true - """) +# # TODO, hooks - run_import_class_test(pytester, errors=1) +# def test_collect_imports_enabled(pytester: Pytester) -> None: +# pytester.makeini(""" +# [pytest] +# collect_imported_tests = true +# """) +# run_import_class_test(pytester, errors=1) # # TODO, hooks @@ -93,141 +94,219 @@ def test_collect_imports_enabled(pytester: Pytester) -> None: def run_import_functions_test( pytester: Pytester, passed: int, errors: int, failed: int -) -> None: - src_dir = pytester.mkdir("src") - tests_dir = pytester.mkdir("tests") - - src_file = src_dir / "foo.py" - +) -> RunResult: # Note that these "tests" should _not_ be treated as tests if `collect_imported_tests = false` # They are normal functions in that case, that happens to have test_* or *_test in the name. # Thus should _not_ be collected! - src_file.write_text( - textwrap.dedent("""\ - def test_function(): - some_random_computation = 5 - return some_random_computation - - def test_bar(): - pass - """), - encoding="utf-8", + pytester.makepyfile( + **{ + "src/foo.py": textwrap.dedent( + """\ + def test_function(): + some_random_computation = 5 + return some_random_computation + + def test_bar(): + pass + """ + ) + } ) - test_file = tests_dir / "foo_test.py" - # Inferred from the comment above, this means that there is _only_ one actual test # which should result in only 1 passing test being ran. - test_file.write_text( - textwrap.dedent("""\ - import sys - import os - - current_file = os.path.abspath(__file__) - current_dir = os.path.dirname(current_file) - parent_dir = os.path.abspath(os.path.join(current_dir, '..')) - sys.path.append(parent_dir) - - from src.foo import * - - class TestDomain: - def test_important(self): - res = test_function() - if res == 5: - pass - """), - encoding="utf-8", + pytester.makepyfile( + **{ + "tests/foo_test.py": textwrap.dedent( + """\ + import sys + import os + + current_file = os.path.abspath(__file__) + current_dir = os.path.dirname(current_file) + parent_dir = os.path.abspath(os.path.join(current_dir, '..')) + sys.path.append(parent_dir) + + from src.foo import * + + class TestDomain: + def test_important(self): + res = test_function() + if res == 5: + pass + """ + ) + } ) result = pytester.runpytest() result.assert_outcomes(passed=passed, errors=errors, failed=failed) - - -def test_collect_function_imports_enabled(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - testpaths = "tests" - collect_imported_tests = true - """) - - run_import_functions_test(pytester, passed=2, errors=0, failed=1) - reprec = pytester.inline_run() - items_collected = reprec.getcalls("pytest_itemcollected") - # Recall that the default is `collect_imported_tests = true`. - # Which means that the normal functions are now interpreted as - # valid tests and `test_function()` will fail. - assert len(items_collected) == 3 - for x in items_collected: - assert x.item._getobj().__name__ in [ - "test_important", - "test_bar", - "test_function", - ] - - -def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: - # Make sure `collect_imported_tests` has no dependence on `testpaths` - pytester.makeini(""" - [pytest] - collect_imported_tests = false - """) - - run_import_functions_test(pytester, passed=1, errors=0, failed=0) - reprec = pytester.inline_run() - items_collected = reprec.getcalls("pytest_itemcollected") - assert len(items_collected) == 1 - for x in items_collected: - assert x.item._getobj().__name__ == "test_important" - - -def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: - # Make sure `collect_imported_tests` has no dependence on `testpaths` - pytester.makeini(""" - [pytest] - collect_imported_tests = true - """) - - run_import_functions_test(pytester, passed=2, errors=0, failed=1) - reprec = pytester.inline_run() - items_collected = reprec.getcalls("pytest_itemcollected") - assert len(items_collected) == 3 - - -def test_hook_behaviour_when_collect_off(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - collect_imported_tests = false - """) - - run_import_functions_test(pytester, passed=1, errors=0, failed=0) - reprec = pytester.inline_run() - - # reports = reprec.getreports("pytest_collectreport") - items_collected = reprec.getcalls("pytest_itemcollected") - modified = reprec.getcalls("pytest_collection_modifyitems") - - # print("Reports: ----------------") - # print(reports) - # for r in reports: - # print(r) - - # TODO this is want I want, I think.... - # - # - # - # - # - - # TODO - # assert(reports.outcome == "passed") - # assert(len(reports.result) == 1) - - # print("Items collected: ----------------") - # print(items_collected) - # print("Modified : ----------------") - - assert len(items_collected) == 1 - for x in items_collected: - assert x.item._getobj().__name__ == "test_important" - - assert len(modified) == 1 + return result + + +# def test_collect_function_imports_enabled(pytester: Pytester) -> None: +# pytester.makeini(""" +# [pytest] +# testpaths = "tests" +# collect_imported_tests = true +# """) + +# run_import_functions_test(pytester, passed=2, errors=0, failed=1) +# reprec = pytester.inline_run() +# items_collected = reprec.getcalls("pytest_itemcollected") +# # Recall that the default is `collect_imported_tests = true`. +# # Which means that the normal functions are now interpreted as +# # valid tests and `test_function()` will fail. +# assert len(items_collected) == 3 +# for x in items_collected: +# assert x.item._getobj().__name__ in [ +# "test_important", +# "test_bar", +# "test_function", +# ] + + +# def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: +# # Make sure `collect_imported_tests` has no dependence on `testpaths` +# pytester.makeini(""" +# [pytest] +# collect_imported_tests = false +# """) + +# run_import_functions_test(pytester, passed=1, errors=0, failed=0) +# reprec = pytester.inline_run() +# items_collected = reprec.getcalls("pytest_itemcollected") +# assert len(items_collected) == 1 +# for x in items_collected: +# assert x.item._getobj().__name__ == "test_important" + + +# def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: +# # Make sure `collect_imported_tests` has no dependence on `testpaths` +# pytester.makeini(""" +# [pytest] +# collect_imported_tests = true +# """) + +# run_import_functions_test(pytester, passed=2, errors=0, failed=1) +# reprec = pytester.inline_run() +# items_collected = reprec.getcalls("pytest_itemcollected") +# assert len(items_collected) == 3 + + +class TestHookBehaviour: + collect_outcomes: dict[str, Any] = {} + + @pytest.mark.parametrize("step", [1, 2, 3]) + def test_hook_behaviour(self, pytester: Pytester, step: int) -> None: + if step == 1: + self._test_hook_default_behaviour(pytester) + elif step == 2: + self._test_hook_behaviour_when_collect_off(pytester) + elif step == 3: + self._test_hook_behaviour() + + @pytest.fixture(scope="class", autouse=True) + def setup_collect_outcomes(self, request: FixtureRequest) -> None: + request.cls.collect_outcomes = {} + + def _test_hook_default_behaviour(self, pytester: Pytester) -> None: + pytester.makepyfile( + **{ + "tests/foo_test.py": textwrap.dedent( + """\ + class TestDomain: + def test_important(self): + pass + """ + ) + } + ) + + result = pytester.runpytest() + result.assert_outcomes(passed=1) + reprec = pytester.inline_run() + reports = reprec.getreports("pytest_collectreport") + modified = reprec.getcalls("pytest_collection_modifyitems") + items_collected = reprec.getcalls("pytest_itemcollected") + + self.collect_outcomes["default"] = { + "result": result.parseoutcomes(), + "modified": modified, + "items_collected": items_collected, + "reports": reports, + } + + def _test_hook_behaviour_when_collect_off(self, pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + collect_imported_tests = false + """) + res = run_import_functions_test(pytester, passed=1, errors=0, failed=0) + reprec = pytester.inline_run() + reports = reprec.getreports("pytest_collectreport") + modified = reprec.getcalls("pytest_collection_modifyitems") + items_collected = reprec.getcalls("pytest_itemcollected") + + self.collect_outcomes["collect_off"] = { + "result": res.parseoutcomes(), + "modified": modified, + "items_collected": items_collected, + "reports": reports, + } + + # Now check that the two tests above did indeed result in the same outcome. + def _test_hook_behaviour(self) -> None: + print("ABCD", self.collect_outcomes) + default = self.collect_outcomes["default"] + collect_off = self.collect_outcomes["collect_off"] + assert default["result"] == collect_off["result"] + + assert len(default["modified"]) == len(collect_off["modified"]) == 1 + + def_modified_record: RecordedHookCall = default["modified"][0] + off_modified_record: RecordedHookCall = collect_off["modified"][0] + def_sess: Session = def_modified_record.__dict__["session"] + off_sess: Session = off_modified_record.__dict__["session"] + + assert def_sess.exitstatus == off_sess.exitstatus + assert def_sess.testsfailed == off_sess.testsfailed + assert def_sess.testscollected == off_sess.testscollected + + def_items = def_modified_record.__dict__["items"] + off_items = off_modified_record.__dict__["items"] + assert len(def_items) == len(off_items) == 1 + assert def_items[0].name == off_items[0].name + + assert ( + len(default["items_collected"]) == len(collect_off["items_collected"]) == 1 + ) + + def_items_record: RecordedHookCall = default["items_collected"][0] + off_items_record: RecordedHookCall = collect_off["items_collected"][0] + def_items = def_items_record.__dict__["item"] + off_items = off_items_record.__dict__["item"] + assert def_items.name == off_items.name + + # TODO: fix diff: + # [ + # , + # - , + # , + # , + # , + # - , + # ? ^ + # + , + # ? ^ + # ] + + # assert len(default['reports']) == len(collect_off['reports']) + # for i in range(len(default['reports'])): + # print("def",default['reports'][i].__dict__) + # print("off",collect_off['reports'][i].__dict__) + + # from pprint import pprint + # pprint(default['reports']) + # pprint(collect_off['reports']) + # assert default['reports'] == collect_off['reports'] From f1821eaa13867864ab9048b217ccbaa77e8e781b Mon Sep 17 00:00:00 2001 From: sven <42868792+FreerGit@users.noreply.github.com> Date: Sun, 17 Nov 2024 20:57:38 +0100 Subject: [PATCH 07/12] WIP - report collection needs triage --- testing/test_collect_imports.py | 278 +++++++++++++++++--------------- 1 file changed, 148 insertions(+), 130 deletions(-) diff --git a/testing/test_collect_imports.py b/testing/test_collect_imports.py index cf8ff9cb983..cc1c5a80015 100644 --- a/testing/test_collect_imports.py +++ b/testing/test_collect_imports.py @@ -13,78 +13,82 @@ # Start of tests for classes -# def run_import_class_test(pytester: Pytester, passed: int = 0, errors: int = 0) -> None: -# src_dir = pytester.mkdir("src") -# tests_dir = pytester.mkdir("tests") -# src_file = src_dir / "foo.py" - -# src_file.write_text( -# textwrap.dedent("""\ -# class Testament(object): -# def __init__(self): -# super().__init__() -# self.collections = ["stamp", "coin"] - -# def personal_property(self): -# return [f"my {x} collection" for x in self.collections] -# """), -# encoding="utf-8", -# ) - -# test_file = tests_dir / "foo_test.py" -# test_file.write_text( -# textwrap.dedent("""\ -# import sys -# import os - -# current_file = os.path.abspath(__file__) -# current_dir = os.path.dirname(current_file) -# parent_dir = os.path.abspath(os.path.join(current_dir, '..')) -# sys.path.append(parent_dir) - -# from src.foo import Testament - -# class TestDomain: -# def test_testament(self): -# testament = Testament() -# assert testament.personal_property() -# """), -# encoding="utf-8", -# ) - -# result = pytester.runpytest() -# result.assert_outcomes(passed=passed, errors=errors) - -# def test_collect_imports_disabled(pytester: Pytester) -> None: -# pytester.makeini(""" -# [pytest] -# testpaths = "tests" -# collect_imported_tests = false -# """) - -# run_import_class_test(pytester, passed=1) - -# # Verify that the state of hooks -# reprec = pytester.inline_run() -# items_collected = reprec.getcalls("pytest_itemcollected") -# assert len(items_collected) == 1 -# for x in items_collected: -# assert x.item._getobj().__name__ == "test_testament" - -# def test_collect_imports_default(pytester: Pytester) -> None: -# run_import_class_test(pytester, errors=1) - -# # TODO, hooks - - -# def test_collect_imports_enabled(pytester: Pytester) -> None: -# pytester.makeini(""" -# [pytest] -# collect_imported_tests = true -# """) - -# run_import_class_test(pytester, errors=1) -# # TODO, hooks + +def run_import_class_test( + pytester: Pytester, passed: int = 0, errors: int = 0 +) -> RunResult: + src_dir = pytester.mkdir("src") + tests_dir = pytester.mkdir("tests") + src_file = src_dir / "foo.py" + + src_file.write_text( + textwrap.dedent("""\ + class Testament(object): + def __init__(self): + super().__init__() + self.collections = ["stamp", "coin"] + + def personal_property(self): + return [f"my {x} collection" for x in self.collections] + """), + encoding="utf-8", + ) + + test_file = tests_dir / "foo_test.py" + test_file.write_text( + textwrap.dedent("""\ + import sys + import os + + current_file = os.path.abspath(__file__) + current_dir = os.path.dirname(current_file) + parent_dir = os.path.abspath(os.path.join(current_dir, '..')) + sys.path.append(parent_dir) + + from src.foo import Testament + + class TestDomain: + def test_testament(self): + testament = Testament() + assert testament.personal_property() + """), + encoding="utf-8", + ) + + result = pytester.runpytest() + result.assert_outcomes(passed=passed, errors=errors) + return result + + +def test_collect_imports_disabled(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + testpaths = "tests" + collect_imported_tests = false + """) + + run_import_class_test(pytester, passed=1) + + # Verify that the state of hooks + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + assert len(items_collected) == 1 + for x in items_collected: + assert x.item._getobj().__name__ == "test_testament" + + +def test_collect_imports_default(pytester: Pytester) -> None: + run_import_class_test(pytester, errors=1) + # TODO + + +def test_collect_imports_enabled(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + collect_imported_tests = true + """) + + run_import_class_test(pytester, errors=1) # End of tests for classes @@ -144,54 +148,54 @@ def test_important(self): return result -# def test_collect_function_imports_enabled(pytester: Pytester) -> None: -# pytester.makeini(""" -# [pytest] -# testpaths = "tests" -# collect_imported_tests = true -# """) - -# run_import_functions_test(pytester, passed=2, errors=0, failed=1) -# reprec = pytester.inline_run() -# items_collected = reprec.getcalls("pytest_itemcollected") -# # Recall that the default is `collect_imported_tests = true`. -# # Which means that the normal functions are now interpreted as -# # valid tests and `test_function()` will fail. -# assert len(items_collected) == 3 -# for x in items_collected: -# assert x.item._getobj().__name__ in [ -# "test_important", -# "test_bar", -# "test_function", -# ] - - -# def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: -# # Make sure `collect_imported_tests` has no dependence on `testpaths` -# pytester.makeini(""" -# [pytest] -# collect_imported_tests = false -# """) - -# run_import_functions_test(pytester, passed=1, errors=0, failed=0) -# reprec = pytester.inline_run() -# items_collected = reprec.getcalls("pytest_itemcollected") -# assert len(items_collected) == 1 -# for x in items_collected: -# assert x.item._getobj().__name__ == "test_important" - - -# def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: -# # Make sure `collect_imported_tests` has no dependence on `testpaths` -# pytester.makeini(""" -# [pytest] -# collect_imported_tests = true -# """) - -# run_import_functions_test(pytester, passed=2, errors=0, failed=1) -# reprec = pytester.inline_run() -# items_collected = reprec.getcalls("pytest_itemcollected") -# assert len(items_collected) == 3 +def test_collect_function_imports_enabled(pytester: Pytester) -> None: + pytester.makeini(""" + [pytest] + testpaths = "tests" + collect_imported_tests = true + """) + + run_import_functions_test(pytester, passed=2, errors=0, failed=1) + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + # Recall that the default is `collect_imported_tests = true`. + # Which means that the normal functions are now interpreted as + # valid tests and `test_function()` will fail. + assert len(items_collected) == 3 + for x in items_collected: + assert x.item._getobj().__name__ in [ + "test_important", + "test_bar", + "test_function", + ] + + +def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: + # Make sure `collect_imported_tests` has no dependence on `testpaths` + pytester.makeini(""" + [pytest] + collect_imported_tests = false + """) + + run_import_functions_test(pytester, passed=1, errors=0, failed=0) + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + assert len(items_collected) == 1 + for x in items_collected: + assert x.item._getobj().__name__ == "test_important" + + +def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: + # Make sure `collect_imported_tests` has no dependence on `testpaths` + pytester.makeini(""" + [pytest] + collect_imported_tests = true + """) + + run_import_functions_test(pytester, passed=2, errors=0, failed=1) + reprec = pytester.inline_run() + items_collected = reprec.getcalls("pytest_itemcollected") + assert len(items_collected) == 3 class TestHookBehaviour: @@ -255,11 +259,12 @@ def _test_hook_behaviour_when_collect_off(self, pytester: Pytester) -> None: "reports": reports, } - # Now check that the two tests above did indeed result in the same outcome. def _test_hook_behaviour(self) -> None: print("ABCD", self.collect_outcomes) default = self.collect_outcomes["default"] collect_off = self.collect_outcomes["collect_off"] + + # Check that the two tests above did indeed result in the same outcome. assert default["result"] == collect_off["result"] assert len(default["modified"]) == len(collect_off["modified"]) == 1 @@ -282,13 +287,14 @@ def _test_hook_behaviour(self) -> None: len(default["items_collected"]) == len(collect_off["items_collected"]) == 1 ) + # Check if the same tests got collected def_items_record: RecordedHookCall = default["items_collected"][0] off_items_record: RecordedHookCall = collect_off["items_collected"][0] def_items = def_items_record.__dict__["item"] off_items = off_items_record.__dict__["item"] assert def_items.name == off_items.name - # TODO: fix diff: + # TODO: fix diff: (This will get cleaned up) # [ # , # - , @@ -301,12 +307,24 @@ def _test_hook_behaviour(self) -> None: # ? ^ # ] - # assert len(default['reports']) == len(collect_off['reports']) - # for i in range(len(default['reports'])): - # print("def",default['reports'][i].__dict__) - # print("off",collect_off['reports'][i].__dict__) + for x in default["reports"]: + print("def", x.__dict__) + + for x in collect_off["reports"]: + print("off", x.__dict__) + + # The two above loops prints: + + # def {'nodeid': '', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # def {'nodeid': 'tests/foo_test.py::TestDomain', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # def {'nodeid': 'tests/foo_test.py', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # def {'nodeid': 'tests', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # def {'nodeid': '.', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # off {'nodeid': '', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # off {'nodeid': 'src', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} + # off {'nodeid': 'tests/foo_test.py::TestDomain', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # off {'nodeid': 'tests/foo_test.py', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # off {'nodeid': 'tests', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 + # off {'nodeid': '.', 'outcome': 'passed', 'longrepr': None, 'result': [, ], 'sections': []} # noqa: E501 - # from pprint import pprint - # pprint(default['reports']) - # pprint(collect_off['reports']) - # assert default['reports'] == collect_off['reports'] + assert len(default["reports"]) == len(collect_off["reports"]) From 022d316addadbe407e813cc902f2439431605cbc Mon Sep 17 00:00:00 2001 From: sven <42868792+FreerGit@users.noreply.github.com> Date: Sun, 24 Nov 2024 23:20:10 +0100 Subject: [PATCH 08/12] Ensure correct collection of reports --- testing/test_collect_imports.py | 59 +++++++++++++-------------------- 1 file changed, 23 insertions(+), 36 deletions(-) diff --git a/testing/test_collect_imports.py b/testing/test_collect_imports.py index cc1c5a80015..1c56c9155e5 100644 --- a/testing/test_collect_imports.py +++ b/testing/test_collect_imports.py @@ -8,6 +8,7 @@ from _pytest.pytester import Pytester from _pytest.pytester import RecordedHookCall from _pytest.pytester import RunResult +from _pytest.reports import CollectReport import pytest @@ -71,7 +72,12 @@ def test_collect_imports_disabled(pytester: Pytester) -> None: # Verify that the state of hooks reprec = pytester.inline_run() + reports = reprec.getreports("pytest_collectreport") + modified = reprec.getcalls("pytest_collection_modifyitems") items_collected = reprec.getcalls("pytest_itemcollected") + + assert len(reports) == 5 + assert len(modified) == 1 assert len(items_collected) == 1 for x in items_collected: assert x.item._getobj().__name__ == "test_testament" @@ -79,7 +85,6 @@ def test_collect_imports_disabled(pytester: Pytester) -> None: def test_collect_imports_default(pytester: Pytester) -> None: run_import_class_test(pytester, errors=1) - # TODO def test_collect_imports_enabled(pytester: Pytester) -> None: @@ -260,7 +265,6 @@ def _test_hook_behaviour_when_collect_off(self, pytester: Pytester) -> None: } def _test_hook_behaviour(self) -> None: - print("ABCD", self.collect_outcomes) default = self.collect_outcomes["default"] collect_off = self.collect_outcomes["collect_off"] @@ -294,37 +298,20 @@ def _test_hook_behaviour(self) -> None: off_items = off_items_record.__dict__["item"] assert def_items.name == off_items.name - # TODO: fix diff: (This will get cleaned up) - # [ - # , - # - , - # , - # , - # , - # - , - # ? ^ - # + , - # ? ^ - # ] - - for x in default["reports"]: - print("def", x.__dict__) - - for x in collect_off["reports"]: - print("off", x.__dict__) - - # The two above loops prints: - - # def {'nodeid': '', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # def {'nodeid': 'tests/foo_test.py::TestDomain', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # def {'nodeid': 'tests/foo_test.py', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # def {'nodeid': 'tests', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # def {'nodeid': '.', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # off {'nodeid': '', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # off {'nodeid': 'src', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} - # off {'nodeid': 'tests/foo_test.py::TestDomain', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # off {'nodeid': 'tests/foo_test.py', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # off {'nodeid': 'tests', 'outcome': 'passed', 'longrepr': None, 'result': [], 'sections': []} # noqa: E501 - # off {'nodeid': '.', 'outcome': 'passed', 'longrepr': None, 'result': [, ], 'sections': []} # noqa: E501 - - assert len(default["reports"]) == len(collect_off["reports"]) + def compare_report(r1: CollectReport, r2: CollectReport) -> None: + assert r1.result[0].name == r2.result[0].name + assert len(r1.result) == len(r2.result) + assert r1.outcome == r2.outcome + + # Function test_important + compare_report(default["reports"][1], collect_off["reports"][2]) + # Class TestDomain + compare_report(default["reports"][2], collect_off["reports"][3]) + # Module foo_test.py + compare_report(default["reports"][3], collect_off["reports"][4]) + + # + 1 since src dir is collected + assert len(default["reports"]) + 1 == len(collect_off["reports"]) + + # Two Dirs will be collected, src and test. + assert len(collect_off["reports"][5].result) == 2 From 6ccb5e75d6dd5494e6ef725cd7a41d3b81116585 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 1 Dec 2024 09:03:24 -0300 Subject: [PATCH 09/12] Fix collection and simplify tests We should not collect classes at the module level when the config is set. Doing so is not only the correct thing, as we will not collect the class, but also lets us avoid having the same logic at the Class collector (as it will not be collected at all now). --- src/_pytest/python.py | 21 +- testing/test_collect_imported_tests.py | 103 ++++++++ testing/test_collect_imports.py | 317 ------------------------- 3 files changed, 108 insertions(+), 333 deletions(-) create mode 100644 testing/test_collect_imported_tests.py delete mode 100644 testing/test_collect_imports.py diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 9c00252c5f8..6e7360c5b7d 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -405,6 +405,7 @@ def collect(self) -> Iterable[nodes.Item | nodes.Collector]: # __dict__ is definition ordered. seen: set[str] = set() dict_values: list[list[nodes.Item | nodes.Collector]] = [] + collect_imported_tests = self.session.config.getini("collect_imported_tests") ihook = self.ihook for dic in dicts: values: list[nodes.Item | nodes.Collector] = [] @@ -417,12 +418,10 @@ def collect(self) -> Iterable[nodes.Item | nodes.Collector]: continue seen.add(name) - if not self.session.config.getini("collect_imported_tests"): - # Do not collect imported functions - if inspect.isfunction(obj) and isinstance(self, Module): - fn_defined_at = obj.__module__ - in_module = self._getobj().__name__ - if fn_defined_at != in_module: + if not collect_imported_tests and isinstance(self, Module): + # Do not collect functions and classes from other modules. + if inspect.isfunction(obj) or inspect.isclass(obj): + if obj.__module__ != self._getobj().__name__: continue res = ihook.pytest_pycollect_makeitem( @@ -750,16 +749,6 @@ def newinstance(self): return self.obj() def collect(self) -> Iterable[nodes.Item | nodes.Collector]: - if not self.config.getini("collect_imported_tests"): - # This entire branch will discard (not collect) a class - # if it is imported (defined in a different module) - if isinstance(self, Class) and isinstance(self.parent, Module): - if inspect.isclass(self._getobj()): - class_defined_at = self._getobj().__module__ - in_module = self.parent._getobj().__name__ - if class_defined_at != in_module: - return [] - if not safe_getattr(self.obj, "__test__", True): return [] if hasinit(self.obj): diff --git a/testing/test_collect_imported_tests.py b/testing/test_collect_imported_tests.py new file mode 100644 index 00000000000..4b264ce6069 --- /dev/null +++ b/testing/test_collect_imported_tests.py @@ -0,0 +1,103 @@ +"""Tests for the `collect_imported_tests` configuration value.""" + +from __future__ import annotations + +import textwrap + +from _pytest.pytester import Pytester +import pytest + + +def setup_import_class_test(pytester: Pytester) -> None: + src_dir = pytester.mkdir("src") + tests_dir = pytester.mkdir("tests") + src_file = src_dir / "foo.py" + + src_file.write_text( + textwrap.dedent("""\ + class Testament: + def test_collections(self): + pass + + def test_testament(): pass + """), + encoding="utf-8", + ) + + test_file = tests_dir / "foo_test.py" + test_file.write_text( + textwrap.dedent("""\ + from foo import Testament, test_testament + + class TestDomain: + def test(self): + testament = Testament() + assert testament + """), + encoding="utf-8", + ) + + pytester.syspathinsert(src_dir) + + +def test_collect_imports_disabled(pytester: Pytester) -> None: + """ + When collect_imported_tests is disabled, only objects in the + test modules are collected as tests, so the imported names (`Testament` and `test_testament`) + are not collected. + """ + pytester.makeini( + """ + [pytest] + testpaths = "tests" + collect_imported_tests = false + """ + ) + + setup_import_class_test(pytester) + result = pytester.runpytest("-v") + result.stdout.fnmatch_lines( + [ + "tests/foo_test.py::TestDomain::test PASSED*", + ] + ) + + # Ensure that the hooks were only called for the collected item. + reprec = result.reprec # type:ignore[attr-defined] + reports = reprec.getreports("pytest_collectreport") + [modified] = reprec.getcalls("pytest_collection_modifyitems") + [item_collected] = reprec.getcalls("pytest_itemcollected") + + assert [x.nodeid for x in reports] == [ + "", + "tests/foo_test.py::TestDomain", + "tests/foo_test.py", + "tests", + ] + assert [x.nodeid for x in modified.items] == ["tests/foo_test.py::TestDomain::test"] + assert item_collected.item.nodeid == "tests/foo_test.py::TestDomain::test" + + +@pytest.mark.parametrize("configure_ini", [False, True]) +def test_collect_imports_enabled(pytester: Pytester, configure_ini: bool) -> None: + """ + When collect_imported_tests is enabled (the default), all names in the + test modules are collected as tests. + """ + if configure_ini: + pytester.makeini( + """ + [pytest] + collect_imported_tests = true + """ + ) + + setup_import_class_test(pytester) + result = pytester.runpytest("-v") + result.stdout.fnmatch_lines( + [ + "tests/foo_test.py::Testament::test_collections PASSED*", + "tests/foo_test.py::test_testament PASSED*", + "tests/foo_test.py::TestDomain::test PASSED*", + ] + ) diff --git a/testing/test_collect_imports.py b/testing/test_collect_imports.py deleted file mode 100644 index 1c56c9155e5..00000000000 --- a/testing/test_collect_imports.py +++ /dev/null @@ -1,317 +0,0 @@ -from __future__ import annotations - -import textwrap -from typing import Any - -from _pytest.fixtures import FixtureRequest -from _pytest.main import Session -from _pytest.pytester import Pytester -from _pytest.pytester import RecordedHookCall -from _pytest.pytester import RunResult -from _pytest.reports import CollectReport -import pytest - - -# Start of tests for classes - - -def run_import_class_test( - pytester: Pytester, passed: int = 0, errors: int = 0 -) -> RunResult: - src_dir = pytester.mkdir("src") - tests_dir = pytester.mkdir("tests") - src_file = src_dir / "foo.py" - - src_file.write_text( - textwrap.dedent("""\ - class Testament(object): - def __init__(self): - super().__init__() - self.collections = ["stamp", "coin"] - - def personal_property(self): - return [f"my {x} collection" for x in self.collections] - """), - encoding="utf-8", - ) - - test_file = tests_dir / "foo_test.py" - test_file.write_text( - textwrap.dedent("""\ - import sys - import os - - current_file = os.path.abspath(__file__) - current_dir = os.path.dirname(current_file) - parent_dir = os.path.abspath(os.path.join(current_dir, '..')) - sys.path.append(parent_dir) - - from src.foo import Testament - - class TestDomain: - def test_testament(self): - testament = Testament() - assert testament.personal_property() - """), - encoding="utf-8", - ) - - result = pytester.runpytest() - result.assert_outcomes(passed=passed, errors=errors) - return result - - -def test_collect_imports_disabled(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - testpaths = "tests" - collect_imported_tests = false - """) - - run_import_class_test(pytester, passed=1) - - # Verify that the state of hooks - reprec = pytester.inline_run() - reports = reprec.getreports("pytest_collectreport") - modified = reprec.getcalls("pytest_collection_modifyitems") - items_collected = reprec.getcalls("pytest_itemcollected") - - assert len(reports) == 5 - assert len(modified) == 1 - assert len(items_collected) == 1 - for x in items_collected: - assert x.item._getobj().__name__ == "test_testament" - - -def test_collect_imports_default(pytester: Pytester) -> None: - run_import_class_test(pytester, errors=1) - - -def test_collect_imports_enabled(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - collect_imported_tests = true - """) - - run_import_class_test(pytester, errors=1) - - -# End of tests for classes -################################# -# Start of tests for functions - - -def run_import_functions_test( - pytester: Pytester, passed: int, errors: int, failed: int -) -> RunResult: - # Note that these "tests" should _not_ be treated as tests if `collect_imported_tests = false` - # They are normal functions in that case, that happens to have test_* or *_test in the name. - # Thus should _not_ be collected! - pytester.makepyfile( - **{ - "src/foo.py": textwrap.dedent( - """\ - def test_function(): - some_random_computation = 5 - return some_random_computation - - def test_bar(): - pass - """ - ) - } - ) - - # Inferred from the comment above, this means that there is _only_ one actual test - # which should result in only 1 passing test being ran. - pytester.makepyfile( - **{ - "tests/foo_test.py": textwrap.dedent( - """\ - import sys - import os - - current_file = os.path.abspath(__file__) - current_dir = os.path.dirname(current_file) - parent_dir = os.path.abspath(os.path.join(current_dir, '..')) - sys.path.append(parent_dir) - - from src.foo import * - - class TestDomain: - def test_important(self): - res = test_function() - if res == 5: - pass - """ - ) - } - ) - - result = pytester.runpytest() - result.assert_outcomes(passed=passed, errors=errors, failed=failed) - return result - - -def test_collect_function_imports_enabled(pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - testpaths = "tests" - collect_imported_tests = true - """) - - run_import_functions_test(pytester, passed=2, errors=0, failed=1) - reprec = pytester.inline_run() - items_collected = reprec.getcalls("pytest_itemcollected") - # Recall that the default is `collect_imported_tests = true`. - # Which means that the normal functions are now interpreted as - # valid tests and `test_function()` will fail. - assert len(items_collected) == 3 - for x in items_collected: - assert x.item._getobj().__name__ in [ - "test_important", - "test_bar", - "test_function", - ] - - -def test_behaviour_without_testpaths_set_and_false(pytester: Pytester) -> None: - # Make sure `collect_imported_tests` has no dependence on `testpaths` - pytester.makeini(""" - [pytest] - collect_imported_tests = false - """) - - run_import_functions_test(pytester, passed=1, errors=0, failed=0) - reprec = pytester.inline_run() - items_collected = reprec.getcalls("pytest_itemcollected") - assert len(items_collected) == 1 - for x in items_collected: - assert x.item._getobj().__name__ == "test_important" - - -def test_behaviour_without_testpaths_set_and_true(pytester: Pytester) -> None: - # Make sure `collect_imported_tests` has no dependence on `testpaths` - pytester.makeini(""" - [pytest] - collect_imported_tests = true - """) - - run_import_functions_test(pytester, passed=2, errors=0, failed=1) - reprec = pytester.inline_run() - items_collected = reprec.getcalls("pytest_itemcollected") - assert len(items_collected) == 3 - - -class TestHookBehaviour: - collect_outcomes: dict[str, Any] = {} - - @pytest.mark.parametrize("step", [1, 2, 3]) - def test_hook_behaviour(self, pytester: Pytester, step: int) -> None: - if step == 1: - self._test_hook_default_behaviour(pytester) - elif step == 2: - self._test_hook_behaviour_when_collect_off(pytester) - elif step == 3: - self._test_hook_behaviour() - - @pytest.fixture(scope="class", autouse=True) - def setup_collect_outcomes(self, request: FixtureRequest) -> None: - request.cls.collect_outcomes = {} - - def _test_hook_default_behaviour(self, pytester: Pytester) -> None: - pytester.makepyfile( - **{ - "tests/foo_test.py": textwrap.dedent( - """\ - class TestDomain: - def test_important(self): - pass - """ - ) - } - ) - - result = pytester.runpytest() - result.assert_outcomes(passed=1) - reprec = pytester.inline_run() - reports = reprec.getreports("pytest_collectreport") - modified = reprec.getcalls("pytest_collection_modifyitems") - items_collected = reprec.getcalls("pytest_itemcollected") - - self.collect_outcomes["default"] = { - "result": result.parseoutcomes(), - "modified": modified, - "items_collected": items_collected, - "reports": reports, - } - - def _test_hook_behaviour_when_collect_off(self, pytester: Pytester) -> None: - pytester.makeini(""" - [pytest] - collect_imported_tests = false - """) - res = run_import_functions_test(pytester, passed=1, errors=0, failed=0) - reprec = pytester.inline_run() - reports = reprec.getreports("pytest_collectreport") - modified = reprec.getcalls("pytest_collection_modifyitems") - items_collected = reprec.getcalls("pytest_itemcollected") - - self.collect_outcomes["collect_off"] = { - "result": res.parseoutcomes(), - "modified": modified, - "items_collected": items_collected, - "reports": reports, - } - - def _test_hook_behaviour(self) -> None: - default = self.collect_outcomes["default"] - collect_off = self.collect_outcomes["collect_off"] - - # Check that the two tests above did indeed result in the same outcome. - assert default["result"] == collect_off["result"] - - assert len(default["modified"]) == len(collect_off["modified"]) == 1 - - def_modified_record: RecordedHookCall = default["modified"][0] - off_modified_record: RecordedHookCall = collect_off["modified"][0] - def_sess: Session = def_modified_record.__dict__["session"] - off_sess: Session = off_modified_record.__dict__["session"] - - assert def_sess.exitstatus == off_sess.exitstatus - assert def_sess.testsfailed == off_sess.testsfailed - assert def_sess.testscollected == off_sess.testscollected - - def_items = def_modified_record.__dict__["items"] - off_items = off_modified_record.__dict__["items"] - assert len(def_items) == len(off_items) == 1 - assert def_items[0].name == off_items[0].name - - assert ( - len(default["items_collected"]) == len(collect_off["items_collected"]) == 1 - ) - - # Check if the same tests got collected - def_items_record: RecordedHookCall = default["items_collected"][0] - off_items_record: RecordedHookCall = collect_off["items_collected"][0] - def_items = def_items_record.__dict__["item"] - off_items = off_items_record.__dict__["item"] - assert def_items.name == off_items.name - - def compare_report(r1: CollectReport, r2: CollectReport) -> None: - assert r1.result[0].name == r2.result[0].name - assert len(r1.result) == len(r2.result) - assert r1.outcome == r2.outcome - - # Function test_important - compare_report(default["reports"][1], collect_off["reports"][2]) - # Class TestDomain - compare_report(default["reports"][2], collect_off["reports"][3]) - # Module foo_test.py - compare_report(default["reports"][3], collect_off["reports"][4]) - - # + 1 since src dir is collected - assert len(default["reports"]) + 1 == len(collect_off["reports"]) - - # Two Dirs will be collected, src and test. - assert len(collect_off["reports"][5].result) == 2 From e2ec64e0374d31e64a59a2b12e4d35de062f07a2 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 1 Dec 2024 09:24:40 -0300 Subject: [PATCH 10/12] Improve docs --- changelog/12749.feature.rst | 20 ++++++++++++++++++-- doc/en/reference/reference.rst | 22 +++++++++++++++++++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/changelog/12749.feature.rst b/changelog/12749.feature.rst index cda0db6c930..c3b7ca5d321 100644 --- a/changelog/12749.feature.rst +++ b/changelog/12749.feature.rst @@ -1,5 +1,21 @@ -New :confval:`collect_imported_tests`: when enabled (the default) pytest will collect classes/functions in test modules even if they are imported from another file. +pytest traditionally collects classes/functions in the test module namespace even if they are imported from another file. -Setting this to False will make pytest collect classes/functions from test files only if they are defined in that file (as opposed to imported there). +For example: + +.. code-block:: python + + # contents of src/domain.py + class Testament: ... + + + # contents of tests/test_testament.py + from domain import Testament + + + def test_testament(): ... + +In this scenario with the default options, pytest will collect the class `Testament` from `tests/test_testament.py` because it starts with `Test`, even though in this case it is a production class being imported in the test module namespace. + +This behavior can now be prevented by setting the new :confval:`collect_imported_tests` configuration option to ``false``, which will make pytest collect classes/functions from test files **only** if they are defined in that file. -- by :user:`FreerGit` diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index 53f9470f756..1e550a115c8 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -1306,7 +1306,7 @@ passed multiple times. The expected format is ``name=value``. For example:: .. versionadded:: 8.4 Setting this to ``false`` will make pytest collect classes/functions from test - files only if they are defined in that file (as opposed to imported there). + files **only** if they are defined in that file (as opposed to imported there). .. code-block:: ini @@ -1315,6 +1315,26 @@ passed multiple times. The expected format is ``name=value``. For example:: Default: ``true`` + pytest traditionally collects classes/functions in the test module namespace even if they are imported from another file. + + For example: + + .. code-block:: python + + # contents of src/domain.py + class Testament: ... + + + # contents of tests/test_testament.py + from domain import Testament + + + def test_testament(): ... + + In this scenario, with the default options, pytest will collect the class `Testament` from `tests/test_testament.py` because it starts with `Test`, even though in this case it is a production class being imported in the test module namespace. + + Set ``collected_imported_tests`` to ``false`` in the configuration file prevents that. + .. confval:: consider_namespace_packages Controls if pytest should attempt to identify `namespace packages `__ From 43865ed1a9642596007c382d212da460e4cdfedb Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 1 Dec 2024 09:30:33 -0300 Subject: [PATCH 11/12] Pass tests directory explicitly --- testing/test_collect_imported_tests.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/testing/test_collect_imported_tests.py b/testing/test_collect_imported_tests.py index 4b264ce6069..158d9342b41 100644 --- a/testing/test_collect_imported_tests.py +++ b/testing/test_collect_imported_tests.py @@ -49,13 +49,12 @@ def test_collect_imports_disabled(pytester: Pytester) -> None: pytester.makeini( """ [pytest] - testpaths = "tests" collect_imported_tests = false """ ) setup_import_class_test(pytester) - result = pytester.runpytest("-v") + result = pytester.runpytest("-v", "tests") result.stdout.fnmatch_lines( [ "tests/foo_test.py::TestDomain::test PASSED*", @@ -93,7 +92,7 @@ def test_collect_imports_enabled(pytester: Pytester, configure_ini: bool) -> Non ) setup_import_class_test(pytester) - result = pytester.runpytest("-v") + result = pytester.runpytest("-v", "tests") result.stdout.fnmatch_lines( [ "tests/foo_test.py::Testament::test_collections PASSED*", From d2327d9deb4b6f54104039817540951b5deced7b Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sun, 1 Dec 2024 09:31:11 -0300 Subject: [PATCH 12/12] Rename test helper --- testing/test_collect_imported_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/test_collect_imported_tests.py b/testing/test_collect_imported_tests.py index 158d9342b41..28b92e17f6f 100644 --- a/testing/test_collect_imported_tests.py +++ b/testing/test_collect_imported_tests.py @@ -8,7 +8,7 @@ import pytest -def setup_import_class_test(pytester: Pytester) -> None: +def setup_files(pytester: Pytester) -> None: src_dir = pytester.mkdir("src") tests_dir = pytester.mkdir("tests") src_file = src_dir / "foo.py" @@ -53,7 +53,7 @@ def test_collect_imports_disabled(pytester: Pytester) -> None: """ ) - setup_import_class_test(pytester) + setup_files(pytester) result = pytester.runpytest("-v", "tests") result.stdout.fnmatch_lines( [ @@ -91,7 +91,7 @@ def test_collect_imports_enabled(pytester: Pytester, configure_ini: bool) -> Non """ ) - setup_import_class_test(pytester) + setup_files(pytester) result = pytester.runpytest("-v", "tests") result.stdout.fnmatch_lines( [