Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add discover_imports in conf, don't collect imported classes named Test* closes #12749` #12810

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,7 @@ Stefanie Molin
Stefano Taschini
Steffen Allner
Stephan Obermann
Sven
Sven-Hendrik Haase
Sviatoslav Sydorenko
Sylvain Marié
Expand Down
5 changes: 5 additions & 0 deletions changelog/12749.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
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`
17 changes: 14 additions & 3 deletions doc/en/reference/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://packaging.python.org/en/latest/guides/packaging-namespace-packages>`__
Expand Down Expand Up @@ -1838,11 +1852,8 @@ passed multiple times. The expected format is ``name=value``. For example::

pytest testing doc


.. confval:: tmp_path_retention_count



How many sessions should we keep the `tmp_path` directories,
according to `tmp_path_retention_policy`.

Expand Down
6 changes: 6 additions & 0 deletions src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ def pytest_addoption(parser: Parser) -> None:
type="args",
default=[],
)
parser.addini(
"collect_imported_tests",
"Whether to collect tests in imported modules outside `testpaths`",
type="bool",
default=True,
)
group = parser.getgroup("general", "Running and selection options")
group._addoption(
"-x",
Expand Down
19 changes: 19 additions & 0 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,15 @@
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):

Check warning on line 422 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L421-L422

Added lines #L421 - L422 were not covered by tests
fn_defined_at = obj.__module__
in_module = self._getobj().__name__

Check warning on line 424 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L424

Added line #L424 was not covered by tests
if fn_defined_at != in_module:
continue

res = ihook.pytest_pycollect_makeitem(
collector=self, name=name, obj=obj
)
Expand Down Expand Up @@ -741,6 +750,16 @@
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()):

Check warning on line 757 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L756-L757

Added lines #L756 - L757 were not covered by tests
class_defined_at = self._getobj().__module__
in_module = self.parent._getobj().__name__

Check warning on line 759 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L759

Added line #L759 was not covered by tests
if class_defined_at != in_module:
return []

Check warning on line 762 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L762

Added line #L762 was not covered by tests
if not safe_getattr(self.obj, "__test__", True):
return []
if hasinit(self.obj):
Expand Down
233 changes: 233 additions & 0 deletions testing/test_collect_imports.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
from __future__ import annotations

import textwrap

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")
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


# End of tests for classes
#################################
# Start of tests for functions


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"

# 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",
)

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",
)

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....
# <CollectReport '' lenresult=1 outcome='passed'>
# <CollectReport 'tests/foo_test.py::TestDomain' lenresult=1 outcome='passed'>
# <CollectReport 'tests/foo_test.py' lenresult=1 outcome='passed'>
# <CollectReport 'tests' lenresult=1 outcome='passed'>
# <CollectReport '.' lenresult=1 outcome='passed'>

# TODO
# assert(reports.outcome == "passed")
# assert(len(reports.result) == 1)

# print("Items collected: ----------------")
# print(items_collected)
# print("Modified : ----------------")

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not completely sure what is the correct behaviour yet, I'll try and figure it out. WIP and all.

assert len(items_collected) == 1
for x in items_collected:
assert x.item._getobj().__name__ == "test_important"

assert len(modified) == 1
Loading