Anssi, thanks a lot for the detailed feedback, it is much appreciated! My 
comments are inlined below:


On Friday 4 May 2012 at 10:03, Anssi Kääriäinen wrote:

> I marked the ticket DDN, there are three reasons:
> 1. If a test case screws up cleanup it will cause problems for
> itself currently, after the patch it will cause problems for the next
> test case which makes debugging much harder.

The same cleanup will still be done, it will just be moved from before the test 
run, to after.

The only way for a test case to screw up would be a bug in Django's 
_pre_setup/_post_teardown implementation, which would cause tricky bugs, no 
matter if the cleanup happens before or after the test.

> 2. Is this backwards incompatible? The behavior was documented.
See notes below.
> 3. Is there some reason why the flush must be done pre-test which
> we are overlooking. Why was it implemented that way originally?

That's a good question. Anyone who wrote to original 
TransactionTestCase/reordering implementation that wants to chime in? :-)
> Perhaps because of the first item?

The first item should get a newly created database, so it should not be a 
problem.  



> Each test case taking care of cleaning up after itself feels like the
> right way to go. There are a couple of reasons:
> - TestCase works already this way: it doesn't do cleanup before, it
> just makes sure it cleans after itself (which is easy as it doesn't
> leave any trash behind).
> - The database is treated specially. The tests trust other tests to
> clean up their state. Settings is an example.
> - As said upthread, this allows for special test case subclasses
> which do more intelligent cleanup. I don't think an intelligent
> TransactionTestCase subclass which doesn't run flush pre-tests is
> possible in the current implementation, as it would then see the trash
> of other TransactionTestCases.

Agreed.  
> But, as said, I left the ticket DDN. None of the issues mentioned
> above seem like a blocker. The backwards incompatibility is the most
> likely blocker - I can't see that changing the behavior is going to
> cause problems, but maybe there are users out there whose test cases
> somehow require pre-test flush?


I have run Djangos own test suite, and my current project's test suite with 
~500 tests and some TransactionTestCases without problems.

This is indeed a change in documented behavior, although I don't think it will 
affect peoples test. In order for a test case to be backward incompatible, a 
user would need to override 
_pre_setup/_fixture_setup/_post_teardown/_fixture_teardown (which could be 
considered private?). Unless that is done, I cannot see how tests can be 
affected by this, but there can certainly be something I am missing here!

I guess we need someone who wrote the initial TransactionTestCase or knows why 
it is the way it is to give his/hers opinion on this not to oversee backward 
incompatible changes.


Andreas

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Reply via email to