id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
31117	ThreadTests fails due to double test_ prefix caused by TestDbCreationTests.	Matthijs Kooijman	Matthijs Kooijman	"
While working on testcases for #26552 and #31051 I ran into some problems with
testing against a local MySQL and Postgres server. I initially thought my
testcases broke things, but it turns out I can see the same broken behaviour on
a clean master. Hence this ticket.

In short, what happens is that
`backends.base.test_creation.TestDbCreationTests` runs `create_test_db` to
verify migration is (not) run depending on the `MIGRATE` settings. However,
this has two problems:

 - This runs `create_test_db` on an already initialized database, leading to
   double initialization (in particular, it adds a *second* `test_` prefix to the
   database name, which produces an unexpected database name.
 - `create_test_db` has side effects that are not properly cleaned up. Some of them
   are reverted by restoring `settings_dict` (see below), but not all of them
   (more on this below).

=== Possible solution

A solution to both issues could be to use a separate database, that is not
normally used by other tests and thus not initialized by the test runner. This
test can then freely call `create_test_db` to initialize it (possible even
actually creating the database), and then call `destroy_test_db` to clean up
everthing again.

I have alredy been working on adding an extra database like this, since this
was also a possible solution to problems I had with
https://github.com/django/django/pull/12166

=== To reproduce

On my system, the partial reverting leads to an exception in a later test due
to a fairly obscure interaction between various parts (details below).

To reproduce this exception, I've used the commands below. Note that the
`multiple_databases.tests` are not related to this bug directly, but used to
work around #31055.

{{{
$ ./runtests.py --settings test_postgresql multiple_database.tests backends.base.test_creation.TestDbCreationTests backends.tests.ThreadTests --parallel 1
  File ""/home/matthijs/docs/src/upstream/django/venv/lib/python3.7/site-packages/psycopg2/__init__.py"", line 126, in connect
    conn = _connect(dsn, connection_factory=connection_factory, **kwasync)
psycopg2.OperationalError: FATAL:  database ""test_test_django_main"" does not exist

$ ./runtests.py --settings test_mysql multiple_database.tests backends.base.test_creation.TestDbCreationTests backends.tests.ThreadTests --parallel 1

  File ""/home/matthijs/docs/src/upstream/django/venv/lib/python3.7/site-packages/MySQLdb/connections.py"", line 179, in __init__
    super(Connection, self).__init__(*args, **kwargs2)
MySQLdb._exceptions.OperationalError: (1044, ""Access denied for user 'test_django'@'localhost' to database 'test_test_django_main'"")
}}}

Interestingly enough, this happens for me locally, but not with the automatic
testing on Jenkins. I originally suspected that this was because my MySQL
permissions were set up strictly (only allowing access to `test_django_*`), but
for postgresql, the error was not about permissions. Also, when I relaxed the
MySQL permissions, the error only changed to `(1049, ""Unknown database
'test_test_django_main'"")`.

Is the configuration for the Jenkins tests published somewhere? Maybe that
would offer a clue about this difference.

=== Analysis

I dug a little deeper to figure out why this exception occurs exactly. This is what happens:

First, `create_test_db` prepends `test_` to the database name, and configures this in two places:

{{{
settings.DATABASES[self.connection.alias][""NAME""] = test_database_name
self.connection.settings_dict[""NAME""] = test_database_name
}}}

See https://github.com/django/django/blob/cebd41e41603c3ca77c5b29d6cd20c1bff43827f/django/db/backends/base/creation.py#L33

Since the test runner has previously called `create_test_db`, the database
name now has a `test_test_` prefix.

At this time, self.connection.settings_dict is the same dict as
`django.db.connections.databases`, so that one is also changed.

At the end of the test, `connection.settings_dict` is restored to a copy
made before the test. This replaces the entire dict instead of modifying the
dict, so `django.db.connections.databases` is *not* restored and still has
the `test_test_` prefix.

{{{
connection.settings_dict = saved_settings
}}}

See https://github.com/django/django/blob/5e00bd1f7717149573df9607b848297a520881d3/tests/backends/base/test_creation.py#L58

Finally, `backends.tests.ThreadTests` creates a new connection object
by calling `connection.cursor()` from within another thread. This new
connection object is initialized using
`django.db.connections.databases['default']`, which has the `test_test_`
prefix.

See https://github.com/django/django/blob/5e00bd1f7717149573df9607b848297a520881d3/tests/backends/tests.py#L634

=== Next steps

Since I was already working on allowing a third database, I'll see if I can fix
this issue as well, probably as a separate pullrequest. Any thoughts on whether
this is the right approach are welcome, as well as thoughts about why this does
not occur on Jenkins.
"	Bug	closed	Testing framework	dev	Normal	fixed			Accepted	1	0	0	0	0	0