New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting env vars for pytest (DJANGO_SETTINGS_MODULE) #6526

Open
alexcouper opened this Issue Sep 19, 2018 · 14 comments

Comments

Projects
None yet
3 participants
@alexcouper

alexcouper commented Sep 19, 2018

Problem

I need to set an env variable for pytest to correctly work with django projects. I reached out on slack and started using the suggested approach:

prep_command(
    name="django-settings-path-env",
    prep_args=["-n", "DJANGO_SETTINGS_MODULE=project2.config.settings"],
    prep_environ=True,
    prep_executable="echo",
    scope="test",
)

python_tests(
    name="tests",
    sources=rglobs("**/test_*.py"),
    # This glob is annoying, but needs to include test files and conftest.py
    dependencies=[
        ":django-settings-path-env",
        ":lib",
        ":pytest-django",
    ],
    compatibility="CPython>=3",
)

All was good, except that if I need to run tests against 2 django projects in one pants command things break:

$ ./pants test src/python/project1:tests src/python/project2:tests

15:40:47 00:02       [run]
                     ============== test session starts ===============
                     platform darwin -- Python 3.4.8, pytest-3.8.0, py-1.6.0, pluggy-0.7.1
                     Django settings: project2.config.settings (from environment variable)
                     rootdir: /Users/alex/Work/opensource/pants-django-tests-issue/.pants.d, inifile: /Users/alex/Work/opensource/pants-django-tests-issue/.pants.d/test/pytest-prep/CPython-3.4.8/ed3debf4b8010fa4f3f4d01e756701a77c55a13f/pytest.ini
                     plugins: cov-2.4.0, timeout-1.2.1, django-3.4.2
                     collected 1 item / 1 errors

                     src/python/project2/bar/tests/test_bar.py . [100%]

                     ===================== ERRORS =====================
                      ERROR collecting pyprep/sources/2fb2e251d3f1ce1035b24617b154567c06f60875/project1/foo/tests/test_foo.py
                     .pants.d/pyprep/sources/2fb2e251d3f1ce1035b24617b154567c06f60875/project1/foo/tests/test_foo.py:7: in <module>
                         from project1.foo.models import Foo
                     .pants.d/pyprep/sources/2fb2e251d3f1ce1035b24617b154567c06f60875/project1/foo/models.py:9: in <module>
                         class Foo(models.Model):
                     .pants.d/pyprep/requirements/CPython-3.4.8/71cf57d0e155e25ce68159af83e67455350952d6/.deps/Django-2.0.8-py3-none-any.whl/django/db/models/base.py:108: in __new__
                         "INSTALLED_APPS." % (module, name)
                     E   RuntimeError: Model class project1.foo.models.Foo doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS.
                      generated xml file: /Users/alex/Work/opensource/pants-django-tests-issue/.pants.d/test/pytest/2e2267bb6aa378cd93ee1cc0fe4ba457212b0e44/junitxml/TEST-2e2267bb6aa378cd93ee1cc0fe4ba457212b0e44.xml
                     ======= 1 passed, 1 error in 0.38 seconds ========

                   src/python/project1:tests                                                       .....   FAILURE
                   src/python/project2:tests                                                       .....   SUCCESS

Alternative solutions

I'd love to provide my own pytest.ini to the test runner. At the moment the pytest run seems to explicitly use the default blank one. Is there anyway I could inject some variables - possibly defined on a python_tests target - into the pytest.ini?

Reproducing

I've created a test project here that reproduces the issue.

To see the issue, run:

./pants test src/python/project1:tests src/python/project2:tests

Note that both of these commands pass:

./pants test src/python/project1:tests
./pants test src/python/project2:tests 
@alexcouper

This comment has been minimized.

Show comment
Hide comment
@alexcouper

alexcouper Sep 19, 2018

@benjyw, I'm slightly hopeful that maybe you can shed some light on how you're running tests in your django world (#6398 (comment))

alexcouper commented Sep 19, 2018

@benjyw, I'm slightly hopeful that maybe you can shed some light on how you're running tests in your django world (#6398 (comment))

@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Sep 19, 2018

Contributor

Yep, let me recollect how we do this and I will follow up.

Contributor

benjyw commented Sep 19, 2018

Yep, let me recollect how we do this and I will follow up.

@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Sep 19, 2018

Contributor

OK, memory refreshed. We don't use prep_command at all. We do this via the conftest.py mechanism.

As a reminder, pytest imports all conftest.py files it finds in every dir that contains test modules it's looking at, and - crucially - also the ones in all parent dirs, up to the source root, in that order. Those files contain hooks that pytest invokes at various points in test running.

So here's how we set up Django for just the apps we care about in a given test run:

  1. We have a conftest.py in our top-level package, that declares an APPS_UNDER_TEST = [].

  2. Each Django app has a conftest.py that appends necessary apps to APPS_UNDER_TEST. E.g.,

from myrootpkg.conftest import APPS_UNDER_TEST
APPS_UNDER_TEST.extend(['polymorphic', 'myrootpkg.app1.apps.App1Config'])
  1. The top-level conftest.py implements the hook pytest_configure() to do setup for everything in APPS_UNDER_TEST (after deduping). Because it's run last, APPS_UNDER_TEST is fully populated.
Contributor

benjyw commented Sep 19, 2018

OK, memory refreshed. We don't use prep_command at all. We do this via the conftest.py mechanism.

As a reminder, pytest imports all conftest.py files it finds in every dir that contains test modules it's looking at, and - crucially - also the ones in all parent dirs, up to the source root, in that order. Those files contain hooks that pytest invokes at various points in test running.

So here's how we set up Django for just the apps we care about in a given test run:

  1. We have a conftest.py in our top-level package, that declares an APPS_UNDER_TEST = [].

  2. Each Django app has a conftest.py that appends necessary apps to APPS_UNDER_TEST. E.g.,

from myrootpkg.conftest import APPS_UNDER_TEST
APPS_UNDER_TEST.extend(['polymorphic', 'myrootpkg.app1.apps.App1Config'])
  1. The top-level conftest.py implements the hook pytest_configure() to do setup for everything in APPS_UNDER_TEST (after deduping). Because it's run last, APPS_UNDER_TEST is fully populated.
@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Sep 19, 2018

Contributor

I'll post an example of our top-level conftest.py in a bit. I may also create an example project in the pantsbuild/pants repo, as a future guide.

Contributor

benjyw commented Sep 19, 2018

I'll post an example of our top-level conftest.py in a bit. I may also create an example project in the pantsbuild/pants repo, as a future guide.

@alexcouper

This comment has been minimized.

Show comment
Hide comment
@alexcouper

alexcouper Sep 19, 2018

@benjyw very clever I'll try that and report back

alexcouper commented Sep 19, 2018

@benjyw very clever I'll try that and report back

@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Sep 19, 2018

Contributor

This is (derived from) our top-level conftest.py, for reference:

from django.conf import settings
from myrootpkg.util.testing_db import TestingDB

# Lower-level conftest.py files can append to this to inform this top-level conftest.py
# that these apps need to be loaded.
APPS_UNDER_TEST = []

def pytest_cmdline_preparse(args):
  args[:] = args + ['--nomigrations']

def pytest_configure():
  if not APPS_UNDER_TEST:
    return

  seen = set()
  deduped_apps_under_test = [x for x in APPS_UNDER_TEST if not (x in seen or seen.add(x))]

  # This is how we ensure a running postgres instance to test against, but you may have your own method.
  test_db = TestingDB()
  # Note: postgres instance may already exist from previous test run. If not, we will start one.
  # Each test invocation uses its own database, so it's fine to run just one global testing instance.
  test_db.ensure_running()

  # Note that we can't do this directly in lower-level conftest.py files, because Django errors if
  # you call settings.configure() more than once.
  settings.configure(
    SECRET_KEY = 'TEST_SECRET_KEY',
    TIME_ZONE = 'UTC',
    USE_TZ = True,
    LOG_QUERY_ENABLED = False,
    PROFILE = False,
    DATABASES = {
      'default': {
        'ENGINE': 'django.db.backends.postgresql',
        'NAME': 'postgres',
        'USER': 'postgres',
        'HOST': test_db.host,
        'PORT': test_db.port,
      }
    },
    # Standard apps that we always (or almost always) need, plus the ones explicitly specified.
    INSTALLED_APPS = (
      'django.contrib.auth',
      'django.contrib.contenttypes',
      'django.contrib.messages',
      'django.contrib.sessions',
      'django.contrib.staticfiles',
      'myrootpkg.django.some_app_we_always_need',
    ) + tuple(deduped_apps_under_test)
  )
Contributor

benjyw commented Sep 19, 2018

This is (derived from) our top-level conftest.py, for reference:

from django.conf import settings
from myrootpkg.util.testing_db import TestingDB

# Lower-level conftest.py files can append to this to inform this top-level conftest.py
# that these apps need to be loaded.
APPS_UNDER_TEST = []

def pytest_cmdline_preparse(args):
  args[:] = args + ['--nomigrations']

def pytest_configure():
  if not APPS_UNDER_TEST:
    return

  seen = set()
  deduped_apps_under_test = [x for x in APPS_UNDER_TEST if not (x in seen or seen.add(x))]

  # This is how we ensure a running postgres instance to test against, but you may have your own method.
  test_db = TestingDB()
  # Note: postgres instance may already exist from previous test run. If not, we will start one.
  # Each test invocation uses its own database, so it's fine to run just one global testing instance.
  test_db.ensure_running()

  # Note that we can't do this directly in lower-level conftest.py files, because Django errors if
  # you call settings.configure() more than once.
  settings.configure(
    SECRET_KEY = 'TEST_SECRET_KEY',
    TIME_ZONE = 'UTC',
    USE_TZ = True,
    LOG_QUERY_ENABLED = False,
    PROFILE = False,
    DATABASES = {
      'default': {
        'ENGINE': 'django.db.backends.postgresql',
        'NAME': 'postgres',
        'USER': 'postgres',
        'HOST': test_db.host,
        'PORT': test_db.port,
      }
    },
    # Standard apps that we always (or almost always) need, plus the ones explicitly specified.
    INSTALLED_APPS = (
      'django.contrib.auth',
      'django.contrib.contenttypes',
      'django.contrib.messages',
      'django.contrib.sessions',
      'django.contrib.staticfiles',
      'myrootpkg.django.some_app_we_always_need',
    ) + tuple(deduped_apps_under_test)
  )
@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Sep 19, 2018

Contributor

One thing to note is that the order in APPS_UNDER_TEST matters - Django expects INSTALLED_APPS to be in dependency order from least to most dependent. But you don't control the order in which leaf-level conftest.py are called. So when adding app1 you have to add all the apps it depends on, in the right order. Then the deduping will do the right thing.

Hope this helps!

Contributor

benjyw commented Sep 19, 2018

One thing to note is that the order in APPS_UNDER_TEST matters - Django expects INSTALLED_APPS to be in dependency order from least to most dependent. But you don't control the order in which leaf-level conftest.py are called. So when adding app1 you have to add all the apps it depends on, in the right order. Then the deduping will do the right thing.

Hope this helps!

@alexcouper

This comment has been minimized.

Show comment
Hide comment
@alexcouper

alexcouper Sep 19, 2018

@benjyw thanks for the pointers.

I guess this means you're not using the django test client in tests then? Or if you are do you create a urls.py that is the union of all django projects urls.py?

alexcouper commented Sep 19, 2018

@benjyw thanks for the pointers.

I guess this means you're not using the django test client in tests then? Or if you are do you create a urls.py that is the union of all django projects urls.py?

@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Sep 21, 2018

Contributor

We're not using Django's test runner, we're using pytest the normal way. We haven't had to deal with urls.py, what's an example of the requirements?

Contributor

benjyw commented Sep 21, 2018

We're not using Django's test runner, we're using pytest the normal way. We haven't had to deal with urls.py, what's an example of the requirements?

@alexcouper

This comment has been minimized.

Show comment
Hide comment
@alexcouper

alexcouper Sep 24, 2018

I'm using the django test client (via pytest) to be able to make fake requests to different endpoints defined in the django projects that I'm testing.

The test client needs urls configured (via ROOT_URLCONF) and this is different for each project.

I think in order to get this unified settings.configure approach to work for me then, I'd either have to create a urls.py that was a union of all django projects urls and make sure that apps didn't override each other, or else try to find a way for tests to patch the ROOT_URLCONF - but altering settings after the fact seems to not be recommended.

I can update the example repo with this problem if you're still interested.

I am interested in knowing if you're just not doing these kinds of tests, or if you solve this in a different way.

from django.core.urlresolvers import reverse
from django.test import Client

client = Client()

class TestGetRecrods(unittest.TestCase):
    def test_get_records(self):
        response = client.get(reverse('records_index'))
        self.assertEqual(200, response.status_code)

alexcouper commented Sep 24, 2018

I'm using the django test client (via pytest) to be able to make fake requests to different endpoints defined in the django projects that I'm testing.

The test client needs urls configured (via ROOT_URLCONF) and this is different for each project.

I think in order to get this unified settings.configure approach to work for me then, I'd either have to create a urls.py that was a union of all django projects urls and make sure that apps didn't override each other, or else try to find a way for tests to patch the ROOT_URLCONF - but altering settings after the fact seems to not be recommended.

I can update the example repo with this problem if you're still interested.

I am interested in knowing if you're just not doing these kinds of tests, or if you solve this in a different way.

from django.core.urlresolvers import reverse
from django.test import Client

client = Client()

class TestGetRecrods(unittest.TestCase):
    def test_get_records(self):
        response = client.get(reverse('records_index'))
        self.assertEqual(200, response.status_code)
@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Sep 24, 2018

Contributor

We're not doing those kinds of tests today. Let me think about this for a bit...

Contributor

benjyw commented Sep 24, 2018

We're not doing those kinds of tests today. Let me think about this for a bit...

@jsirois

This comment has been minimized.

Show comment
Hide comment
@jsirois

jsirois Sep 27, 2018

Member

@alexcouper the OP says env var export works mod running multiple targets at once. Have you tried setting env vars on a per-target basis using conftest.py? The idea would be one test target globs in a conftest.py source ~like so:

import os


_CLEANUPS = {}


def _cleanup(name, value):
  if value is None:
    del os.environ[name]
  else
    os.environ[name] = value


def pytest_runtest_setup(item):
  old_value = os.environ.get('DJANGO_SETTINGS_MODULE')
  _CLEANUPS[item] = lamba: _cleanup('DJANGO_SETTINGS_MODULE', old_value)
  os.environ['DJANGO_SETTINGS_MODULE'] = 'project2.config.settings'


def pytest_runtest_teardown(item, nextitem):
  _CLEANUPS.get(item, lambda: None)()

And another - similar but with different env var values.

Member

jsirois commented Sep 27, 2018

@alexcouper the OP says env var export works mod running multiple targets at once. Have you tried setting env vars on a per-target basis using conftest.py? The idea would be one test target globs in a conftest.py source ~like so:

import os


_CLEANUPS = {}


def _cleanup(name, value):
  if value is None:
    del os.environ[name]
  else
    os.environ[name] = value


def pytest_runtest_setup(item):
  old_value = os.environ.get('DJANGO_SETTINGS_MODULE')
  _CLEANUPS[item] = lamba: _cleanup('DJANGO_SETTINGS_MODULE', old_value)
  os.environ['DJANGO_SETTINGS_MODULE'] = 'project2.config.settings'


def pytest_runtest_teardown(item, nextitem):
  _CLEANUPS.get(item, lambda: None)()

And another - similar but with different env var values.

alexcouper pushed a commit to alexcouper/pants-django-tests-issue that referenced this issue Oct 1, 2018

@alexcouper

This comment has been minimized.

Show comment
Hide comment
@alexcouper

alexcouper Oct 1, 2018

@jsirois thanks for the input.

This doesn't work because typically in a test file, imports of models happen at the top and will be run at collection time - and those result in exceptions:

django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

I've created a branch with this approach.

I also tried doing it at the pytest_collect_file stage but no joy. I think django isn't going to play nicely with this unless I can find a way to run pytest in some kind of multi-session mode.

alexcouper commented Oct 1, 2018

@jsirois thanks for the input.

This doesn't work because typically in a test file, imports of models happen at the top and will be run at collection time - and those result in exceptions:

django.core.exceptions.AppRegistryNotReady: Apps aren't loaded yet.

I've created a branch with this approach.

I also tried doing it at the pytest_collect_file stage but no joy. I think django isn't going to play nicely with this unless I can find a way to run pytest in some kind of multi-session mode.

@benjyw

This comment has been minimized.

Show comment
Hide comment
@benjyw

benjyw Oct 8, 2018

Contributor

Django's global state (as represented by settings and the app registry) is a huge pain when it comes to testing. There is a way to override some settings in tests: https://docs.djangoproject.com/en/2.1/topics/testing/tools/#overriding-settings

This doesn't work on all settings, but it does on ROOT_URLCONF, which would be the one you care about here, I think. And setting that in the test itself (with the @override_settings(ROOT_URLCONF=...) decorator) seems like the right thing to do anyway, since when testing an app you shouldn't need to know how it's slotted in to some larger URL scheme.

Does that make sense?

Contributor

benjyw commented Oct 8, 2018

Django's global state (as represented by settings and the app registry) is a huge pain when it comes to testing. There is a way to override some settings in tests: https://docs.djangoproject.com/en/2.1/topics/testing/tools/#overriding-settings

This doesn't work on all settings, but it does on ROOT_URLCONF, which would be the one you care about here, I think. And setting that in the test itself (with the @override_settings(ROOT_URLCONF=...) decorator) seems like the right thing to do anyway, since when testing an app you shouldn't need to know how it's slotted in to some larger URL scheme.

Does that make sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment