From 1e8f60359cbc3557af1f4934b2994b95903f8d22 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 8 May 2025 18:39:05 +0100 Subject: [PATCH 1/3] Improves help output and testability of commands. #74 --- src/manage/commands.py | 8 ++- src/manage/config.py | 20 +++--- src/manage/list_command.py | 14 ++-- src/manage/logging.py | 31 +++++++-- tests/conftest.py | 128 +++++++++++++++++++++++++++++++++---- tests/test_commands.py | 30 ++++++++- tests/test_installs.py | 62 ------------------ 7 files changed, 193 insertions(+), 100 deletions(-) diff --git a/src/manage/commands.py b/src/manage/commands.py index 4d13a82..ae3b7df 100644 --- a/src/manage/commands.py +++ b/src/manage/commands.py @@ -353,8 +353,8 @@ def __init__(self, args, root=None): set_next = a.lstrip("-/").lower() try: key, value, *opts = cmd_args[set_next] - except LookupError: - raise ArgumentError(f"Unexpected argument: {a}") + except KeyError: + raise ArgumentError(f"Unexpected argument: {a}") from None if value is _NEXT: if sep: if opts: @@ -868,6 +868,10 @@ class HelpCommand(BaseCommand): _create_log_file = False + def __init__(self, args, root=None): + super().__init__([self.CMD], root) + self.args = [a for a in args[1:] if a.isalpha()] + def execute(self): LOGGER.print(COPYRIGHT) self.show_welcome(copyright=False) diff --git a/src/manage/config.py b/src/manage/config.py index 14ecd5e..c9500be 100644 --- a/src/manage/config.py +++ b/src/manage/config.py @@ -35,22 +35,24 @@ def config_bool(v): return v.lower().startswith(("t", "y", "1")) return bool(v) -def _global_file(): + +def load_global_config(cfg, schema): try: from _native import package_get_root except ImportError: - return Path(sys.executable).parent / DEFAULT_CONFIG_NAME - return Path(package_get_root()) / DEFAULT_CONFIG_NAME + file = Path(sys.executable).parent / DEFAULT_CONFIG_NAME + else: + file = Path(package_get_root()) / DEFAULT_CONFIG_NAME + try: + load_one_config(cfg, file, schema=schema) + except FileNotFoundError: + pass + def load_config(root, override_file, schema): cfg = {} - global_file = _global_file() - if global_file: - try: - load_one_config(cfg, global_file, schema=schema) - except FileNotFoundError: - pass + load_global_config(cfg, schema=schema) try: reg_cfg = load_registry_config(cfg["registry_override_key"], schema=schema) diff --git a/src/manage/list_command.py b/src/manage/list_command.py index 462714d..bb4882e 100644 --- a/src/manage/list_command.py +++ b/src/manage/list_command.py @@ -167,12 +167,12 @@ def format_csv(cmd, installs): def format_json(cmd, installs): - print(json.dumps({"versions": installs}, default=str)) + LOGGER.print_raw(json.dumps({"versions": installs}, default=str)) def format_json_lines(cmd, installs): for i in installs: - print(json.dumps(i, default=str)) + LOGGER.print_raw(json.dumps(i, default=str)) def format_bare_id(cmd, installs): @@ -180,18 +180,18 @@ def format_bare_id(cmd, installs): # Don't print useless values (__active-virtual-env, __unmanaged-) if i["id"].startswith("__"): continue - print(i["id"]) + LOGGER.print_raw(i["id"]) def format_bare_exe(cmd, installs): for i in installs: - print(i["executable"]) + LOGGER.print_raw(i["executable"]) def format_bare_prefix(cmd, installs): for i in installs: try: - print(i["prefix"]) + LOGGER.print_raw(i["prefix"]) except KeyError: pass @@ -199,7 +199,7 @@ def format_bare_prefix(cmd, installs): def format_bare_url(cmd, installs): for i in installs: try: - print(i["url"]) + LOGGER.print_raw(i["url"]) except KeyError: pass @@ -223,7 +223,7 @@ def format_legacy(cmd, installs, paths=False): if not seen_default and i.get("default"): tag = f"{tag} *" seen_default = True - print(tag.ljust(17), i["executable"] if paths else i["display-name"]) + LOGGER.print_raw(tag.ljust(17), i["executable"] if paths else i["display-name"]) FORMATTERS = { diff --git a/src/manage/logging.py b/src/manage/logging.py index 3a51641..82af795 100644 --- a/src/manage/logging.py +++ b/src/manage/logging.py @@ -72,15 +72,19 @@ def supports_colour(stream): class Logger: - def __init__(self): - if os.getenv("PYMANAGER_DEBUG"): + def __init__(self, level=None, console=sys.stderr, print_console=sys.stdout): + if level is not None: + self.level = level + elif os.getenv("PYMANAGER_DEBUG"): self.level = DEBUG elif os.getenv("PYMANAGER_VERBOSE"): self.level = VERBOSE else: self.level = INFO - self.console = sys.stderr + self.console = console self.console_colour = supports_colour(self.console) + self.print_console = print_console + self.print_console_colour = supports_colour(self.print_console) self.file = None self._list = None @@ -158,13 +162,19 @@ def would_print(self, *args, always=False, level=INFO, **kwargs): return False return True - def print(self, msg=None, *args, always=False, level=INFO, **kwargs): + def print(self, msg=None, *args, always=False, level=INFO, colours=True, **kwargs): if self._list is not None: - self._list.append(((msg or "") % args, ())) + if args: + self._list.append(((msg or "") % args, ())) + else: + self._list.append((msg or "", ())) if not always and level < self.level: return if msg: - if self.console_colour: + if not colours: + # Don't unescape or replace anything + pass + elif self.print_console_colour: for k in COLOURS: msg = msg.replace(k, COLOURS[k]) else: @@ -176,7 +186,14 @@ def print(self, msg=None, *args, always=False, level=INFO, **kwargs): msg = str(args[0]) else: msg = "" - print(msg, **kwargs, file=self.console) + print(msg, **kwargs, file=self.print_console) + + def print_raw(self, *msg, **kwargs): + kwargs.pop("always", None) + kwargs.pop("level", None) + kwargs.pop("colours", None) + return self.print(kwargs.pop("sep", " ").join(str(s) for s in msg), + always=True, colours=False, **kwargs) LOGGER = Logger() diff --git a/tests/conftest.py b/tests/conftest.py index b8cee5e..68fc24c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,7 +6,7 @@ import sys import winreg -from pathlib import Path +from pathlib import Path, PurePath TESTS = Path(__file__).absolute().parent @@ -18,41 +18,83 @@ setattr(_native, k, getattr(_native_test, k)) +# Importing in order carefully to ensure the variables we override are handled +# correctly by submodules. import manage manage.EXE_NAME = "pymanager-pytest" - import manage.commands manage.commands.WELCOME = "" - -from manage.logging import LOGGER, DEBUG +from manage.logging import LOGGER, DEBUG, ERROR LOGGER.level = DEBUG +import manage.config +import manage.installs + + +# Ensure we don't pick up any settings from configs or the registry + +def _mock_load_global_config(cfg, schema): + cfg.update({ + "base_config": "", + "user_config": "", + "additional_config": "", + }) + +def _mock_load_registry_config(key, schema): + return {} + +manage.config.load_global_config = _mock_load_global_config +manage.config.load_registry_config = _mock_load_registry_config + + +@pytest.fixture +def quiet_log(): + lvl = LOGGER.level + LOGGER.level = ERROR + try: + yield + finally: + LOGGER.level = lvl + + class LogCaptureHandler(list): def skip_until(self, pattern, args=()): return ('until', pattern, args) + def not_logged(self, pattern, args=()): + return ('not', pattern, args) + def __call__(self, *cmp): - it1 = iter(self) + i = 0 for y in cmp: if not isinstance(y, tuple): - op, pat, args = None, y, [] + op, pat, args = None, y, None elif len(y) == 3: op, pat, args = y elif len(y) == 2: op = None pat, args = y + if op == 'not': + for j in range(i, len(self)): + if re.match(pat, self[j][0], flags=re.S): + pytest.fail(f"Should not have found {self[j][0]!r} matching {pat}") + return + continue + while True: try: - x = next(it1) - except StopIteration: + x = self[i] + i += 1 + except IndexError: pytest.fail(f"Not enough elements were logged looking for {pat}") - if op == 'until' and not re.match(pat, x[0]): + if op == 'until' and not re.match(pat, x[0], flags=re.S): continue - assert re.match(pat, x[0]) - assert tuple(x[1]) == tuple(args) + assert re.match(pat, x[0], flags=re.S) + if args is not None: + assert tuple(x[1]) == tuple(args) break @@ -150,3 +192,67 @@ def setup(self, _subkey=None, **keys): def registry(): with RegistryFixture(winreg.HKEY_CURRENT_USER, REG_TEST_ROOT) as key: yield key + + + +def make_install(tag, **kwargs): + run_for = [] + for t in kwargs.get("run_for", [tag]): + run_for.append({"tag": t, "target": kwargs.get("target", "python.exe")}) + run_for.append({"tag": t, "target": kwargs.get("targetw", "pythonw.exe"), "windowed": 1}) + + return { + "company": kwargs.get("company", "PythonCore"), + "id": "{}-{}".format(kwargs.get("company", "PythonCore"), tag), + "sort-version": kwargs.get("sort_version", tag), + "display-name": "{} {}".format(kwargs.get("company", "Python"), tag), + "tag": tag, + "install-for": [tag], + "run-for": run_for, + "prefix": PurePath(kwargs.get("prefix", rf"C:\{tag}")), + "executable": kwargs.get("executable", "python.exe"), + } + + +def fake_get_installs(install_dir): + yield make_install("1.0") + yield make_install("1.0-32", sort_version="1.0") + yield make_install("1.0-64", sort_version="1.0") + yield make_install("2.0-64", sort_version="2.0") + yield make_install("2.0-arm64", sort_version="2.0") + yield make_install("3.0a1-32", sort_version="3.0a1") + yield make_install("3.0a1-64", sort_version="3.0a1") + yield make_install("1.1", company="Company", target="company.exe", targetw="companyw.exe") + yield make_install("1.1-64", sort_version="1.1", company="Company", target="company.exe", targetw="companyw.exe") + yield make_install("1.1-arm64", sort_version="1.1", company="Company", target="company.exe", targetw="companyw.exe") + yield make_install("2.1", sort_version="2.1", company="Company", target="company.exe", targetw="companyw.exe") + yield make_install("2.1-64", sort_version="2.1", company="Company", target="company.exe", targetw="companyw.exe") + + +def fake_get_installs2(install_dir): + yield make_install("1.0-32", sort_version="1.0") + yield make_install("3.0a1-32", sort_version="3.0a1", run_for=["3.0.1a1-32", "3.0-32", "3-32"]) + yield make_install("3.0a1-64", sort_version="3.0a1", run_for=["3.0.1a1-64", "3.0-64", "3-64"]) + yield make_install("3.0a1-arm64", sort_version="3.0a1", run_for=["3.0.1a1-arm64", "3.0-arm64", "3-arm64"]) + + +def fake_get_unmanaged_installs(): + return [] + + +def fake_get_venv_install(virtualenv): + raise LookupError + + +@pytest.fixture +def patched_installs(monkeypatch): + monkeypatch.setattr(manage.installs, "_get_installs", fake_get_installs) + monkeypatch.setattr(manage.installs, "_get_unmanaged_installs", fake_get_unmanaged_installs) + monkeypatch.setattr(manage.installs, "_get_venv_install", fake_get_venv_install) + + +@pytest.fixture +def patched_installs2(monkeypatch): + monkeypatch.setattr(manage.installs, "_get_installs", fake_get_installs2) + monkeypatch.setattr(manage.installs, "_get_unmanaged_installs", fake_get_unmanaged_installs) + monkeypatch.setattr(manage.installs, "_get_venv_install", fake_get_venv_install) diff --git a/tests/test_commands.py b/tests/test_commands.py index 796b503..e498aed 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -1,7 +1,7 @@ import pytest import secrets -from manage import commands -from manage.exceptions import NoInstallsError +from manage import commands, logging +from manage.exceptions import ArgumentError, NoInstallsError def test_pymanager_help_command(assert_log): @@ -55,3 +55,29 @@ def test_exec_with_literal_default(): except NoInstallsError: # This is also an okay result, if the default runtime isn't installed pass + + +def test_legacy_list_command(assert_log, patched_installs): + cmd = commands.ListLegacyCommand(["--list"]) + cmd.show_welcome() + cmd.execute() + assert_log( + # Ensure welcome message is suppressed + assert_log.not_logged(r"Python installation manager.+"), + # Should have a range of values shown, we don't care too much + assert_log.skip_until(r" -V:2\.0\[-64\]\s+Python.*"), + assert_log.skip_until(r" -V:3\.0a1-32\s+Python.*"), + ) + + +def test_legacy_list_command_args(): + with pytest.raises(ArgumentError): + commands.ListLegacyCommand(["--list", "--help"]) + + +def test_legacy_listpaths_command_help(assert_log, patched_installs): + cmd = commands.ListPathsLegacyCommand(["--list-paths"]) + cmd.help() + assert_log( + assert_log.skip_until(r".*List command.+py list.+"), + ) diff --git a/tests/test_installs.py b/tests/test_installs.py index bfa4bca..4a607ef 100644 --- a/tests/test_installs.py +++ b/tests/test_installs.py @@ -5,68 +5,6 @@ from manage import installs -def make_install(tag, **kwargs): - run_for = [] - for t in kwargs.get("run_for", [tag]): - run_for.append({"tag": t, "target": kwargs.get("target", "python.exe")}) - run_for.append({"tag": t, "target": kwargs.get("targetw", "pythonw.exe"), "windowed": 1}) - - return { - "company": kwargs.get("company", "PythonCore"), - "id": "{}-{}".format(kwargs.get("company", "PythonCore"), tag), - "sort-version": kwargs.get("sort_version", tag), - "tag": tag, - "install-for": [tag], - "run-for": run_for, - "prefix": PurePath(kwargs.get("prefix", rf"C:\{tag}")), - "executable": kwargs.get("executable", "python.exe"), - } - - -def fake_get_installs(install_dir): - yield make_install("1.0") - yield make_install("1.0-32", sort_version="1.0") - yield make_install("1.0-64", sort_version="1.0") - yield make_install("2.0-64", sort_version="2.0") - yield make_install("2.0-arm64", sort_version="2.0") - yield make_install("3.0a1-32", sort_version="3.0a1") - yield make_install("3.0a1-64", sort_version="3.0a1") - yield make_install("1.1", company="Company", target="company.exe", targetw="companyw.exe") - yield make_install("1.1-64", sort_version="1.1", company="Company", target="company.exe", targetw="companyw.exe") - yield make_install("1.1-arm64", sort_version="1.1", company="Company", target="company.exe", targetw="companyw.exe") - yield make_install("2.1", sort_version="2.1", company="Company", target="company.exe", targetw="companyw.exe") - yield make_install("2.1-64", sort_version="2.1", company="Company", target="company.exe", targetw="companyw.exe") - - -def fake_get_installs2(install_dir): - yield make_install("1.0-32", sort_version="1.0") - yield make_install("3.0a1-32", sort_version="3.0a1", run_for=["3.0.1a1-32", "3.0-32", "3-32"]) - yield make_install("3.0a1-64", sort_version="3.0a1", run_for=["3.0.1a1-64", "3.0-64", "3-64"]) - yield make_install("3.0a1-arm64", sort_version="3.0a1", run_for=["3.0.1a1-arm64", "3.0-arm64", "3-arm64"]) - - -def fake_get_unmanaged_installs(): - return [] - - -def fake_get_venv_install(virtualenv): - raise LookupError - - -@pytest.fixture -def patched_installs(monkeypatch): - monkeypatch.setattr(installs, "_get_installs", fake_get_installs) - monkeypatch.setattr(installs, "_get_unmanaged_installs", fake_get_unmanaged_installs) - monkeypatch.setattr(installs, "_get_venv_install", fake_get_venv_install) - - -@pytest.fixture -def patched_installs2(monkeypatch): - monkeypatch.setattr(installs, "_get_installs", fake_get_installs2) - monkeypatch.setattr(installs, "_get_unmanaged_installs", fake_get_unmanaged_installs) - monkeypatch.setattr(installs, "_get_venv_install", fake_get_venv_install) - - def test_get_installs_in_order(patched_installs): ii = installs.get_installs("") assert [i["id"] for i in ii] == [ From 8df401b591553331106880387e5abb6319561fc6 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 8 May 2025 19:39:15 +0100 Subject: [PATCH 2/3] Handle UnicodeEncodeErrors when printing emotes --- src/manage/logging.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/manage/logging.py b/src/manage/logging.py index 82af795..7cfb51d 100644 --- a/src/manage/logging.py +++ b/src/manage/logging.py @@ -218,7 +218,10 @@ def __exit__(self, *exc_info): if self._complete: LOGGER.print() else: - LOGGER.print("❌") + try: + LOGGER.print("❌") + except UnicodeEncodeError: + LOGGER.print("x") def __call__(self, progress): if self._complete: @@ -227,7 +230,10 @@ def __call__(self, progress): if progress is None: if self._need_newline: if not self._complete: - LOGGER.print("⏸️") + try: + LOGGER.print("⏸️") + except UnicodeEncodeError: + LOGGER.print("|") self._dots_shown = 0 self._started = False self._need_newline = False @@ -246,6 +252,9 @@ def __call__(self, progress): LOGGER.print(None, "." * dot_count, end="", flush=True) self._need_newline = True if progress >= 100: - LOGGER.print("✅", flush=True) + try: + LOGGER.print("✅", flush=True) + except UnicodeEncodeError: + LOGGER.print(".", flush=True) self._complete = True self._need_newline = False From b40acce2636bda98d7e76e9e998681a4ced263f2 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Thu, 8 May 2025 19:40:59 +0100 Subject: [PATCH 3/3] Shorten code --- src/manage/logging.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/manage/logging.py b/src/manage/logging.py index 7cfb51d..d7aca56 100644 --- a/src/manage/logging.py +++ b/src/manage/logging.py @@ -189,11 +189,10 @@ def print(self, msg=None, *args, always=False, level=INFO, colours=True, **kwarg print(msg, **kwargs, file=self.print_console) def print_raw(self, *msg, **kwargs): - kwargs.pop("always", None) - kwargs.pop("level", None) - kwargs.pop("colours", None) - return self.print(kwargs.pop("sep", " ").join(str(s) for s in msg), - always=True, colours=False, **kwargs) + kwargs["always"] = True + kwargs["colours"] = False + sep = kwargs.pop("sep", " ") + return self.print(sep.join(str(s) for s in msg), **kwargs) LOGGER = Logger()