-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
base: main
Are you sure you want to change the base?
Changes from 5 commits
222457d
fa3b631
68ac4a1
a6ee0bc
eb8592c
935c06d
191456e
f1821ea
022d316
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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` |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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 | ||||||||||
|
||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
Setting this to `false` will make pytest collect classes/functions from test | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
files only if they are defined in that file (as opposed to imported there). | ||||||||||
|
||||||||||
.. code-block:: ini | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||
|
||||||||||
[pytest] | ||||||||||
collect_imported_tests = false | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
|
||||||||||
.. confval:: tmp_path_retention_count | ||||||||||
|
||||||||||
How many sessions should we keep the `tmp_path` directories, | ||||||||||
according to `tmp_path_retention_policy`. | ||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,6 +78,12 @@ | |
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", | ||
|
@@ -958,16 +964,41 @@ | |
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"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Previously this code was placed in In fact, it is important for this to be tested too: the following collection hooks should not receive any item that was discarded due to
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi! Thought I would pick up this issue again after some wait. Is there any examples on how I could test these hooks? Im unsure on how to extract that information from the hooks. Im assuming this is problematic because extensions to pytest could be listening to these hooks? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Usually we write a
Exactly. |
||
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) | ||
keepduplicates = self.config.getoption("keepduplicates") | ||
# 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"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same comment here, this logic should be moved to |
||
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: | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,168 @@ | ||||||
from __future__ import annotations | ||||||
|
||||||
import textwrap | ||||||
|
||||||
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") | ||||||
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) | ||||||
|
||||||
|
||||||
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" | ||||||
collect_imported_tests = true | ||||||
""") | ||||||
|
||||||
run_import_class_test(pytester, errors=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" | ||||||
|
||||||
# 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 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) | ||||||
|
||||||
|
||||||
def test_collect_function_imports_disabled(pytester: Pytester) -> None: | ||||||
pytester.makeini(""" | ||||||
[pytest] | ||||||
# testpaths = "tests" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this was meant to be:
Suggested change
But I don't think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ment delete that commented line but forgot. You're right There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh in that case make sure to add a comment/docstring mentioning that if you want to keep the test. |
||||||
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) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you move this closer to the other options alphabetically?