-
Notifications
You must be signed in to change notification settings - Fork 299
Fix for apps that don't load contenttypes module. #345
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
Conversation
Codecov Report
@@ Coverage Diff @@ ## develop #345 +/- ## =========================================== + Coverage 76.37% 76.38% +<.01% =========================================== Files 50 50 Lines 6024 6025 +1 =========================================== + Hits 4601 4602 +1 Misses 1423 1423
Continue to review full report at Codecov.
|
I agree that a test would be nice, but this might be one of those cases where the setup for that test would be extreme overkill since it involves things that happen at import time.
What do you think of adding a # pragma: no cover to the if clause that checks for contenttypes in INSTALLED_APPS? It seems like that might be a clue to future readers that this was considered and declared "not worth testing."
PR django-json-api#319 brought support for generic relations. Unfortunately apps that don't add contenttypes to it's INSTALLED_APPS would crash and burn: ``` RuntimeError: Model class django.contrib.contenttypes.models.ContentType doesn't declare an explicit app_label and isn't in an application in INSTALLED_APPS. ``` Also using `type(something) is object()` comparison as a safer alternative to `type(something) is type(None)`. If `something` happened to be `None` we would enter a branch that was never supposed to run.
36b0c58 to
20933e6
Compare
Added.
👍
PR #319 brought support for generic relations. Unfortunately apps that don't add contenttypes to it's INSTALLED_APPS would crash and burn:
I would like to add a test for this, but it would require two separate Django configs. Not sure how you would want to handle that in py.test.
Also using
type(something) is object()comparison as a safer alternative totype(something) is type(None). Ifsomethinghappened to beNonewe would enter a branch that was never supposed to run.