[Django] #32668: Separate test-collection setup from runtests.py's setup() for use in get_app_test_labels()
Django
get_app_test_labels()
------------------------------------------------+------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
This is another clean-up follow-up to
[https://github.com/django/django/pull/4106 PR #4106], similar to
([https://github.com/django/django/pull/14276 PR #14276].
TLDR: Refactor out from `runtests.py`'s
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L135
setup()] and
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L263
teardown()] simpler `setup_test_collection()` and
`teardown_test_collection()` functions for use in the new
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L337-L342
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
[https://github.com/django/django/blob/413c15ef2e3d3958fb641a023bc1e2d15fb2b228/tests/runtests.py#L332
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 (always
needed), and one for setting up the test run (needed only for
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L301
django_tests()] but not `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 [https://github.com/django/django/pull/4106 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 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32668>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: nobody
Cleanup/optimization |
Component: Testing framework | Version: dev
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Description changed by Chris Jerdonek:
Old description:
New description:
This is another clean-up follow-up to
[https://github.com/django/django/pull/4106 PR #4106], similar to
[https://github.com/django/django/pull/14276 PR #14276].
TLDR: Refactor out from `runtests.py`'s 125-line
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L135
setup()] and
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L263
teardown()] simpler `setup_test_collection()` and
`teardown_test_collection()` functions for use in the new
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L337-L342
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
[https://github.com/django/django/blob/413c15ef2e3d3958fb641a023bc1e2d15fb2b228/tests/runtests.py#L332
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
[https://github.com/django/django/blob/54e94640ace261b14cf8cdb1fae3dc6f068a5f87/tests/runtests.py#L301
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 [https://github.com/django/django/pull/4106 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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:1>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* stage: Unreviewed => Accepted
Comment:
Yes, thank you for the good explanation Chris, this makes sense.
> 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.
I agree `setup()` is a ''little opaque'' so +1.
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:2>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Chris Jerdonek):
* owner: nobody => Chris Jerdonek
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:3>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Chris Jerdonek):
* has_patch: 0 => 1
Comment:
Here is a first PR: https://github.com/django/django/pull/14477
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:4>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"0d2816133c7e81966bec36b2e3af2fa1ff4f12c2" 0d281613]:
{{{
#!CommitTicketReference repository=""
revision="0d2816133c7e81966bec36b2e3af2fa1ff4f12c2"
Refs #32668 -- Simplified get_test_modules() in runtests.py.
This simplifies runtests.py's get_test_modules() in a few ways. For
example, it changes the function to yield strings instead of returning
pairs of strings, which simplifies the calling code.
This commit also changes SUBDIRS_TO_SKIP from a list to a dict since
the directories to skip depend on the parent directory.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:7>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"90f41c2d9165e6e5b7c8f2229f8d01ecc4b99281" 90f41c2]:
{{{
#!CommitTicketReference repository=""
revision="90f41c2d9165e6e5b7c8f2229f8d01ecc4b99281"
Refs #32668 -- Made setup()'s test_labels argument optional in
runtests.py.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:5>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"ffc0d57a04141b6bc8f1263849cfcc9566cf63a8" ffc0d57a]:
{{{
#!CommitTicketReference repository=""
revision="ffc0d57a04141b6bc8f1263849cfcc9566cf63a8"
Refs #32668 -- Refactored away module_found_in_labels in runtests.py's
setup().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:6>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:8>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Chris Jerdonek):
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/14484
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:9>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by GitHub <noreply@…>):
In [changeset:"9812b486b5237933559c1bea8a11eff2b0d2af49" 9812b486]:
{{{
#!CommitTicketReference repository=""
revision="9812b486b5237933559c1bea8a11eff2b0d2af49"
Refs #32668 -- Simplified start_at/start_after logic in runtests.py's
setup().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:10>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"9b9cea04b98b66a68b7588cc4c34401f5f7d7c12" 9b9cea04]:
{{{
#!CommitTicketReference repository=""
revision="9b9cea04b98b66a68b7588cc4c34401f5f7d7c12"
Refs #32668 -- Added gis_enabled argument to get_test_modules().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:11>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"87bb746ea6bb4bf6fb9f87065d5daaca857a130c" 87bb746e]:
{{{
#!CommitTicketReference repository=""
revision="87bb746ea6bb4bf6fb9f87065d5daaca857a130c"
Refs #32668 -- Renamed setup()/teardown() to
setup_run_tests()/teardown_run_tests() in runtests.py.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:15>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"9389d4d3dbfed050045d514c19bc11680f67c6d6" 9389d4d3]:
{{{
#!CommitTicketReference repository=""
revision="9389d4d3dbfed050045d514c19bc11680f67c6d6"
Refs #32668 -- Changed bisect_tests() and paired_tests() to use only
setup_collect_tests().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:14>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b3083d5bd244ca90848aaa2a1c8e7ba7f9cc72f4" b3083d5b]:
{{{
#!CommitTicketReference repository=""
revision="b3083d5bd244ca90848aaa2a1c8e7ba7f9cc72f4"
Refs #32668 -- Refactored out setup_collect_tests() in runtests.py.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:12>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"e96e93618c608a422a6fee81de2f932ca3094d81" e96e936]:
{{{
#!CommitTicketReference repository=""
revision="e96e93618c608a422a6fee81de2f932ca3094d81"
Refs #32668 -- Passed setup()'s return value to run_tests() instead of
get_installed().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:13>
Django
get_app_test_labels()
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: assigned => closed
* resolution: => fixed
--
Ticket URL: <https://code.djangoproject.com/ticket/32668#comment:16>