Skip to content

Commit

Permalink
pythongh-109566: Fix regrtest code adding Python options
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
vstinner committed Sep 26, 2023
1 parent 4390c13 commit 3321352
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 38 deletions.
2 changes: 1 addition & 1 deletion Lib/test/__main__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
from test.libregrtest.main import main
main(reexec=True)
main(_add_python_opts=True)
7 changes: 4 additions & 3 deletions Lib/test/libregrtest/cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ def __init__(self, **kwargs) -> None:
self.threshold = None
self.fail_rerun = False
self.tempdir = None
self.no_reexec = False
self._add_python_opts = True

super().__init__(**kwargs)

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

Expand Down Expand Up @@ -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
Expand Down
65 changes: 42 additions & 23 deletions Lib/test/libregrtest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -70,7 +70,11 @@ def __init__(self, ns: Namespace, reexec: bool = False):
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:
Expand All @@ -97,7 +101,6 @@ def __init__(self, ns: Namespace, reexec: bool = False):
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
Expand Down Expand Up @@ -486,32 +489,48 @@ def run_tests(self, selected: TestTuple, tests: TestList | None) -> int:
# processes.
return self._run_tests(selected, tests)

def _reexecute_python(self):
if self.python_cmd:
# Do nothing if --python=cmd option is used
return
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'))

python_opts = [
'-u', # Unbuffered stdout and stderr
'-W', 'default', # Add warnings filter 'default'
'-bb', # Error on bytes/str comparison
'-E', # Ignore PYTHON* environment variables
]
# Error on bytes/str comparison
if sys.flags.bytes_warning < 2:
python_opts.append('-bb')

cmd = [*sys.orig_argv, "--no-reexec"]
# Ignore PYTHON* environment variables
if not sys.flags.ignore_environment:
python_opts.append('-E')

if not python_opts:
return

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,
Expand All @@ -527,8 +546,8 @@ def _init(self):
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()

Expand Down Expand Up @@ -556,7 +575,7 @@ def main(self, tests: TestList | None = None):
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)
26 changes: 15 additions & 11 deletions Lib/test/test_regrtest.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,6 @@ def check_ci_mode(self, args, use_resources):
# 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)
Expand Down Expand Up @@ -413,6 +412,11 @@ def test_slow_ci(self):
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:
Expand Down Expand Up @@ -1984,22 +1988,23 @@ def test_config(self):
# -E option
self.assertTrue(config['use_environment'], 0)
# test if get_config() is not available
def test_unbuffered(self):
def test_python_opts(self):
# -u option
self.assertFalse(sys.stdout.line_buffering)
self.assertFalse(sys.stderr.line_buffering)
self.assertTrue(sys.__stdout__.write_through)
self.assertTrue(sys.__stderr__.write_through)
def test_python_opts(self):
# -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}',
Expand All @@ -2010,11 +2015,10 @@ def test_python_opts(self):
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):
Expand Down

0 comments on commit 3321352

Please sign in to comment.