Code

#20579 closed Bug (fixed)

Define the expected state of the database between test cases

Reported by: aaugustin Owned by: aaugustin
Component: Testing framework Version: master
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.

Attachments (0)

Change History (5)

comment:1 Changed 10 months ago 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?

comment:3 Changed 10 months ago 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.

comment:4 Changed 10 months ago 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.

comment:5 Changed 10 months ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.