Test discovery in unittest will only attempt to import modules that are importable; i.e. their names are valid Python identifiers. If an import fails during discovery this will be recorded as an error and test discovery will continue. Issue 6568.
diff --git a/Lib/test/test_unittest.py b/Lib/test/test_unittest.py
index 79ee982..459334b 100644
--- a/Lib/test/test_unittest.py
+++ b/Lib/test/test_unittest.py
@@ -3469,31 +3469,19 @@
class TestDiscovery(TestCase):
# Heavily mocked tests so I can avoid hitting the filesystem
- def test_get_module_from_path(self):
+ def test_get_name_from_path(self):
loader = unittest.TestLoader()
- old_import = __import__
- def restore_import():
- __builtin__.__import__ = old_import
- __builtin__.__import__ = lambda *_: None
- self.addCleanup(restore_import)
-
- expected_module = object()
- def del_module():
- del sys.modules['bar.baz']
- sys.modules['bar.baz'] = expected_module
- self.addCleanup(del_module)
-
loader._top_level_dir = '/foo'
- module = loader._get_module_from_path('/foo/bar/baz.py')
- self.assertEqual(module, expected_module)
+ name = loader._get_name_from_path('/foo/bar/baz.py')
+ self.assertEqual(name, 'bar.baz')
if not __debug__:
# asserts are off
return
with self.assertRaises(AssertionError):
- loader._get_module_from_path('/bar/baz.py')
+ loader._get_name_from_path('/bar/baz.py')
def test_find_tests(self):
loader = unittest.TestLoader()
@@ -3509,7 +3497,7 @@
os.path.isdir = original_isdir
path_lists = [['test1.py', 'test2.py', 'not_a_test.py', 'test_dir',
- 'test.foo', 'another_dir'],
+ 'test.foo', 'test-not-a-module.py', 'another_dir'],
['test3.py', 'test4.py', ]]
os.listdir = lambda path: path_lists.pop(0)
self.addCleanup(restore_listdir)
@@ -3525,16 +3513,16 @@
os.path.isfile = isfile
self.addCleanup(restore_isfile)
- loader._get_module_from_path = lambda path: path + ' module'
+ loader._get_module_from_name = lambda path: path + ' module'
loader.loadTestsFromModule = lambda module: module + ' tests'
loader._top_level_dir = '/foo'
suite = list(loader._find_tests('/foo', 'test*.py'))
- expected = [os.path.join('/foo', name) + ' module tests' for name in
- ('test1.py', 'test2.py')]
- expected.extend([os.path.join('/foo', 'test_dir', name) + ' module tests' for name in
- ('test3.py', 'test4.py')])
+ expected = [name + ' module tests' for name in
+ ('test1', 'test2')]
+ expected.extend([('test_dir.%s' % name) + ' module tests' for name in
+ ('test3', 'test4')])
self.assertEqual(suite, expected)
def test_find_tests_with_package(self):
@@ -3577,7 +3565,7 @@
def __eq__(self, other):
return self.path == other.path
- loader._get_module_from_path = lambda path: Module(path)
+ loader._get_module_from_name = lambda name: Module(name)
def loadTestsFromModule(module, use_load_tests):
if use_load_tests:
raise self.failureException('use_load_tests should be False for packages')
@@ -3592,15 +3580,12 @@
# We should have loaded tests from the test_directory package by calling load_tests
# and directly from the test_directory2 package
self.assertEqual(suite,
- ['load_tests',
- os.path.join('/foo', 'test_directory2') + ' module tests'])
- self.assertEqual(Module.paths, [os.path.join('/foo', 'test_directory'),
- os.path.join('/foo', 'test_directory2')])
+ ['load_tests', 'test_directory2' + ' module tests'])
+ self.assertEqual(Module.paths, ['test_directory', 'test_directory2'])
# load_tests should have been called once with loader, tests and pattern
self.assertEqual(Module.load_tests_args,
- [(loader, os.path.join('/foo', 'test_directory') + ' module tests',
- 'test*')])
+ [(loader, 'test_directory' + ' module tests', 'test*')])
def test_discover(self):
loader = unittest.TestLoader()
@@ -3640,6 +3625,25 @@
self.assertEqual(loader._top_level_dir, top_level_dir)
self.assertEqual(_find_tests_args, [(start_dir, 'pattern')])
+ def test_discover_with_modules_that_fail_to_import(self):
+ loader = unittest.TestLoader()
+
+ listdir = os.listdir
+ os.listdir = lambda _: ['test_this_does_not_exist.py']
+ isfile = os.path.isfile
+ os.path.isfile = lambda _: True
+ def restore():
+ os.path.isfile = isfile
+ os.listdir = listdir
+ self.addCleanup(restore)
+
+ suite = loader.discover('.')
+ self.assertEqual(suite.countTestCases(), 1)
+ test = list(list(suite)[0])[0] # extract test from suite
+
+ with self.assertRaises(ImportError):
+ test.test_this_does_not_exist()
+
def test_command_line_handling_parseArgs(self):
# Haha - take that uninstantiable class
program = object.__new__(TestProgram)
diff --git a/Lib/unittest/loader.py b/Lib/unittest/loader.py
index 21520f5..31c343b 100644
--- a/Lib/unittest/loader.py
+++ b/Lib/unittest/loader.py
@@ -1,7 +1,9 @@
"""Loading unittests."""
import os
+import re
import sys
+import traceback
import types
from fnmatch import fnmatch
@@ -19,6 +21,26 @@
return K
+# what about .pyc or .pyo (etc)
+# we would need to avoid loading the same tests multiple times
+# from '.py', '.pyc' *and* '.pyo'
+VALID_MODULE_NAME = re.compile(r'[_a-z]\w*\.py$', re.IGNORECASE)
+
+
+def _make_failed_import_test(name, suiteClass):
+ message = 'Failed to import test module: %s' % name
+ if hasattr(traceback, 'format_exc'):
+ # Python 2.3 compatibility
+ # format_exc returns two frames of discover.py as well
+ message += '\n%s' % traceback.format_exc()
+
+ def testImportFailure(self):
+ raise ImportError(message)
+ attrs = {name: testImportFailure}
+ ModuleImportFailure = type('ModuleImportFailure', (case.TestCase,), attrs)
+ return suiteClass((ModuleImportFailure(name),))
+
+
class TestLoader(object):
"""
This class is responsible for loading tests according to various criteria
@@ -161,17 +183,17 @@
tests = list(self._find_tests(start_dir, pattern))
return self.suiteClass(tests)
-
- def _get_module_from_path(self, path):
- """Load a module from a path relative to the top-level directory
- of a project. Used by discovery."""
+ def _get_name_from_path(self, path):
path = os.path.splitext(os.path.normpath(path))[0]
- relpath = os.path.relpath(path, self._top_level_dir)
- assert not os.path.isabs(relpath), "Path must be within the project"
- assert not relpath.startswith('..'), "Path must be within the project"
+ _relpath = os.path.relpath(path, self._top_level_dir)
+ assert not os.path.isabs(_relpath), "Path must be within the project"
+ assert not _relpath.startswith('..'), "Path must be within the project"
- name = relpath.replace(os.path.sep, '.')
+ name = _relpath.replace(os.path.sep, '.')
+ return name
+
+ def _get_module_from_name(self, name):
__import__(name)
return sys.modules[name]
@@ -181,14 +203,20 @@
for path in paths:
full_path = os.path.join(start_dir, path)
- # what about __init__.pyc or pyo (etc)
- # we would need to avoid loading the same tests multiple times
- # from '.py', '.pyc' *and* '.pyo'
- if os.path.isfile(full_path) and path.lower().endswith('.py'):
+ if os.path.isfile(full_path):
+ if not VALID_MODULE_NAME.match(path):
+ # valid Python identifiers only
+ continue
+
if fnmatch(path, pattern):
# if the test file matches, load it
- module = self._get_module_from_path(full_path)
- yield self.loadTestsFromModule(module)
+ name = self._get_name_from_path(full_path)
+ try:
+ module = self._get_module_from_name(name)
+ except:
+ yield _make_failed_import_test(name, self.suiteClass)
+ else:
+ yield self.loadTestsFromModule(module)
elif os.path.isdir(full_path):
if not os.path.isfile(os.path.join(full_path, '__init__.py')):
continue
@@ -197,7 +225,8 @@
tests = None
if fnmatch(path, pattern):
# only check load_tests if the package directory itself matches the filter
- package = self._get_module_from_path(full_path)
+ name = self._get_name_from_path(full_path)
+ package = self._get_module_from_name(name)
load_tests = getattr(package, 'load_tests', None)
tests = self.loadTestsFromModule(package, use_load_tests=False)
diff --git a/Lib/unittest/suite.py b/Lib/unittest/suite.py
index 5fba259..730b4d9 100644
--- a/Lib/unittest/suite.py
+++ b/Lib/unittest/suite.py
@@ -1,6 +1,7 @@
"""TestSuite"""
from . import case
+from . import util
class TestSuite(object):
@@ -17,7 +18,7 @@
self.addTests(tests)
def __repr__(self):
- return "<%s tests=%s>" % (_strclass(self.__class__), list(self))
+ return "<%s tests=%s>" % (util.strclass(self.__class__), list(self))
def __eq__(self, other):
if not isinstance(other, self.__class__):