Opened 11 years ago
Closed 11 years ago
#20579 closed Bug (fixed)
Define the expected state of the database between test cases
Reported by: | Aymeric Augustin | Owned by: | Aymeric Augustin |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Release blocker | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
As pointed out in 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.
Change History (5)
comment:1 by , 11 years ago
comment:3 by , 11 years ago
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.
comment:4 by , 11 years ago
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.
comment:5 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
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:
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 intounittest.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?