[Django] #20579: Define the expected state of the database between test cases
Django
---------------------------------------------+---------------------------
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: new
Component: Testing framework | Version: master
Severity: Release blocker | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------------+---------------------------
As pointed out in [https://code.djangoproject.com/ticket/20483#comment:39
this commment], the fix for #20483 doesn't use the correct set of content
types and permissions, because the database is flushed at the end of the
test case, not at the beginning. That behavior was introduced in #18271.
The goal of this ticket is to find a good compromise between:
- making the state of the database between test cases as deterministic as
possible,
- preserving the optimization of #20483,
- not introducing backwards incompatibilities,
- minimizing the amount of changes — we're after 1.6 alpha.
--
Ticket URL: <https://code.djangoproject.com/ticket/20579>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
I had spent half an hour carefully writing down my research, going back to
the introduction of TransactionTestCase, only to lose my text to a bad key
combo. Lesson learnt: Firefox doesn't consider Trac's comment field as an
input field, and won't preserve its content across Back / Next :'( I'm too
lazy to re-write it all.
----
To sum up, I was proposing to:
- Trigger post_syncdb at the beginning of TransactionTestCase => ensures
required CTs and permissions exist.
- Flush without triggering post_syncdb at the end of TransactionTestCase
=> avoids creating thousands of CTs and permissions, preserving the
benefits of #20483.
This introduces one noticeable change compared to Django 1.5: after a
TransactionTestCase, CTs and permissions tables will be empty. This isn't
a problem for CTs because they're created on the fly when they're missing
-- get_for_model actually does_create_for_model. It could be a problem for
permissions.
----
It could also be a problem for doctests. Carl says: ''there is no longer
any special support for doctests in Django (or well, there is, but it is
deprecated); doctests are still fully "supported" in the sense that they
are part of Python and can be integrated with a unittest test suite in the
manner recommended in the Python docs; that integration has to be
explicit, doctests are not automatically discovered; see
http://docs.python.org/2/library/doctest.html#unittest-api''.
Currently, the reordering of the test suite runs all subclasses
`django.test.TestCase` first, and then everything else -- including
TransactionTestCase and doctests turned into `unittest.TestCase`. As a
consequence, if someone's running doctests that rely on permissions, this
proposal would break them.
To play safe, we could perform this steps only when available_apps isn't
None. This makes available_apps even more of a hack specific to Django's
own test suite, but it never was anything else anyway... Thoughts?
--
Ticket URL: <https://code.djangoproject.com/ticket/20579#comment:1>
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* has_patch: 0 => 1
Comment:
https://github.com/django/django/pull/1261
--
Ticket URL: <https://code.djangoproject.com/ticket/20579#comment:2>
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by akaariai):
I think the patch is OK. Though the setup/teardown for TXTestCase is
getting a bit complex...
In the long term it might be better to do some test class reordering: run
first TestCases, then TransactionTestCases, do a flush + reload, and then
run the rest. This would mean that TXTestCase really has an empty db at
the start of the run, and that the rest of tests start running with
properly filled database.
To me it seems the ultimate solution is to do the flush + data reloading
not as part of test case, but as an intermediary step done by the testing
code. The TestCase class has visibility to both the previous and next test
and could maybe do some more intelligent reloading based on that
information. I don't know if this is easy or even possible to do.
--
Ticket URL: <https://code.djangoproject.com/ticket/20579#comment:3>
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: new
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by aaugustin):
This proposal for test case reordering still assumes that all
TransactionTestCase define available_apps. I'm not sure we can provide a
generic solution, and the current code (hack?) looks good enough for
Django's own test suite.
Indeed, our problem is that we want to express "between X and X" in terms
of "before X" and "after X", and we don't have enough context to optimize
things. One possibility would be to add a global variable to track
information about the state of the database, so the next test case could
know what the previous did. Still, as long as we try to maintain strict
backwards compatibility, there's no silver bullet.
--
Ticket URL: <https://code.djangoproject.com/ticket/20579#comment:4>
Django
Reporter: aaugustin | Owner: aaugustin
Type: Bug | Status: closed
Component: Testing framework | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Aymeric Augustin <aymeric.augustin@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"55cbd65985bfad02512a64a4cb8468140f15ee84"]:
{{{
#!CommitTicketReference repository=""
revision="55cbd65985bfad02512a64a4cb8468140f15ee84"
Fixed #20579 -- Improved TransactionTestCase.available_apps.
Also moved its documentation to the 'advanced' section. It doesn't
belong to the 'overview'. Same for TransactionTestCase.reset_sequences.
When available_apps is set, after a TransactionTestCase, the database
is now totally empty. post_syncdb is fired at the beginning of the next
TransactionTestCase.
Refs #20483.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20579#comment:5>