-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add discover_imports in conf, don't collect imported classes named Test* closes #12749`
#12810
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
Changes from 2 commits
222457d
fa3b631
68ac4a1
a6ee0bc
eb8592c
935c06d
191456e
f1821ea
022d316
6ccb5e7
e2ec64e
43865ed
d2327d9
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,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` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| from __future__ import annotations | ||
|
|
||
| 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) |
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.
Nice, changelog entry.
We also need to add documentation for that confval in
reference.rst.I'm not sure about the name too, but at the moment I don't have another suggestion (however we really should strive to find a more appropriate name).
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.
treat_imports_as_tests?
imports_are_tests?
crawl_imports?
Uh oh!
There was an error while loading. Please reload this page.
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.
As per my comment, perhaps we should focus on being more "strict" about what we collect from modules.
collect_?something? = "all"/"local-only"But we should wait to see what other maintainers think.
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.
Alright, now after a some hours (more than id like to admit) I now have something that works. Both for functions and classes. Let me know how we wanna handle the toggle of the feature. Have a good weekend!