Re: Test reordering and TransactionTestCase cleanup

[Ramiro Morales]

Anssi,

Thanks for the review,

On Tue, Jul 10, 2012 at 2:16 AM, Anssi Kääriäinen
 wrote:
>
> I spotted one error:
>   https://github.com/ramiro/django/compare/pr45_t18271#L7R472 - The
> line should say conn.cursor(), not connection.cursor().

Fixed in the last iteration of the branch.

>
> I ran the full test suite on SQLite, and got this error:
> ==
> ERROR: test_pass_connection_between_threads
> (regressiontests.backends.tests.ThreadTests)
> --
> Traceback (most recent call last):
>   File "/home/akaariai/Programming/django/tests/regressiontests/
> backends/tests.py", line 595, in test_pass_connection_between_threads
> self.assertTrue(isinstance(exceptions[0], DatabaseError))
> IndexError: list index out of range
>
> I don't know if this is related to this patch or not. The error does
> not happen when running the backends test alone. This might be related
> to test ordering

Exactly, this is what I've found is happening:

The problems shows when you run together (minimal expression):

A. Any test from the servers `regression` tests (that subclass
   TransactiontestCase via LiveServerTestCase)
B. The test that is generating the error: A test from the `backends`
   regression tests (that subclass TestCase)

e.g.

 runtests.py --settings=test_sqlite servers.LiveServerViews.test_404
backends.ThreadTests.test_pass_connection_between_threads

For SQLite, memory-hosted databases, LiveServerTestCase sets main thread's
'default' DB connection's allow_thread_sharing to True.

The `backends` tests:

* Spawns a second helper thread
* It clobbers this thread's connection with the main thread's one.
* Both main and helper threads 'default' connections' allow_`thread_sharing`
  values are initialized or assumed be False and then the effects of such
  value are then verified.

Currently, tests that subclass TestCase always run first so the order is
always B before A and no problem occurs. Even when the main thread default
connection `allow_thread_sharing` value state is preserved across test cases
after all the LiveserverTestCase have finished.

But when the patch is applied such order isn't guaranteed anymore (all the
tests that inherit from unittest.TestCase are run intermixed w/o a defined
orderiing.)

The test discovery seems to always order them so A runs before B, the main
thread connection allow_thread_sharing=True values survives, causes a
condition that isn't expected by the B test and so the error occurs.

Maybe we can fix the LiveServerTestCase so it doesn't leak the munging of
`allow_thread_sharing` it performs by resetting its value in its
classTearDown() method?.


>
> And a question:
>   This change in https://github.com/ramiro/django/compare/pr45_t18271#L9R502
> says that changing a TestCase to TransactionTestCase might solve some
> errors caused by test ordering. I don't get why this is true.

It was text I had moved from a spot below in the same document. It applied to
the changes introduced in 1.1 and is actually inaccurate now. My fault,
I've reworded it.

Regards,

-- 
Ramiro Morales

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

Re: Test reordering and TransactionTestCase cleanup

[Anssi Kääriäinen]

On 10 heinä, 00:35, Ramiro Morales  wrote:
> On Mon, May 7, 2012 at 6:50 PM, Anssi Kääriäinen
>
>  wrote:
>
> > I would like to just get rid of the sequence resets. Oracle doesn't do
> > it currently, TestCase doesn't do it, and IMO assuming the IDs are
> > going to start from 1 is an assumption one should not make.
>
> > Objections to just getting rid of the sequence resets altogether (with
> > the opt-in flag)?
>
> Hi all,
>
> Based on Andreas' work and discussion on this thread, I've:
>
> 1. If the user specifies the `reset_sequences` flag, what is performed
>    during test case setup is actually that: A reset of DB sequences.
>    There is no more flushing of tables.
>
>    For this a method was added to DB backends DatabaseOperation classes.
>
> 2. Dropped resetting of sequences from the DB flush now performed during
>    test case teardown code. For this, a stealth option was added to the
>    `flush` management command.
>
> 3. Expanded and re-factored [related] documentation a bit.
>
> 1. and 2. mean that now:
>
> * Pre test case execution: There is no DB table flushing nor reset of
>   sequences (the latter can be asked for the developer by using
>   reset_sequences.
>
> * Post test case execution: Flushing of DB tables is performed.
>
> https://github.com/ramiro/django/compare/pr45_t18271
>
> So far, I count two favorable from core devs: Karen and Anssi.
>
> Any further feedback about the proposed changes is welcome.
> Especially reports of results of people running their own test suites.
>
> I ran the Django test suite w/o against Postgres, MySQL and sqlite3
> w/o problems.
>
> Hopefully I will be able to commit something along the lines soon.

I don't know if this is the proper forum to give a review... but here
goes anyways :)

I spotted one error:
  https://github.com/ramiro/django/compare/pr45_t18271#L7R472 - The
line should say conn.cursor(), not connection.cursor().

And a question:
  This change in https://github.com/ramiro/django/compare/pr45_t18271#L9R502
says that changing a TestCase to TransactionTestCase might solve some
errors caused by test ordering. I don't get why this is true.

I ran the full test suite on SQLite, and got this error:
==
ERROR: test_pass_connection_between_threads
(regressiontests.backends.tests.ThreadTests)
--
Traceback (most recent call last):
  File "/home/akaariai/Programming/django/tests/regressiontests/
backends/tests.py", line 595, in test_pass_connection_between_threads
self.assertTrue(isinstance(exceptions[0], DatabaseError))
IndexError: list index out of range

I don't know if this is related to this patch or not. The error does
not happen when running the backends test alone. This might be related
to test ordering, or just a random test failure (one which I haven't
seen before, though). I have no time to rerun the full test suite just
now.

  - Anssi

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

Re: Test reordering and TransactionTestCase cleanup

[Ramiro Morales]

On Mon, May 7, 2012 at 6:50 PM, Anssi Kääriäinen
 wrote:
>
> I would like to just get rid of the sequence resets. Oracle doesn't do
> it currently, TestCase doesn't do it, and IMO assuming the IDs are
> going to start from 1 is an assumption one should not make.
>
> Objections to just getting rid of the sequence resets altogether (with
> the opt-in flag)?

Hi all,

Based on Andreas' work and discussion on this thread, I've:

1. If the user specifies the `reset_sequences` flag, what is performed
   during test case setup is actually that: A reset of DB sequences.
   There is no more flushing of tables.

   For this a method was added to DB backends DatabaseOperation classes.

2. Dropped resetting of sequences from the DB flush now performed during
   test case teardown code. For this, a stealth option was added to the
   `flush` management command.

3. Expanded and re-factored [related] documentation a bit.

1. and 2. mean that now:

* Pre test case execution: There is no DB table flushing nor reset of
  sequences (the latter can be asked for the developer by using
  reset_sequences.

* Post test case execution: Flushing of DB tables is performed.

https://github.com/ramiro/django/compare/pr45_t18271

So far, I count two favorable from core devs: Karen and Anssi.

Any further feedback about the proposed changes is welcome.
Especially reports of results of people running their own test suites.

I ran the Django test suite w/o against Postgres, MySQL and sqlite3
w/o problems.

Hopefully I will be able to commit something along the lines soon.

Regards,

--
Ramiro Morales

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

Re: Test reordering and TransactionTestCase cleanup

[Karen Tracey]

On Mon, May 7, 2012 at 5:50 PM, Anssi Kääriäinen wrote:

> I would like to just get rid of the sequence resets. Oracle doesn't do
> it currently, TestCase doesn't do it, and IMO assuming the IDs are
> going to start from 1 is an assumption one should not make.
>
> Objections to just getting rid of the sequence resets altogether (with
> the opt-in flag)?
>

I did not realize Django code was doing something explicit to reset
sequences during the database flush between TransactionTestCases, I thought
it just happened as a side-effect of the method used to clear the DB. I
don't believe tests should be relying on pk values starting with 1, if
that's the only reason the sequence resets are there I think we should get
rid of them.

Karen

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

Re: Test reordering and TransactionTestCase cleanup

[Anssi Kääriäinen]

On May 8, 12:50 am, Anssi Kääriäinen  wrote:
> On May 5, 4:31 pm, Andreas Pelme  wrote:
>
>
>
>
>
>
>
>
>
> > > On May 4, 2:30 pm, Karen Tracey  
> > > wrote:
> > > Thanks for the link. While reading the previous threads I spotted at
> > > one blocker issue: the first TransactionTestCase will not start with
> > > zeroed database sequence values. To prevent this one would need to
> > > flush the DB before the first TransactionTestCase. In addition, if the
> > > goal is to get rid of test ordering, then one would need to flush
> > > before every test case which isn't good. Getting rid of the
> > > requirement to reset sequences between test cases would be good.
> > > Oracle doesn't reset the sequences reliably currently just for
> > > example. But I don't think we can change that for backwards
> > > compatibility reasons. While tests that rely on sequence values are
> > > IMHO broken, changing this would result in numerous broken tests for
> > > users upgrading Django. Notably normal TestCases do not do sequence
> > > resetting at all currently.
>
> > > This is a blocker: why change the flushing from pre-test to post-test
> > > if the end result is that you can't run the tests in arbitrary order
> > > anyways? Or if you want to be able to run them in arbitrary order you
> > > will need to run at least the sequence resets pre-test anyways.
> > > Resetting sequences can be really expensive, so there goes any
> > > potential savings in speed.
>
> > > Ideas?
>
> > Agreed, it is indeed backward incompatible. However, I think we can make 
> > the upgrade for those affected very anyways:
>
> >  * Document this change properly in the release notes and in the testing 
> > docs
> >  * Document that tests should not depend on hard coded primary key values 
> > (this is already hinted in the testing docs where 
> > TestCase/TransactionTestCase is described)
> >  * Add a reset_sequences attribute on TransactionTestCase, that can be 
> > applied to existing TransactionTestCase tests that already uses hard coded 
> > primary key value - which will reset sequences *before* the test run, just 
> > like before. (And those tests will also flush the database after they are 
> > run)
>
> > I have updated the pull request with the reset_sequences attribute and an 
> > initial draft for the release notes and 
> > documentation:https://github.com/django/django/pull/45
>
> > 1.1 introduced a far greater change to testing behavior (the TestCase with 
> > transaction/rollback), if that change was considered acceptable from a 
> > backward compatibility perspective, this change should be too. When that 
> > happened, the same problem appeared since TestCase does not reset 
> > sequences. (Yeah, just because it was done in the past does not make it OK 
> > now, but that should give some perspective).
>
> > I guess it boils down to where to draw the line for backward compatibility 
> > here. :-) I personally think it is acceptable since we can offer a very 
> > easy fix for the existing (already broken) tests out there.
>
> I would like to just get rid of the sequence resets. Oracle doesn't do
> it currently, TestCase doesn't do it, and IMO assuming the IDs are
> going to start from 1 is an assumption one should not make.
>
> Objections to just getting rid of the sequence resets altogether (with
> the opt-in flag)?
>
>  - Anssi

Just another idea: Maybe the right approach is to add flags to
TransactionTestCase (reset_sequences, track_state, track_fixtures,
more_ponies,...) and then recommend use of own subclasses in testing.
Something like:

class MyTestCase(TransactionTestCase):
reset_sequences = False
track_state = True # (flush only dirty tables).
track_fixtures = True # (Track if fixture data needs reloading
between tests).

class LoginLogouTests(MyTestCase):
fixtures = ['auth-user.json']
def test_login(self):
...

The defaults should be conservative (safe but slow). If there are
mysterious test case failures it will be easy to just comment out all
the flags to check if the test failures are explained by dirty state.
Similarly you could have "MySeleniumTestCase" subclass for selenium
tests.

This also allows deprecation of reset_sequences (or changing the
default from True to False) if we want to do so in the future.

It seems hard to provide subclasses in Django core, as that will lead
to combinatorial explosion down the road. Maybe mixins is the right
approach. I haven't used mixins based approaches, so I don't know how
easy it is to do the above using mixins.

 - Anssi

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

Re: Test reordering and TransactionTestCase cleanup

[Andreas Pelme]

On Saturday 5 May 2012 at 00:39, Anssi Kääriäinen wrote:
> On May 4, 2:30 pm, Karen Tracey  wrote:
> Thanks for the link. While reading the previous threads I spotted at
> one blocker issue: the first TransactionTestCase will not start with
> zeroed database sequence values. To prevent this one would need to
> flush the DB before the first TransactionTestCase. In addition, if the
> goal is to get rid of test ordering, then one would need to flush
> before every test case which isn't good. Getting rid of the
> requirement to reset sequences between test cases would be good.
> Oracle doesn't reset the sequences reliably currently just for
> example. But I don't think we can change that for backwards
> compatibility reasons. While tests that rely on sequence values are
> IMHO broken, changing this would result in numerous broken tests for
> users upgrading Django. Notably normal TestCases do not do sequence
> resetting at all currently.
>  
> This is a blocker: why change the flushing from pre-test to post-test
> if the end result is that you can't run the tests in arbitrary order
> anyways? Or if you want to be able to run them in arbitrary order you
> will need to run at least the sequence resets pre-test anyways.
> Resetting sequences can be really expensive, so there goes any
> potential savings in speed.
>  
> Ideas?

Agreed, it is indeed backward incompatible. However, I think we can make the 
upgrade for those affected very anyways:

 * Document this change properly in the release notes and in the testing docs
 * Document that tests should not depend on hard coded primary key values (this 
is already hinted in the testing docs where TestCase/TransactionTestCase is 
described)
 * Add a reset_sequences attribute on TransactionTestCase, that can be applied 
to existing TransactionTestCase tests that already uses hard coded primary key 
value - which will reset sequences *before* the test run, just like before. 
(And those tests will also flush the database after they are run)

I have updated the pull request with the reset_sequences attribute and an 
initial draft for the release notes and documentation:
https://github.com/django/django/pull/45

1.1 introduced a far greater change to testing behavior (the TestCase with 
transaction/rollback), if that change was considered acceptable from a backward 
compatibility perspective, this change should be too. When that happened, the 
same problem appeared since TestCase does not reset sequences. (Yeah, just 
because it was done in the past does not make it OK now, but that should give 
some perspective).

I guess it boils down to where to draw the line for backward compatibility 
here. :-) I personally think it is acceptable since we can offer a very easy 
fix for the existing (already broken) tests out there.


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.

Re: Test reordering and TransactionTestCase cleanup

[Andreas Pelme]

On Friday 4 May 2012 at 13:30, Karen Tracey wrote:
> On Fri, May 4, 2012 at 4:46 AM, Andreas Pelme  (mailto:andr...@pelme.se)> wrote:
> > 
> > That's a good question. Anyone who wrote to original 
> > TransactionTestCase/reordering implementation that wants to chime in? :-)
> 
> I worked on the test speedups that introduced TransactionTestCase and
> I added the re-ordering, but the behavior of database flush being done
> at the beginning of a test existed before that work. The conversation
> that led to addition of the reordering is here:
> 
> http://groups.google.com/group/django-developers/browse_thread/thread/1e4f4c840b180895/
> 
> doctests (which we have no more?) play a prominent role in that
> discussion. While we have gotten rid of doctests in Django's own
> suite, we still support apps which may use doctests in their code, so
> anything we do to change when the DB is cleared needs to take that
> into account. On the table then was the idea of adding cleanup after
> doctests, possibly that would need to be re-considered if you want to
> move the database clearing to the end of everything rather than the
> beginning of TransactionTestCase.


#12408 is related here, which is caused by TransactionTestCase's not cleaning 
up after themselves. Rather than changing the test order to TestCase -> 
doctests -> TransactionTestCase I think the solution below is better.

Karens proposed a fix for this (#2) in the above thread [1], it basically says:

1) Make TransactionTestCase clean up after itself
2) Order the test suite to make sure doctests are always run *last*

That would run all unittest-style test cases first (the order of TestCase / 
TransactionTestCase will not be important), and last, all doctests.

A test suite that have doctests that *expects* state to be left from a 
TransactionTestCase before it would be broken by this change. In my opinion, a 
test suite with such expectations is already very broken and needs to be fixed 
any way, and should not be considered a blocker to this change (in that case, 
#12408 is a blocker too, since that effect will be the same).

I have updated my pull request to make unittest.TestCase subclasses to run 
first (i.e. doctests will run after all unit tests):
https://github.com/django/django/pull/45

Am I missing something? This seems like the most reliable way to solve this to 
me.

(I originally discovered this when I tried to run my tests with an alternative 
test runner, which does not order the tests in any particular way. I will still 
not be able to run doctests that does not clean up after themselves in 
arbitrary order, but as long as I document that's not supported with my test 
runner, I don't consider that to be a problem.)

Andreas

[1] https://groups.google.com/d/msg/django-developers/Hk9MhAsYCJU/fB4rj7F5SXEJ

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

Re: Test reordering and TransactionTestCase cleanup

[Anssi Kääriäinen]

On May 4, 2:30 pm, Karen Tracey  wrote:
> On Fri, May 4, 2012 at 4:46 AM, Andreas Pelme  wrote:
>
> > That's a good question. Anyone who wrote to original 
> > TransactionTestCase/reordering implementation that wants to chime in? :-)
>
> I worked on the test speedups that introduced TransactionTestCase and
> I added the re-ordering, but the behavior of database flush being done
> at the beginning of a test existed before that work. The conversation
> that led to addition of the reordering is here:
>
> http://groups.google.com/group/django-developers/browse_thread/thread...
>
> doctests (which we have no more?) play a prominent role in that
> discussion. While we have gotten rid of doctests in Django's own
> suite, we still support apps which may use doctests in their code, so
> anything we do to change when the DB is cleared needs to take that
> into account. On the table then was the idea of adding cleanup after
> doctests, possibly that would need to be re-considered if you want to
> move the database clearing to the end of everything rather than the
> beginning of TransactionTestCase.

Thanks for the link. While reading the previous threads I spotted at
one blocker issue: the first TransactionTestCase will not start with
zeroed database sequence values. To prevent this one would need to
flush the DB before the first TransactionTestCase. In addition, if the
goal is to get rid of test ordering, then one would need to flush
before every test case which isn't good. Getting rid of the
requirement to reset sequences between test cases would be good.
Oracle doesn't reset the sequences reliably currently just for
example. But I don't think we can change that for backwards
compatibility reasons. While tests that rely on sequence values are
IMHO broken, changing this would result in numerous broken tests for
users upgrading Django. Notably normal TestCases do not do sequence
resetting at all currently.

This is a blocker: why change the flushing from pre-test to post-test
if the end result is that you can't run the tests in arbitrary order
anyways? Or if you want to be able to run them in arbitrary order you
will need to run at least the sequence resets pre-test anyways.
Resetting sequences can be really expensive, so there goes any
potential savings in speed.

Ideas?

 - Anssi

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

Re: Test reordering and TransactionTestCase cleanup

[Karen Tracey]

On Fri, May 4, 2012 at 4:46 AM, Andreas Pelme  wrote:
>
> That's a good question. Anyone who wrote to original 
> TransactionTestCase/reordering implementation that wants to chime in? :-)

I worked on the test speedups that introduced TransactionTestCase and
I added the re-ordering, but the behavior of database flush being done
at the beginning of a test existed before that work. The conversation
that led to addition of the reordering is here:

http://groups.google.com/group/django-developers/browse_thread/thread/1e4f4c840b180895/

doctests (which we have no more?) play a prominent role in that
discussion. While we have gotten rid of doctests in Django's own
suite, we still support apps which may use doctests in their code, so
anything we do to change when the DB is cleared needs to take that
into account. On the table then was the idea of adding cleanup after
doctests, possibly that would need to be re-considered if you want to
move the database clearing to the end of everything rather than the
beginning of TransactionTestCase.

Karen

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

Re: Test reordering and TransactionTestCase cleanup

[Anssi Kääriäinen]

On May 4, 10:09 am, Andreas Pelme  wrote:
> Here's the ticket:
>
> https://code.djangoproject.com/ticket/18271
>
> … and an initial patch:
>
> https://github.com/django/django/pull/45

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.
2. Is this backwards incompatible? The behavior was documented.
3. Is there some reason why the flush must be done pre-test which
we are overlooking. Why was it implemented that way originally?
Perhaps because of the first item?

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.

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?

 - Anssi

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

Re: Test reordering and TransactionTestCase cleanup

[Andreas Pelme]

On Thursday 3 May 2012 at 19:23, Anssi Kääriäinen wrote:
> On May 3, 7:29 pm, Andreas Pelme  wrote:
> > I am trying to run my Django test suite with an alternative test runner 
> > (py.test), and found some issues with test isolation.
> >  
> > TransactionTestCase does currently not clean up after itself (i.e. flush 
> > the database), but instead assumes that the next test will flush the 
> > database. It is generally a good idea to restore the state after the test 
> > run, and let other tests start out with a known state.
> >  
> > Djangos default testrunner reorders the test suite to run all TestCase 
> > tests before TransactionTestCases, which avoids this problem. I cannot find 
> > this reordering documented anywhere, or even commented anywhere in the 
> > implementation (test/simple.py: reorder_suite and 
> > DjangoTestSuiteRunner.build_suite).
> >  
> > I propose to move the flush command out of _fixture_setup to _post_teardown 
> > in TransactionTestCase. This makes it possible for arbitrary execution 
> > order of the test suite. Performance could potentially be an issue, since 
> > it is "lazy" today and flushes the database just right before it is needed. 
> > That would however only affect the last test in the run (i.e. it will 
> > truncate the tables, and then the database itself will be teared down).
> >  
> > Are there any objections or anything that might break with such a change? 
> > Are there any specific reasons it works the way it does?
>  
> I don't know if anything will break...
>  
> I think this is a good idea. In addition to the above issues this
> would allow the test ran tell the flushing which tables are dirty,
> which not. Currently the transaction_regress for example uses just a
> single table. But after each test, all tables will be truncated, and
> the contenttypes and permissions will be reloaded. I bet we could
> speed up the testing _a lot_ if the cleaning up could be made more
> intelligent. Making it robust is somewhat hard but should not be
> impossible. Of course, the "StateTrackingTestCase" should not be done
> in the same patch at all... just a possibility for future work.


There are indeed some work that can be done to speed up the test running. :)

Here's the ticket:
  
https://code.djangoproject.com/ticket/18271  

… and an initial patch:

https://github.com/django/django/pull/45

Is there any sane way to test this except to run the test suite itself?

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.

Re: Test reordering and TransactionTestCase cleanup

[Andreas Pelme]

On Thursday 3 May 2012 at 22:14, Ramiro Morales wrote:
> On Thu, May 3, 2012 at 1:29 PM, Andreas Pelme  (mailto:andr...@pelme.se)> wrote:
> > I am trying to run my Django test suite with an alternative test runner 
> > (py.test), and found some issues with test isolation.
> > 
> > TransactionTestCase does currently not clean up after itself (i.e. flush 
> > the database), but instead assumes that the next test will flush the 
> > database.
> > [...]
> > 
> > Djangos default testrunner reorders the test suite to run all TestCase 
> > tests before TransactionTestCases, which avoids this problem.
> 
> Just a quick related note
> 
> There is a [1]proposal to extend this reordering to be:
> 
> * TestCases
> * doctests
> * TransactionTestCases
> 
> So doctests aren't affected either.
> 
> I intend to commit a fix for it soon.
> 
> -- 
> Ramiro Morales
> 
> 1. https://code.djangoproject.com/ticket/12408

This won't be an issue if TransactionTestCase resets the database after itself.

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.

Re: Test reordering and TransactionTestCase cleanup

[Andreas Pelme]

On Thursday 3 May 2012 at 21:17, Karen Tracey wrote:
> On Thu, May 3, 2012 at 12:29 PM, Andreas Pelme  (mailto:andr...@pelme.se)> wrote:
> > 
> > Djangos default testrunner reorders the test suite to run all TestCase 
> > tests before TransactionTestCases, which avoids this problem. I cannot find 
> > this reordering documented anywhere,
> 
> It is documented:
> https://docs.djangoproject.com/en/1.4/topics/testing/#django.test.TransactionTestCase
> 
> Mentioned in both the 3rd paragraph and the note.
Thanks -- I'm not sure how I missed that. I'll make sure my patch updates the 
docs too.

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.

Re: Test reordering and TransactionTestCase cleanup

[Ramiro Morales]

On Thu, May 3, 2012 at 1:29 PM, Andreas Pelme  wrote:
> I am trying to run my Django test suite with an alternative test runner 
> (py.test), and found some issues with test isolation.
>
> TransactionTestCase does currently not clean up after itself (i.e. flush the 
> database), but instead assumes that the next test will flush the database.
> [...]
>
> Djangos default testrunner reorders the test suite to run all TestCase tests 
> before TransactionTestCases, which avoids this problem.

Just a quick related note

There is a [1]proposal to extend this reordering to be:

* TestCases
* doctests
* TransactionTestCases

So doctests aren't affected either.

I intend to commit a fix for it soon.

-- 
Ramiro Morales

1. https://code.djangoproject.com/ticket/12408

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

Re: Test reordering and TransactionTestCase cleanup

[Karen Tracey]

On Thu, May 3, 2012 at 12:29 PM, Andreas Pelme  wrote:
>
> Djangos default testrunner reorders the test suite to run all TestCase tests 
> before TransactionTestCases, which avoids this problem. I cannot find this 
> reordering documented anywhere,

It is documented:
https://docs.djangoproject.com/en/1.4/topics/testing/#django.test.TransactionTestCase

Mentioned in both the 3rd paragraph and the note.

Karen

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

Re: Test reordering and TransactionTestCase cleanup

[Anssi Kääriäinen]

On May 3, 7:29 pm, Andreas Pelme  wrote:
> I am trying to run my Django test suite with an alternative test runner 
> (py.test), and found some issues with test isolation.
>
> TransactionTestCase does currently not clean up after itself (i.e. flush the 
> database), but instead assumes that the next test will flush the database. It 
> is generally a good idea to restore the state after the test run, and let 
> other tests start out with a known state.
>
> Djangos default testrunner reorders the test suite to run all TestCase tests 
> before TransactionTestCases, which avoids this problem. I cannot find this 
> reordering documented anywhere, or even commented anywhere in the 
> implementation (test/simple.py: reorder_suite and 
> DjangoTestSuiteRunner.build_suite).
>
> I propose to move the flush command out of _fixture_setup to _post_teardown 
> in TransactionTestCase. This makes it possible for arbitrary execution order 
> of the test suite. Performance could potentially be an issue, since it is 
> "lazy" today and flushes the database just right before it is needed. That 
> would however only affect the last test in the run (i.e. it will truncate the 
> tables, and then the database itself will be teared down).
>
> Are there any objections or anything that might break with such a change? Are 
> there any specific reasons it works the way it does?

I don't know if anything will break...

I think this is a good idea. In addition to the above issues this
would allow the test ran tell the flushing which tables are dirty,
which not. Currently the transaction_regress for example uses just a
single table. But after each test, all tables will be truncated, and
the contenttypes and permissions will be reloaded. I bet we could
speed up the testing _a lot_ if the cleaning up could be made more
intelligent. Making it robust is somewhat hard but should not be
impossible. Of course, the "StateTrackingTestCase" should not be done
in the same patch at all... just a possibility for future work.

 - Anssi

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

Test reordering and TransactionTestCase cleanup

[Andreas Pelme]

I am trying to run my Django test suite with an alternative test runner 
(py.test), and found some issues with test isolation.

TransactionTestCase does currently not clean up after itself (i.e. flush the 
database), but instead assumes that the next test will flush the database. It 
is generally a good idea to restore the state after the test run, and let other 
tests start out with a known state.

Djangos default testrunner reorders the test suite to run all TestCase tests 
before TransactionTestCases, which avoids this problem. I cannot find this 
reordering documented anywhere, or even commented anywhere in the 
implementation (test/simple.py: reorder_suite and 
DjangoTestSuiteRunner.build_suite).

I propose to move the flush command out of _fixture_setup to _post_teardown in 
TransactionTestCase. This makes it possible for arbitrary execution order of 
the test suite. Performance could potentially be an issue, since it is "lazy" 
today and flushes the database just right before it is needed. That would 
however only affect the last test in the run (i.e. it will truncate the tables, 
and then the database itself will be teared down).

Are there any objections or anything that might break with such a change? Are 
there any specific reasons it works the way it does?

I will be happy to create a ticket and patch for this, I would get to input in 
case I miss something obvious.

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