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.

2 files changed, 66 insertions, 44 deletions

diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py
index 02717d8c..a9b2b35 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.libregrtest.utils import removepy, count, format_duration, printlist
 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 @@ class Regrtest:
         # 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 @@ class Regrtest:
         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 @@ class Regrtest:
         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 @@ class Regrtest:
             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 @@ class Regrtest:
                 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 @@ class Regrtest:
 
         # 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 4217847..aa2409b 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)