-
Notifications
You must be signed in to change notification settings - Fork 348
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
Refine the django.conf module check to see if the settings really are configured #668
Conversation
d20ab49
to
0f8f48c
Compare
configured flag in the Django settings object if django.conf has already | ||
been imported. | ||
|
||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case this needs to be fixed anyway, please remove the newline at the end.
(Inner) test fails during collection.. This fixes it: diff --git i/tests/test_django_settings_module.py w/tests/test_django_settings_module.py
index a7f716d..98b3f8e 100644
--- i/tests/test_django_settings_module.py
+++ w/tests/test_django_settings_module.py
@@ -368,18 +368,24 @@ def test_no_ds_but_django_conf_imported(testdir, monkeypatch):
testdir.makepyfile("""
import os
- # line copied from hypothesis/extras/django.py
- from django.conf import settings as django_settings
-
- from pytest_django.lazy_django import django_settings_is_configured
-
def test_django_settings_is_configured():
+ # Line copied from hypothesis/extras/django.py
+ from django.conf import settings as django_settings
+
+ from pytest_django.lazy_django import django_settings_is_configured
+
assert django_settings_is_configured() is False
def test_env():
+ # Line copied from hypothesis/extras/django.py
+ from django.conf import settings as django_settings
+
assert 'DJANGO_SETTINGS_MODULE' not in os.environ
def test_cfg(pytestconfig):
+ # Line copied from hypothesis/extras/django.py
+ from django.conf import settings as django_settings
+
assert pytestconfig.option.ds is None
""")
r = testdir.runpytest_subprocess('-s') It still fails then without your patch, but only with Especially for |
0f8f48c
to
8a881a4
Compare
I've modified my patch to import django.conf.settings and then I'm guessing the QA check is broken because of the recent flake8 release? At least it's not in code I touched :-) |
Ah, and, obviously in the end, having that patch on pytest_django makes it not interfere with my other tests! |
|
||
# Don't let pytest poke into this object, generating a | ||
# django.core.exceptions.ImproperlyConfigured | ||
del django_settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow.. inspect.isclass
/ isinstance
triggers the ImproperlyConfigured already..
What do you mean? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will rebase this after #670.
Ah, no, I did my patch before I saw yours; I just meant that the original issue was indeed fixed :) |
… configured Some modules, e.g. hypothesis, load some Django modules for their own fixtures. This means that the `django.conf` module sometimes gets loaded, even though it's never used (and the settings are not initialized). This causes unrelated test failures when pytest_django is installed, as pytest_django considers that having a loaded django.conf means the settings are set up and ready to be modified. The Django settings object provides a flag to check this condition, which we now use. Add a regression test that mimics what hypothesis did which makes pytest-django fail. Closes pytest-dev#599.
8a881a4
to
d8ef4f1
Compare
I see, rebased. |
Codecov Report
@@ Coverage Diff @@
## master #668 +/- ##
=========================================
+ Coverage 94.38% 94.4% +0.01%
=========================================
Files 33 33
Lines 1834 1840 +6
Branches 156 156
=========================================
+ Hits 1731 1737 +6
Misses 78 78
Partials 25 25
Continue to review full report at Codecov.
|
Some modules, e.g. hypothesis, load some Django modules for their own fixtures.
This means that the
django.conf
module sometimes gets loaded, even though it'snever used (and the settings are not initialized).
This causes unrelated test failures when pytest_django is installed, as
pytest_django considers that having a loaded django.conf means the settings are
set up and ready to be modified. The Django settings object provides a flag to
check this condition, which we now use.
Add a regression test that mimics what hypothesis did which makes pytest-django
fail.
Closes #599.