Skip to content

bpo-41718: regrtest saved_test_environment avoids imports #24934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 22, 2021
Merged

bpo-41718: regrtest saved_test_environment avoids imports #24934

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
21 changes: 13 additions & 8 deletions Lib/test/libregrtest/runtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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

Expand Down
90 changes: 56 additions & 34 deletions Lib/test/libregrtest/save_env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -73,20 +65,36 @@ def __init__(self, testname, verbose=0, quiet=False, *, pgo=False):
'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):
Expand Down Expand Up @@ -145,8 +153,10 @@ def restore___import__(self, import_):
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]

Expand All @@ -161,30 +171,36 @@ def restore_asyncore_socket_map(self, saved_map):
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):
# Can't easily revert the logging state
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):
Expand All @@ -208,32 +224,34 @@ def restore_threading__dangling(self, saved):

# Same for Process objects
def get_multiprocessing_process__dangling(self):
if not multiprocessing:
return None
multiprocessing_process = self.try_get_module('multiprocessing.process')
Copy link
Member

Choose a reason for hiding this comment

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

Hm. The previous code checked the multiprocessing.process imported successful or not. Do we need keep it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The current code ignores ImportError. What do you mean?

try:
    import _multiprocessing, multiprocessing.process
except ImportError:
    multiprocessing = None

Copy link
Member

Choose a reason for hiding this comment

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

Oh, sorry. Wrong comment in here. You raised the SkipTestEnviroment in here and catch it in __enter__(). So it is same as the old codes.

# 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])
Expand Down Expand Up @@ -264,8 +282,10 @@ def restore_locale(self, saved):
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):
Expand All @@ -276,26 +296,28 @@ def resource_info(self):
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}")
Expand Down