Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 38 additions & 23 deletions Lib/test/test_unittest/test_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,54 +42,69 @@ def test_get_name_from_path(self):
loader._get_name_from_path('/bar/baz.py')

def test_find_tests(self):
path_lists = [[
# Valid module and package names
'test2.py', 'test1.py', 'not_a_test.py', 'test_dir',
'測試2.py', '測試1.py', '不是測試.py', '測試_資料夾',
# Invalid names
'test.foo', 'test-not-a-module.py', 'not-a-package_dir',
'another_dir',
'測試.付歐歐', '測試-不是-一個-模組.py',
'測試-不是-一個-模組_資料夾', '另外的_資料夾'],
# Valid names; test case tests that these work from inside
# of a package directory
['test4.py', 'test3.py'],
['測試4.py', '測試3.py', ]]

loader = unittest.TestLoader()

original_listdir = os.listdir
def restore_listdir():
os.listdir = original_listdir
original_isfile = os.path.isfile
def restore_isfile():
os.path.isfile = original_isfile
original_isdir = os.path.isdir
def restore_isdir():
os.path.isdir = original_isdir
original_isfile = os.path.isfile

path_lists = [['test2.py', 'test1.py', 'not_a_test.py', 'test_dir',
'test.foo', 'test-not-a-module.py', 'another_dir'],
['test4.py', 'test3.py', ]]
os.listdir = lambda path: path_lists.pop(0)
self.addCleanup(restore_listdir)

def isdir(path):
return path.endswith('dir')
os.path.isdir = isdir
self.addCleanup(restore_isdir)
os.path.isdir = lambda path: path.endswith(('dir', '資料夾'))

def isfile(path):
# another_dir is not a package and so shouldn't be recursed into
return not path.endswith('dir') and not 'another_dir' in path
# Mocking isfile to pretend that path names that end in dir or
# 資料夾 are directories rather than files.
# Additionally, another_dir and 另外的_資料夾 are supposed to be
# directories that are not python packages. We simulate that by
# returning False here when unittest asks us if __init__.py is
# present in those directories.
return (not (path.endswith(('dir', '資料夾')))
and 'another_dir' not in path
and '另外的_資料夾' not in path)
os.path.isfile = isfile
self.addCleanup(restore_isfile)

self.addCleanup(setattr, os, 'listdir', original_listdir)
self.addCleanup(setattr, os.path, 'isdir', original_isdir)
self.addCleanup(setattr, os.path, 'isfile', original_isfile)

loader._get_module_from_name = lambda path: path + ' module'
orig_load_tests = loader.loadTestsFromModule

def loadTestsFromModule(module, pattern=None):
# This is where load_tests is called.
base = orig_load_tests(module, pattern=pattern)
return base + [module + ' tests']

loader.loadTestsFromModule = loadTestsFromModule
loader.suiteClass = lambda thing: thing

top_level = os.path.abspath('/foo')
loader._top_level_dir = top_level
suite = list(loader._find_tests(top_level, 'test*.py'))
suite = list(loader._find_tests(top_level, '[t測]*.py'))

# The test suites found should be sorted alphabetically for reliable
# execution order.
expected = [[name + ' module tests'] for name in
('test1', 'test2', 'test_dir')]
expected.extend([[('test_dir.%s' % name) + ' module tests'] for name in
expected = [['%s module tests' % name] for name in
('test1', 'test2', 'test_dir', '測試1', '測試2', '測試_資料夾')]
expected.extend([['test_dir.%s module tests' % name] for name in
('test3', 'test4')])
expected.extend([['測試_資料夾.%s module tests' % name] for name in
('測試3', '測試4')])
expected.sort()
self.assertEqual(suite, expected)

def test_find_tests_socket(self):
Expand Down
20 changes: 13 additions & 7 deletions Lib/unittest/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@

__unittest = True

# what about .pyc (etc)
# we would need to avoid loading the same tests multiple times
# from '.py', *and* '.pyc'
VALID_MODULE_NAME = re.compile(r'[_a-z]\w*\.py$', re.IGNORECASE)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be better to keep this global and update it to use r'[^\W\d]\w*\.py$', but as @serhiy-storchaka pointed out in #68451 (comment) this doesn't match all identifiers.

Perhaps a is_valid_module_name() global function could be added instead -- this will still provide a way to control what is valid, instead of hardcoding the check in _find_test_path().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added this as a private function in loader: _is_valid_module_filename(). The code that makes use of the function makes use of it to tell if a filename is a valid python module name. A different code path operates on directory names. I think I put the amount of logic into the function that you were thinking (the equivalent of what the regex replaced). Let me know if you were thinking of something with a larger scope.



class _FailedTest(case.TestCase):
_testMethodName = None
Expand All @@ -34,6 +29,12 @@ def testFailure():
return testFailure


def _is_valid_module_filename(module_filename):
root, ext = os.path.splitext(module_filename)
if not (ext == '.py' and root.isidentifier()):
return False
return True

def _make_failed_import_test(name, suiteClass):
message = 'Failed to import test module: %s\n%s' % (
name, traceback.format_exc())
Expand Down Expand Up @@ -371,18 +372,19 @@ def _find_test_path(self, full_path, pattern):
"""
basename = os.path.basename(full_path)
if os.path.isfile(full_path):
if not VALID_MODULE_NAME.match(basename):
if not _is_valid_module_filename(basename):
# valid Python identifiers only
return None, False
if not self._match_path(basename, full_path, pattern):
return None, False

# if the test file matches, load it
name = self._get_name_from_path(full_path)
try:
module = self._get_module_from_name(name)
except case.SkipTest as e:
return _make_skipped_test(name, e, self.suiteClass), False
except:
except Exception:
error_case, error_message = \
_make_failed_import_test(name, self.suiteClass)
self.errors.append(error_message)
Expand Down Expand Up @@ -411,6 +413,10 @@ def _find_test_path(self, full_path, pattern):
load_tests = None
tests = None
name = self._get_name_from_path(full_path)

if not name.isidentifier():
return None, False
Comment thread
abadger marked this conversation as resolved.

try:
package = self._get_module_from_name(name)
except case.SkipTest as e:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix unittest test discovery to find tests in files and directories with
non-ascii characters in their filename