#33277: SimpleTestCase does not block database connections in threads
-----------------------------------+--------------------------------------
Reporter: Daniel Hahler | Owner: nobody
Type: Uncategorized | Status: new
Component: Testing framework | Version: 3.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-----------------------------------+--------------------------------------
Description changed by Daniel Hahler:
Old description:
> Due to {{{ConnectionHandler}}}'s connections being thread-local [1] new
> connections will be used in new threads, which then do not have been
> patched for disallowed methods [2].
>
> Given {{{test_simpletestcase.py}}}:
>
> {{{
> import threading
>
> from django.db import connection
> from django.test import SimpleTestCase
>
> class MySimpleTestCase(SimpleTestCase):
> def test_this(self):
> try:
> with connection.cursor() as cursor:
> cursor.execute("SELECT 1")
> raise Exception("should have failed")
> except AssertionError:
> pass
>
> res = []
>
> def thread_func():
> res.append(1)
> try:
> with connection.cursor() as cursor:
> cursor.execute("SELECT 1")
> raise Exception("should have failed")
> except AssertionError:
> pass
> res.append(2)
>
> t = threading.Thread(target=thread_func)
> t.start()
> t.join()
> assert res == [1, 2], res
> }}}
>
> {{{./manage.py test test_simpletestcase.py}}} fails like this:
> {{{
> Exception in thread Thread-1:
> Traceback (most recent call last):
> File "/usr/lib/python3.9/threading.py", line 973, in _bootstrap_inner
> self.run()
> File "/usr/lib/python3.9/threading.py", line 910, in run
> self._target(*self._args, **self._kwargs)
> File "…/test_simpletestcase.py", line 23, in thread_func
> raise Exception("should have failed")
> Exception: should have failed
> F
> ======================================================================
> FAIL: test_this (test_simpletestcase.MySimpleTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
> File "…/test_simpletestcase.py", line 31, in test_this
> assert res == [1, 2], res
> AssertionError: [1]
>
> ----------------------------------------------------------------------
> Ran 1 test in 0.006s
>
> FAILED (failures=1)
> }}}
>
> (Note that there is some handling of {{{connection.settings_dict}}} for
> workers of the test runner, which is only slightly related:
> https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/test/runner.py#L327-L335)
>
> A possible solution might be to use the existing
> [https://docs.djangoproject.com/en/3.2/ref/signals/#connection-created
> connection_created] signal to raise an exception when a connections was
> created (although that would happen only after the fact - a new pre-
> connect signal could be used/added for this).
>
> Given that the test DB names are not prefixed with {{{test_}}} with
> {{{SimpleTestCase}}} you might accidentally change the production DB from
> within your tests when something like a {{{ThreadPoolExecutor}}} is being
> used when mixing sync with async etc.
>
> 1:
> https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/utils/connection.py#L41
> 2:
> https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/test/testcases.py#L183
New description:
Due to {{{ConnectionHandler}}}'s connections being thread-local [1] new
connections will be used in new threads, which then do not have been
patched for disallowed methods [2].
Given {{{test_simpletestcase.py}}}:
{{{
import threading
from django.db import connection
from django.test import SimpleTestCase
class MySimpleTestCase(SimpleTestCase):
def test_this(self):
try:
with connection.cursor() as cursor:
cursor.execute("SELECT 1")
raise Exception("should have failed")
except AssertionError:
pass
res = []
def thread_func():
res.append(1)
try:
with connection.cursor() as cursor:
cursor.execute("SELECT 1")
raise Exception("should have failed")
except AssertionError:
pass
res.append(2)
t = threading.Thread(target=thread_func)
t.start()
t.join()
assert res == [1, 2], res
}}}
{{{./manage.py test test_simpletestcase.py}}} fails like this:
{{{
Exception in thread Thread-1:
Traceback (most recent call last):
File "/usr/lib/python3.9/threading.py", line 973, in _bootstrap_inner
self.run()
File "/usr/lib/python3.9/threading.py", line 910, in run
self._target(*self._args, **self._kwargs)
File "…/test_simpletestcase.py", line 23, in thread_func
raise Exception("should have failed")
Exception: should have failed
F
======================================================================
FAIL: test_this (test_simpletestcase.MySimpleTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "…/test_simpletestcase.py", line 31, in test_this
assert res == [1, 2], res
AssertionError: [1]
----------------------------------------------------------------------
Ran 1 test in 0.006s
FAILED (failures=1)
}}}
(Note that there is some handling of {{{connection.settings_dict}}} for
workers of the test runner, which is only slightly related:
https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/test/runner.py#L327-L335)
A possible solution might be to use the existing
[https://docs.djangoproject.com/en/3.2/ref/signals/#connection-created
connection_created] signal to raise an exception when a connections was
created (although that would happen only after the fact - a new pre-
connect signal could be used/added for this).
Given that the test DB names are not prefixed with {{{test_}}} with
{{{SimpleTestCase}}} you might accidentally change the production DB from
within your tests when something like a {{{ThreadPoolExecutor}}} is being
used when mixing sync with async etc.
Note: pytest-django monkeypatches
{{{django.db.backends.base.base.BaseDatabaseWrapper.ensure_connection}}}
to block DB access, which appears to work better in this regard (across
threads).
1:
https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/utils/connection.py#L41
2:
https://github.com/django/django/blob/dfa1145a22042dcf9e504a5a7edd5557e3e0d07c/django/test/testcases.py#L183
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33277#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/065.244d7aa4679a5559cd27cbe8394dcd26%40djangoproject.com.
