This issue tracker has been migrated to GitHub ,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2015年06月08日 18:31 by r.david.murray, last changed 2022年04月11日 14:58 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| addCleanupClass.patch | vajrasky, 2015年07月02日 15:21 | review | ||
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 9190 | merged | lisroach, 2018年09月11日 22:02 | |
| PR 10794 | merged | xtreak, 2018年11月29日 12:42 | |
| Messages (18) | |||
|---|---|---|---|
| msg245030 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2015年06月08日 18:31 | |
Since tearDownClass isn't run if setUpClass fails, there is a need for the class level equivalent of addCleanup. addClassCleanup? |
|||
| msg245293 - (view) | Author: Tal Einat (taleinat) * (Python committer) | Date: 2015年06月13日 06:31 | |
Is this really needed? One can use try/except/raise, and since addClassCleanup() would only be called from setUpClass(), I don't quite see the utility of it. |
|||
| msg245295 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2015年06月13日 06:43 | |
It would be nice for symmetry. I mean, setUpClass isn't needed either, and we have it ;). however, we actually have two contexts this would be called from - setUpClass and setUpModule; both share their internals. So we probably need a decoupled cleanups implementation, and two new binding points to it. |
|||
| msg245299 - (view) | Author: Tal Einat (taleinat) * (Python committer) | Date: 2015年06月13日 07:02 | |
I'm not convinced this would be worth the effort required to implement and maintain it. Can someone find examples from existing test suites where this would clearly be useful? For example, a setUpClass() or setUpModule() function with multiple try/finally clauses. |
|||
| msg245301 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2015年06月13日 07:23 | |
addCleanup() is helpful because it can be used in test methods. addClassCleanup() and addModuleCleanup() can't be used in test methods, and setUpClass() and setUpModule() are used less than setUp(), therefore the benefit of these methods are less than of addCleanup(). |
|||
| msg245318 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2015年06月13日 14:57 | |
Not having addClassCleanup means that my setUpClass method will have four try/excepts in it, as will my tearDownClass (I have four fixtures I'm setting up for the tests). So, no, it isn't strictly needed, but it is prettier :). As Robert says, though, it makes for a nice symmetry in the API. Basically, I like to see tearDown deprecated, as I find the setup/addcleanup pattern much easier to write and, more importantly, to read. |
|||
| msg245323 - (view) | Author: Tal Einat (taleinat) * (Python committer) | Date: 2015年06月13日 19:06 | |
Ahh, that makes sense. Sounds good to me! |
|||
| msg245530 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2015年06月19日 22:16 | |
As further motivation, I actually tried to implement this using try/except. First I wrote a loop in tearDownClass that ran each of the cleanups inside a try/except and printed the exception if there was one. But of course that doesn't run if setUpClass fails. So I started to add a similar try/except loop in setUpClass...and then realized that in order to have the cleanups run correctly, I needed to add each cleanup to a list on the class if and only if the corresponding setup succeeded, and then run only those cleanups in the tearDownClass. So, I ended up implementing addClassCleanup. However, in order to make it generic (I have two classes where I need it currently, and will probably be adding more), I have a base class with setUpClass that calls a safeSetUpClass inside a try/except, my safeSetUpClass on the actual test class does the setup and calls to addClassCleanUp, and my tearDownClass does the loop over the accumulated cleanups. So, yeah, it would be really handy to have this as an actual feature :) |
|||
| msg245531 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2015年06月19日 22:33 | |
contextlib.ExitStack could help you. |
|||
| msg245534 - (view) | Author: R. David Murray (r.david.murray) * (Python committer) | Date: 2015年06月19日 22:51 | |
That would not make it simpler. In fact it would make the test code more complex. I'd still need the safeSetUpClass dodge (or repeat the try/except/log in every setUpClass method), and I'd have to have a wrapper function that I passed each cleanup to as an argument to wrap it in a try/except/log, since close would just propagate the exceptions. I think implementing this parallel to addCleanUp is much cleaner, even if the feature isn't (yet :) in the stdlib. |
|||
| msg245540 - (view) | Author: Tal Einat (taleinat) * (Python committer) | Date: 2015年06月20日 05:24 | |
I'm convinced. +1 |
|||
| msg246084 - (view) | Author: Vajrasky Kok (vajrasky) * | Date: 2015年07月02日 15:21 | |
Here is the patch. |
|||
| msg275890 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2016年09月11日 23:58 | |
So, thank you for the patch. However - there's no need for a metaclass here, and its actively harmful to comprehending the code. As I suggested earlier, please decouple the cleanups implementation and then consume it from the two places that it will be needed (testcase and testsuite). |
|||
| msg275895 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2016年09月12日 00:10 | |
Btw some things to be aware of in addressing this: - we will need tests that catchable exceptions raised from a setUpModule or setUpClass cause cleanups already registered to run - if (and I'd need to check this) setUpModule or setUpClass can nest - I think setUpModule may, in package.module contexts) - that non-local cleanups definitively run too |
|||
| msg326531 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2018年09月27日 08:01 | |
There is a design issue with the order of cleaning up. tearDown() is purposed to release resources acquired in setUp(). Resources acquired in test methods will be released in doCleanup() if use addCleanup(), but since doCleanup() is called after tearDown(), resources will be released not in reversed order of the time of acquiring. This causes issues if resources acquired in a test method depends on the resource acquired in setUp(). Adding doClassCleanups() and doModuleCleanups() with the same semantic will introduce this problem at class and module level. |
|||
| msg326533 - (view) | Author: Robert Collins (rbcollins) * (Python committer) | Date: 2018年09月27日 08:14 | |
Serhiy, thats not a design flaw, its a feature. in a class hierarchy, setup and teardown ordering is undefined: implementors can choose whatever order they want to execute in. e.g. class A(TestCase): def setUp(self): super().setUp() acquire1() class B(A): def setUp(self): acquire2() super().setUp() class C(B): def setUp(self): super().setUp() acquire3() will acquire 2, then 1, then 3. tearDown() is similarly ill defined. Secondly, consider class Example(TestCase): def setUp(self): self._1 = acquire() self.addCleanUp(acquire()) self._3 = acquire() def tearDown(self): self._3.cleanUp() self._1.cleanUp() super().tearDown() As such, there is no ordering of cleanup + teardown that is 'right' in all cases. The question then becomes, which ordering *most facilitates* migration from tearDown to cleanup. The allowable orders are: - with a more complex implementation, per-class (no) - before tearDown starts - after tearDown finishes The common case tearDown tends to look like this: def tearDown(self): <local cleanup> super().tearDown() so, by placing doCleanup after tearDown, we make it possible for base classes in a test hierarchy to migrate to cleanups without breaking compatibility with child classes. The cost, as you note is that we make it harder for child classes to migrate until the base class has migrated. But it is strictly better to permit the base class to migrate, because in projects you cannot be sure of all your subclasses out there in the wild, but you can be sure of all your base classes. |
|||
| msg329494 - (view) | Author: Lisa Roach (lisroach) * (Python committer) | Date: 2018年11月09日 02:34 | |
New changeset 0f221d09cad46bee38d1b7a7822772df66c53028 by Lisa Roach in branch 'master': bpo-24412: Adds cleanUps for setUpClass and setUpModule. (GH-9190) https://github.com/python/cpython/commit/0f221d09cad46bee38d1b7a7822772df66c53028 |
|||
| msg330624 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) | Date: 2018年11月28日 17:54 | |
/home/serhiy/py/cpython/Lib/unittest/test/test_runner.py:259: DeprecationWarning: Please use assertEqual instead. self.assertEquals(e, 'cleanup1') |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022年04月11日 14:58:17 | admin | set | github: 68600 |
| 2019年06月10日 21:23:12 | lisroach | set | status: open -> closed stage: patch review -> resolved |
| 2018年11月29日 12:42:36 | xtreak | set | pull_requests: + pull_request10041 |
| 2018年11月28日 17:54:07 | serhiy.storchaka | set | messages: + msg330624 |
| 2018年11月09日 02:34:38 | lisroach | set | nosy:
+ lisroach messages: + msg329494 |
| 2018年09月27日 08:14:16 | rbcollins | set | messages: + msg326533 |
| 2018年09月27日 08:01:15 | serhiy.storchaka | set | versions: + Python 3.8, - Python 3.6 |
| 2018年09月27日 08:01:06 | serhiy.storchaka | set | messages: + msg326531 |
| 2018年09月11日 22:02:55 | lisroach | set | stage: needs patch -> patch review pull_requests: + pull_request8628 |
| 2017年09月28日 06:58:44 | taleinat | set | nosy:
- taleinat |
| 2017年06月29日 08:17:15 | dablanc | set | nosy:
+ dablanc |
| 2016年09月12日 00:10:18 | rbcollins | set | messages: + msg275895 |
| 2016年09月11日 23:58:15 | rbcollins | set | messages: + msg275890 |
| 2015年07月02日 15:21:57 | vajrasky | set | files:
+ addCleanupClass.patch nosy: + vajrasky messages: + msg246084 keywords: + patch |
| 2015年06月22日 22:16:50 | axitkhurana | set | nosy:
+ axitkhurana |
| 2015年06月20日 05:24:11 | taleinat | set | messages: + msg245540 |
| 2015年06月19日 22:51:12 | r.david.murray | set | messages: + msg245534 |
| 2015年06月19日 22:33:02 | serhiy.storchaka | set | messages: + msg245531 |
| 2015年06月19日 22:16:50 | r.david.murray | set | keywords:
+ easy messages: + msg245530 |
| 2015年06月13日 19:06:56 | taleinat | set | messages: + msg245323 |
| 2015年06月13日 14:57:16 | r.david.murray | set | messages: + msg245318 |
| 2015年06月13日 07:23:01 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg245301 |
| 2015年06月13日 07:02:53 | taleinat | set | messages: + msg245299 |
| 2015年06月13日 06:43:10 | rbcollins | set | messages: + msg245295 |
| 2015年06月13日 06:31:31 | taleinat | set | nosy:
+ taleinat messages: + msg245293 |
| 2015年06月08日 18:52:50 | barry | set | nosy:
+ barry |
| 2015年06月08日 18:31:39 | r.david.murray | create | |