Misleading Exception on empty test class: ImportError: Start directory is not importable

[thepapermen]

Django 1.6 test runner produces misleading exception if you try to run a test case which does not contain any methods with names starting with test.

Issues of this type are time-consuming to debug, since the exception puts you on the wrong path of searching for faulty and/or circular imports.

To reproduce:

"""
This file is placed in application/tests.py. `application` directory does contain an __init__.py file.
"""
from django.test import TestCase

class AnEmptyTest(TestCase):
    """
    Test case without method names that start with `test`.
    """
    pass

./manage.py test application.tests.AnEmptyTest will raise an error: ImportError: Start directory is not importable: 'application.tests.AnEmptyTest'. If you add an empty method with name starting with test, test runner will run fine.

from django.test import TestCase

class ANonEmptyTest(TestCase):
    """
    This will run fine.
    """
    def test_something(self):
        pass

Desired behavior for AnEmptyTest: launch test runner and run zero tests.

[Preston Timmons]

This exception comes from the unittest discover method.

By adding an empty unittest on the Python path, this can be reproduced outside of Django with python -m unittest discover path.to.EmptyTestCase.

Marking this as accepted for now, but fixing it would require us to further mask the unittest discovery behavior in Django's test runner.

[Carl Meyer]

Yeah, I agree this is bad behavior, but its bad behavior that should really be fixed upstream in unittest, not in Django.

I'll leave this open in case anyone suggests a really slick way to work around it in Django, and just so we can continue to track the issue and any upstream progress, but in general I don't think it's up to Django to fix bugs in the Python standard library.

[thepapermen]

I was going to report the bug upstream, but I wasn't able to reproduce it with python's unittest module. Indeed, it looks like it is a Django test runner bug.

I'm testing it with python2.7:amd64/sid 2.7.5-8 uptodate.

import unittest

class EmptyTestCase(unittest.TestCase):
    pass

Here is the output of python -m unittest discover /path/to/python/module path.to.EmptyTestCase:

----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK

[Carl Meyer]

Yeah, on a closer look, you're right. Although this error message is reproducible with python -m unittest discover path.to.EmptyTestCase, it's not tied to an empty test case; it's simply because in unittest "discover" does not accept dotted paths to test case classes, only file paths or dotted package paths to start discovery at. Django's discovery runner attempts to combine unittest's discovery ability with its ability to load tests directly from a dotted module / testcase path, by autodetecting which one each provided test label is, and the bug here lies in that autodetection. We assume that if we try loadTestsFromName and no tests are found, that the given label must have been a package instead, and so we try discovery. When the given label was in fact an empty test case class, this error occurs.

In order to fix this, we'll need to do some further checks in the if not (tests and tests.countTestCases()) block to verify that the provided label actually maps to a package before we try discovery on it.

[Carl Meyer]

BTW, in your provided example command line of python -m unittest discover /path/to/python/module path.to.EmptyTestCase, the path.to.EmptyTestCase portion is not having the effect you think it does. python -m unittest discover, unlike the Django runner, accepts only one discovery start dir. The second argument, if given, is actually the test-files pattern (i.e. to override the default of test*.py). So no matter how many tests you add to EmptyTestCase, your provided command line will never run any tests.

[thepapermen]

Hi! Thank you! Sorry for posting wrong second argument! In my case the command python -m unittest discover $(pwd) test.EmptyTestCase did actually try to run all the tests in the directory provided as first argument (before posting the previous reply I have tried to add real test methods to the test class and did insure that they do run, just as a sanity check). Still, I didn't perform enough research. My bad. :P

[Preston Timmons]

Good catch.

I posted a test case for this here:

​https://github.com/prestontimmons/django/commit/0170eff8350285e47aa2561d475725d4436df3cc

Simply changing the check to if tests is None seems to fix the problem and passes all discovery tests, but for some reason it causes test_runner.tests.DeprecationDisplayTest to fail.

Is there ever a case that discovery should run if loadTestsFromName is called?

[Carl Meyer]

Yes, there is such a case, though apparently we don't have good tests for it. If a test label is some.package, where some/package/ is a directory with an __init__.py and then various test files within it, loadTestsFromName will successfully return a test suite containing zero tests (it only looks directly in the some.package module namespace, doesn't recurse to contained modules). In that case, what the user almost certainly meant was "run all the tests within some.package", so we run discovery using some.package (i.e. some/package/) as the start dir.

[Preston Timmons]

Ah, yes. Looks like we're missing a test for that.

That indeed fails if we only check if tests is None.

I went ahead and added a test for this case in my branch.

​https://github.com/prestontimmons/django/compare/ticket-21206

[Preston Timmons]

I added a fix and PR here:

​https://github.com/django/django/pull/1705

I'm not happy with the use of import_module. If anyone has another way to tell a dotted path is a package, I'll try it out.

[Carl Meyer]

I suggested a few small changes on the PR, but basically I think you've got the right approach. Thanks for taking this one on!

[Preston Timmons]

Added final updates in pull request: ​https://github.com/django/django/pull/1705/files

The pull adds two commits. One that fixes this issue, and one that adds two additional test cases that weren't covered in the original discover runner tests.

I think this is ready to go.

[Tim Graham <timograham@…>]

In aef019de6113e6c750e3ba49dc1e9f2a179eb0ca:

Fixed #21206 -- No longer run discovery if the test label doesn't point to a package or directory.

Thanks thepapermen for the report and Carl Meyer for the review.