Browse Source

Fixed #19031 -- Added a warning when using override_settings with 'DATABASES'

Joeri Bekker 12 years ago
parent
commit
66f3d57b79

+ 12 - 1
django/test/signals.py

@@ -1,5 +1,6 @@
 import os
 import time
+import warnings
 
 from django.conf import settings
 from django.db import connections
@@ -9,11 +10,14 @@ from django.utils.functional import empty
 
 template_rendered = Signal(providing_args=["template", "context"])
 
-setting_changed = Signal(providing_args=["setting", "value"])
+setting_changed = Signal(providing_args=["setting", "value", "enter"])
 
 # Most setting_changed receivers are supposed to be added below,
 # except for cases where the receiver is related to a contrib app.
 
+# Settings that may not work well when using 'override_settings' (#19031)
+COMPLEX_OVERRIDE_SETTINGS = set(['DATABASES'])
+
 
 @receiver(setting_changed)
 def update_connections_time_zone(**kwargs):
@@ -74,8 +78,15 @@ def language_changed(**kwargs):
         if kwargs['setting'] == 'LOCALE_PATHS':
             trans_real._translations = {}
 
+
 @receiver(setting_changed)
 def file_storage_changed(**kwargs):
     if kwargs['setting'] in ('MEDIA_ROOT', 'DEFAULT_FILE_STORAGE'):
         from django.core.files.storage import default_storage
         default_storage._wrapped = empty
+
+
+@receiver(setting_changed)
+def complex_setting_changed(**kwargs):
+    if kwargs['enter'] and kwargs['setting'] in COMPLEX_OVERRIDE_SETTINGS:
+        warnings.warn("Overriding setting %s can lead to unexpected behaviour." % kwargs['setting'])

+ 2 - 2
django/test/utils.py

@@ -228,7 +228,7 @@ class override_settings(object):
         settings._wrapped = override
         for key, new_value in self.options.items():
             setting_changed.send(sender=settings._wrapped.__class__,
-                                 setting=key, value=new_value)
+                                 setting=key, value=new_value, enter=True)
 
     def disable(self):
         settings._wrapped = self.wrapped
@@ -236,7 +236,7 @@ class override_settings(object):
         for key in self.options:
             new_value = getattr(settings, key, None)
             setting_changed.send(sender=settings._wrapped.__class__,
-                                 setting=key, value=new_value)
+                                 setting=key, value=new_value, enter=False)
 
 
 def compare_xml(want, got):

+ 7 - 1
docs/ref/signals.txt

@@ -553,7 +553,8 @@ This signal is sent when the value of a setting is changed through the
 :func:`django.test.utils.override_settings` decorator/context manager.
 
 It's actually sent twice: when the new value is applied ("setup") and when the
-original value is restored ("teardown").
+original value is restored ("teardown"). Use the ``enter`` argument to
+distinguish between the two.
 
 Arguments sent with this signal:
 
@@ -567,6 +568,11 @@ Arguments sent with this signal:
     The value of the setting after the change. For settings that initially
     don't exist, in the "teardown" phase, ``value`` is ``None``.
 
+``enter``
+    .. versionadded:: 1.7
+
+        A boolean; ``True`` if the setting is applied, ``False`` if restored.
+
 template_rendered
 -----------------
 

+ 3 - 0
docs/releases/1.7.txt

@@ -38,6 +38,9 @@ Minor features
   contains extra parameters passed to the ``content-type`` header on a file
   upload.
 
+* The ``enter`` argument was added to the
+  :data:`~django.test.signals.setting_changed` signal.
+
 Backwards incompatible changes in 1.7
 =====================================
 

+ 17 - 0
docs/topics/testing/overview.txt

@@ -1403,6 +1403,23 @@ The decorator can also be applied to test case classes::
     the original ``LoginTestCase`` is still equally affected by the
     decorator.
 
+.. warning::
+
+    The settings file contains some settings that are only consulted during
+    initialization of Django internals. If you change them with
+    ``override_settings``, the setting is changed if you access it via the
+    ``django.conf.settings`` module, however, Django's internals access it
+    differently. Effectively, using ``override_settings`` with these settings
+    is probably not going to do what you expect it to do.
+
+    We do not recommend using ``override_settings`` with :setting:`DATABASES`.
+    Using ``override_settings`` with :setting:`CACHES` is possible, but a bit
+    tricky if you are using internals that make using of caching, like
+    :mod:`django.contrib.sessions`. For example, you will have to reinitialize
+    the session backend in a test that uses cached sessions and overrides
+    :setting:`CACHES`.
+
+
 You can also simulate the absence of a setting by deleting it after settings
 have been overriden, like this::
 

+ 23 - 0
tests/settings_tests/tests.py

@@ -203,6 +203,29 @@ class SettingsTests(TestCase):
             'ALLOWED_INCLUDE_ROOTS', '/var/www/ssi/')
 
 
+class TestComplexSettingOverride(TestCase):
+    def setUp(self):
+        self.old_warn_override_settings = signals.COMPLEX_OVERRIDE_SETTINGS.copy()
+        signals.COMPLEX_OVERRIDE_SETTINGS.add('TEST_WARN')
+
+    def tearDown(self):
+        signals.COMPLEX_OVERRIDE_SETTINGS = self.old_warn_override_settings
+        self.assertFalse('TEST_WARN' in signals.COMPLEX_OVERRIDE_SETTINGS)
+
+    def test_complex_override_warning(self):
+        """Regression test for #19031"""
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+
+            override = override_settings(TEST_WARN='override')
+            override.enable()
+            self.assertEqual('override', settings.TEST_WARN)
+            override.disable()
+
+            self.assertEqual(len(w), 1)
+            self.assertEqual('Overriding setting TEST_WARN can lead to unexpected behaviour.', str(w[-1].message))
+
+
 class TrailingSlashURLTests(TestCase):
     """
     Tests for the MEDIA_URL and STATIC_URL settings.