From c15b0c27927afd141d311dd9bfe68540cf44a8d6 Mon Sep 17 00:00:00 2001 From: Simon Charette Date: Thu, 16 Apr 2015 16:19:30 -0400 Subject: [PATCH] Fixed #24652 -- Disallowed query execution in SimpleTestCase subclasses. Thanks to Tim and Anssi for the review. --- django/test/testcases.py | 29 +++++++++++++++++++++++++++++ docs/releases/1.9.txt | 6 ++++++ docs/topics/testing/tools.txt | 11 +++++++++++ tests/test_utils/tests.py | 18 ++++++++++++++++++ 4 files changed, 64 insertions(+) diff --git a/django/test/testcases.py b/django/test/testcases.py index 421134572d..b17b3921d8 100644 --- a/django/test/testcases.py +++ b/django/test/testcases.py @@ -140,6 +140,19 @@ class _AssertTemplateNotUsedContext(_AssertTemplateUsedContext): return '%s was rendered.' % self.template_name +class _CursorFailure(object): + def __init__(self, cls_name, wrapped): + self.cls_name = cls_name + self.wrapped = wrapped + + def __call__(self): + raise AssertionError( + "Database queries aren't allowed in SimpleTestCase. " + "Either use TestCase or TransactionTestCase to ensure proper test isolation or " + "set %s.allow_database_queries to True to silence this failure." % self.cls_name + ) + + class SimpleTestCase(unittest.TestCase): # The class we'll use for the test client self.client. @@ -148,6 +161,10 @@ class SimpleTestCase(unittest.TestCase): _overridden_settings = None _modified_settings = None + # Tests shouldn't be allowed to query the database since + # this base class doesn't enforce any isolation. + allow_database_queries = False + @classmethod def setUpClass(cls): super(SimpleTestCase, cls).setUpClass() @@ -157,9 +174,17 @@ class SimpleTestCase(unittest.TestCase): if cls._modified_settings: cls._cls_modified_context = modify_settings(cls._modified_settings) cls._cls_modified_context.enable() + if not cls.allow_database_queries: + for alias in connections: + connection = connections[alias] + connection.cursor = _CursorFailure(cls.__name__, connection.cursor) @classmethod def tearDownClass(cls): + if not cls.allow_database_queries: + for alias in connections: + connection = connections[alias] + connection.cursor = connection.cursor.wrapped if hasattr(cls, '_cls_modified_context'): cls._cls_modified_context.disable() delattr(cls, '_cls_modified_context') @@ -777,6 +802,10 @@ class TransactionTestCase(SimpleTestCase): # This can be slow; this flag allows enabling on a per-case basis. serialized_rollback = False + # Since tests will be wrapped in a transaction, or serialized if they + # are not available, we allow queries to be run. + allow_database_queries = True + def _pre_setup(self): """Performs any pre-test setup. This includes: diff --git a/docs/releases/1.9.txt b/docs/releases/1.9.txt index 28bd915a23..76e8085fbd 100644 --- a/docs/releases/1.9.txt +++ b/docs/releases/1.9.txt @@ -505,6 +505,12 @@ Miscellaneous * The ``django.contrib.sites.models.Site.domain`` field was changed to be :attr:`~django.db.models.Field.unique`. +* In order to enforce test isolation, database queries are not allowed + by default in :class:`~django.test.SimpleTestCase` tests anymore. You + can disable this behavior by setting the + :attr:`~django.test.SimpleTestCase.allow_database_queries` class attribute + to ``True`` on your test class. + .. _deprecated-features-1.9: Features deprecated in 1.9 diff --git a/docs/topics/testing/tools.txt b/docs/topics/testing/tools.txt index 3e5cd63a12..aa66afb1d9 100644 --- a/docs/topics/testing/tools.txt +++ b/docs/topics/testing/tools.txt @@ -606,6 +606,17 @@ features like: then you should use :class:`~django.test.TransactionTestCase` or :class:`~django.test.TestCase` instead. +.. attribute:: SimpleTestCase.allow_database_queries + + .. versionadded:: 1.9 + + :class:`~SimpleTestCase` disallows database queries by default. This + helps to avoid executing write queries which will affect other tests + since each ``SimpleTestCase`` test isn't run in a transaction. If you + aren't concerned about this problem, you can disable this behavior by + setting the ``allow_database_queries`` class attribute to ``True`` on + your test class. + ``SimpleTestCase`` inherits from ``unittest.TestCase``. .. warning:: diff --git a/tests/test_utils/tests.py b/tests/test_utils/tests.py index 57c0e6c2e7..dd68acdff9 100644 --- a/tests/test_utils/tests.py +++ b/tests/test_utils/tests.py @@ -914,3 +914,21 @@ class OverrideSettingsTests(TestCase): with self.settings(STATICFILES_DIRS=[test_path]): finder = get_finder('django.contrib.staticfiles.finders.FileSystemFinder') self.assertIn(expected_location, finder.locations) + + +class DisallowedDatabaseQueriesTests(SimpleTestCase): + def test_disallowed_database_queries(self): + expected_message = ( + "Database queries aren't allowed in SimpleTestCase. " + "Either use TestCase or TransactionTestCase to ensure proper test isolation or " + "set DisallowedDatabaseQueriesTests.allow_database_queries to True to silence this failure." + ) + with self.assertRaisesMessage(AssertionError, expected_message): + Car.objects.first() + + +class AllowedDatabaseQueriesTests(SimpleTestCase): + allow_database_queries = True + + def test_allowed_database_queries(self): + Car.objects.first()