Thanks for picking up on this one, Karen. I thought I would have time
to look at this, but life conspired against me.
> So, the latest patch on #8183 (8138alternate-nodoctestxaction.diff) does not
> enclose doctest runs in a rolled-back transaction. Thus the effects of
> doctests can still bleed over into subsequent tests. This doesn't actually
> cause any failures or errors when running the Django test suite, as far as
> I've seen. I'm inclined to think it's an OK alternative at least for the
> short term, but would be interested in hearing other opinions.
I can't say this concerns me particularly. Doctests are nice and all,
but they aren't a particularly sophisticated testing option. IMHO if
you have complex testing requirements, you should probably be looking
at moving beyond doctests.
> (Longer term
> I'm thinking perhaps something along the lines of Joseph's patch on #5624
> for specifying doctest fixtures could be developed into a general mechanism
> for a doctest to specify some operational needs including fixtures,
> transactional control, isolation...or that may be overkill.)
This is always an option, but it's an option we can add later if the
need arises.
> Some other to-dos I have on my list:
>
> 1 - Documentation is currently completely lacking.
> 2 - Think some more about TransactionTestCase. This isn't used by any of
> the Django tests but I think needs to be provided in case anyone has tests
> that really do need to make use of transactions internally. I think,
> though, there is currently a bit of a problem with mixing
> TransactionTestCases with TestCases since the former clears the DB on entry
> but not afterward, so its effects are not cleaned up, while TestCases assume
> the DB is clean on entry.
I agree that the TransactionTestCase needs to be provided, even if we
aren't using it.
The mixing of TransactionTestCase and TestCase doesn't worry me so
much; using doctests and TestCase requires that you are aware of the
way that they interact; this just introduces a different set of
expectations that we need to manage.
For me, a bigger concern is the change in behaviour of TestCase.
Moving to a transaction-based TestCase needs to be a zero-impact
operation, but under the current patch, it isn't. With the existing
code, a TestCase could assume that the database was clean at the start
of each test; this isn't the case after applying this patch.
> 3 - Anything else anyone notices?
I've seen the messages go back and forth about the timezone problem,
but the solution you have found makes me a little nervous. Modifying
the behavior of normal runtime code to fix test behavior strikes me as
the wrong thing to do. In particular, this modification means that
under Postgres, there is no longer any such thing as a passive read
operation on the database (e.g., retrieving a list of blog entries).
Every database connection will force a commit.
I'm not sure I have a better solution, but you asked if there was
anything I noticed :-)
Yours,
Russ Magee %-)
> So, the latest patch on #8183 (8138alternate-nodoctestxaction.diff) does notI can't say this concerns me particularly. Doctests are nice and all,
> enclose doctest runs in a rolled-back transaction. Thus the effects of
> doctests can still bleed over into subsequent tests. This doesn't actually
> cause any failures or errors when running the Django test suite, as far as
> I've seen. I'm inclined to think it's an OK alternative at least for the
> short term, but would be interested in hearing other opinions.
but they aren't a particularly sophisticated testing option. IMHO if
you have complex testing requirements, you should probably be looking
at moving beyond doctests.
[snip]
>2 - Think some more about TransactionTestCase. This isn't used by any ofI agree that the TransactionTestCase needs to be provided, even if we
> the Django tests but I think needs to be provided in case anyone has tests
> that really do need to make use of transactions internally. I think,
> though, there is currently a bit of a problem with mixing
> TransactionTestCases with TestCases since the former clears the DB on entry
> but not afterward, so its effects are not cleaned up, while TestCases assume
> the DB is clean on entry.
aren't using it.
The mixing of TransactionTestCase and TestCase doesn't worry me so
much; using doctests and TestCase requires that you are aware of the
way that they interact; this just introduces a different set of
expectations that we need to manage.
For me, a bigger concern is the change in behaviour of TestCase.
Moving to a transaction-based TestCase needs to be a zero-impact
operation, but under the current patch, it isn't. With the existing
code, a TestCase could assume that the database was clean at the start
of each test; this isn't the case after applying this patch.
I've seen the messages go back and forth about the timezone problem,
> 3 - Anything else anyone notices?
but the solution you have found makes me a little nervous. Modifying
the behavior of normal runtime code to fix test behavior strikes me as
the wrong thing to do. In particular, this modification means that
under Postgres, there is no longer any such thing as a passive read
operation on the database (e.g., retrieving a list of blog entries).
Every database connection will force a commit.
I'm not sure I have a better solution, but you asked if there was
anything I noticed :-)
Yeah, I agree. doctests are the 80% testing tool; if you need more
control, that's what formal test cases are for.
> Of the three #2 sounds most attractive to me, but I have no idea if it is
> feasible.
I also agree, and I *think* it ought to be possible by modifying the
way the test runner is constructed during discovery. Failing that, I
think keeping TestCase as it is in 1.0 and introducing a new
FasterTestCase would be the better approach -- that way people have to
opt-in to the new behavior, and can decide if the caveats are worth
it.
Jacob
#3 sounds like the nicest option to me. #1 introduces even more
slowdowns caused by flush operations; #2 might be possible, but I
suspect it will be a complex solution to implement. #3 is a simple
approach to implement, easy to explain, and opt-in for any existing
usage, so it should be zero impact.
If I'm wrong on the complexity of #2, it might be preferable, but I
don't think it will be easy.
Russ %-)
Each test should be a unit, and should be able to be invoked
independently. If one test is depending on another test being executed
first, that's a problem with the test case that can be provoked right
now by running
$ manage test app.OrderDependentTestCase
so the problem with our test suite needs to be fixed, regardless.
As for making the whole thing opt-in, I'm of two minds. Introducing
potential test failures (however legitimate) as part of an upgrade to
the test system is something I'd rather avoid doing. On the other
hand, this will only affect people that have pre-existing problems
with their test suite (just problems they don't know about), and the
performance improvement is a huge boost that it seems a pity to
relegate it to sitting in the corner until it gets noticed.
I suppose we can manage this with a release note highlighting that
v1.1 will highlight any order dependent tests. Affected users can
replace TestCase with TransactionTestCase to make the problem go away,
or they can fix their tests.
Russ %-)
Some weeks ago I wanted to write an "always_rollback" decorator, but found
that it was not that easy, since Postgres transactions can't be nested. The
first COMMIT commits everything. [1]
I want to run readonly unittests on my live data. I looked at your
patch, and
saw the way you do it: you redefine the transaction methods to "nop".
Are other developers interested in "readonly unitests on live data", too?
Of course you the tests must not alter data on the filesystem something
like this.
Thomas
[1] http://www.mail-archive.com/django...@googlegroups.com/msg58024.html
http://www.nnseek.com/e/comp.databases.postgresql/nested_transactions_49494491t.html
Karen Tracey schrieb:
> One of the item on the list for 1.1 is "Run Django test cases inside a
> transaction". The ticket for this is #8138:
>
> http://code.djangoproject.com/ticket/8138
> ...
>
--
Thomas Guettler, http://www.thomas-guettler.de/
E-Mail: guettli (*) thomas-guettler + de
There is two issues at work here.
1) Is #5625 a good idea?
2) Do we need to implement it right now?
Regardless of the answer to (1), (2) is the more pressing concern -
given that we're days away from a v1.1 feature freeze, and we're
probably behind schedule as it is, now isn't the time to start with
unnecessary feature creep. As far as I can tell, #5625 can be safely
deferred to v1.2 without losing functionality or being boxed into a
corner by #8138. Given that there isn't any urgency brought on by a
need to meet v1.1 commitments, I suggest that we should defer this
discussion to the next planning cycle.
Russ %-)
You both are indeed correct. I certainly think that the current patch can go in today as presented. The Ellington test suite is passing with a 10x speedup. We can get it to 40x speedup if we change out doctests that load fixtures into unit tests (which probably makes more sense anyway). From 2 hours down to 3 minutes is amazing.
Thanks for all the hard work, and I'm really excited to see this going into Django. Django development will be much improved with this change.