From 9abba715e3225b8e4c4b7dd0ed528ef3a3057bea Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 26 Sep 2023 23:59:11 +0200 Subject: [PATCH] gh-109566: Fix regrtest code adding Python options (#109926) * On Windows, use subprocess.run() instead of os.execv(). * Only add needed options * Rename reexec parameter to _add_python_opts. * Rename --no-reexec option to --dont-add-python-opts. --- Lib/test/__main__.py | 2 +- Lib/test/libregrtest/cmdline.py | 7 ++-- Lib/test/libregrtest/main.py | 65 +++++++++++++++++++++------------ Lib/test/test_regrtest.py | 28 ++++++++------ 4 files changed, 63 insertions(+), 39 deletions(-) diff --git a/Lib/test/__main__.py b/Lib/test/__main__.py index 42553fa3286..82b50ad2c6e 100644 --- a/Lib/test/__main__.py +++ b/Lib/test/__main__.py @@ -1,2 +1,2 @@ from test.libregrtest.main import main -main(reexec=True) +main(_add_python_opts=True) diff --git a/Lib/test/libregrtest/cmdline.py b/Lib/test/libregrtest/cmdline.py index bc969969e06..c180bb76222 100644 --- a/Lib/test/libregrtest/cmdline.py +++ b/Lib/test/libregrtest/cmdline.py @@ -184,7 +184,7 @@ class Namespace(argparse.Namespace): self.threshold = None self.fail_rerun = False self.tempdir = None - self.no_reexec = False + self._add_python_opts = True super().__init__(**kwargs) @@ -344,7 +344,8 @@ def _create_parser(): help='override the working directory for the test run') group.add_argument('--cleanup', action='store_true', help='remove old test_python_* directories') - group.add_argument('--no-reexec', action='store_true', + group.add_argument('--dont-add-python-opts', dest='_add_python_opts', + action='store_false', help="internal option, don't use it") return parser @@ -425,7 +426,7 @@ def _parse_args(args, **kwargs): if MS_WINDOWS: ns.nowindows = True # Silence alerts under Windows else: - ns.no_reexec = True + ns._add_python_opts = False # When both --slow-ci and --fast-ci options are present, # --slow-ci has the priority diff --git a/Lib/test/libregrtest/main.py b/Lib/test/libregrtest/main.py index e1cb22afcc9..c31d5ff187c 100644 --- a/Lib/test/libregrtest/main.py +++ b/Lib/test/libregrtest/main.py @@ -48,7 +48,7 @@ class Regrtest: directly to set the values that would normally be set by flags on the command line. """ - def __init__(self, ns: Namespace, reexec: bool = False): + def __init__(self, ns: Namespace, _add_python_opts: bool = False): # Log verbosity self.verbose: int = int(ns.verbose) self.quiet: bool = ns.quiet @@ -70,7 +70,11 @@ class Regrtest: self.want_cleanup: bool = ns.cleanup self.want_rerun: bool = ns.rerun self.want_run_leaks: bool = ns.runleaks - self.want_reexec: bool = (reexec and not ns.no_reexec) + + ci_mode = (ns.fast_ci or ns.slow_ci) + self.want_add_python_opts: bool = (_add_python_opts + and ns._add_python_opts + and ci_mode) # Select tests if ns.match_tests: @@ -97,7 +101,6 @@ class Regrtest: self.worker_json: StrJSON | None = ns.worker_json # Options to run tests - self.ci_mode: bool = (ns.fast_ci or ns.slow_ci) self.fail_fast: bool = ns.failfast self.fail_env_changed: bool = ns.fail_env_changed self.fail_rerun: bool = ns.fail_rerun @@ -486,32 +489,48 @@ class Regrtest: # processes. return self._run_tests(selected, tests) - def _reexecute_python(self): - if self.python_cmd: - # Do nothing if --python=cmd option is used + def _add_python_opts(self): + python_opts = [] + + # Unbuffered stdout and stderr + if not sys.stdout.write_through: + python_opts.append('-u') + + # Add warnings filter 'default' + if 'default' not in sys.warnoptions: + python_opts.extend(('-W', 'default')) + + # Error on bytes/str comparison + if sys.flags.bytes_warning < 2: + python_opts.append('-bb') + + # Ignore PYTHON* environment variables + if not sys.flags.ignore_environment: + python_opts.append('-E') + + if not python_opts: return - python_opts = [ - '-u', # Unbuffered stdout and stderr - '-W', 'default', # Add warnings filter 'default' - '-bb', # Error on bytes/str comparison - '-E', # Ignore PYTHON* environment variables - ] - - cmd = [*sys.orig_argv, "--no-reexec"] + cmd = [*sys.orig_argv, "--dont-add-python-opts"] cmd[1:1] = python_opts # Make sure that messages before execv() are logged sys.stdout.flush() sys.stderr.flush() + cmd_text = shlex.join(cmd) try: - os.execv(cmd[0], cmd) - # execv() do no return and so we don't get to this line on success - except OSError as exc: - cmd_text = shlex.join(cmd) - print_warning(f"Failed to reexecute Python: {exc!r}\n" + if hasattr(os, 'execv') and not MS_WINDOWS: + os.execv(cmd[0], cmd) + # execv() do no return and so we don't get to this line on success + else: + import subprocess + proc = subprocess.run(cmd) + sys.exit(proc.returncode) + except Exception as exc: + print_warning(f"Failed to change Python options: {exc!r}\n" f"Command: {cmd_text}") + # continue executing main() def _init(self): # Set sys.stdout encoder error handler to backslashreplace, @@ -527,8 +546,8 @@ class Regrtest: self.tmp_dir = get_temp_dir(self.tmp_dir) def main(self, tests: TestList | None = None): - if self.want_reexec and self.ci_mode: - self._reexecute_python() + if self.want_add_python_opts: + self._add_python_opts() self._init() @@ -556,7 +575,7 @@ class Regrtest: sys.exit(exitcode) -def main(tests=None, reexec=False, **kwargs): +def main(tests=None, _add_python_opts=False, **kwargs): """Run the Python suite.""" ns = _parse_args(sys.argv[1:], **kwargs) - Regrtest(ns, reexec=reexec).main(tests=tests) + Regrtest(ns, _add_python_opts=_add_python_opts).main(tests=tests) diff --git a/Lib/test/test_regrtest.py b/Lib/test/test_regrtest.py index 2b77300c079..3ece31be9af 100644 --- a/Lib/test/test_regrtest.py +++ b/Lib/test/test_regrtest.py @@ -382,7 +382,6 @@ class ParseArgsTestCase(unittest.TestCase): # Check Regrtest attributes which are more reliable than Namespace # which has an unclear API regrtest = main.Regrtest(ns) - self.assertTrue(regrtest.ci_mode) self.assertEqual(regrtest.num_workers, -1) self.assertTrue(regrtest.want_rerun) self.assertTrue(regrtest.randomize) @@ -413,6 +412,11 @@ class ParseArgsTestCase(unittest.TestCase): regrtest = self.check_ci_mode(args, use_resources) self.assertEqual(regrtest.timeout, 20 * 60) + def test_dont_add_python_opts(self): + args = ['--dont-add-python-opts'] + ns = cmdline._parse_args(args) + self.assertFalse(ns._add_python_opts) + @dataclasses.dataclass(slots=True) class Rerun: @@ -1984,22 +1988,23 @@ class ArgsTestCase(BaseTestCase): # -E option self.assertTrue(config['use_environment'], 0) - # test if get_config() is not available - def test_unbuffered(self): - # -u option - self.assertFalse(sys.stdout.line_buffering) - self.assertFalse(sys.stderr.line_buffering) - def test_python_opts(self): + # -u option + self.assertTrue(sys.__stdout__.write_through) + self.assertTrue(sys.__stderr__.write_through) + # -W default option self.assertTrue(sys.warnoptions, ['default']) + # -bb option self.assertEqual(sys.flags.bytes_warning, 2) + # -E option self.assertTrue(sys.flags.ignore_environment) """) testname = self.create_test(code=code) + # Use directly subprocess to control the exact command line cmd = [sys.executable, "-m", "test", option, f'--testdir={self.tmptestdir}', @@ -2010,11 +2015,10 @@ class ArgsTestCase(BaseTestCase): text=True) self.assertEqual(proc.returncode, 0, proc) - def test_reexec_fast_ci(self): - self.check_reexec("--fast-ci") - - def test_reexec_slow_ci(self): - self.check_reexec("--slow-ci") + def test_add_python_opts(self): + for opt in ("--fast-ci", "--slow-ci"): + with self.subTest(opt=opt): + self.check_reexec(opt) class TestUtils(unittest.TestCase):