Opened 3 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 Chris Jerdonek)

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 Chris Jerdonek, 3 years ago

Description: modified (diff)

comment:2 by Carlton Gibson, 3 years ago

Triage Stage: UnreviewedAccepted

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.

comment:3 by Chris Jerdonek, 3 years ago

Owner: changed from nobody to Chris Jerdonek
Status: newassigned

comment:4 by Chris Jerdonek, 3 years ago

Has patch: set

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 90f41c2:

Refs #32668 -- Made setup()'s test_labels argument optional in runtests.py.

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In ffc0d57a:

Refs #32668 -- Refactored away module_found_in_labels in runtests.py's setup().

comment:7 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 0d281613:

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.

comment:8 by Mariusz Felisiak, 3 years ago

Has patch: unset

comment:10 by GitHub <noreply@…>, 3 years ago

In 9812b486:

Refs #32668 -- Simplified start_at/start_after logic in runtests.py's setup().

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 9b9cea04:

Refs #32668 -- Added gis_enabled argument to get_test_modules().

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In b3083d5b:

Refs #32668 -- Refactored out setup_collect_tests() in runtests.py.

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In e96e936:

Refs #32668 -- Passed setup()'s return value to run_tests() instead of get_installed().

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 9389d4d3:

Refs #32668 -- Changed bisect_tests() and paired_tests() to use only setup_collect_tests().

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

In 87bb746e:

Refs #32668 -- Renamed setup()/teardown() to setup_run_tests()/teardown_run_tests() in runtests.py.

comment:16 by Mariusz Felisiak, 3 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top