From 13ad5325c45a94615d843a252e10cd2cfd2a346b Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Thu, 16 Nov 2017 16:14:03 +0100 Subject: [PATCH 1/5] bpo-31324: Optimize support.match_test() * Rename support._match_test() to support.match_test(): make it public * Remove support.match_tests global variable. It is replaced with a new support.set_match_tests() function, so match_test() doesn't have to check each time if patterns were modified. * Rewrite match_test(): use different code paths depending on the kind of patterns for best performances. Co-Authored-By: Serhiy Storchaka --- Lib/test/libregrtest/main.py | 4 +-- Lib/test/libregrtest/runtest.py | 2 +- Lib/test/support/__init__.py | 56 ++++++++++++++++++++++++++------- Lib/test/test_support.py | 42 +++++++++++++++++++++++++ 4 files changed, 89 insertions(+), 15 deletions(-) diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index 9871a28dbf2d2b3..ce01c8ce586d659 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -257,12 +257,12 @@ def _list_cases(self, suite): if isinstance(test, unittest.TestSuite): self._list_cases(test) elif isinstance(test, unittest.TestCase): - if support._match_test(test): + if support.match_test(test): print(test.id()) def list_cases(self): support.verbose = False - support.match_tests = self.ns.match_tests + support.set_match_tests(self.ns.match_tests) for test in self.selected: abstest = get_abs_module(self.ns, test) diff --git a/Lib/test/libregrtest/runtest.py b/Lib/test/libregrtest/runtest.py index dbd463435c781bd..12bf422c902dc1c 100644 --- a/Lib/test/libregrtest/runtest.py +++ b/Lib/test/libregrtest/runtest.py @@ -102,7 +102,7 @@ def runtest(ns, test): if use_timeout: faulthandler.dump_traceback_later(ns.timeout, exit=True) try: - support.match_tests = ns.match_tests + support.set_match_tests(ns.match_tests) # reset the environment_altered flag to detect if a test altered # the environment support.environment_altered = False diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 527cf7fbf95328c..e653116c8169e31 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -278,7 +278,6 @@ def get_attribute(obj, name): # small sizes, to make sure they work.) real_max_memuse = 0 failfast = False -match_tests = None # _original_stdout is meant to hold stdout at the time regrtest began. # This may be "the real" stdout, or IDLE's emulation of stdout, or whatever. @@ -1900,21 +1899,54 @@ def _run_suite(suite): raise TestFailed(err) -def _match_test(test): - global match_tests +# By default, don't filter tests +_match_test_func = None - if match_tests is None: + +def match_test(test): + # Function used by support.run_unittest() and regrtest --list-cases + if _match_test_func is None: return True - test_id = test.id() + else: + return _match_test_func(test.id()) + + +def _is_full_match_test(pattern): + # If a pattern contains at least one dot, it's considered as a full test + # identifier like 'test.test_os.FileTests.test_access'. + # + # Reject patterns which contain fnmatch patterns: '*', '?', '[...]' + # or '[!...]'. For example, reject 'test_access*'. + return ('.' in pattern) and all(char not in '?*[]' for char in pattern) - for match_test in match_tests: - if fnmatch.fnmatchcase(test_id, match_test): - return True - for name in test_id.split("."): - if fnmatch.fnmatchcase(name, match_test): +def set_match_tests(patterns): + global _match_test_func + + if not patterns: + _match_test_func = None + elif all(_is_full_match_test(pattern) for pattern in patterns): + # Simple case: all patterns are full test identifier. + # The test.bisect utility only uses such full test identifiers. + _match_test_func = set(patterns).__contains__ + else: + regex = '|'.join(map(fnmatch.translate, patterns)) + # The search *is* case sensitive on purpose: + # don't use flags=re.IGNORECASE + regex_match = re.compile(regex).match + + def match_test_regex(test_id): + if regex_match(test_id): + # The regex matchs the whole identifier like + # 'test.test_os.FileTests.test_access' return True - return False + else: + # Try to match parts of the test identifier. + # For example, split 'test.test_os.FileTests.test_access' + # into: 'test', 'test_os', 'FileTests' and 'test_access'. + return any(map(regex_match, test_id.split("."))) + + _match_test_func = match_test_regex def run_unittest(*classes): @@ -1931,7 +1963,7 @@ def run_unittest(*classes): suite.addTest(cls) else: suite.addTest(unittest.makeSuite(cls)) - _filter_suite(suite, _match_test) + _filter_suite(suite, match_test) _run_suite(suite) #======================================================================= diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py index 4a577efbeb9ccb2..55a1d3d16f5b8ee 100644 --- a/Lib/test/test_support.py +++ b/Lib/test/test_support.py @@ -483,6 +483,48 @@ def test_optim_args_from_interpreter_flags(self): with self.subTest(opts=opts): self.check_options(opts, 'optim_args_from_interpreter_flags') + def test_match_test(self): + class Test: + def __init__(self, test_id): + self.test_id = test_id + + def id(self): + return self.test_id + + test_access = Test('test.test_os.FileTests.test_access') + test_chdir = Test('test.test_os.Win32ErrorTests.test_chdir') + + old_match = support._match_test_func + try: + # match all + support.set_match_tests([]) + self.assertTrue(support.match_test(test_access)) + self.assertTrue(support.match_test(test_chdir)) + + # match the full test identifier + support.set_match_tests([test_access.id()]) + self.assertTrue(support.match_test(test_access)) + self.assertFalse(support.match_test(test_chdir)) + + # match the module name + support.set_match_tests(['test_os']) + self.assertTrue(support.match_test(test_access)) + self.assertTrue(support.match_test(test_chdir)) + + # Test '*' pattern + support.set_match_tests(['test_*']) + self.assertTrue(support.match_test(test_access)) + self.assertTrue(support.match_test(test_chdir)) + + # Test case sensitivity + support.set_match_tests(['filetests']) + self.assertFalse(support.match_test(test_access)) + support.set_match_tests(['FileTests']) + self.assertTrue(support.match_test(test_access)) + finally: + support._match_test_func = old_match + + # XXX -follows a list of untested API # make_legacy_pyc # is_resource_enabled From bdec1ff9c21961e4a0c983262f626c7e8ea8d5ca Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 21 Nov 2017 15:04:54 +0100 Subject: [PATCH 2/5] set_match_tests() does nothing if patterns don't change --- Lib/test/support/__init__.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index e653116c8169e31..10961e54a6df88a 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -1901,6 +1901,7 @@ def _run_suite(suite): # By default, don't filter tests _match_test_func = None +_match_test_patterns = None def match_test(test): @@ -1921,14 +1922,18 @@ def _is_full_match_test(pattern): def set_match_tests(patterns): - global _match_test_func + global _match_test_func, _match_test_patterns + + if patterns == _match_test_patterns: + # No change: no need to recompile patterns. + return if not patterns: - _match_test_func = None + func = None elif all(_is_full_match_test(pattern) for pattern in patterns): # Simple case: all patterns are full test identifier. # The test.bisect utility only uses such full test identifiers. - _match_test_func = set(patterns).__contains__ + func = set(patterns).__contains__ else: regex = '|'.join(map(fnmatch.translate, patterns)) # The search *is* case sensitive on purpose: @@ -1946,7 +1951,11 @@ def match_test_regex(test_id): # into: 'test', 'test_os', 'FileTests' and 'test_access'. return any(map(regex_match, test_id.split("."))) - _match_test_func = match_test_regex + func = match_test_regex + + _match_test_patterns = patterns + _match_test_func = func + def run_unittest(*classes): From 7b2df8098624c9b137998d21780076f90482bd8d Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 21 Nov 2017 15:11:10 +0100 Subject: [PATCH 3/5] Add more tests Use also support.swap_attr(). --- Lib/test/test_support.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_support.py b/Lib/test/test_support.py index 55a1d3d16f5b8ee..4756defa5f79b74 100644 --- a/Lib/test/test_support.py +++ b/Lib/test/test_support.py @@ -494,8 +494,7 @@ def id(self): test_access = Test('test.test_os.FileTests.test_access') test_chdir = Test('test.test_os.Win32ErrorTests.test_chdir') - old_match = support._match_test_func - try: + with support.swap_attr(support, '_match_test_func', None): # match all support.set_match_tests([]) self.assertTrue(support.match_test(test_access)) @@ -521,8 +520,20 @@ def id(self): self.assertFalse(support.match_test(test_access)) support.set_match_tests(['FileTests']) self.assertTrue(support.match_test(test_access)) - finally: - support._match_test_func = old_match + + # Test pattern containing '.' and a '*' metacharacter + support.set_match_tests(['*test_os.*.test_*']) + self.assertTrue(support.match_test(test_access)) + self.assertTrue(support.match_test(test_chdir)) + + # Multiple patterns + support.set_match_tests([test_access.id(), test_chdir.id()]) + self.assertTrue(support.match_test(test_access)) + self.assertTrue(support.match_test(test_chdir)) + + support.set_match_tests(['test_access', 'DONTMATCH']) + self.assertTrue(support.match_test(test_access)) + self.assertFalse(support.match_test(test_chdir)) # XXX -follows a list of untested API From ac956e33edfcf92de7fddb9493e1793fbdd628de Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 21 Nov 2017 15:53:41 +0100 Subject: [PATCH 4/5] Address Serhiy's comments --- Lib/test/support/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 10961e54a6df88a..0c332301c974094 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -1913,12 +1913,13 @@ def match_test(test): def _is_full_match_test(pattern): - # If a pattern contains at least one dot, it's considered as a full test - # identifier like 'test.test_os.FileTests.test_access'. + # If a pattern contains at least one dot, it's considered + # as a full test identifier. + # Example: 'test.test_os.FileTests.test_access'. # # Reject patterns which contain fnmatch patterns: '*', '?', '[...]' # or '[!...]'. For example, reject 'test_access*'. - return ('.' in pattern) and all(char not in '?*[]' for char in pattern) + return ('.' in pattern) and (not re.search(r'[?*\[\]]', pattern)) def set_match_tests(patterns): @@ -1930,7 +1931,7 @@ def set_match_tests(patterns): if not patterns: func = None - elif all(_is_full_match_test(pattern) for pattern in patterns): + elif all(map(_is_full_match_test, patterns)): # Simple case: all patterns are full test identifier. # The test.bisect utility only uses such full test identifiers. func = set(patterns).__contains__ From c28b3c0fb7d06e2ba9a044a9906db8558584aa13 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 21 Nov 2017 23:17:39 +0100 Subject: [PATCH 5/5] set_match_tests() copies patterns --- Lib/test/support/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 0c332301c974094..71d9c2c87959050 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -1954,7 +1954,8 @@ def match_test_regex(test_id): func = match_test_regex - _match_test_patterns = patterns + # Create a copy since patterns can be mutable and so modified later + _match_test_patterns = tuple(patterns) _match_test_func = func