Opened 2 years ago

Closed 21 months ago

#21206 closed Bug (fixed)

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

Reported by: thepapermen Owned by: nobody
Component: Testing framework Version: 1.6-beta-1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


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/ `application` directory does contain an file.
from django.test import TestCase

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

./ 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):

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

Change History (15)

comment:1 Changed 2 years ago by prestontimmons

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

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

comment:2 Changed 2 years ago by prestontimmons

  • Triage Stage changed from Unreviewed to Accepted

comment:3 Changed 2 years ago by carljm

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.

comment:4 Changed 2 years ago by thepapermen

Replying to carljm & prestontimmons:

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):

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

Ran 0 tests in 0.000s

Last edited 2 years ago by thepapermen (previous) (diff)

comment:5 Changed 2 years ago by carljm

Yeah, on a closer look, you're right. Although this error message is reproducible with python -m unittest discover, 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.

comment:6 Changed 2 years ago by carljm

BTW, in your provided example command line of python -m unittest discover /path/to/python/module, the 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.

comment:7 Changed 2 years ago by 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

It looks like

Version 4, edited 2 years ago by thepapermen (previous) (next) (diff)

comment:8 Changed 2 years ago by prestontimmons

Good catch.

I posted a test case for this here:

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?

comment:9 Changed 2 years ago by carljm

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 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.

comment:10 Changed 2 years ago by prestontimmons

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.

comment:11 Changed 2 years ago by prestontimmons

I added a fix and PR here:

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.

comment:12 Changed 2 years ago by carljm

  • Has patch set
  • Patch needs improvement set

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!

comment:13 Changed 21 months ago by prestontimmons

Added final updates in pull request:

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.

comment:14 Changed 21 months ago by prestontimmons

  • Patch needs improvement unset
  • Summary changed from Misleading Exception on emty test class: ImportError: Start directory is not importable to Misleading Exception on empty test class: ImportError: Start directory is not importable

comment:15 Changed 21 months ago by Tim Graham <timograham@…>

  • Resolution set to fixed
  • Status changed from new to closed

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.

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