#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 , 4 years ago
Description: | modified (diff) |
---|
comment:3 by , 4 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 , 4 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 , 4 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(test, reverse=reverse) else: yield test
comment:6 by , 4 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:7 by , 4 years ago
Has patch: | set |
---|
comment:8 by , 4 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
comment:10 by , 4 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):