From 3c93153f7db5dd9b06f229e61978fd9199b3c097 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 14 May 2019 15:49:16 +0200 Subject: [PATCH] bpo-36915: regrtest always remove tempdir of worker processes (GH-13312) When using multiprocessing (-jN option), worker processes now create their temporary directory inside the temporary directory of the main process. So the main process is able to remove temporary directories of worker processes even if they crash or when they are killed by regrtest on KeyboardInterrupt (CTRL+c). Rework also how multiprocessing arguments are parsed in main.py. --- Lib/test/libregrtest/main.py | 90 +++++++++++-------- Lib/test/libregrtest/runtest_mp.py | 20 +++-- .../2019-05-14-14-12-24.bpo-36915.58b7pH.rst | 3 + 3 files changed, 69 insertions(+), 44 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2019-05-14-14-12-24.bpo-36915.58b7pH.rst diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index 02717d8c7b132b..a9b2b352d120e6 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -1,6 +1,5 @@ import datetime import faulthandler -import json import locale import os import platform @@ -22,22 +21,6 @@ from test import support -# When tests are run from the Python build directory, it is best practice -# to keep the test files in a subfolder. This eases the cleanup of leftover -# files using the "make distclean" command. -if sysconfig.is_python_build(): - TEMPDIR = sysconfig.get_config_var('abs_builddir') - if TEMPDIR is None: - # bpo-30284: On Windows, only srcdir is available. Using abs_builddir - # mostly matters on UNIX when building Python out of the source tree, - # especially when the source tree is read only. - TEMPDIR = sysconfig.get_config_var('srcdir') - TEMPDIR = os.path.join(TEMPDIR, 'build') -else: - TEMPDIR = tempfile.gettempdir() -TEMPDIR = os.path.abspath(TEMPDIR) - - class Regrtest: """Execute a test suite. @@ -98,7 +81,10 @@ def __init__(self): # used by --junit-xml self.testsuite_xml = None + # misc self.win_load_tracker = None + self.tmp_dir = None + self.worker_test_name = None def get_executed(self): return (set(self.good) | set(self.bad) | set(self.skipped) @@ -177,6 +163,13 @@ def parse_args(self, kwargs): if ns.xmlpath: support.junit_xml_list = self.testsuite_xml = [] + worker_args = ns.worker_args + if worker_args is not None: + from test.libregrtest.runtest_mp import parse_worker_args + ns, test_name = parse_worker_args(ns.worker_args) + ns.worker_args = worker_args + self.worker_test_name = test_name + # Strip .py extensions. removepy(ns.args) @@ -186,7 +179,7 @@ def find_tests(self, tests): self.tests = tests if self.ns.single: - self.next_single_filename = os.path.join(TEMPDIR, 'pynexttest') + self.next_single_filename = os.path.join(self.tmp_dir, 'pynexttest') try: with open(self.next_single_filename, 'r') as fp: next_test = fp.read().strip() @@ -544,29 +537,54 @@ def save_xml_result(self): for s in ET.tostringlist(root): f.write(s) - def main(self, tests=None, **kwargs): - global TEMPDIR - self.ns = self.parse_args(kwargs) - + def create_temp_dir(self): if self.ns.tempdir: - TEMPDIR = self.ns.tempdir - elif self.ns.worker_args: - ns_dict, _ = json.loads(self.ns.worker_args) - TEMPDIR = ns_dict.get("tempdir") or TEMPDIR + self.tmp_dir = self.ns.tempdir + + if not self.tmp_dir: + # When tests are run from the Python build directory, it is best practice + # to keep the test files in a subfolder. This eases the cleanup of leftover + # files using the "make distclean" command. + if sysconfig.is_python_build(): + self.tmp_dir = sysconfig.get_config_var('abs_builddir') + if self.tmp_dir is None: + # bpo-30284: On Windows, only srcdir is available. Using + # abs_builddir mostly matters on UNIX when building Python + # out of the source tree, especially when the source tree + # is read only. + self.tmp_dir = sysconfig.get_config_var('srcdir') + self.tmp_dir = os.path.join(self.tmp_dir, 'build') + else: + self.tmp_dir = tempfile.gettempdir() - os.makedirs(TEMPDIR, exist_ok=True) + self.tmp_dir = os.path.abspath(self.tmp_dir) + os.makedirs(self.tmp_dir, exist_ok=True) # Define a writable temp dir that will be used as cwd while running # the tests. The name of the dir includes the pid to allow parallel # testing (see the -j option). - test_cwd = 'test_python_{}'.format(os.getpid()) - test_cwd = os.path.join(TEMPDIR, test_cwd) + pid = os.getpid() + if self.worker_test_name is not None: + test_cwd = 'worker_{}'.format(pid) + else: + test_cwd = 'test_python_{}'.format(pid) + test_cwd = os.path.join(self.tmp_dir, test_cwd) + return test_cwd + + def main(self, tests=None, **kwargs): + self.ns = self.parse_args(kwargs) + + test_cwd = self.create_temp_dir() - # Run the tests in a context manager that temporarily changes the CWD to a - # temporary and writable directory. If it's not possible to create or - # change the CWD, the original CWD will be used. The original CWD is - # available from support.SAVEDCWD. + # Run the tests in a context manager that temporarily changes the CWD + # to a temporary and writable directory. If it's not possible to + # create or change the CWD, the original CWD will be used. + # The original CWD is available from support.SAVEDCWD. with support.temp_cwd(test_cwd, quiet=True): + # When using multiprocessing, worker processes will use test_cwd + # as their parent temporary directory. So when the main process + # exit, it removes also subdirectories of worker processes. + self.ns.tempdir = test_cwd self._main(tests, kwargs) def getloadavg(self): @@ -588,9 +606,9 @@ def _main(self, tests, kwargs): print(msg, file=sys.stderr, flush=True) sys.exit(2) - if self.ns.worker_args is not None: + if self.worker_test_name is not None: from test.libregrtest.runtest_mp import run_tests_worker - run_tests_worker(self.ns.worker_args) + run_tests_worker(self.ns, self.worker_test_name) if self.ns.wait: input("Press any key to continue...") @@ -611,7 +629,7 @@ def _main(self, tests, kwargs): # If we're on windows and this is the parent runner (not a worker), # track the load average. - if sys.platform == 'win32' and (self.ns.worker_args is None): + if sys.platform == 'win32' and self.worker_test_name is None: from test.libregrtest.win_utils import WindowsLoadTracker try: diff --git a/Lib/test/libregrtest/runtest_mp.py b/Lib/test/libregrtest/runtest_mp.py index 42178471ef1da9..aa2409b4ef7985 100644 --- a/Lib/test/libregrtest/runtest_mp.py +++ b/Lib/test/libregrtest/runtest_mp.py @@ -33,6 +33,12 @@ def must_stop(result, ns): return False +def parse_worker_args(worker_args): + ns_dict, test_name = json.loads(worker_args) + ns = types.SimpleNamespace(**ns_dict) + return (ns, test_name) + + def run_test_in_subprocess(testname, ns): ns_dict = vars(ns) worker_args = (ns_dict, testname) @@ -42,8 +48,6 @@ def run_test_in_subprocess(testname, ns): '-u', # Unbuffered stdout and stderr '-m', 'test.regrtest', '--worker-args', worker_args] - if ns.pgo: - cmd += ['--pgo'] # Running the child from the same working directory as regrtest's original # invocation ensures that TEMPDIR for the child is the same when @@ -56,15 +60,15 @@ def run_test_in_subprocess(testname, ns): cwd=support.SAVEDCWD) -def run_tests_worker(worker_args): - ns_dict, testname = json.loads(worker_args) - ns = types.SimpleNamespace(**ns_dict) - +def run_tests_worker(ns, test_name): setup_tests(ns) - result = runtest(ns, testname) + result = runtest(ns, test_name) + print() # Force a newline (just in case) - print(json.dumps(result), flush=True) + + # Serialize TestResult as list in JSON + print(json.dumps(list(result)), flush=True) sys.exit(0) diff --git a/Misc/NEWS.d/next/Tests/2019-05-14-14-12-24.bpo-36915.58b7pH.rst b/Misc/NEWS.d/next/Tests/2019-05-14-14-12-24.bpo-36915.58b7pH.rst new file mode 100644 index 00000000000000..4eebfb4832510d --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2019-05-14-14-12-24.bpo-36915.58b7pH.rst @@ -0,0 +1,3 @@ +The main regrtest process now always removes all temporary directories of +worker processes even if they crash or if they are killed on +KeyboardInterrupt (CTRL+c).