gh-109276: libregrtest: add RunTests.work_dir

vstinner · vstinner · commit 31696293b735 · 2023-09-12T01:51:11.000+02:00
* WorkerThread now always creates a temporary directory, even on
  Emscripten and WASI: it's used as the working directory of the test
  worker process.
* Fix Emscripten and WASI: start the test worker process in the
  Python source code directory, where 'python.js' and 'python.wasm'
  can be found. Then worker_process() goes to the temporary directory
  created to run tests.
* --cleanup now also removes "temporary" directories of test worker
  processes (in /tmp).
diff --git a/Lib/test/libregrtest/cmdline.py b/Lib/test/libregrtest/cmdline.py
@@ -331,7 +331,8 @@ def _create_parser():
                        help='writes JUnit-style XML results to the specified '
                             'file')
     group.add_argument('--tempdir', metavar='PATH',
-                       help='override the working directory for the test run')
+                       help='Override the working directory for the test run. '
+                            'It not be too long and only use ASCII characters.')
     group.add_argument('--cleanup', action='store_true',
                        help='remove old test_python_* directories')
     return parser
diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py
@@ -3,6 +3,7 @@
 import re
 import sys
 import time
+import tempfile
 
 from test import support
 from test.support import os_helper
@@ -20,7 +21,7 @@
     StrPath, StrJSON, TestName, TestList, TestTuple, FilterTuple,
     strip_py_suffix, count, format_duration,
     printlist, get_temp_dir, get_work_dir, exit_timeout,
-    display_header, cleanup_temp_dir)
+    display_header, cleanup_temp_dir, set_temp_dir_environ)
 
 
 class Regrtest:
@@ -393,6 +394,7 @@ def create_run_tests(self, tests: TestTuple):
             gc_threshold=self.gc_threshold,
             use_resources=self.use_resources,
             python_cmd=self.python_cmd,
+            work_dir=os.getcwd(),
         )
 
     def _run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
@@ -465,10 +467,18 @@ def main(self, tests: TestList | None = None):
 
         strip_py_suffix(self.cmdline_args)
 
+        if self.tmp_dir:
+            self.tmp_dir = os.path.abspath(os.path.expanduser(self.tmp_dir))
+            set_temp_dir_environ(os.environ, self.tmp_dir)
         self.tmp_dir = get_temp_dir(self.tmp_dir)
+        # Don't use set_temp_dir_environ() on get_temp_dir() result, since many
+        # tests fail if the temporary directory is non-ASCII or is too long.
 
         if self.want_cleanup:
             cleanup_temp_dir(self.tmp_dir)
+            tmp_dir2 = os.path.abspath(tempfile.gettempdir())
+            if tmp_dir2 != self.tmp_dir:
+                cleanup_temp_dir(tmp_dir2)
             sys.exit(0)
 
         if self.want_wait:
diff --git a/Lib/test/libregrtest/run_workers.py b/Lib/test/libregrtest/run_workers.py
@@ -9,7 +9,7 @@
 import threading
 import time
 import traceback
-from typing import Literal, TextIO
+from typing import Literal
 
 from test import support
 from test.support import os_helper
@@ -21,7 +21,7 @@
 from .single import PROGRESS_MIN_TIME
 from .utils import (
     StrPath, StrJSON, TestName, MS_WINDOWS,
-    format_duration, print_warning)
+    format_duration, print_warning, WORKER_DIR_PREFIX)
 from .worker import create_worker_process, USE_PROCESS_GROUP
 
 if MS_WINDOWS:
@@ -156,10 +156,10 @@ def mp_result_error(
         return MultiprocessResult(test_result, stdout, err_msg)
 
     def _run_process(self, runtests: RunTests, output_fd: int, json_fd: int,
-                     tmp_dir: StrPath | None = None) -> int:
+                     start_work_dir: StrPath | None = None) -> int:
         try:
             popen = create_worker_process(runtests, output_fd, json_fd,
-                                          tmp_dir)
+                                          start_work_dir)
 
             self._killed = False
             self._popen = popen
@@ -235,33 +235,39 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
             if MS_WINDOWS:
                 json_fd = msvcrt.get_osfhandle(json_fd)
 
+            # gh-93353: Check for leaked temporary files in the parent process,
+            # since the deletion of temporary files can happen late during
+            # Python finalization: too late for libregrtest.
+            tmp_dir = tempfile.mkdtemp(prefix=WORKER_DIR_PREFIX)
+            tmp_dir = os.path.abspath(tmp_dir)
+
             kwargs = {}
             if match_tests:
                 kwargs['match_tests'] = match_tests
             worker_runtests = self.runtests.copy(
                 tests=tests,
                 json_fd=json_fd,
+                work_dir=tmp_dir,
                 **kwargs)
 
-            # gh-93353: Check for leaked temporary files in the parent process,
-            # since the deletion of temporary files can happen late during
-            # Python finalization: too late for libregrtest.
-            if not support.is_wasi:
-                # Don't check for leaked temporary files and directories if Python is
-                # run on WASI. WASI don't pass environment variables like TMPDIR to
-                # worker processes.
-                tmp_dir = tempfile.mkdtemp(prefix="test_python_")
-                tmp_dir = os.path.abspath(tmp_dir)
-                try:
-                    retcode = self._run_process(worker_runtests,
-                                                stdout_fd, json_fd, tmp_dir)
-                finally:
-                    tmp_files = os.listdir(tmp_dir)
-                    os_helper.rmtree(tmp_dir)
+            # Starting working directory of the worker process.
+            #
+            # Emscripten and WASI Python must start in the Python source code
+            # directory to get 'python.js' or 'python.wasm' file.
+            #
+            # Then worker_process() calls change_cwd(runtests.work_dir).
+            if support.is_emscripten or support.is_wasi:
+                start_work_dir = os_helper.SAVEDCWD
             else:
+                start_work_dir = tmp_dir
+
+            try:
                 retcode = self._run_process(worker_runtests,
-                                            stdout_fd, json_fd)
-                tmp_files = ()
+                                            stdout_fd, json_fd,
+                                            start_work_dir)
+            finally:
+                tmp_files = os.listdir(tmp_dir)
+                os_helper.rmtree(tmp_dir)
             stdout_file.seek(0)
 
             try:
@@ -280,7 +286,7 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
                 if worker_json:
                     result = TestResult.from_json(worker_json)
                 else:
-                    err_msg = f"empty JSON"
+                    err_msg = "empty JSON"
             except Exception as exc:
                 # gh-101634: Catch UnicodeDecodeError if stdout cannot be
                 # decoded from encoding
diff --git a/Lib/test/libregrtest/runtests.py b/Lib/test/libregrtest/runtests.py
@@ -39,6 +39,7 @@ class RunTests:
     # On Unix, it's a file descriptor.
     # On Windows, it's a handle.
     json_fd: int | None = None
+    work_dir: StrPath = None
 
     def copy(self, **override):
         state = dataclasses.asdict(self)
diff --git a/Lib/test/libregrtest/utils.py b/Lib/test/libregrtest/utils.py
@@ -17,6 +17,8 @@
 
 
 MS_WINDOWS = (sys.platform == 'win32')
+WORK_DIR_PREFIX = 'test_python_'
+WORKER_DIR_PREFIX = f'{WORK_DIR_PREFIX}worker_'
 
 # bpo-38203: Maximum delay in seconds to exit Python (call Py_Finalize()).
 # Used to protect against threading._shutdown() hang.
@@ -338,9 +340,7 @@ def get_build_info():
 
 
 def get_temp_dir(tmp_dir):
-    if tmp_dir:
-        tmp_dir = os.path.expanduser(tmp_dir)
-    else:
+    if not 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.
@@ -359,6 +359,12 @@ def get_temp_dir(tmp_dir):
     return os.path.abspath(tmp_dir)
 
 
+def set_temp_dir_environ(environ: dict, tmp_dir: StrPath) -> None:
+    environ['TMPDIR'] = tmp_dir
+    environ['TEMP'] = tmp_dir
+    environ['TMP'] = tmp_dir
+
+
 def fix_umask():
     if support.is_emscripten:
         # Emscripten has default umask 0o777, which breaks some tests.
@@ -370,21 +376,18 @@ def fix_umask():
             os.umask(old_mask)
 
 
-def get_work_dir(*, parent_dir: StrPath = '', worker: bool = False):
+def get_work_dir(*, parent_dir: StrPath | None = None) -> StrPath:
     # 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).
     # Emscripten and WASI have stubbed getpid(), Emscripten has only
     # milisecond clock resolution. Use randint() instead.
-    if sys.platform in {"emscripten", "wasi"}:
+    if support.is_emscripten or support.is_wasi:
         nounce = random.randint(0, 1_000_000)
     else:
         nounce = os.getpid()
 
-    if worker:
-        work_dir = 'test_python_worker_{}'.format(nounce)
-    else:
-        work_dir = 'test_python_{}'.format(nounce)
+    work_dir = WORK_DIR_PREFIX + str(nounce)
     work_dir += os_helper.FS_NONASCII
     if parent_dir:
         work_dir = os.path.join(parent_dir, work_dir)
@@ -570,7 +573,7 @@ def display_header():
 def cleanup_temp_dir(tmp_dir: StrPath):
     import glob
 
-    path = os.path.join(glob.escape(tmp_dir), 'test_python_*')
+    path = os.path.join(glob.escape(tmp_dir), WORK_DIR_PREFIX + '*')
     print("Cleanup %s directory" % tmp_dir)
     for name in glob.glob(path):
         if os.path.isdir(name):
diff --git a/Lib/test/libregrtest/worker.py b/Lib/test/libregrtest/worker.py
@@ -1,7 +1,7 @@
 import subprocess
 import sys
 import os
-from typing import TextIO, NoReturn
+from typing import NoReturn
 
 from test import support
 from test.support import os_helper
@@ -11,15 +11,15 @@
 from .single import run_single_test
 from .utils import (
     StrPath, StrJSON, FilterTuple, MS_WINDOWS,
-    get_work_dir, exit_timeout)
+    exit_timeout, set_temp_dir_environ)
 
 
 USE_PROCESS_GROUP = (hasattr(os, "setsid") and hasattr(os, "killpg"))
 
 
 def create_worker_process(runtests: RunTests,
                           output_fd: int, json_fd: int,
-                          tmp_dir: StrPath | None = None) -> subprocess.Popen:
+                          start_work_dir: StrPath | None = None) -> subprocess.Popen:
     python_cmd = runtests.python_cmd
     worker_json = runtests.as_json()
 
@@ -33,10 +33,7 @@ def create_worker_process(runtests: RunTests,
            worker_json]
 
     env = dict(os.environ)
-    if tmp_dir is not None:
-        env['TMPDIR'] = tmp_dir
-        env['TEMP'] = tmp_dir
-        env['TMP'] = tmp_dir
+    set_temp_dir_environ(env, runtests.work_dir)
 
     # Running the child from the same working directory as regrtest's original
     # invocation ensures that TEMPDIR for the child is the same when
@@ -48,6 +45,7 @@ def create_worker_process(runtests: RunTests,
         stderr=output_fd,
         text=True,
         close_fds=True,
+        cwd=start_work_dir,
     )
     if not MS_WINDOWS:
         kwargs['pass_fds'] = [json_fd]
@@ -67,8 +65,9 @@ def create_worker_process(runtests: RunTests,
             os.set_handle_inheritable(json_fd, False)
 
 
-def worker_process(worker_json: StrJSON) -> NoReturn:
-    runtests = RunTests.from_json(worker_json)
+def worker_process(runtests: RunTests) -> NoReturn:
+    set_temp_dir_environ(os.environ, os.getcwd())
+
     test_name = runtests.tests[0]
     match_tests: FilterTuple | None = runtests.match_tests
     json_fd: int = runtests.json_fd
@@ -77,7 +76,6 @@ def worker_process(worker_json: StrJSON) -> NoReturn:
         import msvcrt
         json_fd = msvcrt.open_osfhandle(json_fd, os.O_WRONLY)
 
-
     setup_test_dir(runtests.test_dir)
     setup_process()
 
@@ -100,13 +98,12 @@ def main():
     if len(sys.argv) != 2:
         print("usage: python -m test.libregrtest.worker JSON")
         sys.exit(1)
-    worker_json = sys.argv[1]
-
-    work_dir = get_work_dir(worker=True)
+    worker_json: StrJSON = sys.argv[1]
 
     with exit_timeout():
-        with os_helper.temp_cwd(work_dir, quiet=True):
-            worker_process(worker_json)
+        runtests = RunTests.from_json(worker_json)
+        with os_helper.change_cwd(runtests.work_dir):
+            worker_process(runtests)
 
 
 if __name__ == "__main__":
