[issue24412] setUpClass equivalent for addCleanup

[Lisa Roach]

Change by Lisa Roach :


--
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
pull_requests: +10041

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

/home/serhiy/py/cpython/Lib/unittest/test/test_runner.py:259: 
DeprecationWarning: Please use assertEqual instead.
  self.assertEquals(e, 'cleanup1')

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Lisa Roach]

Lisa Roach  added the comment:


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


--
nosy: +lisroach

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Robert Collins]

Robert Collins  added the comment:

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):
  
  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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Serhiy Storchaka]

Change by Serhiy Storchaka :


--
versions: +Python 3.8 -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Lisa Roach]

Change by Lisa Roach :


--
pull_requests: +8628
stage: needs patch -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Tal Einat]

Change by Tal Einat :


--
nosy:  -taleinat

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Daryl Blanc]

Changes by Daryl Blanc :


--
nosy: +dablanc

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Robert Collins]

Robert Collins added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Robert Collins]

Robert Collins added the comment:

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Vajrasky Kok]

Vajrasky Kok added the comment:

Here is the patch.

--
keywords: +patch
nosy: +vajrasky
Added file: http://bugs.python.org/file39844/addCleanupClass.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Akshit Khurana]

Changes by Akshit Khurana axitkhur...@gmail.com:


--
nosy: +axitkhurana

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

contextlib.ExitStack could help you.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[R. David Murray]

R. David Murray added the comment:

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.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[R. David Murray]

R. David Murray added the comment:

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 :)

--
keywords: +easy

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Tal Einat]

Tal Einat added the comment:

I'm convinced. +1

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[R. David Murray]

R. David Murray added the comment:

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.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Tal Einat]

Tal Einat added the comment:

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.

--
nosy: +taleinat

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Tal Einat]

Tal Einat added the comment:

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.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Robert Collins]

Robert Collins added the comment:

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.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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().

--
nosy: +serhiy.storchaka

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Tal Einat]

Tal Einat added the comment:

Ahh, that makes sense. Sounds good to me!

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[Barry A. Warsaw]

Changes by Barry A. Warsaw ba...@python.org:


--
nosy: +barry

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24412] setUpClass equivalent for addCleanup

[R. David Murray]

New submission from R. David Murray:

Since tearDownClass isn't run if setUpClass fails, there is a need for the 
class level equivalent of addCleanup.  addClassCleanup?

--
components: Library (Lib)
messages: 245030
nosy: ezio.melotti, michael.foord, r.david.murray, rbcollins
priority: normal
severity: normal
stage: needs patch
status: open
title: setUpClass equivalent for addCleanup
type: enhancement
versions: Python 3.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24412
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com