From 532e063fc2bf9e6e80550670ddc5dc5d2b1d2450 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Mon, 22 Mar 2021 23:52:13 +0100 Subject: [PATCH] bpo-41718: regrtest saved_test_environment avoids imports (GH-24934) Reduce the number of modules imported by libregrtest. saved_test_environment no longer imports modules at startup, but try to get them from sys.modules. If an module is missing, skip the test. It also sets directly support.environment_altered. runtest() now now two saved_test_environment instances: one before importing the test module, one after importing it. Remove imports from test.libregrtest.save_env: * asyncio * logging * multiprocessing * shutil * sysconfig * urllib.request * warnings When a test method imports a module (ex: warnings) and the test has a side effect (ex: add a warnings filter), the side effect is not detected, because the module was not imported when Python enters the saved_test_environment context manager. --- Lib/test/libregrtest/runtest.py | 21 +++++--- Lib/test/libregrtest/save_env.py | 90 ++++++++++++++++++++------------ 2 files changed, 69 insertions(+), 42 deletions(-) diff --git a/Lib/test/libregrtest/runtest.py b/Lib/test/libregrtest/runtest.py index e46cc31caea..b4ef9fbb245 100644 --- a/Lib/test/libregrtest/runtest.py +++ b/Lib/test/libregrtest/runtest.py @@ -211,6 +211,10 @@ def _test_module(the_module): support.run_unittest(tests) +def save_env(ns, test_name): + return saved_test_environment(test_name, ns.verbose, ns.quiet, pgo=ns.pgo) + + def _runtest_inner2(ns, test_name): # Load the test function, run the test function, handle huntrleaks # and findleaks to detect leaks @@ -229,12 +233,13 @@ def _runtest_inner2(ns, test_name): test_runner = functools.partial(_test_module, the_module) try: - if ns.huntrleaks: - # Return True if the test leaked references - refleak = dash_R(ns, test_name, test_runner) - else: - test_runner() - refleak = False + with save_env(ns, test_name): + if ns.huntrleaks: + # Return True if the test leaked references + refleak = dash_R(ns, test_name, test_runner) + else: + test_runner() + refleak = False finally: cleanup_test_droppings(test_name, ns.verbose) @@ -268,7 +273,7 @@ def _runtest_inner(ns, test_name, display_failure=True): try: clear_caches() - with saved_test_environment(test_name, ns.verbose, ns.quiet, pgo=ns.pgo) as environment: + with save_env(ns, test_name): refleak = _runtest_inner2(ns, test_name) except support.ResourceDenied as msg: if not ns.quiet and not ns.pgo: @@ -298,7 +303,7 @@ def _runtest_inner(ns, test_name, display_failure=True): if refleak: return FAILED - if environment.changed: + if support.environment_altered: return ENV_CHANGED return PASSED diff --git a/Lib/test/libregrtest/save_env.py b/Lib/test/libregrtest/save_env.py index 4c9c692400b..f0bfcf38999 100644 --- a/Lib/test/libregrtest/save_env.py +++ b/Lib/test/libregrtest/save_env.py @@ -1,21 +1,15 @@ -import asyncio import builtins import locale -import logging import os -import shutil import sys -import sysconfig import threading -import urllib.request -import warnings from test import support from test.support import os_helper from test.libregrtest.utils import print_warning -try: - import _multiprocessing, multiprocessing.process -except ImportError: - multiprocessing = None + + +class SkipTestEnvironment(Exception): + pass # Unit tests are supposed to leave the execution environment unchanged @@ -33,15 +27,13 @@ class saved_test_environment: #stuff Unless quiet is True, a warning is printed to stderr if any of - the saved items was changed by the test. The attribute 'changed' - is initially False, but is set to True if a change is detected. + the saved items was changed by the test. The support.environment_altered + attribute is set to True if a change is detected. If verbose is more than 1, the before and after state of changed items is also printed. """ - changed = False - def __init__(self, testname, verbose=0, quiet=False, *, pgo=False): self.testname = testname self.verbose = verbose @@ -73,20 +65,36 @@ class saved_test_environment: 'urllib.requests._url_tempfiles', 'urllib.requests._opener', ) + def get_module(self, name): + # function for restore() methods + return sys.modules[name] + + def try_get_module(self, name): + # function for get() methods + try: + return self.get_module(name) + except KeyError: + raise SkipTestEnvironment + def get_urllib_requests__url_tempfiles(self): - return list(urllib.request._url_tempfiles) + urllib_request = self.try_get_module('urllib.request') + return list(urllib_request._url_tempfiles) def restore_urllib_requests__url_tempfiles(self, tempfiles): for filename in tempfiles: os_helper.unlink(filename) def get_urllib_requests__opener(self): - return urllib.request._opener + urllib_request = self.try_get_module('urllib.request') + return urllib_request._opener def restore_urllib_requests__opener(self, opener): - urllib.request._opener = opener + urllib_request = self.get_module('urllib.request') + urllib_request._opener = opener def get_asyncio_events__event_loop_policy(self): + self.try_get_module('asyncio') return support.maybe_get_event_loop_policy() def restore_asyncio_events__event_loop_policy(self, policy): + asyncio = self.get_module('asyncio') asyncio.set_event_loop_policy(policy) def get_sys_argv(self): @@ -145,8 +153,10 @@ class saved_test_environment: builtins.__import__ = import_ def get_warnings_filters(self): + warnings = self.try_get_module('warnings') return id(warnings.filters), warnings.filters, warnings.filters[:] def restore_warnings_filters(self, saved_filters): + warnings = self.get_module('warnings') warnings.filters = saved_filters[1] warnings.filters[:] = saved_filters[2] @@ -161,23 +171,28 @@ class saved_test_environment: asyncore.socket_map.update(saved_map) def get_shutil_archive_formats(self): + shutil = self.try_get_module('shutil') # we could call get_archives_formats() but that only returns the # registry keys; we want to check the values too (the functions that # are registered) return shutil._ARCHIVE_FORMATS, shutil._ARCHIVE_FORMATS.copy() def restore_shutil_archive_formats(self, saved): + shutil = self.get_module('shutil') shutil._ARCHIVE_FORMATS = saved[0] shutil._ARCHIVE_FORMATS.clear() shutil._ARCHIVE_FORMATS.update(saved[1]) def get_shutil_unpack_formats(self): + shutil = self.try_get_module('shutil') return shutil._UNPACK_FORMATS, shutil._UNPACK_FORMATS.copy() def restore_shutil_unpack_formats(self, saved): + shutil = self.get_module('shutil') shutil._UNPACK_FORMATS = saved[0] shutil._UNPACK_FORMATS.clear() shutil._UNPACK_FORMATS.update(saved[1]) def get_logging__handlers(self): + logging = self.try_get_module('logging') # _handlers is a WeakValueDictionary return id(logging._handlers), logging._handlers, logging._handlers.copy() def restore_logging__handlers(self, saved_handlers): @@ -185,6 +200,7 @@ class saved_test_environment: pass def get_logging__handlerList(self): + logging = self.try_get_module('logging') # _handlerList is a list of weakrefs to handlers return id(logging._handlerList), logging._handlerList, logging._handlerList[:] def restore_logging__handlerList(self, saved_handlerList): @@ -208,32 +224,34 @@ class saved_test_environment: # Same for Process objects def get_multiprocessing_process__dangling(self): - if not multiprocessing: - return None + multiprocessing_process = self.try_get_module('multiprocessing.process') # Unjoined process objects can survive after process exits - multiprocessing.process._cleanup() + multiprocessing_process._cleanup() # This copies the weakrefs without making any strong reference - return multiprocessing.process._dangling.copy() + return multiprocessing_process._dangling.copy() def restore_multiprocessing_process__dangling(self, saved): - if not multiprocessing: - return - multiprocessing.process._dangling.clear() - multiprocessing.process._dangling.update(saved) + multiprocessing_process = self.get_module('multiprocessing.process') + multiprocessing_process._dangling.clear() + multiprocessing_process._dangling.update(saved) def get_sysconfig__CONFIG_VARS(self): # make sure the dict is initialized + sysconfig = self.try_get_module('sysconfig') sysconfig.get_config_var('prefix') return (id(sysconfig._CONFIG_VARS), sysconfig._CONFIG_VARS, dict(sysconfig._CONFIG_VARS)) def restore_sysconfig__CONFIG_VARS(self, saved): + sysconfig = self.get_module('sysconfig') sysconfig._CONFIG_VARS = saved[1] sysconfig._CONFIG_VARS.clear() sysconfig._CONFIG_VARS.update(saved[2]) def get_sysconfig__INSTALL_SCHEMES(self): + sysconfig = self.try_get_module('sysconfig') return (id(sysconfig._INSTALL_SCHEMES), sysconfig._INSTALL_SCHEMES, sysconfig._INSTALL_SCHEMES.copy()) def restore_sysconfig__INSTALL_SCHEMES(self, saved): + sysconfig = self.get_module('sysconfig') sysconfig._INSTALL_SCHEMES = saved[1] sysconfig._INSTALL_SCHEMES.clear() sysconfig._INSTALL_SCHEMES.update(saved[2]) @@ -264,8 +282,10 @@ class saved_test_environment: locale.setlocale(lc, setting) def get_warnings_showwarning(self): + warnings = self.try_get_module('warnings') return warnings.showwarning def restore_warnings_showwarning(self, fxn): + warnings = self.get_module('warnings') warnings.showwarning = fxn def resource_info(self): @@ -276,26 +296,28 @@ class saved_test_environment: yield name, getattr(self, get_name), getattr(self, restore_name) def __enter__(self): - self.saved_values = dict((name, get()) for name, get, restore - in self.resource_info()) + self.saved_values = [] + for name, get, restore in self.resource_info(): + try: + original = get() + except SkipTestEnvironment: + continue + + self.saved_values.append((name, get, restore, original)) return self def __exit__(self, exc_type, exc_val, exc_tb): saved_values = self.saved_values - del self.saved_values + self.saved_values = None # Some resources use weak references support.gc_collect() - # Read support.environment_altered, set by support helper functions - self.changed |= support.environment_altered - - for name, get, restore in self.resource_info(): + for name, get, restore, original in saved_values: current = get() - original = saved_values.pop(name) # Check for changes to the resource's value if current != original: - self.changed = True + support.environment_altered = True restore(original) if not self.quiet and not self.pgo: print_warning(f"{name} was modified by {self.testname}")