﻿id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
32655	Deprecate DiscoverRunner.build_suite()'s extra_tests argument.	Chris Jerdonek	Jacob Walls	"Currently, if you pass a string like `'abc'` rather than a `TestCase` instance as one of the items in the `extra_tests` argument to `DiscoverRunner.build_suite()`, you will get a not-too-helpful error that looks something like this:

{{{
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/test/runner.py"", line 759, in run_tests
    suite = self.build_suite(test_labels, extra_tests)
  File "".../django/test/runner.py"", line 641, in build_suite
    all_tests.extend(iter_test_cases(extra_tests))
  File "".../django/test/utils.py"", line 249, in iter_test_cases
    yield from iter_test_cases(test)
  File "".../django/test/utils.py"", line 249, in iter_test_cases
    yield from iter_test_cases(test)
  File "".../django/test/utils.py"", line 249, in iter_test_cases
    yield from iter_test_cases(test)
  [Previous line repeated 991 more times]
  File "".../django/test/utils.py"", line 245, in iter_test_cases
    if isinstance(test, TestCase):
RecursionError: maximum recursion depth exceeded while calling a Python object
}}}

This is because [https://github.com/django/django/blob/23fa29f6a6659e0f600d216de6bcb79e7f6818c9/django/test/utils.py#L238-L249 iter_test_cases()] goes into an infinite loop when it gets to `iter_test_cases('a')` (iterating the first element of `'abc'`).

Since passing a string is a plausible mistake, I think it would be worth doing something to short-circuit this case and prevent a `RecursionError`. The following would accomplish this with a pointer to the problem and without adding too much complexity:

{{{
diff --git a/django/test/utils.py b/django/test/utils.py
index e977db8a10..ec4dc1837d 100644
--- a/django/test/utils.py
+++ b/django/test/utils.py
@@ -244,6 +244,8 @@ def iter_test_cases(tests):
     for test in tests:
         if isinstance(test, TestCase):
             yield test
+        elif isinstance(test, str):
+            # Prevent an unfriendly RecursionError that can happen with strings.
+            raise TypeError(f'test {test!r} must be a test case or test suite not string')
         else:
             # Otherwise, assume it is a test suite.
             yield from iter_test_cases(test)
}}}

It would result in the following:

{{{
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/test/runner.py"", line 758, in run_tests
    suite = self.build_suite(test_labels, extra_tests)
  File "".../django/test/runner.py"", line 641, in build_suite
    all_tests.extend(iter_test_cases(extra_tests))
  File "".../django/test/utils.py"", line 248, in iter_test_cases
    raise TypeError(f'test {test!r} must be a test case or test suite not string')
TypeError: test 'abc' must be a test case or test suite not string
}}}

Note that prior to #32489 (""Add a generator function in runner.py to iterate over a test suite's test cases""), the error looked like the following, which was better than what it is now, but not quite as good as what I propose above:

{{{
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/test/runner.py"", line 724, in run_tests
    suite = self.build_suite(test_labels, extra_tests)
  File "".../django/test/runner.py"", line 615, in build_suite
    suite.addTest(test)
  File "".../unittest/suite.py"", line 47, in addTest
    raise TypeError(""{} is not callable"".format(repr(test)))
TypeError: 'abc' is not callable
}}}
"	Cleanup/optimization	closed	Testing framework	4.0	Normal	fixed	iter_test_cases,DiscoverRunner,extra_tests		Ready for checkin	1	0	0	0	0	0
