Make DiscoverRunner skip creating a test database if not needed

[Tim Graham]

Inspired by ​https://gist.github.com/zenweasel/7495758 and ​https://www.caktusgroup.com/blog/2013/10/02/skipping-test-db-creation/, DiscoverRunner could skip creating the test database if no TransactionTestCase or its subclasses are used.

I'd suggest adding an attribute on the test classes such as _needs_database rather than using isinstance(test, TransactionTestCase) in those patches. This would allow compatibility in case Django users have other classes using the database that don't inherit from Django's classes (though I'm not aware of anyone doing this).

[Simon Charette]

I suggest we expose a _needed_databases attribute instead which returns set(connections) if self.multi_db else {DEFAULT_DB_ALIAS} to optimize the case where only the default database is needed as well.

[Adam Johnson]

N.B. pytest-django does this already, it makes the test databases pytest fixtures and attaches them to TransactionTestCase etc., if you run a set of tests without such classes then the fixture is never invoked.

[Simon Charette]

I wonder if we could unify SimpleTestCase.allow_database_queries, TransactionTestCase.mutli_db and this feature under a single SimpleTestCase.databases attribute.

The databases attribute could be set to None on SimpleTestCase and to {DEFAULT_DB_ALIAS} on TransactionTestCase with a deprecation shim that turns multi_db = True into set(connections). The discover runner could simply create the union of all tests's databases and the allow_database_queries functionality could be reused to block accessed to all connection aliases not defined in the test's databases.

[Simon Charette]

Here's a ​POC of what I had in mind. The patch is still missing documentation, appropriate warnings and probably doesn't correctly deal with mirroring setups but it correctly only creates the required databases while still respecting allow_database_queries and multi_db.

[Adam Johnson]

The PR looks like a good start... do you have time to continue working on it? I'll happily help with review, since I was bitten by multi_db in #29513.

[Simon Charette]

​PR

[Tim Graham <timograham@…>]

In 8c775391:

Refs #28478 -- Deprecated TestCase's allow_database_queries and multi_db in favor of databases.

[Tim Graham <timograham@…>]

In 41e73de3:

Fixed #28478 -- Make DiscoverRunner skip creating unused test databases.

SimpleTestCase.databases makes it possible to determine the set of
databases required to run the discovered tests.

[Tim Graham]

A failure appeared after 41e73de39df31c4b13d65462bfeedde6924226d8:

./tests/runtests.py --settings=test_postgres postgres_tests
Testing against Django installed in '/home/tim/code/django/django' with up to 3 processes
PostgreSQL 90515
Creating test database for alias 'default'…
Cloning test database for alias 'default'…
Cloning test database for alias 'default'…
Cloning test database for alias 'default'…
System check identified no issues (0 silenced).
...
======================================================================
ERROR: test_datetime_prepare_value (postgres_tests.test_ranges.TestFormField)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/python3.7.0/lib/python3.7/unittest/case.py", line 59, in testPartExecutor
    yield
  File "/opt/python3.7.0/lib/python3.7/unittest/case.py", line 615, in run
    testMethod()
  File "/home/tim/code/django/django/test/utils.py", line 370, in inner
    with self as context:
  File "/home/tim/code/django/django/test/utils.py", line 338, in __enter__
    return self.enable()
  File "/home/tim/code/django/django/test/utils.py", line 419, in enable
    self.disable()
  File "/home/tim/code/django/django/test/utils.py", line 437, in disable
    raise exc
  File "/home/tim/code/django/django/test/utils.py", line 415, in enable
    setting=key, value=new_value, enter=True,
  File "/home/tim/code/django/django/dispatch/dispatcher.py", line 175, in send
    for receiver in self._live_receivers(sender)
  File "/home/tim/code/django/django/dispatch/dispatcher.py", line 175, in <listcomp>
    for receiver in self._live_receivers(sender)
  File "/home/tim/code/django/django/test/signals.py", line 74, in update_connections_time_zone
    conn.ensure_timezone()
  File "/home/tim/code/django/django/db/backends/postgresql/base.py", line 198, in ensure_timezone
    self.ensure_connection()
  File "/home/tim/code/django/django/db/backends/base/base.py", line 216, in ensure_connection
    self.connect()
  File "/home/tim/code/django/django/db/utils.py", line 89, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/home/tim/code/django/django/db/backends/base/base.py", line 216, in ensure_connection
    self.connect()
  File "/home/tim/code/django/django/db/backends/base/base.py", line 194, in connect
    self.connection = self.get_new_connection(conn_params)
  File "/home/tim/code/django/django/db/backends/postgresql/base.py", line 178, in get_new_connection
    connection = Database.connect(**conn_params)
  File "/home/tim/.virtualenvs/django37/lib/python3.7/site-packages/psycopg2/__init__.py", line 130, in connect
    conn = _connect(dsn, connection_factory=connection_factory, **kwasync)
django.db.utils.OperationalError: FATAL:  database "djangotests2_2" does not exist

Add --parallel=1 and there's no problem.

[Tim Graham]

The problem also affects other test apps when run in isolation on PostgreSQL.

[Simon Charette]

I'll look into it today.

[Simon Charette]

Some tests were failing because the patch wasn't mocking all connection interaction attempts. If test database creation is skipped then any attempts to connect to it will necessarily fail.

The current patch prevents that from happening by mocking the connect method but it unveiled a few SimpleTestCase that happened to silently interact with the database.

The only part missing at this point is to decide whether or not we want to make skipUnlessDBFeature and friends usage on SimpleTestCase without databases override error out or go through a deprecation period where it automatically adds DEFAULT_CONNECTION_ALIAS to databases and raise a warning instead. Given the use case is quite nice I think error'ing out should be fine but given we had some instances of this pattern in the test suite I thought it was still worth discussing.

[Tim Graham]

I don't expect user projects use skipUnlessDBFeature much (it would be more like libraries that test against different databases) so I don't a deprecation adds much benefit.

[Tim Graham <timograham@…>]

In a02a6fd5:

Refs #28478 -- Prevented connection creation in model_indexes tests.

Entering a SchemaEditor instance creates a connection but it isn't needed
for this test.

[Tim Graham <timograham@…>]

In a96b9019:

Refs #28478 -- Prevented timezone assignment for unusable PostgreSQL connections.

[Tim Graham <timograham@…>]

In f5b63508:

Refs #28478 -- Prevented connection attempts against disallowed databases in tests.

Mocking connect as well as cursor methods makes sure an appropriate error
message is surfaced when running a subset of test attempting to access a
a disallowed database.

[Tim Graham <timograham@…>]

In b181aba7:

Refs #28478 -- Prevented database feature based skipping on tests disallowing queries.

Database features may require a connection to be established to determine
whether or not they are enabled.

[Carlton Gibson <carlton.gibson@…>]

In 7071f8f2:

Fixed #30193, Refs #28478 -- Avoided PostgreSQL connection health checks on initialization.

This addressed a regression introduced by a96b9019320ed8236659ee520a7a017c1bafbc6f as identified by Ran Benita.

[Carlton Gibson <carlton.gibson@…>]

In 7f25344c:

[2.2.x] Fixed #30193, Refs #28478 -- Avoided PostgreSQL connection health checks on initialization.

This addressed a regression introduced by a96b9019320ed8236659ee520a7a017c1bafbc6f as identified by Ran Benita.
Backport of 7071f8f2729295b0da77b6c651966dc32c71f1ab from master

[Mariusz Felisiak <felisiak.mariusz@…>]

In b61ea567:

Refs #28478 -- Removed support for TestCase's allow_database_queries and multi_db per deprecation timeline.