[Django] #32532: Provide friendlier error if a file path is passed as a test label when running tests
Django
running tests
-------------------------------------+-------------------------------------
Reporter: Chris | Owner: nobody
Jerdonek |
Type: | Status: new
Cleanup/optimization |
Component: Testing | Version: 3.1
framework | Keywords: DiscoverRunner,test
Severity: Normal | labels
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Currently, if a user passes a file path as a test label to
`DiscoverRunner`, they will get a somewhat unfriendly error that looks
something like the following:
{{{
$ ./runtests.py test_runner/tests.py
Testing against Django installed in '.../django/django' with up to 8
processes
Traceback (most recent call last):
File "./runtests.py", line 593, in <module>
options.timing,
File "./runtests.py", line 325, in django_tests
failures = test_runner.run_tests(test_labels or get_installed())
File ".../django/django/test/runner.py", line 721, in run_tests
suite = self.build_suite(test_labels, extra_tests)
File ".../django/django/test/runner.py", line 612, in build_suite
suite.addTests(tests)
File ".../unittest/suite.py", line 57, in addTests
for test in tests:
TypeError: 'NoneType' object is not iterable
}}}
This is because `DiscoverRunner.build_suite()` starts out with `tests =
None` in each iteration of the loop and doesn't have an `else` clause for
the file path case:
https://github.com/django/django/blob/6f5dbe9dbe45b23b3befe4f1cd2ea13b6049ab96/django/test/runner.py#L574-L600
I imagine this is a common error because the help string tells the user to
use "paths":
> "Optional path(s) to test modules; e.g. "i18n" or
"i18n.tests.TranslationTests.test_lazy_objects".
but without saying that file paths aren't allowed.
A more friendly error message could tell the user that they provided a
path to a file, but that only dotted module paths and directory paths are
supported.
--
Ticket URL: <https://code.djangoproject.com/ticket/32532>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
running tests
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage:
labels | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
Another option would be to support passing file paths, but that could be
done separately, after first doing the easier thing of providing a nicer
error message.
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:1>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
labels |
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* easy: 0 => 1
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:2>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Accepted
labels |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Chris Jerdonek):
I would suggest that this not be done until after #32540, since it affects
the `if` logic.
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:3>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Accepted
labels |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* owner: nobody => Chris Jerdonek
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:4>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Accepted
labels |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Chris Jerdonek):
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/14177
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:5>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Accepted
labels |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"235d23cf995eb958922a747289dfbcd9220e1195" 235d23cf]:
{{{
#!CommitTicketReference repository=""
revision="235d23cf995eb958922a747289dfbcd9220e1195"
Refs #32532 -- Added DiscoverRunner.load_tests_for_label().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:6>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Accepted
labels |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* has_patch: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:7>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Accepted
labels |
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
PR: https://github.com/django/django/pull/14180
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:8>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Accepted
labels |
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* easy: 1 => 0
Comment:
With my posted patch, the new error will look like this:
{{{
$ ./runtests.py test_runner/tests.py
Testing against Django installed in '.../django/django' with up to 8
processes
Traceback (most recent call last):
...
RuntimeError: One of the test labels is a path to a file:
'test_runner/tests.py'
Test labels that are paths but not importable as names can only be
directories.
}}}
(By the way, I unchecked "easy" because this wasn't necessarily so easy
given that `is_discoverable()` had to be broken up to get the info needed
for the patch.)
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:9>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
labels | 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/32532#comment:10>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Cleanup/optimization | Status: assigned
Component: Testing framework | Version: 3.1
Severity: Normal | Resolution:
Keywords: DiscoverRunner,test | Triage Stage: Ready for
labels | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"0af81b22b5289de878c6ebf93d0579dc31599539" 0af81b2]:
{{{
#!CommitTicketReference repository=""
revision="0af81b22b5289de878c6ebf93d0579dc31599539"
Refs #32532 -- Replaced is_discoverable() with try_importing().
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:11>
Django
running tests
-------------------------------------+-------------------------------------
Type: | Jerdonek
Component: Testing framework | Version: 3.1
Keywords: DiscoverRunner,test | Triage Stage: Ready for
labels | checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"a89e975caf69ffdbec527f9fe84824e73a7c9cac" a89e975c]:
{{{
#!CommitTicketReference repository=""
revision="a89e975caf69ffdbec527f9fe84824e73a7c9cac"
Fixed #32532 -- Made DiscoverRunner raise RuntimeError when a test label
is a file path.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/32532#comment:12>