Add missing __test__ check for discovery
#2099
Conversation
| @@ -502,6 +502,8 @@ def _get_xunit_func(obj, name): | |||
| class Class(PyCollector): | |||
| """ Collector for test methods. """ | |||
| def collect(self): | |||
| if not getattr(self.obj, "__test__", True): | |||
| return | |||
There was a problem hiding this comment.
I think you have to return [] here.
| assert True | ||
| """) | ||
| result = testdir.runpytest() | ||
| assert "cannot collect test class 'TestFoo'" not in result.stdout.str() |
There was a problem hiding this comment.
Please also ensure something about the output, I suggest this:
result.stdout.fnmatch_lines([
'collected 0 items',
'*no tests ran in*',
])| @@ -1,6 +1,9 @@ | |||
| 3.0.5.dev0 | |||
There was a problem hiding this comment.
Since this is a behavior change, I think we should play safe here and merge this into features. Could you please change the target branch of the PR?
| @@ -502,6 +502,8 @@ def _get_xunit_func(obj, name): | |||
| class Class(PyCollector): | |||
| """ Collector for test methods. """ | |||
| def collect(self): | |||
| if not getattr(self.obj, "__test__", True): | |||
There was a problem hiding this comment.
pytest.compat has a safe_getattr which returns a default when exceptions happen - shouldn't this be used here? (see #214)
| @@ -85,6 +85,19 @@ def pytest_collect_file(path, parent): | |||
| assert len(nodes) == 1 | |||
| assert isinstance(nodes[0], pytest.File) | |||
|
|
|||
| def test_can_skip_class_with_test_attr(self, testdir): | |||
| """Assure warning is not outputted when using `__test__=True` (See #2007)""" | |||
|
Addressed those comments and changed the target branch 👍 Not sure I got the |
| class TestFoo(): | ||
| __test__ = False | ||
| def __init__(self): | ||
| return self.__init__() |
There was a problem hiding this comment.
while this will hopefully never be called, its still a endless recursion
There was a problem hiding this comment.
Haha, oh yeah, let's get rid of that.
Yeah sorry for not being clearer: the So your While we are at it, I think we could reword the entry a bit to convey to end-users better what was fixed, perhaps: Other than that the rest LGTM for merging! Thanks again! |
|
Thanks for the clarification @nicoddemus. I'll get the hang of wording these |
|
LGTM, thanks @lwm for the patience! @The-Compiler could you review the recent changes and merge if you are OK with them? |
| @@ -13,6 +13,10 @@ New Features | |||
| Changes | |||
| ------- | |||
|
|
|||
| * It is now possible to skip test classes from being collected by setting a | |||
| ``__test__`` attribute to ``False`` in the class body (`#2007`_).* Thanks | |||
There was a problem hiding this comment.
Oh, yes, fixed that one!
| @@ -85,6 +85,22 @@ def pytest_collect_file(path, parent): | |||
| assert len(nodes) == 1 | |||
| assert isinstance(nodes[0], pytest.File) | |||
|
|
|||
| def test_can_skip_class_with_test_attr(self, testdir): | |||
| """Assure warning is not outputted when using `__test__=False` (See #2007)""" | |||
There was a problem hiding this comment.
Just noticed that this docstring seems to be out-dated.
|
I think it's ready to go. @The-Compiler please feel free to merge if you are also OK with it. |
|
Looks good, thanks! |
Closes #2007.
Not sure if the test is in the right place?