Opened 3 years ago
Closed 3 years ago
#33359 closed Cleanup/optimization (needsinfo)
Don't close database connections after every TestCase
Reported by: | Ran Benita | Owned by: | nobody |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Adam Johnson | Triage Stage: | Unreviewed |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Django's TestCase
closes all DB connections at tearDownClass
: https://github.com/django/django/blob/4.0/django/test/testcases.py#L1235-L1236
This makes it impossible to wrap a TestCase
inside a larger DB transaction. This is something that an external project, pytest-django, would like to do. See here for more discussion: https://forum.djangoproject.com/t/why-does-django-close-db-connections-between-test-classes/10782
Besides the pytest-django angle (which Django itself should not really consider), DB connection setup can be somewhat heavy and so needlessly closing them adds overhead to the test run.
We've tried to figure out why it was added. It seems to involve databases which perform SQL queries during *cursor* setup but it does not make much sense to me (see Django forum discussion).
We've run the Django CI with these lines removed (including Oracle) and all of the tests pass.
Change History (3)
comment:1 by , 3 years ago
comment:2 by , 3 years ago
Cc: | added |
---|
comment:3 by , 3 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
This change crashes many tests for non-in-memory database on SQLite, e.g.:
$ ./runtests.py backends queries test_utils ... File "/django/django/db/backends/sqlite3/base.py", line 268, in create_cursor return self.connection.cursor(factory=SQLiteCursorWrapper) django.db.utils.ProgrammingError: Cannot operate on a closed database.
It's tempting to remove these two lines, but more research is needed.
Agreed, this can be "heavy" -- do you mind running with sqlite & postgres locally and report timings with and without your patch?
What I am a bit worried though is that the state of the connection is essentially undefined after running a test, so I wonder if we should add a
reset
method or similar on the connection to restore it to it's initial state; for instance see docs aboutserver_reset_query
in pgbouncer:We might have to rerun connection initialization queries after that though.