Omits test_ prefix from database name when running subset of tests

[Matthijs Kooijman]

While debugging some test framework issues wrt mysql, I noticed a problem where the test runner would try to access the test database without prefixing test_, leading to an access denied error (because my permissions are set up tightly).

What I suspect happens is that this subset of tests only uses the default database, so only that one is set up by DisoveryRunner.setup_databases. This is confirmed by using a debugger, which shows ​databases only contains 'default'. Then, it runs the check management command, which looks at settings.DATABASES, which still contains the settings for default and other. This in turn causes a connection to the other database to be made, but since the name of that database is not modified by ​create_test_db, that still refers to the original name, and the connection fails.

To reproduce, I have a clean master (c33eb6dcd0c211f8f02b2976fe3b3463f0a54498), with the following tests/test_mysql.py:

DATABASES = {                                                                                                           
    'default': {                                                                                                        
        'ENGINE': 'django.db.backends.mysql',                                                                           
        'HOST': 'localhost',                                                                                            
        'USER': 'test_django',                                                                                          
        'PASSWORD': 'XXX',                                                                                 
        # Django prepends test_ to this name...                                                                         
        'NAME': 'django_main',                                                                                          
    },                                                                                                                  
    'other': {                                                                                                          
        'ENGINE': 'django.db.backends.mysql',                                                                           
        'HOST': 'localhost',                                                                                            
        'USER': 'test_django',                                                                                          
        'PASSWORD': 'XXX',                                                                                 
        # Django prepends test_ to this name...                                                                         
        'NAME': 'django_other',                                                                                         
    }                                                                                                                   
}                                                                                                                       
                                                                                                                        
SECRET_KEY = "django_tests_secret_key"                                                                                  
                                                                                                                        
# Use a fast hasher to speed up tests.                                                                                  
PASSWORD_HASHERS = [                                                                                                    
    'django.contrib.auth.hashers.MD5PasswordHasher',                                                                    
]

Then inside tests, I run:

 ./runtests.py --settings test_mysql --parallel 1 timezones 

I think the --parallel 1 is not strictly needed, but might make things easier to debug. With the above, I get:

Creating test database for alias 'default'...
Destroying test database for alias 'default'...
Testing against Django installed in '/home/matthijs/docs/src/upstream/django/django'
Traceback (most recent call last):
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 220, in ensure_connection
    self.connect()
  File "/home/matthijs/docs/src/upstream/django/django/utils/asyncio.py", line 24, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 197, in connect
    self.connection = self.get_new_connection(conn_params)
  File "/home/matthijs/docs/src/upstream/django/django/utils/asyncio.py", line 24, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/mysql/base.py", line 233, in get_new_connection
    return Database.connect(**conn_params)
  File "/home/matthijs/docs/src/upstream/django/venv/lib/python3.7/site-packages/MySQLdb/__init__.py", line 84, in Connect
    return Connection(*args, **kwargs)
  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 'django_other'")

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "./runtests.py", line 566, in <module>
    options.start_at, options.start_after, options.pdb,
  File "./runtests.py", line 308, in django_tests
    extra_tests=extra_tests,
  File "/home/matthijs/docs/src/upstream/django/django/test/runner.py", line 687, in run_tests
    self.run_checks()
  File "/home/matthijs/docs/src/upstream/django/django/test/runner.py", line 625, in run_checks
    call_command('check', verbosity=self.verbosity)
  File "/home/matthijs/docs/src/upstream/django/django/core/management/__init__.py", line 168, in call_command
    return command.execute(*args, **defaults)
  File "/home/matthijs/docs/src/upstream/django/django/core/management/base.py", line 369, in execute
    output = self.handle(*args, **options)
  File "/home/matthijs/docs/src/upstream/django/django/core/management/commands/check.py", line 64, in handle
    fail_level=getattr(checks, options['fail_level']),
  File "/home/matthijs/docs/src/upstream/django/django/core/management/base.py", line 395, in check
    include_deployment_checks=include_deployment_checks,
  File "/home/matthijs/docs/src/upstream/django/django/core/management/base.py", line 382, in _run_checks
    return checks.run_checks(**kwargs)
  File "/home/matthijs/docs/src/upstream/django/django/core/checks/registry.py", line 72, in run_checks
    new_errors = check(app_configs=app_configs)
  File "/home/matthijs/docs/src/upstream/django/django/core/checks/model_checks.py", line 34, in check_all_models
    errors.extend(model.check(**kwargs))
  File "/home/matthijs/docs/src/upstream/django/django/db/models/base.py", line 1276, in check
    *cls._check_constraints(),
  File "/home/matthijs/docs/src/upstream/django/django/db/models/base.py", line 1842, in _check_constraints
    connection.features.supports_table_check_constraints or
  File "/home/matthijs/docs/src/upstream/django/django/utils/functional.py", line 48, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/mysql/features.py", line 97, in supports_column_check_constraints
    if self.connection.mysql_is_mariadb:
  File "/home/matthijs/docs/src/upstream/django/django/utils/functional.py", line 48, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/mysql/base.py", line 364, in mysql_is_mariadb
    return 'mariadb' in self.mysql_server_info.lower()
  File "/home/matthijs/docs/src/upstream/django/django/utils/functional.py", line 48, in __get__
    res = instance.__dict__[self.name] = self.func(instance)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/mysql/base.py", line 351, in mysql_server_info
    with self.temporary_connection() as cursor:
  File "/usr/lib/python3.7/contextlib.py", line 112, in __enter__
    return next(self.gen)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 604, in temporary_connection
    with self.cursor() as cursor:
  File "/home/matthijs/docs/src/upstream/django/django/utils/asyncio.py", line 24, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 260, in cursor
    return self._cursor()
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 236, in _cursor
    self.ensure_connection()
  File "/home/matthijs/docs/src/upstream/django/django/utils/asyncio.py", line 24, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 220, in ensure_connection
    self.connect()
  File "/home/matthijs/docs/src/upstream/django/django/db/utils.py", line 90, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 220, in ensure_connection
    self.connect()
  File "/home/matthijs/docs/src/upstream/django/django/utils/asyncio.py", line 24, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/base/base.py", line 197, in connect
    self.connection = self.get_new_connection(conn_params)
  File "/home/matthijs/docs/src/upstream/django/django/utils/asyncio.py", line 24, in inner
    return func(*args, **kwargs)
  File "/home/matthijs/docs/src/upstream/django/django/db/backends/mysql/base.py", line 233, in get_new_connection
    return Database.connect(**conn_params)
  File "/home/matthijs/docs/src/upstream/django/venv/lib/python3.7/site-packages/MySQLdb/__init__.py", line 84, in Connect
    return Connection(*args, **kwargs)
  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)
django.db.utils.OperationalError: (1044, "Access denied for user 'test_django'@'localhost' to database 'django_other'")

I am not quite familiar with this code, and this is already a distraction from a distraction from a distraction from the actual project I was working on, so I'm going to leave this here for others to fix :-)

[Carlton Gibson]

OK, wowser, yes. Good one.

Running all the tests we create both DBs:

$ ./runtests.py  --parallel=1 
Testing against Django installed in '.../django'
Creating test database for alias 'default'...
Creating test database for alias 'other'...
System check identified no issues (14 silenced).

vs

$ ./runtests.py  --parallel=1 timezones
Testing against Django installed in '.../django'
Creating test database for alias 'default'...
System check identified no issues (0 silenced).

How best we should disable the other alias in this circumstance is a good question, but presumably this should work.
Thanks for the report.

[Simon Charette]

This is likely related to #28478.

I guess DiscoverRunner should make sure only checks against the tested databases are run when it calls run_checks

​https://github.com/django/django/blob/d8e233352877c37c469687287e7761e05bdae94e/django/test/runner.py#L633-L636

[Matthijs Kooijman]

Simon, you set "has patch", but I cannot see any patch. Was that unintentional?

As for the fix, I wonder if maybe the connections object should just be trimmed to only contain the used databases, so all other code that iterates the databases will only see the ones actually in use? I believe there is code in multiple places now that has special casing for when a subset of databases is in use to not use all of the connections but only some, that could maybe also be simplified. On the other hand, this seems like an obvious implementation for using a subset of databases, so I guess there is a reason this implementation is not used right now?

[Simon Charette]

Matthijs, you can see links to Github PRs in the ticket description at the top.

​https://github.com/django/django/pull/12201

As for the fix, I wonder if maybe the connections object should just be trimmed to only contain the used databases, so all other code that iterates the databases will only see the ones actually in use

You'd have to give it a shot to make sure but I'm afraid it might break backward compatibility in subtle ways.

To me this issue is really similar to the ones discovered during #26351 where checks that require database accesses should be special cased.

[Matthijs Kooijman]

Matthijs, you can see links to Github PRs in the ticket description at the top.

Ah, I see. I quickly looked through those, but thought that if the PR link was added as a property there, it should also show up in the activity log below. But I suspect this PR link is added automatically based on the PR description or something like that, so it does not show up in the log. Oh well, found it now, thanks!

I had a quick look at the PR, but I do not think I have enough experience with this code to add anything useful there (and it looks like others are chipping in just fine). In any case, thanks for picking this up :-)

[Mariusz Felisiak]

Marking as "needs improvement" due to ​Simon's comment.

[Simon Charette]

​https://github.com/django/django/pull/12396

[Mariusz Felisiak <felisiak.mariusz@…>]

In 0b83c8c:

Refs #31055 -- Added --database option to the check management command.

This avoids enabling the database checks unless they are explicitly
requested and allows to disable on a per-alias basis which is required
when only creating a subset of the test databases.

This also removes unnecessary BaseCommand._run_checks() hook.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 430e796:

Refs #31055 -- Made DiscoverRunner skip running system checks on unused test databases.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 71756bdf:

Fixed #31055 -- Made constraint checks support databases aware.

[GitHub <noreply@…>]

In 708c534:

Refs #31055 -- Fixed Model.check() call in ConstraintsTests.test_check_constraints_required_db_features().

[Mariusz Felisiak <felisiak.mariusz@…>]

In 5c8441a:

Refs #31055 -- Made long column names checks support databases aware.

[Mariusz Felisiak <felisiak.mariusz@…>]

In e8d3088:

Refs #31055 -- Allowed database queries in invalid_models_tests.test_models.FieldNamesTests.

[Mariusz Felisiak <felisiak.mariusz@…>]

In c8d3cbdb:

Refs #31055 -- Doc'd 'databases' argument of check functions.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 54684a3:

[3.2.x] Refs #31055 -- Doc'd 'databases' argument of check functions.

Backport of c8d3cbdba809285ced3d9ba3cbb63bec422b8e48 from main