Fixed #12658 -- Fixed test discovery so ImportErrors aren't ignored when both…

Fixed #12658 -- Fixed test discovery so ImportErrors aren't ignored when both `tests` and `models` are packages. Thanks schinckel for the report and boxm for a patch for the issue.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@16382 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/test/simple.py

 import unittest as real_unittest
+
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.db.models import get_app, get_apps
@@ -6,6 +7,8 @@ from django.test import _doctest as doctest
 from django.test.utils import setup_test_environment, teardown_test_environment
 from django.test.testcases import OutputChecker, DocTestRunner, TestCase
 from django.utils import unittest
+from django.utils.importlib import import_module
+from django.utils.module_loading import module_has_submodule
 
 __all__ = ('DjangoTestRunner', 'DjangoTestSuiteRunner', 'run_tests')
 
@@ -24,27 +27,25 @@ class DjangoTestRunner(unittest.TextTestRunner):
         super(DjangoTestRunner, self).__init__(*args, **kwargs)
 
 def get_tests(app_module):
+    parts = app_module.__name__.split('.')
+    prefix, last = parts[:-1], parts[-1]
     try:
-        app_path = app_module.__name__.split('.')[:-1]
-        test_module = __import__('.'.join(app_path + [TEST_MODULE]), {}, {}, TEST_MODULE)
-    except ImportError, e:
+        test_module = import_module('.'.join(prefix + [TEST_MODULE]))
+    except ImportError:
         # Couldn't import tests.py. Was it due to a missing file, or
         # due to an import error in a tests.py that actually exists?
-        import os.path
-        from imp import find_module
-        try:
-            mod = find_module(TEST_MODULE, [os.path.dirname(app_module.__file__)])
-        except ImportError:
-            # 'tests' module doesn't exist. Move on.
+        # app_module either points to a models.py file, or models/__init__.py
+        # Tests are therefore either in same directory, or one level up
+        if last == 'models':
+            app_root = import_module('.'.join(prefix))
+        else:
+            app_root = app_module
+
+        if not module_has_submodule(app_root, TEST_MODULE):
             test_module = None
         else:
-            # The module exists, so there must be an import error in the
-            # test module itself. We don't need the module; so if the
-            # module was a single file module (i.e., tests.py), close the file
-            # handle returned by find_module. Otherwise, the test module
-            # is a directory, and there is nothing to close.
-            if mod[0]:
-                mod[0].close()
+            # The module exists, so there must be an import error in the test
+            # module itself.
             raise
     return test_module
 

tests/regressiontests/test_runner/invalid_app/__init__.py

+# Example of app layout that causes issue #12658:
+# * Both `models` and `tests` are packages.
+# * The tests raise a ImportError exception.
+# `test_runner` tests performs test discovery on this app.

tests/regressiontests/test_runner/invalid_app/tests/__init__.py

+# Tests that raise ImportError should not fail silently.
+# This is a support fixture for one test case in test_runner
+
+raise ImportError

tests/regressiontests/test_runner/tests.py

@@ -8,11 +8,18 @@ import warnings
 from django.core.exceptions import ImproperlyConfigured
 from django.core.management import call_command
 from django.test import simple
+from django.test.simple import get_tests
 from django.test.utils import get_warnings_state, restore_warnings_state
 from django.utils import unittest
+from django.utils.importlib import import_module
+
 from regressiontests.admin_scripts.tests import AdminScriptTestCase
 
 
+TEST_APP_OK = 'regressiontests.test_runner.valid_app.models'
+TEST_APP_ERROR = 'regressiontests.test_runner.invalid_app.models'
+
+
 class DjangoTestRunnerTests(unittest.TestCase):
     def setUp(self):
         self._warnings_state = get_warnings_state()
@@ -203,3 +210,16 @@ class CustomTestRunnerOptionsTests(AdminScriptTestCase):
         out, err = self.run_django_admin(args)
         self.assertNoOutput(err)
         self.assertOutput(out, 'bar:foo:31337')
+
+
+class ModulesTestsPackages(unittest.TestCase):
+    def test_get_tests(self):
+        "Check that the get_tests helper function can find tests in a directory"
+        module = import_module(TEST_APP_OK)
+        tests = get_tests(module)
+        self.assertIsInstance(tests, type(module))
+
+    def test_import_error(self):
+        "Test for #12658 - Tests with ImportError's shouldn't fail silently"
+        module = import_module(TEST_APP_ERROR)
+        self.assertRaises(ImportError, get_tests, module)

tests/regressiontests/test_runner/valid_app/__init__.py

+# Example of app layout to verify that the fix for #12658 doesn't break test
+# discovery when both `models` and `tests` are packages.
+# `test_runner` tests perform test discovery on this app.