#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 django-updates+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.244d7aa4679a5559cd27cbe8394dcd26%40djangoproject.com.