gh-120678: pyrepl: Include globals from modules passed with `-i` by AlexWaygood · Pull Request #120904 · python/cpython · GitHub
Skip to content
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
3 changes: 3 additions & 0 deletions Lib/_pyrepl/__main__.py
6 changes: 5 additions & 1 deletion Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2614,14 +2614,18 @@ def force_not_colorized(func):
def wrapper(*args, **kwargs):
import _colorize
original_fn = _colorize.can_colorize
variables = {"PYTHON_COLORS": None, "FORCE_COLOR": None}
variables: dict[str, str | None] = {
"PYTHON_COLORS": None, "FORCE_COLOR": None, "NO_COLOR": None
}
try:
for key in variables:
variables[key] = os.environ.pop(key, None)
os.environ["NO_COLOR"] = "1"
_colorize.can_colorize = lambda: False
return func(*args, **kwargs)
finally:
_colorize.can_colorize = original_fn
del os.environ["NO_COLOR"]
for key, value in variables.items():
if value is not None:
os.environ[key] = value
Expand Down
13 changes: 12 additions & 1 deletion Lib/test/test_pyrepl/support.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
from code import InteractiveConsole
from functools import partial
from typing import Iterable
Expand Down Expand Up @@ -100,8 +101,18 @@ def handle_all_events(
)


def make_clean_env() -> dict[str, str]:
clean_env = os.environ.copy()
for k in clean_env.copy():
if k.startswith("PYTHON"):
clean_env.pop(k)
clean_env.pop("FORCE_COLOR", None)
clean_env.pop("NO_COLOR", None)
return clean_env


class FakeConsole(Console):
def __init__(self, events, encoding="utf-8"):
def __init__(self, events, encoding="utf-8") -> None:
self.events = iter(events)
self.encoding = encoding
self.screen = []
Expand Down
114 changes: 106 additions & 8 deletions Lib/test/test_pyrepl/test_pyrepl.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import itertools
import os
import pathlib
import re
import rlcompleter
import select
import subprocess
Expand All @@ -21,7 +22,8 @@
more_lines,
multiline_input,
code_to_events,
clean_screen
clean_screen,
make_clean_env,
)
from _pyrepl.console import Event
from _pyrepl.readline import ReadlineAlikeReader, ReadlineConfig
Expand Down Expand Up @@ -487,6 +489,18 @@ def prepare_reader(self, events):
reader.can_colorize = False
return reader

def test_stdin_is_tty(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was helpful to me when debugging weird behavior so I decided to keep it.

# Used during test log analysis to figure out if a TTY was available.
if os.isatty(sys.stdin.fileno()):
return
self.skipTest("stdin is not a tty")

def test_stdout_is_tty(self):
# Used during test log analysis to figure out if a TTY was available.
if os.isatty(sys.stdout.fileno()):
return
self.skipTest("stdout is not a tty")

def test_basic(self):
reader = self.prepare_reader(code_to_events("1+1\n"))

Expand Down Expand Up @@ -888,12 +902,7 @@ def setUp(self):
# Cleanup from PYTHON* variables to isolate from local
# user settings, see #121359. Such variables should be
# added later in test methods to patched os.environ.
clean_env = os.environ.copy()
for k in clean_env.copy():
if k.startswith("PYTHON"):
clean_env.pop(k)

patcher = patch('os.environ', new=clean_env)
patcher = patch('os.environ', new=make_clean_env())
self.addCleanup(patcher.stop)
patcher.start()

Expand All @@ -920,6 +929,84 @@ def test_exposed_globals_in_repl(self):

self.assertTrue(case1 or case2 or case3 or case4, output)

def _assertMatchOK(
self, var: str, expected: str | re.Pattern, actual: str
) -> None:
if isinstance(expected, re.Pattern):
self.assertTrue(
expected.match(actual),
f"{var}={actual} does not match {expected.pattern}",
)
else:
self.assertEqual(
actual,
expected,
f"expected {var}={expected}, got {var}={actual}",
)
Comment thread
ambv marked this conversation as resolved.

@force_not_colorized
def _run_repl_globals_test(self, expectations, *, as_file=False, as_module=False):
clean_env = make_clean_env()
clean_env["NO_COLOR"] = "1" # force_not_colorized doesn't touch subprocesses

with tempfile.TemporaryDirectory() as td:
blue = pathlib.Path(td) / "blue"
blue.mkdir()
mod = blue / "calx.py"
mod.write_text("FOO = 42", encoding="utf-8")
commands = [
"print(f'{" + var + "=}')" for var in expectations
] + ["exit"]
if as_file and as_module:
self.fail("as_file and as_module are mutually exclusive")
elif as_file:
output, exit_code = self.run_repl(
commands,
cmdline_args=[str(mod)],
env=clean_env,
)
elif as_module:
output, exit_code = self.run_repl(
commands,
cmdline_args=["-m", "blue.calx"],
env=clean_env,
cwd=td,
)
else:
self.fail("Choose one of as_file or as_module")

if "can't use pyrepl" in output:
self.skipTest("pyrepl not available")

self.assertEqual(exit_code, 0)
for var, expected in expectations.items():
with self.subTest(var=var, expected=expected):
if m := re.search(rf"[\r\n]{var}=(.+?)[\r\n]", output):
self._assertMatchOK(var, expected, actual=m.group(1))
else:
self.fail(f"{var}= not found in output")

self.assertNotIn("Exception", output)
self.assertNotIn("Traceback", output)

def test_inspect_keeps_globals_from_inspected_file(self):
expectations = {
"FOO": "42",
"__name__": "'__main__'",
"__package__": "None",
# "__file__" is missing in -i, like in the basic REPL
Comment thread
ambv marked this conversation as resolved.
}
self._run_repl_globals_test(expectations, as_file=True)

def test_inspect_keeps_globals_from_inspected_module(self):
expectations = {
"FOO": "42",
"__name__": "'__main__'",
"__package__": "'blue'",
"__file__": re.compile(r"^'.*calx.py'$"),
Comment thread
ambv marked this conversation as resolved.
}
self._run_repl_globals_test(expectations, as_module=True)

def test_dumb_terminal_exits_cleanly(self):
env = os.environ.copy()
env.update({"TERM": "dumb"})
Expand Down Expand Up @@ -981,16 +1068,27 @@ def test_not_wiping_history_file(self):
self.assertIn("spam", output)
self.assertNotEqual(pathlib.Path(hfile.name).stat().st_size, 0)

def run_repl(self, repl_input: str | list[str], env: dict | None = None) -> tuple[str, int]:
def run_repl(
self,
repl_input: str | list[str],
env: dict | None = None,
*,
cmdline_args: list[str] | None = None,
cwd: str | None = None,
) -> tuple[str, int]:
assert pty
master_fd, slave_fd = pty.openpty()
cmd = [sys.executable, "-i", "-u"]
if env is None:
cmd.append("-I")
if cmdline_args is not None:
cmd.extend(cmdline_args)
process = subprocess.Popen(
cmd,
stdin=slave_fd,
stdout=slave_fd,
stderr=slave_fd,
cwd=cwd,
text=True,
close_fds=True,
env=env if env else os.environ,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Fix regression in the new REPL that meant that globals from files passed
using the ``-i`` argument would not be included in the REPL's global
namespace. Patch by Alex Waygood.
50 changes: 49 additions & 1 deletion Modules/main.c