#32489 closed Cleanup/optimization (fixed)
Add a generator function in runner.py to iterate over a test suite's test cases
| Reported by: | Chris Jerdonek | Owned by: | Chris Jerdonek |
|---|---|---|---|
| Component: | Testing framework | Version: | dev |
| Severity: | Normal | Keywords: | test suite, iterator, |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
There are four functions or methods in test/runner.py that need to iterate over a test suite's test cases:
- DiscoverRunner._get_databases(self, suite)
- partition_suite_by_type(suite, classes, bins, reverse=False)
- partition_suite_by_case(suite)
- filter_tests_by_tags(suite, tags, exclude_tags)
Each of these functions is a little harder to understand than it needs to be because each needs to contain the logic of how to iterate over a test suite, which isn't obvious. In particular, they're all implemented as recursive functions, since test suites can contain test suites.
If runner.py contained a single helper function that accepts a test suite instance and returns an iterator of the test suite's test cases, then each of those four functions could be simplified. They could contain a simple for loop over the iterator's return value and not have to be recursive.
The helper function would be pretty simple and could itself be recursive (but it wouldn't need to be). It could be called something like iter_test_cases(suite).
Change History (14)
comment:1 by , 5 years ago
| Description: | modified (diff) |
|---|
comment:3 by , 5 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
Sounds sensible to me. As you say, these are all just slightly different enough to be hard to understand.
You proposed implementation seems fine. We might just need to be careful with the groupby in partition_suite_by_case().
comment:4 by , 5 years ago
Thanks. To lower the bar for PR's, I would suggest that the first PR could start by adding the function and changing just one of the existing functions to use it (e.g. partition_suite_by_type()). Subsequent PR's could change one or more of the other functions to use it.
comment:5 by , 5 years ago
Also, I just noticed that the proposed iter_test_cases() should probably also sport a reverse keyword argument, to support e.g. partition_suite_by_type(). That way partition_suite_by_type() won't need to construct the full list before reversing it.
The new proposed implementation of iter_test_cases() would be (following the pattern of partition_suite_by_type()):
def iter_test_cases(suite, reverse=False): """Return an iterator over a test suite's unittest.TestCase objects.""" # Django permits custom TestSuite classes, so use the caller's class. suite_class = type(suite) if reverse: suite = reversed(tuple(suite)) for test in suite: if isinstance(test, suite_class): yield from iter_test_cases(suite, reverse=reverse) else: yield test
comment:6 by , 5 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:7 by , 5 years ago
| Has patch: | set |
|---|
comment:8 by , 5 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:10 by , 5 years ago
I posted a follow-up PR with some additional simplifications made possible by the change in the first PR:
https://github.com/django/django/pull/14085
If the function were implemented recursively, I think it could be something like this (untested):