Reset values of settings fixture does not work on mutable types?

[gforcada]

trafficstars

Hi,

together with @jnns we stumbled upon this issue:

we have a dictionary in our settings_test.py where one key is just holding a boolean value.

In one test we modify that boolean value and on the next test the default value set on settings_test.py is not there, but rather the value set on the previous test.

On regular, plain values, i.e. MY_SETTING = 3 the value is reset perfectly from test to test, but seems that it does not handle mutable types like dictionaries, could it be?

[lezeroq]

I also experienced issue with settings fixture. I used it in two different tests, and in first test everything was ok, but after second test setting value was not restored. And problem was a SettingsWrapper._to_restore class attribute (the same variable for two different fixtures). https://github.com/pytest-dev/pytest-django/blob/master/pytest_django/fixtures.py#L243

[floer32]

I'm seeing something like this when dealing with nested mutable types.

[floer32]

Agree with @lezeroq that _to_restore being a class level attribute may be part of the problem. When I tried to work around this, and also throw some extra copy.deepcopy() around, I'm still having an issue though. Digging deeper...

I saw some stuff like this that could be related but maybe not...

Update Indeed it does seem related.

We do not recommend altering the DATABASES setting. Altering the CACHES setting is possible, but a bit tricky if you are using internals that make using of caching, like django.contrib.sessions. For example, you will have to reinitialize the session backend in a test that uses cached sessions and overrides CACHES.

The DATABASES setting is the one giving me the problem. Hm. Wonder if I'll be able to find a workaround...

[blueyed]

You might want to try something like the following:

@pytest.fixture
def massage_settings_path(mocker):
    """Re-import settings anew, and restore them afterwards.

    This is required for DJANGO_CONFIGURATION to have an effect."""
    from django.conf import settings
    assert settings.configured
    orig_settings = settings

    restore = []
    reload_modules = ['project.settings', 'django.conf']
    for m in reload_modules:
        try:
            restore.append((m, sys.modules.pop(m)))
        except KeyError:  # pragma: no cover
            pass

    # Reload, but keep some settings.
    # (see django.test.utils.setup_test_environment).
    from django.conf import settings
    for k in ('SECURE_SSL_REDIRECT', 'ALLOWED_HOSTS'):
        setattr(settings, k, getattr(orig_settings, k))

    unpatch = []
    for name in [x for x in sys.modules
                 if x not in reload_modules and hasattr(sys.modules[x],
                                                        'settings')]:
        sc = sys.modules[name].settings.__class__
        if ('{}.{}'.format(sc.__module__, sc.__name__) ==
                'django.conf.Settings'):
            unpatch.append(name)
            setattr(sys.modules[name], 'settings', settings)

    # Re-setup Django for logging config.
    import django
    django.setup()

    yield settings

    for k, v in restore:
        sys.modules[k] = v

    for k in unpatch:
        assert sys.modules[k].settings is settings
        sys.modules[k].settings = orig_settings

    from django.conf import settings
    settings._wrapped = orig_settings._wrapped

    # Re-setup Django for logging config.
    django.setup()


@pytest.fixture
def dev(request, mocker):
    mocker.patch.dict(os.environ, clear=False,
                      DJANGO_CONFIGURATION='Dev')
    return request.getfuncargvalue('massage_settings_path')


@pytest.fixture
def prod(request, mocker):
    mocker.patch.dict(os.environ, clear=False,
                      DJANGO_CONFIGURATION='Production')
    return request.getfuncargvalue('massage_settings_path_with_monitoring')

I am using this via the dev or prod fixtures to test those configurations then, but we are not changing DATABASES in our tests. It might work for this, since it takes care of monkey-patching all places where settings is used.

[blueyed]

This might be different from the original issue though.

@gforcada @lezeroq Can you provide a failing test for your use case? I am not sure if moving initialization of _to_restore to __init__ would help - normally this should not be nested, is it?

[cybergrind]

@blueyed it definitely helps.

We're using https://github.com/tipsi/pytest-tipsi-django/blob/1cd89fdd088ba5822df8bd1e8d8ebcfa60bfa918/pytest_tipsi_django/django_fixtures.py#L106 for several months and we're heavily utilizing tests nesting, scopes and transactions on different scopes.

You can see example tests for it here: https://github.com/tipsi/pytest-tipsi-django/blob/1cd89fdd088ba5822df8bd1e8d8ebcfa60bfa918/test_django_plugin/app/tests/test_settings.py

But our fixtures policies have stricter finalization requirements, all non-required fixtures must be finalized before a test started even if the fixture has wider scope level (eg. we're finalizing fixtures with scope=module if the test doesn't ask for it)

[richtier]

I solve this by explicitly defining an autousef fixture in project's conftest.py:

@pytest.fixture(autouse=True)
def reset_feature_flags(settings):
    settings.FEATURE_FLAGS = {**settings.FEATURE_FLAGS}

then in the tests I can do:

def test_foo(settings):
    settings.FEATURE_FLAGS['THING_ON'] = True

This results in my settings.FEATURE_FLAGS being changed in tests, but reset after every test is ran.

note my settings looks like

FEATURE_FLAGS = {
    'THING_ON': False,
}

so documentation could be updated giving similar guidance, or the SettingsWrapper could be updated to loop over the settings and if the setting is a dictionary, magically handle it by doing settings.THE_SETTING = {**settings.THE_SETTING}

[cybergrind]

@richtier the only one thing you should keep in mind, that your fixture resets flags before every test run, not after. if you want to change its behavior to after you can do that by changing implementation:

@pytest.fixture(autouse=True)
def reset_feature_flags(settings):
    yield
    settings.FEATURE_FLAGS = {**settings.FEATURE_FLAGS}

it will make a difference when you start having more fixtures working with settings and start getting some strange errors in tests =)

[richtier]

@cybergrind Good point. the resetting should inded occur after

However, if we do settings.FEATURE_FLAGS = {**settings.FEATURE_FLAGS} after then we're not resetting them: we're using the values that were mutated in the tests.

I think we need to do something like the following to reset after:

@pytest.fixture(autouse=True)
def reset_feature_flags(settings):
    original =   {**settings.FEATURE_FLAGS}
    yield
    settings.FEATURE_FLAGS = original

[yamila-moreno]

Hi! I found a related problem, and I really don't understand the given solutions for my case. This is my test:

import pytest                                                    
                                                                
from entities.models.user import User                            


pytestmark = pytest.mark.django_db                               

def test_pagination_with_users_and_pagination(client, settings):                     
    # given                                                                          
    settings.REST_FRAMEWORK['PAGE_SIZE'] = 2 # default in settings.py is 10
    u1 = User.objects.create_user(email='[email protected]', password='secret')          
    u2 = User.objects.create_user(email='[email protected]', password='secret')          
    u3 = User.objects.create_user(email='[email protected]', password='secret')        
    u4 = User.objects.create_user(email='[email protected]', password='secret')         
    u5 = User.objects.create_user(email='[email protected]', password='secret')         
                                                                                     
    # when                                                                           
    response = client.get('/qapi/users/')                                            
    # then (this fails because next == '' as there is no a new page)
    assert response['next'] == 'http://testserver/qapi/users/?page=2'                

I'm expecting only two users in the request.data object, but get 5. Testing it live I receive the expected values.

Any hint would be appreciated,

[yamila-moreno]

I continued digging into this and I found that in my case the problem seems to be related to the way Django Rest Framework loads the settings, that can't be overriden.

https://github.com/encode/django-rest-framework/issues/6030#issuecomment-482178061

[Geekfish]

@yamila-moreno

In the ticket you linked there has been some work to make DRF api_settings reload automatically when Django settings are changed. But it's not enough, because the settings are assigned to the view attributes when the view gets imported, not when it runs. So by the time you override them it's too late, they've already been assigned.

One of these could work as a workaround:

• Mock the paginator attribute directly, rather than the setting (using the monkeypatch fixture, pytest-mock package, or just plain standard library mock.
• Reload rest_framework.pagination.PageNumberPagination (or whichever pagination class you use), after changing the settings using imp.reload_module. But this feels a bit hacky / would make the test way less readable.

[yamila-moreno]

@Geekfish thanks for your response; I got to the same conclusion.

This is what I did; I used Django Settings to ensure I can change them in runtime; it may be useful for someone that falls in this thread:

from django.conf import settings
from rest_framework.pagination import PageNumberPagination
from rest_framework.response import Response


class CustomHeaderPagination(PageNumberPagination):
    """
    This Serializer is used to put the count, next and prev in the headers.
    Code taken from: https://github.com/tbeadle/django-rest-framework-link-header-pagination
    """
    page_size = None
    page_size_query_param = 'page_size'

    def get_paginated_response(self, data):
        next_url = self.get_next_link() or ''
        prev_url = self.get_previous_link() or ''
        count = self.page.paginator.count

        headers = {
            'next':  next_url,
            'prev': prev_url,
            'count': count
        }

        return Response(data, headers=headers)

    def get_page_size(self, request):
        """
        This is a hack due to:
        https://github.com/encode/django-rest-framework/issues/6030#issuecomment-482178061
        """
        page_size = super().get_page_size(request)
        if page_size is None:
           return settings.REST_FRAMEWORK['PAGE_SIZE']

        return page_size

Now, in my test, I can use the settings fixtures provided by pytest-django.

[gh640]

(I believe) I met the same issue but in my case the cause was that I just didn't understand the correct way to use the fixture.

The override is not cleared with:

@pytest.fixture()
def login_required_middleware_disabled(settings):
    settings.MIDDLEWARE.remove('myapp.middleware.login_required_middleware')

The problem was fixed if I used the following code:

@pytest.fixture()
def login_required_middleware_disabled(settings):
    middleware = settings.MIDDLEWARE[:]
    middleware.remove('myapp.middleware.login_required_middleware')
    settings.MIDDLEWARE = middleware

The key difference is to re-set the settings.MIDDLEWARE attribute.

It's necessary to re-set the attribute to override the setting since the magic is done in SettingsWrapper.__setattr__():

class SettingsWrapper(object):
    _to_restore = []

    ...

    def __setattr__(self, attr, value):
        from django.test import override_settings

        override = override_settings(**{attr: value})
        override.enable()
        self._to_restore.append(override)

    ...

https://github.com/pytest-dev/pytest-django/blob/v3.5.0/pytest_django/fixtures.py#L305-L335

Using the following pattern worked too but it seems too much.

@pytest.fixture()
def login_required_middleware_disabled(settings):
    original = settings.MIDDLEWARE
    middleware = settings.MIDDLEWARE[:]
    middleware.remove('myapp.middleware.login_required_middleware')
    settings.MIDDLEWARE = middleware
    yield
    settings.MIDDLEWARE = original

I hope this helps for people down the road.

The versions:

• Python 3.7.2
• pytest 4.6.2
• pytest-django 3.5.0

[danihodovic]

I ran into the same issue when modifying the MIDDLEWARE settings.

@pytest.mark.django_db
def test_disable_cache_control(settings, client):
    settings.MIDDLEWARE += ["jobs.middleware.DisableCacheControl"]

after one test modified the middleware, other tests would inherit the same middleware. I think this is counter-intuitive behavior. I think Django's default test settings decorator resets the middleware entirely https://docs.djangoproject.com/en/2.2/topics/testing/tools/#django.test.SimpleTestCase.modify_settings

Thanks @gh640 for the tip, I resolved it in a similar manner for a single test

@pytest.mark.django_db
def test_disable_cache_control(settings, client):
    # https://github.com/pytest-dev/pytest-django/issues/601#issuecomment-505280173
    original_middleware = settings.MIDDLEWARE
    try:
        middleware = settings.MIDDLEWARE[:]
        settings.MIDDLEWARE = middleware + ["jobs.middleware.DisableCacheControl"]
        res = client.get("/")
        assert "cache-control" not in res
    finally:
        settings.MIDDLEWARE = original_middleware

[blueyed]

Thanks for the info so far - I've not looked into it in detail, but is sounds like we should /could have this fixed? Therefore please consider creating a PR for it.

[mschoettle]

~Although it does not completely match the issue description it happens to me for settings.DEBUG as well.~

Sorry, my bad. I think it is unrelated since it only happens if settings.DEBUG is accessed directly in a module (such as URL conf) which if there are two tests, the first one will cause the URLs to be loaded and the outcome is set at that point according to the setting.