#8138: Switch django tests to use transactions
----------------------------------------+-----------------------------------
Reporter: mremolt | Owner: nobody
Status: new | Milestone: post-1.0
Component: Testing framework | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
----------------------------------------+-----------------------------------
Comment (by kmtracey):
I've looked at this a little more and figured out a few things.
First, it never seemed well-understood from the mailing list discussion
why tests that used the test Client had to continue to use the
flush/reload and even then there were failures. The problem is that
`django.test.client.ClientHandler`'s `__call__` sends the
`request_finished` signal, which is connected (in `django.db.__init__`) to
`close_connection`, which severs the connection to the database. So if a
test POSTed via the test Client in order to login, for example, the
session changes made during the POST were rolled back when the POST's
`request_finished` signal severed the DB connection apparently mid-
transaction, and subsequent stuff done by the testcase that relied on
being logged in would fail.
Disconnecting `close_connection` from from `request_finished` solves this
problem, so that tests that use the test Client do not automatically need
to use the old way, and may take advantage of the rollback approach. In
the patch I'll attach the signal is disconnected/reconnected around the
sending of the signal...this is probably overkill. I think it would be OK
to just disconnect `close_connection` from that signal once and be done
with it but I haven't had time to go back and experiment with that since I
tried the first way.
Second, having the fixture teardown code attempt to detect whether a
commit had been performed by the test code (despite the test case being a
type that supposedly didn't use transactions) and fallback to flush/reload
didn't seem to be working too well. I tried tracking down a few of the
failures caused by these and eventually decided to try a different
approach, which is: if it's a test case that says it doesn't need
transactions to work, monkey patch the transaction commit/rollback and
enter/leave transaction management routines so that even if they are
called by the code being tested, they do nothing, for the duration of the
test.
I don't know how the monkey patch idea will be received, but this approach
seems to work better. Doing it this way, none of the existing Django
tescases need to use the old flush/reload approach. That is, the tests
that do actually call commit or rollback (like admin_views, which call
admin views that are wrapped in commit_on_success) don't actually need
those calls to do anything in order for the test to properly test what it
is trying to test. Things work fine when those routines don't do
anything, and the test fixture teardown is able to reliably un-do what the
test has done, leaving things clean for the next test.
Which is not to say there were no remaining failures, but what was left
was somewhat easier to deal with. First and easiest there were a few
tests that assume objects they created would be assigned pk's starting
with 1. This is an invalid assumption when rollback is being used to un-
do the test effects since autoincrement sequence numbers are not
necessarily reset (sqlite seems to reset them, the others don't). These
were pretty easy to modify to retrieve and use the pk instead of assuming
1, 2, etc.
Second was a failure in the views test resulting from the Site's
SITE_CACHE not being cleared during fixture loading. The flush command
sends out the post-syncdb signal, which Sites connects to to initialize a
default site and clear the cache, so this SITE_CACHE was always clear on
test start using the old approach. With the rollback approach, the views
test would pass if it was run by itself, but would fail if run with
comment_tests, which apparently does something to cause SITE_CACHE to get
an entry for site 1. The value (default since comment_tests doesn't load
anything into the Site model) didn't match what was expected by views
(which loads a Site via a fixture, with values different from the
default). In the patch I'll attach this is fixed by calling the Site's
clear_cache function during fixture loading. Not particularly elegant and
it points out a subtle difference between the old and new approaches --
but this is the only problem I have found resulting from that difference.
Third and one I haven't entirely figured out is this failure, only with
PostgreSQL, in the fixtures test:
{{{
======================================================================
FAIL: Doctest: modeltests.fixtures.models.__test__.API_TESTS
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/kmt/tmp/django/trunk/django/test/_doctest.py", line 2180, in
runTest
raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for
modeltests.fixtures.models.__test__.API_TESTS
File "/home/kmt/tmp/django/trunk/tests/modeltests/fixtures/models.py",
line unknown line number, in API_TESTS
----------------------------------------------------------------------
File "/home/kmt/tmp/django/trunk/tests/modeltests/fixtures/models.py",
line ?, in modeltests.fixtures.models.__test__.API_TESTS
Failed example:
management.call_command('dumpdata', 'fixtures', format='json')
Expected:
[{"pk": 3, "model": "fixtures.article", "fields": {"headline": "Time
to reform copyright", "pub_date": "2006-06-16 13:00:00"}}, {"pk": 2,
"model": "fixtures.article", "fields": {"headline": "Poker has no place on
ESPN", "pub_date": "2006-06-16 12:00:00"}}, {"pk": 1, "model":
"fixtures.article", "fields": {"headline": "Python program becomes self
aware", "pub_date": "2006-06-16 11:00:00"}}]
Got:
[{"pk": 3, "model": "fixtures.article", "fields": {"headline": "Time
to reform copyright", "pub_date": "2006-06-16 14:00:00"}}, {"pk": 2,
"model": "fixtures.article", "fields": {"headline": "Poker has no place on
ESPN", "pub_date": "2006-06-16 13:00:00"}}, {"pk": 1, "model":
"fixtures.article", "fields": {"headline": "Python program becomes self
aware", "pub_date": "2006-06-16 12:00:00"}}]
----------------------------------------------------------------------
Ran 2 tests in 25.922s
FAILED (failures=1)
Destroying test database...
}}}
The difference is in the times, they are an hour off. Luke also mentioned
seeing this on the mailing list, only his times were off by 6 hours. I'd
guess Luke's real time zone was 6 hours off from 'America/Chicago' while I
am 1 hour off from 'American/Chicago'. I can make this failure go away by
setting the TIME_ZONE setting to match my real time zone, but I haven't
figured out why this is necessary only when using the rollback approach --
how did the times workout properly without a TIME_ZONE setting in the
flush/reload approach?
I think that was it for problems encountered. With this approach the test
suite runs about 6x faster on sqlite, 8-12x faster on MySQL/InnoDB,
PostgreSQL, and Oracle, on my (oldish to antique) machines, with no
failures other than what I see using the current trunk code (InnoDB's
failures are well-known, I think, I also see a PostgreSQL failure related
to #9758/#9006 with both old and new test approaches, and my Oracle
install on a Windows box has a quartet of failures that may be related to
Windows file and charset issues that I haven't bothered to track down
since I barely know how to spell Oracle).
That leaves MySQL/MyISAM, which of course fails miserably with the
rollback approach since its rollback doesn't actually do anything. I
added a routine that attempts to detect whether rollback works in the test
DB, and if it does not the fixture setup/teardown falls back to the old
way. It occurs to me now that I didn't do anything to deal with the
!DocTest runner's rollback not working (nor does the !DocTest runner
monkey patch the transaction routines)...there's probably more somebody
needs to understand about the !DocTest runner with the new approach, but
I've run out of time to look at this for a few days probably so if anybody
else has time to look at it go ahead. I also suspect the "does rollback
work" routine could be done more elegantly; what I've put in is pretty
much a quick hack to try to get tests basically working with the patch on
MySQL/MyISAM.
--
Ticket URL: <http://code.djangoproject.com/ticket/8138#comment:18>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---
