[Django] #32591: Change DiscoverRunner always to run _FailedTest "tests" first

17 views
Skip to first unread message

Django

unread,
Mar 24, 2021, 4:57:09 PM3/24/21
to django-...@googlegroups.com
#32591: Change DiscoverRunner always to run _FailedTest "tests" first
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: nobody
Jerdonek |
Type: | Status: new
Cleanup/optimization |
Component: Testing | Version: dev
framework | Keywords:
Severity: Normal | DiscoverRunner,_FailedTest
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Currently, `DiscoverRunner`'s default test order is to run `_FailedTest`
objects in a last "catch-all" test-type group, and to order them in no
particular way within that last group. Since Django already knows those
tests have failed, it seems like it would be better for the user if Django
always ran / reported those failed tests first. That would give the user
quicker feedback. Also, when `--failfast` is used, it would result in
tests failing faster (namely right away), instead of possibly waiting for
many tests to complete before getting to the `_FailedTest`.

This could be implemented by changing
[https://github.com/django/django/blob/f5a22442176878d07e80ae5ba0af1dd8d6910fbd/django/test/runner.py#L813
reorder_tests()] to prepend its list of classes with a bin for
`_FailedTest` objects.

--
Ticket URL: <https://code.djangoproject.com/ticket/32591>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Mar 25, 2021, 3:18:20 AM3/25/21
to django-...@googlegroups.com
#32591: Change DiscoverRunner always to run _FailedTest "tests" first
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,_FailedTest |
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:

Hey Chris — I'm happy to accept this.

With pytest I find the `-x --lf` combo to be quite effective. (That's not
quite what you're proposing here I think, since `--lf` will select only
the tests that previously failed — but "Carry on if all those passed" is
probably better behaviour...)

I don't know if folks will have ''thoughts'' on the exact behaviour
they're after, but I suspect we'll find out in review 😀

Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/32591#comment:1>

Django

unread,
Mar 25, 2021, 3:28:19 AM3/25/21
to django-...@googlegroups.com
#32591: Change DiscoverRunner always to run _FailedTest "tests" first
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned

Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,_FailedTest |
Has patch: 0 | Needs documentation: 0

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/32591#comment:2>

Django

unread,
Mar 26, 2021, 4:10:33 AM3/26/21
to django-...@googlegroups.com
#32591: Change DiscoverRunner always to run _FailedTest "tests" first
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,_FailedTest |
Has patch: 1 | Needs documentation: 0

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/14191

--
Ticket URL: <https://code.djangoproject.com/ticket/32591#comment:3>

Django

unread,
Mar 26, 2021, 8:04:57 AM3/26/21
to django-...@googlegroups.com
#32591: Change DiscoverRunner always to run _FailedTest "tests" first
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: closed

Component: Testing framework | Version: dev
Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Accepted
DiscoverRunner,_FailedTest |
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* status: assigned => closed
* resolution: => fixed


Comment:

In [changeset:"dcb06c2c6889d04506b48f025622357e2926c709" dcb06c2c]:
{{{
#!CommitTicketReference repository=""
revision="dcb06c2c6889d04506b48f025622357e2926c709"
Fixed #32591 -- Made DiscoverRunner order _FailedTest objects first.

Failures detected when loading tests are ordered before all of the
above for quicker feedback. This includes things like test modules that
couldn't be found or that couldn't be loaded due to syntax errors.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/32591#comment:4>

Reply all
Reply to author
Forward
0 new messages