Opened 7 years ago

Last modified 7 years ago

#29225 closed Bug

The non-test sqlite database can be easily destroyed during tests — at Version 2

Reported by: gh720 Owned by: nobody
Component: Testing framework Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by gh720)

I don't know if it is a bug or just a nasty feature, that appears to be undocumented, or a well known caveat that I don't know about.

With a few seemingly innocent steps you can get your working sqlite database destroyed during tests.
Suppose you hold a few database configs in settings.DATABASES:

DATABASES = {  'sqlite1': { ... }, 'sqlite2': { ... } }

And you want to select one and leave it as the default.
The simplest solution that comes to mind is
DATABASES['default'] = DATABASES['sqlite1'] .

And now there is a funny piece of code, which is a part of the teardown procedure:

https://github.com/django/django/blob/master/django/db/backends/base/creation.py
...
# Restore the original database name
        if old_database_name is not None:
            settings.DATABASES[self.connection.alias]["NAME"] = old_database_name
            self.connection.settings_dict["NAME"] = old_database_name

Suppose we run a test.
In case of in-memory sqlite testing the "url" of in-memory database (something like file:memorydb_sqlite?mode=memory&cache=shared gets replaced with the real sqlite db file path by the code above.
The teardown procedure is repeated for every database config we have.
Guess what happens in the second iteration?
Real sqlite db is destroyed because self.connection.settings_dict is actually the same object for both configs.

May be this one is well known and every developer is aware of that?
But still it is to easy to step into this trap in my opinion.

I guess it should be documented somewhere or the code above somehow amended.
May be some flag showing that the database is already destroyed would be enough.
I don't know Django well enough to say whether that restoring old name is necessary or not.

Change History (2)

comment:1 by gh720, 7 years ago

Description: modified (diff)

comment:2 by gh720, 7 years ago

Description: modified (diff)
Note: See TracTickets for help on using tickets.
Back to Top