Improves help output and testability of commands.

#74

Author: zooba

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)

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)

src/manage/list_command.py

@@ -167,39 +167,39 @@ 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):
     for i in 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
 
 
 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 = {

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()

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)

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.+"),
+    )