Improve test coverage

Author: zooba

scripts/test-firstrun.py

@@ -0,0 +1,25 @@
+"""Simple script to allow running manage/firstrun.py without rebuilding.
+
+You'll need to build the test module (_msbuild_test.py).
+"""
+
+import os
+import pathlib
+import sys
+
+
+ROOT = pathlib.Path(__file__).absolute().parent.parent / "src"
+sys.path.append(str(ROOT))
+
+
+import _native
+if not hasattr(_native, "coinitialize"):
+    import _native_test
+    for k in dir(_native_test):
+        if k[:1] not in ("", "_"):
+            setattr(_native, k, getattr(_native_test, k))
+
+
+import manage.commands
+cmd = manage.commands.FirstRun([], ROOT)
+sys.exit(cmd.execute() or 0)

src/manage/firstrun.py

@@ -2,19 +2,6 @@
 import sys
 import time
 
-
-if __name__ == "__main__":
-    __package__ = "manage"
-    sys.path.append(os.path.dirname(os.path.dirname(__file__)))
-
-    import _native
-    if not hasattr(_native, "coinitialize"):
-        import _native_test
-        for k in dir(_native_test):
-            if k[:1] not in ("", "_"):
-                setattr(_native, k, getattr(_native_test, k))
-
-
 from . import logging
 from .pathutils import Path
 
@@ -160,7 +147,7 @@ def do_global_dir_on_path(cmd):
             initial, kind = winreg.QueryValueEx(key, "Path")
         LOGGER.debug("Initial path: %s", initial)
         if kind not in (winreg.REG_SZ, winreg.REG_EXPAND_SZ) or not isinstance(initial, str):
-            LOGGER.debug("Value kind is %s and not REG_[EXPAND_]SZ. Aborting.")
+            LOGGER.debug("Value kind is %s and not REG_[EXPAND_]SZ. Aborting.", kind)
             return
         for p in initial.split(";"):
             if not p:
@@ -390,49 +377,3 @@ def first_run(cmd):
                      "manager!W! from your Start menu, or !B!py install "
                      "--configure!W! from the terminal.", wrap=True)
         line_break()
-
-
-if __name__ == "__main__":
-    class TestCommand:
-        enabled = True
-        global_dir = Path(os.path.expandvars(r"%LocalAppData%\Python\bin"))
-        explicit = False
-        confirm = True
-        check_app_alias = True
-        check_long_paths = True
-        check_py_on_path = True
-        check_any_install = True
-        check_global_dir = True
-        check_default_tag = True
-
-        def get_installs(self, *args, **kwargs):
-            import json
-            root = Path(os.path.expandvars(r"%LocalAppData%\Python"))
-            result = []
-            for d in root.iterdir():
-                inst = d / "__install__.json"
-                try:
-                    result.append(json.loads(inst.read_text()))
-                except FileNotFoundError:
-                    pass
-            return result
-
-        def _ask(self, fmt, *args, yn_text="Y/n", expect_char="y"):
-            if not self.confirm:
-                return True
-            LOGGER.print(f"{fmt} [{yn_text}] ", *args, end="")
-            try:
-                resp = input().casefold()
-            except Exception:
-                return False
-            return not resp or resp.startswith(expect_char.casefold())
-
-        def ask_yn(self, fmt, *args):
-            "Returns True if the user selects 'yes' or confirmations are skipped."
-            return self._ask(fmt, *args)
-
-        def ask_ny(self, fmt, *args):
-            "Returns True if the user selects 'no' or confirmations are skipped."
-            return self._ask(fmt, *args, yn_text="y/N", expect_char="n")
-
-    first_run(TestCommand())

tests/conftest.py

@@ -146,7 +146,8 @@ def localserver():
 
 
 class FakeConfig:
-    def __init__(self, installs=[]):
+    def __init__(self, global_dir, installs=[]):
+        self.global_dir = global_dir
         self.installs = list(installs)
         self.shebang_can_run_anything = True
         self.shebang_can_run_anything_silently = False
@@ -161,8 +162,8 @@ def get_install_to_run(self, tag):
 
 
 @pytest.fixture
-def fake_config():
-    return FakeConfig()
+def fake_config(tmp_path):
+    return FakeConfig(tmp_path / "bin")
 
 
 REG_TEST_ROOT = r"Software\Python\PyManagerTesting"

tests/test_firstrun.py

@@ -1,5 +1,6 @@
 import os
 import pytest
+import winreg
 
 from pathlib import Path
 
@@ -105,3 +106,134 @@ def test_welcome(assert_log):
     assert_log(".*Welcome.*", assert_log.end_of_log())
     welcome()
     assert_log(".*Welcome.*", assert_log.end_of_log())
+
+
+
+def test_firstrun_command(monkeypatch):
+    from manage import commands
+
+    called_first_run = False
+    called_show_usage = False
+
+    def fake_first_run(*args):
+        nonlocal called_first_run
+        called_first_run = True
+
+    def fake_show_usage(*args):
+        nonlocal called_show_usage
+        called_show_usage = True
+
+    monkeypatch.setattr(firstrun, "first_run", fake_first_run)
+    monkeypatch.setattr(commands.FirstRun, "confirm", False)
+    monkeypatch.setattr(commands.FirstRun, "show_usage", fake_show_usage)
+    cmd = commands.find_command(["**first_run"], None)
+    cmd.execute()
+    assert called_first_run
+    assert called_show_usage
+
+
+def test_install_configure_command(monkeypatch):
+    from manage import commands
+
+    called_first_run = False
+    called_show_usage = False
+
+    def fake_first_run(*args):
+        nonlocal called_first_run
+        called_first_run = True
+
+    def fake_show_usage(*args):
+        nonlocal called_show_usage
+        called_show_usage = True
+
+    monkeypatch.setattr(firstrun, "first_run", fake_first_run)
+    monkeypatch.setattr(commands.FirstRun, "confirm", False)
+    monkeypatch.setattr(commands.FirstRun, "show_usage", fake_show_usage)
+    cmd = commands.find_command(["install", "--configure"], None)
+    cmd.execute()
+    assert called_first_run
+    assert not called_show_usage
+
+
+def _create_key_read_only(key, subkey, *args, **kwargs):
+    return winreg.OpenKeyEx(key, subkey)
+
+
+def _raise_oserror(*args, **kwargs):
+    raise OSError("injected error")
+
+
+@pytest.fixture
+def protect_reg(monkeypatch):
+    import _native
+    monkeypatch.setattr(winreg, "CreateKeyEx", _create_key_read_only)
+    monkeypatch.setattr(winreg, "SetValueEx", _raise_oserror)
+    monkeypatch.setattr(_native, "broadcast_settings_change", lambda *a: None)
+
+
+def test_do_global_dir_open_fail(protect_reg, fake_config, assert_log, monkeypatch):
+    monkeypatch.setattr(winreg, "OpenKeyEx", _raise_oserror)
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(assert_log.skip_until("Failed to update PATH.+"))
+
+
+def test_do_global_dir_read_fail(protect_reg, fake_config, assert_log, monkeypatch):
+    monkeypatch.setattr(winreg, "QueryValueEx", _raise_oserror)
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(assert_log.skip_until("Failed to update PATH.+"))
+
+
+def test_do_global_dir_read_kind_fail(protect_reg, fake_config, assert_log, monkeypatch):
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (100, winreg.REG_DWORD))
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(
+        assert_log.skip_until("Initial path: %s", (100,)),
+        ("Value kind is %s.+", (winreg.REG_DWORD,)),
+    )
+
+
+def test_do_global_dir_path_already_set(protect_reg, fake_config, assert_log, monkeypatch):
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (f"{fake_config.global_dir};b;c", winreg.REG_SZ))
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(assert_log.skip_until("Path is already found"))
+
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (f"a;{fake_config.global_dir};c", winreg.REG_SZ))
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(assert_log.skip_until("Path is already found"))
+
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (f"a;b;{fake_config.global_dir}", winreg.REG_SZ))
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(assert_log.skip_until("Path is already found"))
+
+
+def test_do_global_dir_path_lost_race(protect_reg, fake_config, assert_log, monkeypatch):
+    paths = ["a;b", "a;b;c"]
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: (paths.pop(), winreg.REG_SZ))
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(
+        assert_log.skip_until("New path: %s", None),
+        "Path is added successfully",
+        "PATH has changed.+",
+    )
+
+
+def test_do_global_dir_write_same_kind(protect_reg, fake_config, monkeypatch):
+    saved = []
+    monkeypatch.setattr(winreg, "SetValueEx", lambda *a: saved.append(a))
+
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: ("a;", winreg.REG_SZ))
+    firstrun.do_global_dir_on_path(fake_config)
+    assert saved[-1][1:] == ("Path", 0, winreg.REG_SZ, f"a;{fake_config.global_dir}")
+
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: ("a", winreg.REG_EXPAND_SZ))
+    firstrun.do_global_dir_on_path(fake_config)
+    assert saved[-1][1:] == ("Path", 0, winreg.REG_EXPAND_SZ, f"a;{fake_config.global_dir}")
+
+
+def test_do_global_dir_path_fail_broadcast(protect_reg, fake_config, assert_log, monkeypatch):
+    import _native
+    monkeypatch.setattr(_native, "broadcast_settings_change", _raise_oserror)
+    monkeypatch.setattr(winreg, "QueryValueEx", lambda *a: ("a;", winreg.REG_SZ))
+    monkeypatch.setattr(winreg, "SetValueEx", lambda *a: None)
+    firstrun.do_global_dir_on_path(fake_config)
+    assert_log(assert_log.skip_until("Failed to notify of PATH environment.+"))