From 66f3d57b79eee0381c29ee4c76582d6b182bfad9 Mon Sep 17 00:00:00 2001 From: Joeri Bekker Date: Sat, 18 May 2013 13:43:51 +0200 Subject: [PATCH] Fixed #19031 -- Added a warning when using override_settings with 'DATABASES' --- django/test/signals.py | 13 ++++++++++++- django/test/utils.py | 4 ++-- docs/ref/signals.txt | 8 +++++++- docs/releases/1.7.txt | 3 +++ docs/topics/testing/overview.txt | 17 +++++++++++++++++ tests/settings_tests/tests.py | 23 +++++++++++++++++++++++ 6 files changed, 64 insertions(+), 4 deletions(-) diff --git a/django/test/signals.py b/django/test/signals.py index a96bdff3b3..de8802b381 100644 --- a/django/test/signals.py +++ b/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']) diff --git a/django/test/utils.py b/django/test/utils.py index 70ce640239..dc3e5952ea 100644 --- a/django/test/utils.py +++ b/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): diff --git a/docs/ref/signals.txt b/docs/ref/signals.txt index 0253832b8d..d4f261cadb 100644 --- a/docs/ref/signals.txt +++ b/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 ----------------- diff --git a/docs/releases/1.7.txt b/docs/releases/1.7.txt index 8d967ff469..2cf1cd326e 100644 --- a/docs/releases/1.7.txt +++ b/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 ===================================== diff --git a/docs/topics/testing/overview.txt b/docs/topics/testing/overview.txt index 6f75a3b7a8..73f2daa294 100644 --- a/docs/topics/testing/overview.txt +++ b/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:: diff --git a/tests/settings_tests/tests.py b/tests/settings_tests/tests.py index bfd8d607a9..87ed4d4f5e 100644 --- a/tests/settings_tests/tests.py +++ b/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.