Call Python 3.8 doClassCleanups #8033
Conversation
There was a problem hiding this comment.
Thanks @PetterS, LGTM with a few comments. The extensive tests are nice.
I see there is also a new addModuleCleanup but that'd be more tricky to support since the code for setUpModule/tearDownModule is not in the unittest plugin for some reason.
src/_pytest/unittest.py
Outdated
| cls, | ||
| "setup_method", | ||
| "teardown_method", | ||
| "<no cleanup function>", |
There was a problem hiding this comment.
I prefer not to do this trickery. I suggest allowing None and passing None here instead.
src/_pytest/unittest.py
Outdated
| ): | ||
| setup = getattr(obj, setup_name, None) | ||
| teardown = getattr(obj, teardown_name, None) | ||
| if setup is None and teardown is None: | ||
| return None | ||
|
|
||
| cleanup = getattr(obj, cleanup_name, lambda *args: None) | ||
|
|
||
| def docleanup(self): |
There was a problem hiding this comment.
I prefer to inline it. The setup code already does it, it's not too bad and will help with showing proper coverage for the different cases.
There was a problem hiding this comment.
Done.
Linter would not let me assign a lambda to a variable so it looks like this now.
| setup(self, request.function) | ||
| else: | ||
| setup() | ||
| except Exception: |
There was a problem hiding this comment.
Would add a comment here about why only Exception is caught while below finally is used (-> to match what unittest does).
testing/test_unittest.py
Outdated
|
|
||
|
|
||
| @pytest.mark.skipif( | ||
| sys.version_info < (3, 1), reason="Feature introduced in Python 3.1" |
There was a problem hiding this comment.
Since we only support Python>=3.6, the Python 3.1 skips can be removed.
| def cleanup(): | ||
| cls.values.append(1) | ||
| cls.addClassCleanup(cleanup) | ||
| def test_one(self): |
There was a problem hiding this comment.
Perhaps also add test_two to the class tests, to ensure that the cleanup indeed only runs once.
testing/test_unittest.py
Outdated
| cls.addClassCleanup(cleanup) | ||
| def test_one(self): | ||
| pass | ||
| class Second(unittest.TestCase): |
There was a problem hiding this comment.
I'm not sure if the fact that this is a unittest.TestCase is part of the test or not. If the purpose is just to check the MyTestCase.values value, then using a regular pytest function test would remove this ambiguity.
|
I think this patch still can be improved. The |
|
Cheers! This will help our internal development! |
|
@bluetech Do you know when the next release of pytest will be? |
|
Cheers, thanks! |
pytest calls doClassCleanups() directly: pytest-dev/pytest#8033. This ends up calling our "backport" on Python 3.6 even when the test case doesn't use any class cleanups or the @classCleanups decorator, resulting in the following error: AttributeError: type object 'TestFsRefs' has no attribute '_class_cleanups' Fix it by allowing our doClassCleanups() to be called unconditionally. Signed-off-by: Omar Sandoval <osandov@osandov.com>
Fixes #8032
Pytest already supports doCleanups; I added tests for this.