Opened 4 years ago

Closed 4 years ago

#32609 closed Cleanup/optimization (fixed)

Added runtests.py support for directory path test labels.

Reported by: Chris Jerdonek Owned by: Chris Jerdonek
Component: Testing framework Version: dev
Severity: Normal Keywords: DiscoverRunner, runtests
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 Chris Jerdonek)

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.

Change History (10)

comment:1 by Chris Jerdonek, 4 years ago

Description: modified (diff)

comment:2 by Tim Graham, 4 years ago

Triage Stage: UnreviewedAccepted

comment:3 by Chris Jerdonek, 4 years ago

Owner: changed from nobody to Chris Jerdonek
Status: newassigned

comment:4 by Chris Jerdonek, 4 years ago

Has patch: set

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.

comment:5 by GitHub <noreply@…>, 4 years ago

In 413c15ef:

Refs #32609 -- Simplified test_labels_set construction in runtests.py's setup().

Follow up to 7cf3a5786bc76374e743fbc0c1a1c8470a61f6c0.

comment:6 by Mariusz Felisiak, 4 years ago

Has patch: unset

comment:7 by Chris Jerdonek, 4 years ago

Has patch: set

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.

Last edited 4 years ago by Chris Jerdonek (previous) (diff)

comment:8 by Mariusz Felisiak, 4 years ago

Summary: runtests.py setup should use DiscoverRunner's test label logicAdded runtests.py support for directory path test labels.

comment:9 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In de4f620:

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/

Note: See TracTickets for help on using tickets.
Back to Top