[Django] #32489: Add a generator function in runner.py to iterate over a test suite's test cases
Django
cases
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: nobody
Jerdonek |
Type: | Status: new
Cleanup/optimization |
Component: Testing | Version: master
framework | Keywords: test
Severity: Normal | suite,iterator,
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
There are four functions or methods in
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py
test/runner.py] that need to iterate over a test suite's test cases:
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L684-L699
DiscoverRunner._get_databases()]
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L792-L815
partition_suite_by_type()]
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L818-L828
partition_suite_by_case()]
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L831-L848
filter_tests_by_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)`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32489>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
cases
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage:
suite,iterator, | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
New description:
There are four functions or methods in
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py
test/runner.py] that need to iterate over a test suite's test cases:
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L684-L699
DiscoverRunner._get_databases(self, suite)]
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L792-L815
partition_suite_by_type(suite, classes, bins, reverse=False)]
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L818-L828
partition_suite_by_case(suite)]
*
[https://github.com/django/django/blob/64a0d1ef6e7a6739148996e9584bbb61fe3dcc60/django/test/runner.py#L831-L848
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)`.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:1>
Django
cases
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage:
suite,iterator, | Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
If the function were implemented recursively, I think it could be
something like this (untested):
{{{#!python
def iter_test_cases(suite):
suite_class = type(suite)
for test in suite:
if isinstance(test, suite_class):
yield from iter_test_cases(suite)
else:
yield test
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:2>
Django
cases
Reporter: Chris Jerdonek | Owner: nobody
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Nick Pope):
* stage: Unreviewed => Accepted
Comment:
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()`.
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:3>
Django
cases
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test suite,iterator, | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:4>
Django
cases
Reporter: Chris Jerdonek | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test suite,iterator, | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Chris Jerdonek):
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.
(I can update my code snippet above to support passing `reverse`.)
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:5>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
suite,iterator, |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Chris Jerdonek
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:6>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: master
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Accepted
suite,iterator, |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:7>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: test | Triage Stage: Ready for
suite,iterator, | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:8>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Component: Testing framework | Version: dev
Keywords: test | Triage Stage: Ready for
suite,iterator, | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"22c9af0eae5be38618b5facbb55f5d62c661d9af" 22c9af0e]:
{{{
#!CommitTicketReference repository=""
revision="22c9af0eae5be38618b5facbb55f5d62c661d9af"
Fixed #32489 -- Added iter_test_cases() to iterate over a TestSuite.
This also makes partition_suite_by_type(), partition_suite_by_case(),
filter_tests_by_tags(), and DiscoverRunner._get_databases() to use
iter_test_cases().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:9>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Testing framework | Version: dev
Severity: Normal | Resolution: fixed
Keywords: test | Triage Stage: Ready for
suite,iterator, | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
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
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:10>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Testing framework | Version: dev
Severity: Normal | Resolution: fixed
Keywords: test | Triage Stage: Ready for
suite,iterator, | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"465fdffda0507fa407bf78f5823cd83730b5ffb7" 465fdffd]:
{{{
#!CommitTicketReference repository=""
revision="465fdffda0507fa407bf78f5823cd83730b5ffb7"
Refs #32489 -- Simplified filter_tests_by_tags().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:13>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Testing framework | Version: dev
Severity: Normal | Resolution: fixed
Keywords: test | Triage Stage: Ready for
suite,iterator, | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"cc12894017c3b941c51f80b8d8e50da758f67802" cc12894]:
{{{
#!CommitTicketReference repository=""
revision="cc12894017c3b941c51f80b8d8e50da758f67802"
Refs #32489 -- Removed unneeded partition_suite_by_type().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:11>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Testing framework | Version: dev
Severity: Normal | Resolution: fixed
Keywords: test | Triage Stage: Ready for
suite,iterator, | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"8ff75a3d9ec4290ad7108b2242ac8e6e2aa5d5ea" 8ff75a3]:
{{{
#!CommitTicketReference repository=""
revision="8ff75a3d9ec4290ad7108b2242ac8e6e2aa5d5ea"
Refs #32489 -- Simplified partition_suite_by_case().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:12>
Django
cases
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: closed
Component: Testing framework | Version: dev
Severity: Normal | Resolution: fixed
Keywords: test | Triage Stage: Ready for
suite,iterator, | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"d8a4bcffdb116ca094d0da45ec4644f22133e02e" d8a4bcff]:
{{{
#!CommitTicketReference repository=""
revision="d8a4bcffdb116ca094d0da45ec4644f22133e02e"
Refs #32489 -- Doc'd and tested iter_test_cases() support for an iterable
of tests.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32489#comment:14>