manage.py flush raises error if there are unsynchronized applications

[Russell Keith-Magee]

If an application has been added to INSTALLED_APPS, but has not yet been synchronized, ./manage.py flush will raise a "no such table:" error.

How to reproduce:

1. Create a new project with a bunch of apps
2. Call manage.py syncdb
3. Add some new apps to INSTALLED_APPS (e.g., add contrib.admin to the list)
4. Call manage.py flush

This will raise an error because the tables for contrib.admin haven't been created, but the flush command will try to delete those tables.

The solution here will be to check the tables that already exist in the database before attempting to destroy them. Syncdb already does a version of this, checking the tables that already exist before trying to create; the same logic needs to be applied to flush.

[Julien Phalip]

The proposed patch is a bit simpler than replicating syncdb's behaviour. It works when tested locally with my apps, but unfortunately I could not find a way to write regression tests for it. Here's a first sketch below:

>>> _old_installed_apps = settings.INSTALLED_APPS
>>> settings.INSTALLED_APPS += ['?????']
>>> management.call_command('flush', verbosity=0, interactive=False)
>>> settings.INSTALLED_APPS = _old_installed_apps

The problem is: what should we put in place of the "?????". All contrib apps with models are included by default in the test suite, so we can't play with those. So, I guess that to properly test this, one would need to be able to include apps dynamically in the test suite, a problem which #7835 is trying to solve. Looks like a "chicken or the egg" puzzle :)

Any suggestion?

[Julien Phalip]

Patch, but without regression tests yet...

[Julien Phalip]

In the case where no regression tests could be written for this, here's a suggestion, based on the assumption that this patch is trivial enough and does not break anything in the Django test suite:

• Check this patch in.
• Open a ticket T saying that there should be a regression test for this.
• Create new test API and close #7835.
• Write regression tests thanks to the new API and fix T.

[Russell Keith-Magee]

Replying to julien:

So, I guess that to properly test this, one would need to be able to include apps dynamically in the test suite, a problem which #7835 is trying to solve. Looks like a "chicken or the egg" puzzle :)

This is one of the rare occasions where it is acceptable to not include a unit test with a patch. There are a number of areas of Django that aren't unit tested - for example, most of the behaviour of syncdb is not formally tested. This isn't ideal, but sometimes it is unavoidable - usually because the cost of setting up the infrastructure required for testing these edge cases vastly exceeds the benefit that will be returned, especially given the frequency with which those areas of code are modified is taken in to consideration.

I'll take a look at this patch tonight.

[Russell Keith-Magee]

(In [9535]) Fixed #9717 -- Corrected a problem where django-admin.py flush would attempt to flush database tables that had not yet been created. This occurred when an application had been added to INSTALLED_APPS, but had not yet been synchronized. Thanks to Julien Phalip for the patch.

[Russell Keith-Magee]

(In [9536]) [1.0.X] Fixed #9717 -- Corrected a problem where django-admin.py flush would attempt to flush database tables that had not yet been created. This occurred when an application had been added to INSTALLED_APPS, but had not yet been synchronized. Thanks to Julien Phalip for the patch.

Merge of [9535] from trunk.