Opened 4 years ago
Closed 3 years ago
#32668 closed Cleanup/optimization (fixed)
Separate test-collection setup from runtests.py's setup() for use in get_app_test_labels()
Reported by: | Chris Jerdonek | Owned by: | Chris Jerdonek |
---|---|---|---|
Component: | Testing framework | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
This is another clean-up follow-up to PR #4106, similar to PR #14276.
TLDR: Refactor out from runtests.py
's 125-line setup() and teardown() simpler setup_test_collection()
and teardown_test_collection()
functions for use in the new get_app_test_labels() (used for bisect_tests()
and paired_tests()
). (The exact names aren't so important.)
Longer version:
Currently, runtests.py
's setup()
function and its role in getting the list of default test labels for bisect_tests()
, paired_tests()
, and test_runner.run_tests() is a bit complicated and harder to understand than it needs to be.
There are a couple reasons for this. First, setup()
is actually doing two kinds of setup: one for collecting the default test labels, and one for setting up the test run (needed only for django_tests() but never bisect_tests()
and paired_tests()
). Second, the way the list of default test labels is obtained is a bit roundabout. Namely, a bunch of apps are added to INSTALLED_APPS
, and then runtests.py
's get_installed()
is called to read from INSTALLED_APPS
. However, for test-collection, INSTALLED_APPS
doesn't actually need to be modified. You can see a side effect of this in the fact that get_installed()
doesn't just return test labels. It also returns the following modules, which no longer contain any tests (this is the thing that PR #4106 from six years ago fixed):
django.contrib.admin
, django.contrib.auth
, django.contrib.contenttypes
, django.contrib.flatpages
, django.contrib.messages
, django.contrib.redirects
, django.contrib.sessions
, django.contrib.sites
, django.contrib.staticfiles
.
By extracting out from setup()
a setup_test_collection()
function that just collects and returns the top-level, filtered test labels, I think things will become a lot easier to understand and work with -- not just for bisect_tests()
and paired_tests()
, but also for the main django_tests()
case.
Change History (16)
comment:1 by , 4 years ago
Description: | modified (diff) |
---|
comment:2 by , 4 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:4 by , 3 years ago
Has patch: | set |
---|
Here is a first PR: https://github.com/django/django/pull/14477
comment:8 by , 3 years ago
Has patch: | unset |
---|
comment:16 by , 3 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Yes, thank you for the good explanation Chris, this makes sense.
I agree
setup()
is a little opaque so +1.Thanks.