Skip to content
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

Refs #20349 -- Avoided loading testing libraries when not needed. #15203

Merged
merged 1 commit into from Jan 25, 2022

Conversation

collinanderson
Copy link
Contributor

@collinanderson collinanderson commented Dec 15, 2021

This avoids loading django.test, unittest, and xml libraries when not needed.

See https://code.djangoproject.com/ticket/20349

This should give slightly faster startup time and slightly lower memory usage.

@collinanderson collinanderson changed the title Import setting_changed from django.core.signals Avoid loading django.test, unittest, and xml libraries when not needed. Dec 15, 2021
@charettes
Copy link
Member

charettes commented Dec 15, 2021

I so wish there was an easy way to test for that to prevent future regressions.

What if we called python -m django --settings=settings_install_all_contrib_apps mng_cmd_asserts_no_django_test in a subprocess.

Where the management command is placed in a test app referenced by settings_install_all_contrib_apps.INSTALLED_APPS and asserts that 'django.test' not in sys.modules?

@collinanderson
Copy link
Contributor Author

Makes sense, but I might need some help making that test. Where should it go? (admin_scripts/tests.py?) Is there already a settings_install_all_contrib_apps somewhere?

@charettes
Copy link
Member

Where should it go? (admin_scripts/tests.py?)

I'm not sure of where it should live. Maybe in test_utils or in a new test_modules one?

Is there already a settings_install_all_contrib_apps somewhere?

It doesn't exist yet but it could live as a module in the test app? I guess it could have all DB backend compatible contrib apps installed.

@ngnpope
Copy link
Member

ngnpope commented Dec 15, 2021

I so wish there was an easy way to test for that to prevent future regressions.

Maybe import-linter could be used? There are a whole bunch of other import patterns that we've tidied up in the past which would be nice to enforce, e.g. #11689.

@felixxm
Copy link
Member

felixxm commented Jan 25, 2022

@collinanderson Thanks 👍

@felixxm felixxm changed the title Avoid loading django.test, unittest, and xml libraries when not needed. Refs #20349 -- Avoided loading testing libraries when not needed. Jan 25, 2022
@felixxm felixxm merged commit 890bfa3 into django:main Jan 25, 2022
@collinanderson
Copy link
Contributor Author

Thanks. Yeah, I'm not super good at tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants