Opened 15 months ago

Last modified 3 months ago

#28478 new Cleanup/optimization

Make DiscoverRunner skip creating a test database if not needed

Reported by: Tim Graham Owned by: nobody
Component: Testing framework Version: master
Severity: Normal Keywords:
Cc: Adam (Chainz) Johnson Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no


Inspired by and, 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).

Change History (7)

comment:1 Changed 15 months ago by 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.

comment:2 Changed 14 months ago by Adam (Chainz) 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.

comment:3 Changed 14 months ago by 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 accesses to all connection aliases not defined in the test's databases.

Last edited 14 months ago by Simon Charette (previous) (diff)

comment:4 Changed 14 months ago by 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.

comment:5 Changed 4 months ago by Adam (Chainz) Johnson

Cc: Adam (Chainz) Johnson added

comment:6 Changed 4 months ago by Adam (Chainz) 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.

comment:7 Changed 3 months ago by Simon Charette

Has patch: set
Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top