[Django] #32609: runtests.py setup should use DiscoverRunner's test label logic
Django
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: nobody
Jerdonek |
Type: | Status: new
Cleanup/optimization |
Component: Testing | Version: dev
framework | Keywords:
Severity: Normal | DiscoverRunner,runtests
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
I noticed that `runtests.py` does its own rudimentary "parsing" of the
provided test labels here:
https://github.com/django/django/blob/548dce50cf548e777a0c34b5485a146a0606ae73/tests/runtests.py#L128-L132
However, it would be better if it used the same logic as
`DiscoverRunner.build_suite()`:
https://github.com/django/django/blob/548dce50cf548e777a0c34b5485a146a0606ae73/django/test/runner.py#L612
There are a few reasons for this. First, it would let `runtests` take into
account the test tags when determining which app modules apply. Second, it
would centralize the test label logic, which should simplify maintenance.
(For example, I was previously unaware of this code path, which explains
why some things mysteriously weren't working before.) Third, it might even
permit test labels to be directory paths when used with `runtests.py`, as
a free side effect. (Currently, directory paths don't seem to work with
`runtests.py`, I believe for this reason.)
--
Ticket URL: <https://code.djangoproject.com/ticket/32609>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage:
DiscoverRunner,runtests | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> I noticed that `runtests.py` does its own rudimentary "parsing" of the
> provided test labels here:
> https://github.com/django/django/blob/548dce50cf548e777a0c34b5485a146a0606ae73/tests/runtests.py#L128-L132
>
> However, it would be better if it used the same logic as
> `DiscoverRunner.build_suite()`:
> https://github.com/django/django/blob/548dce50cf548e777a0c34b5485a146a0606ae73/django/test/runner.py#L612
>
> There are a few reasons for this. First, it would let `runtests` take
> into account the test tags when determining which app modules apply.
> Second, it would centralize the test label logic, which should simplify
> maintenance. (For example, I was previously unaware of this code path,
> which explains why some things mysteriously weren't working before.)
> Third, it might even permit test labels to be directory paths when used
> with `runtests.py`, as a free side effect. (Currently, directory paths
> don't seem to work with `runtests.py`, I believe for this reason.)
New description:
I noticed that `runtests.py` does its own rudimentary "parsing" of the
provided test labels here:
https://github.com/django/django/blob/548dce50cf548e777a0c34b5485a146a0606ae73/tests/runtests.py#L128-L132
However, it would be better if it used the same logic as
`DiscoverRunner.build_suite()`:
https://github.com/django/django/blob/548dce50cf548e777a0c34b5485a146a0606ae73/django/test/runner.py#L612
There are a few reasons for this. First, it would let `runtests` take into
account the test tags when determining which app modules apply. Second, it
would centralize the test label logic, which should simplify maintenance.
(For example, I was previously unaware of this code path, which explains
why some things mysteriously weren't working before.) Third, it might even
permit test labels to be directory paths when used with `runtests.py`, as
a free side effect. (Currently, directory paths don't seem to work with
`runtests.py`, I believe for this reason.) A fourth is that `runtests.py`
will fail faster if a bad test label is passed.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:1>
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
DiscoverRunner,runtests |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:2>
Django
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,runtests |
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/32609#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,runtests |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
Here is a first PR: https://github.com/django/django/pull/14276
I want to wait until #32611 is resolved before doing more because there is
overlap, but this is a good change to make in isolation.
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,runtests |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"413c15ef2e3d3958fb641a023bc1e2d15fb2b228" 413c15ef]:
{{{
#!CommitTicketReference repository=""
revision="413c15ef2e3d3958fb641a023bc1e2d15fb2b228"
Refs #32609 -- Simplified test_labels_set construction in runtests.py's
setup().
Follow up to 7cf3a5786bc76374e743fbc0c1a1c8470a61f6c0.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:5>
Django
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,runtests |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:6>
Django
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,runtests |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/14507
This PR isn't the approach I had planned when filing this ticket, but it
achieves the main part of what I was after, which is this part of my
original comment:
> Third, it might even permit test labels to be directory paths when used
with runtests.py, as a free side effect. (Currently, directory paths don't
seem to work with `runtests.py`, I believe for this reason.)
In other words, it fixes this issue that Carlton encountered when running
`runtests.py`:
https://github.com/django/django/pull/14180#pullrequestreview-624099857
As for the rest of the points, I'm not sure any longer if it's possible or
desirable to call `DiscoverRunner.build_suite()`s logic from within
`runtests.py`. The reason is that there's a bootstrapping issue: To load
tests you need to do `runtests.py`'s setup, but to do `runtests.py`'s
setup, you first have to know what the needed test modules are. This is
after developing a better understanding of `runtests.py` in the course of
working on #32668.
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
DiscoverRunner,runtests |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: dev
Severity: Normal | Resolution:
DiscoverRunner,runtests | 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/32609#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Chris Jerdonek | Owner: Chris
Type: | Jerdonek
Component: Testing framework | Version: dev
Keywords: | Triage Stage: Ready for
DiscoverRunner,runtests | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"de4f6201835043ba664e8dcbdceffc4b0955ce29" de4f620]:
{{{
#!CommitTicketReference repository=""
revision="de4f6201835043ba664e8dcbdceffc4b0955ce29"
Fixed #32609 -- Updated runtests.py to support directory path test labels.
For example, with this change, the following now works from the tests
directory:
$ ./runtests.py view_tests/tests/
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32609#comment:10>