Skip to content

gh-109566: Fix regrtest code adding Python options #109926

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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