#32655: Improve error if a string is passed as an extra_test to
DiscoverRunner.build_suite()
-------------------------------------+-------------------------------------
               Reporter:  Chris      |          Owner:  nobody
  Jerdonek                           |
                   Type:             |         Status:  new
  Cleanup/optimization               |
              Component:  Testing    |        Version:  4.0
  framework                          |       Keywords:
               Severity:  Normal     |  
iter_test_cases,DiscoverRunner,extra_tests
           Triage Stage:             |      Has patch:  0
  Unreviewed                         |
    Needs documentation:  0          |    Needs tests:  0
Patch needs improvement:  0          |  Easy pickings:  0
                  UI/UX:  0          |
-------------------------------------+-------------------------------------
 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):
 +            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
 }}}

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

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/052.819ee6529f56430af1f4e25c8eda40af%40djangoproject.com.

Reply via email to