Testing speedup checked in

[Karen Tracey]

r9756 contains the changes to make django.test.TestCase use transaction rollback (where available) instead of database flush/reload for resetting the database.  This can provide a nice speedup when running large test suites -- the Django/sqlite buildbot went from around 20 minutes for the test run to 3, which is pretty cool.  Other backends see even better results (excepting MySQL/MyISAM which doesn't do transactions and therefore sees no improvement).

For the most part this change should be invisible, except for the speed improvement when running tests.  However, if you have tests that need to actually test the effects of transaction commit or rollback, you will now need to use a django.test.TransactionTestCase instead of a django.test.TestCase.  Also, in some cases the change can reveal previously-unnoticed test case flaws.  For example in some Django tests assumptions were made that primary key values would be assigned starting at one, which isn't true for all backends when rollback is used to reset the test database.  Such problems are usually easily fixed, but a quick fix, if changing the test case code is not immediately possible, is to just switch the test to be a TransactionTestCase. 

The doc for the changes is here: http://docs.djangoproject.com/en/dev/topics/testing/#testcase

I don't know of any existing problems with the changes.  If you observe any please let me know and/or open a ticket.  Though I tried to test as many backends/platforms as I could I doubt I've covered every possible combination.

Many people contributed to getting this done:

- Marc Remolt proposed the idea initially and provided a first implementation.
- Luke Plant made some improvements as the result of initial testing.
- Ramiro Morales helped me track down the last mysterious error introduced by the change.
- Eric Holscher provided feedback regarding the effect of the change on the Ellington testsuite (faster and worth the possible test hiccups).
- Russell Keith-Magee provided guidance and feedback from initial proposal through the end.
- Hopefully there's no one I forgot to list -- if so my apologies.

Thanks to all who have helped!

Karen

[Ivan Sagalaev]

Karen Tracey wrote:
> For the most part this change should be invisible, except for the speed
> improvement when running tests. However, if you have tests that need to
> actually test the effects of transaction commit or rollback, you will
> now need to use a django.test.TransactionTestCase instead of a
> django.test.TestCase.

Just ran the new code with our tests and about 2/3 of them became
broken. My first hypothesis is it's caused by testing views with
self.client. We use TransactionMiddleware that explicitly commits
transaction on view's successful return. So the first test that does
"self.client.get(...)" leaves fixtures data in the database and all
subsequent attempts to load the same fixtures are failing.

Now I'm thinking about somehow disabling TransactionMiddleware from
within test environment. But is seems hacky...

[Ivan Sagalaev]

Sorry, wrote too soon. Disabling TransactionMiddleware didn't help. I'll
investigate further.

[Karen Tracey]

On Fri, Jan 16, 2009 at 7:47 AM, Ivan Sagalaev <man...@softwaremaniacs.org> wrote:

Karen Tracey wrote:
> For the most part this change should be invisible, except for the speed
> improvement when running tests.  However, if you have tests that need to
> actually test the effects of transaction commit or rollback, you will
> now need to use a django.test.TransactionTestCase instead of a
> django.test.TestCase.

Just ran the new code with our tests and about 2/3 of them became
broken. My first hypothesis is it's caused by testing views with
self.client. We use TransactionMiddleware that explicitly commits
transaction on view's successful return. So the first test that does
"self.client.get(...)" leaves fixtures data in the database and all
subsequent attempts to load the same fixtures are failing.

Hmm, not sure what is going on here.  Django's admin_views testscase uses client and fixtures, and doesn't have a problem.  It should not happen that data loaded by one TestCase persists in the database to another test.  TestCase setup starts a transaction and disables access to the transaction routines, so commits done by the called code don't actually get committed.  TestCase cleanup rolls back the transaction started by TestCase setup.  So I'm concerned how you are getting bleed over from one test to the next...that shouldn't happen.

Karen

[Ivan Sagalaev]

Karen Tracey wrote:
> Hmm, not sure what is going on here. Django's admin_views testscase
> uses client and fixtures, and doesn't have a problem. It should not
> happen that data loaded by one TestCase persists in the database to
> another test. TestCase setup starts a transaction and disables access
> to the transaction routines, so commits done by the called code don't
> actually get committed. TestCase cleanup rolls back the transaction
> started by TestCase setup. So I'm concerned how you are getting bleed
> over from one test to the next...that shouldn't happen.

I've found it. My fault, as usually :-). But an interesting corner case
too.

We're using mysql_replicated db backend that basically has two
connections. The first ("master") is active by default and is used when
loading fixtures. Then a test calls a db-read-only view and all its
interactions with the db go intto another ("slave") connection which
doesn't see any data changed in the master because it didn't commit.
Thus a test fails because the db is absolutely empty.

Now I'm going to implement a switch for the backend ensuring that only
one connection is used and it'll be used in test environment.

[Alex Gaynor]

I'm seeing some failures exclusively under PgSQL: http://dpaste.com/109831/ .  I'm not sure what's causing them, but it's just the admin views tests, no other ones.

Alex


--
"I disapprove of what you say, but I will defend to the death your right to say it." --Voltaire
"The people's good is the highest law."--Cicero

[Karen Tracey]

On Fri, Jan 16, 2009 at 9:15 AM, Alex Gaynor <alex....@gmail.com> wrote:

I'm seeing some failures exclusively under PgSQL: http://dpaste.com/109831/ .  I'm not sure what's causing them, but it's just the admin views tests, no other ones.

(PgSQL is PostgreSQL ... ?)

I see those too, on my PostgreSQL test machine.  But I saw them before the test rollback changes as well.  When I looked into them I concluded they were related to #9006/#9758.  They all involve attempts to update a page with multiple inline-edited objects, and it seems the DB returns the objects in a different order on the POST than it did when the form was originally created on the GET, which results in the admin/form code failing to match up the actual instances to the form data, resulting in redisplay of the form with "Please correct the errors below" (and status code of 200) instead of the expected redirect from a successful form post.

For me, these were not introduced by the test rollback changes, they were pre-existing problems that I haven't had time to look into much more closely than what I've written above and in one of those tickets.  Are you only seeing these after r9756? 

Karen

[Alex Gaynor]

PgSQL is indeed PostgreSQL.  Yes, I do still get the failures pre-fast tests, I guess I'd never run those tests.  I would like to say that when I went back to test that the tests were unbearably slow, so that makes me super happy about the new fast tests, thanks for all your hard work.

[Karen Tracey]

On Fri, Jan 16, 2009 at 9:13 AM, Ivan Sagalaev <man...@softwaremaniacs.org> wrote:

I've found it. My fault, as usually :-). But an interesting corner case
too.

We're using mysql_replicated db backend that basically has two
connections. The first ("master") is active by default and is used when
loading fixtures. Then a test calls a db-read-only view and all its
interactions with the db go intto another ("slave") connection which
doesn't see any data changed in the master because it didn't commit.
Thus a test fails because the db is absolutely empty.

Now I'm going to implement a switch for the backend ensuring that only
one connection is used and it'll be used in test environment.

Yes, that's an interesting case, and certainly not one covered in my testing.

If you actually want to test that your master/slave setup is working and doing all the right stuff with commits getting shadowed to the slave, a TransactionTestCase should work for that.  But for the bulk of your tests that are actually testing other stuff, limiting the test to using one connection sounds like a good idea.

Karen

[Karen Tracey]

On Fri, Jan 16, 2009 at 9:43 AM, Alex Gaynor <alex....@gmail.com> wrote:

PgSQL is indeed PostgreSQL.  Yes, I do still get the failures pre-fast tests, I guess I'd never run those tests.  I would like to say that when I went back to test that the tests were unbearably slow, so that makes me super happy about the new fast tests, thanks for all your hard work.

Yeah, one consequence of the new faster tests may be people start running the full suite more often and notice lurking errors that have been out there but just unseen.  The machines where I have Oracle and PostgreSQL set up are old and slow and took 8+ hours to run the full Django suite prior to these changes, meaning I would really never run the full suite for those backends.  Now I can run them in an hour or so -- still not real speedy but it's at least feasible to run the full suite.

Sadly, only a few of my test setups pass completely clean.  I see the errors you pointed out on PostgreSQL, others related to referential integrity & fixtures on InnoDB, some Windows temp file problems, some sqlite problems related to a specific sqlite/Python level, some Oracle errors that I think are something wrong in my DB setup, etc.  On my wish list is getting some of these resolved, but I haven't had time to focus on that yet.  What I did for the rollback test changes was manually verify that any errors/failures I saw with the changes were identical to what I saw without them. 

I do think all the problems I see (with the exception of the Oracle ones which I suspect are database setup related, but I've not had a chance to investigate much there) are either known with open tickets or known unfixable (e.g. sqlite bug for a specific version).  It might be nice if we had a way to flag such things as "expected to fail (at the moment, in this configuration)" so that it's easier to run the full suite on a wide range of configurations and get a quick confirmation from the output that the results are as expected, even if there are known failures in some cases at the moment.  I think I've seen this idea discussed before...but again I haven't had much time to look into it yet.

Karen

[Matt Boersma]

As another data point, our nightly run of the test suite against
Oracle just completed. We see the same test failures we had
previously--nothing new--but it was *much* faster as hoped.

Python 2.5, Django 1.0.x: 24510 secs.
Python 2.5: Django trunk: 3024 secs.

Previously, trunk took about ten minutes longer than the 1.0.x
branch. (This is on a puny Pentium 3 box with 512MB RAM running both
Oracle 10g and the django tests, so don't fret about the absolute
times.)

So let me thank profusely Karen and everyone else who pitched in on
this change! It will make a very practical difference in helping Ian
Kelly and I to keep the Oracle backend in shape.

Matt


On Jan 16, 8:17 am, "Karen Tracey" <kmtra...@gmail.com> wrote:

[mrts]

Great thanks and praises to everybody involved!

Getting down from 1346 to 204 seconds is such a relief...