style: modernise typing and clean up lint findings
Repository-wide mechanical cleanup so `ruff check src tests` has a chance of passing; no behavioural changes. - Add `from __future__ import annotations` where PEP 604 unions are used. This has to come first: pyproject declares requires-python >= 3.9, where `X | None` is not evaluable at runtime unless annotations are stringified. - Replace typing.List/Dict/Tuple/Set with the builtin generics and Optional[X] with X | None, then drop the imports that became unused. The four actions/*/__init__.py files needed this by hand because ruff leaves unused imports in __init__.py alone (possible re-exports). - Strip shebangs from 72 importable modules. None of them are executable or invoked directly; the entry points are console_scripts and runpy. - Flatten nested `with` blocks, collapse needless-bool returns, and apply the remaining mechanical ruff fixes (PIE810, FLY002, PERF102, FURB192, RUF059, I001). - Pass check=False explicitly to the four subprocess.run() calls that inspect returncode themselves. That is the existing default. Two rewrites are visible to mocks, so their tests move with them: subprocess.run(stdout=PIPE, stderr=PIPE) became capture_output=True, and open(path, "r", ...) lost the redundant mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,3 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
"""
|
||||
Integration tests for the `pkgmgr branch` CLI wiring.
|
||||
|
||||
|
||||
@@ -67,51 +67,53 @@ class ConfigDefaultsIntegrationTest(unittest.TestCase):
|
||||
# Provide fake pkgmgr module so your functions resolve pkg_root correctly
|
||||
fake_pkgmgr = types.SimpleNamespace(__file__=str(pkg_root / "__init__.py"))
|
||||
|
||||
with patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}):
|
||||
with patch.dict(os.environ, {"HOME": str(home)}):
|
||||
# A) load_config should fall back to <pkg_root>/config/defaults.yaml
|
||||
merged = load_config(user_config_path)
|
||||
with (
|
||||
patch.dict(sys.modules, {"pkgmgr": fake_pkgmgr}),
|
||||
patch.dict(os.environ, {"HOME": str(home)}),
|
||||
):
|
||||
# A) load_config should fall back to <pkg_root>/config/defaults.yaml
|
||||
merged = load_config(user_config_path)
|
||||
|
||||
self.assertEqual(
|
||||
merged["directories"]["repositories"], "/opt/Repositories"
|
||||
)
|
||||
self.assertEqual(
|
||||
merged["directories"]["binaries"], "/usr/local/bin"
|
||||
)
|
||||
self.assertEqual(
|
||||
merged["directories"]["repositories"], "/opt/Repositories"
|
||||
)
|
||||
self.assertEqual(
|
||||
merged["directories"]["binaries"], "/usr/local/bin"
|
||||
)
|
||||
|
||||
# user-only key must still exist (user config merges over defaults)
|
||||
self.assertEqual(merged["directories"]["user_only"], "/home/user")
|
||||
# user-only key must still exist (user config merges over defaults)
|
||||
self.assertEqual(merged["directories"]["user_only"], "/home/user")
|
||||
|
||||
self.assertIn("repositories", merged)
|
||||
self.assertTrue(
|
||||
any(
|
||||
r.get("provider") == "github"
|
||||
and r.get("account") == "acme"
|
||||
and r.get("repository") == "demo"
|
||||
for r in merged["repositories"]
|
||||
)
|
||||
self.assertIn("repositories", merged)
|
||||
self.assertTrue(
|
||||
any(
|
||||
r.get("provider") == "github"
|
||||
and r.get("account") == "acme"
|
||||
and r.get("repository") == "demo"
|
||||
for r in merged["repositories"]
|
||||
)
|
||||
)
|
||||
|
||||
# B) update_default_configs should copy defaults.yaml to ~/.config/pkgmgr/
|
||||
before_config_yaml = (user_cfg_dir / "config.yaml").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
# B) update_default_configs should copy defaults.yaml to ~/.config/pkgmgr/
|
||||
before_config_yaml = (user_cfg_dir / "config.yaml").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
|
||||
config_cmd._update_default_configs(user_config_path)
|
||||
config_cmd._update_default_configs(user_config_path)
|
||||
|
||||
self.assertTrue((user_cfg_dir / "defaults.yaml").is_file())
|
||||
copied_defaults = yaml.safe_load(
|
||||
(user_cfg_dir / "defaults.yaml").read_text(encoding="utf-8")
|
||||
)
|
||||
self.assertEqual(
|
||||
copied_defaults["directories"]["repositories"],
|
||||
"/opt/Repositories",
|
||||
)
|
||||
self.assertTrue((user_cfg_dir / "defaults.yaml").is_file())
|
||||
copied_defaults = yaml.safe_load(
|
||||
(user_cfg_dir / "defaults.yaml").read_text(encoding="utf-8")
|
||||
)
|
||||
self.assertEqual(
|
||||
copied_defaults["directories"]["repositories"],
|
||||
"/opt/Repositories",
|
||||
)
|
||||
|
||||
after_config_yaml = (user_cfg_dir / "config.yaml").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
self.assertEqual(after_config_yaml, before_config_yaml)
|
||||
after_config_yaml = (user_cfg_dir / "config.yaml").read_text(
|
||||
encoding="utf-8"
|
||||
)
|
||||
self.assertEqual(after_config_yaml, before_config_yaml)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
import os
|
||||
import tempfile
|
||||
import unittest
|
||||
@@ -134,9 +132,7 @@ class TestInstallReposIntegration(unittest.TestCase):
|
||||
Make _ensure_repo_dir() believe that the repo directories
|
||||
already exist so that it does not attempt cloning.
|
||||
"""
|
||||
if path in (repo_system_dir, repo_nix_dir):
|
||||
return True
|
||||
return False
|
||||
return path in (repo_system_dir, repo_nix_dir)
|
||||
|
||||
mock_resolve.side_effect = fake_resolve
|
||||
mock_exists_install.side_effect = fake_exists_install
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
"""
|
||||
CLI integration tests for `pkgmgr mirror`.
|
||||
|
||||
@@ -21,7 +19,6 @@ import runpy
|
||||
import sys
|
||||
import unittest
|
||||
from contextlib import ExitStack, redirect_stderr, redirect_stdout
|
||||
from typing import Dict, List, Optional
|
||||
from unittest.mock import MagicMock, PropertyMock, patch
|
||||
|
||||
|
||||
@@ -29,7 +26,7 @@ class TestIntegrationMirrorCommands(unittest.TestCase):
|
||||
"""Integration tests for `pkgmgr mirror` commands."""
|
||||
|
||||
def _run_pkgmgr(
|
||||
self, args: List[str], extra_env: Optional[Dict[str, str]] = None
|
||||
self, args: list[str], extra_env: dict[str, str] | None = None
|
||||
) -> str:
|
||||
"""Execute pkgmgr with the given arguments and return captured output."""
|
||||
original_argv = list(sys.argv)
|
||||
@@ -151,8 +148,7 @@ class TestIntegrationMirrorCommands(unittest.TestCase):
|
||||
code = exc.code if isinstance(exc.code, int) else None
|
||||
if code not in (0, None):
|
||||
raise AssertionError(
|
||||
"%r failed with exit code %r.\n\nOutput:\n%s"
|
||||
% (cmd_repr, exc.code, buffer.getvalue())
|
||||
f"{cmd_repr!r} failed with exit code {exc.code!r}.\n\nOutput:\n{buffer.getvalue()}"
|
||||
)
|
||||
|
||||
return buffer.getvalue()
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
"""
|
||||
Integration test for mirror probing + provisioning after refactor.
|
||||
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
"""
|
||||
Integration tests for recursive capability resolution and installer shadowing.
|
||||
|
||||
@@ -17,7 +15,6 @@ import shutil
|
||||
import tempfile
|
||||
import unittest
|
||||
from collections.abc import Sequence
|
||||
from typing import List, Tuple
|
||||
from unittest.mock import patch
|
||||
|
||||
import pkgmgr.actions.install as install_mod
|
||||
@@ -29,7 +26,7 @@ from pkgmgr.actions.install.installers.os_packages.arch_pkgbuild import (
|
||||
)
|
||||
from pkgmgr.actions.install.installers.python import PythonInstaller
|
||||
|
||||
InstallerSpec = Tuple[str, object]
|
||||
InstallerSpec = tuple[str, object]
|
||||
|
||||
|
||||
class TestRecursiveCapabilitiesIntegration(unittest.TestCase):
|
||||
@@ -54,7 +51,7 @@ class TestRecursiveCapabilitiesIntegration(unittest.TestCase):
|
||||
repo_dir: str,
|
||||
installers: Sequence[InstallerSpec],
|
||||
selected_repos=None,
|
||||
) -> List[str]:
|
||||
) -> list[str]:
|
||||
"""
|
||||
Run install_repos() with a custom INSTALLERS list and capture which
|
||||
installer labels actually run.
|
||||
@@ -69,7 +66,7 @@ class TestRecursiveCapabilitiesIntegration(unittest.TestCase):
|
||||
else:
|
||||
all_repos = selected_repos
|
||||
|
||||
called_installers: List[str] = []
|
||||
called_installers: list[str] = []
|
||||
|
||||
patched_installers = []
|
||||
for label, inst in installers:
|
||||
|
||||
@@ -21,62 +21,46 @@ class TestTokenResolverIntegration(unittest.TestCase):
|
||||
|
||||
resolver = TokenResolver()
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# 1) ENV: empty
|
||||
# ------------------------------------------------------------------
|
||||
with patch.dict("os.environ", {}, clear=True):
|
||||
# ------------------------------------------------------------------
|
||||
# 2) GH CLI is available
|
||||
# ------------------------------------------------------------------
|
||||
with patch(
|
||||
def validate_side_effect(
|
||||
provider_kind: str,
|
||||
host: str,
|
||||
token: str,
|
||||
) -> bool:
|
||||
return False # gh + keyring invalid
|
||||
|
||||
with (
|
||||
patch.dict("os.environ", {}, clear=True),
|
||||
patch(
|
||||
"pkgmgr.core.credentials.providers.gh.shutil.which",
|
||||
return_value="/usr/bin/gh",
|
||||
):
|
||||
with patch(
|
||||
"pkgmgr.core.credentials.providers.gh.subprocess.check_output",
|
||||
return_value="gh-invalid-token\n",
|
||||
):
|
||||
# ------------------------------------------------------------------
|
||||
# 3) Keyring returns an existing (invalid) token
|
||||
# ------------------------------------------------------------------
|
||||
with patch(
|
||||
"pkgmgr.core.credentials.providers.keyring._import_keyring"
|
||||
) as mock_import_keyring:
|
||||
mock_keyring = mock_import_keyring.return_value
|
||||
mock_keyring.get_password.return_value = "keyring-invalid-token"
|
||||
),
|
||||
patch(
|
||||
"pkgmgr.core.credentials.providers.gh.subprocess.check_output",
|
||||
return_value="gh-invalid-token\n",
|
||||
),
|
||||
patch(
|
||||
"pkgmgr.core.credentials.providers.keyring._import_keyring"
|
||||
) as mock_import_keyring,
|
||||
patch(
|
||||
"pkgmgr.core.credentials.providers.prompt.sys.stdin.isatty",
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"pkgmgr.core.credentials.providers.prompt.getpass",
|
||||
return_value="new-valid-token",
|
||||
),
|
||||
patch(
|
||||
"pkgmgr.core.credentials.resolver.validate_token",
|
||||
side_effect=validate_side_effect,
|
||||
) as validate_mock,
|
||||
):
|
||||
mock_keyring = mock_import_keyring.return_value
|
||||
mock_keyring.get_password.return_value = "keyring-invalid-token"
|
||||
|
||||
# ------------------------------------------------------------------
|
||||
# 4) Prompt is allowed and returns a NEW token
|
||||
# ------------------------------------------------------------------
|
||||
with patch(
|
||||
"pkgmgr.core.credentials.providers.prompt.sys.stdin.isatty",
|
||||
return_value=True,
|
||||
):
|
||||
with patch(
|
||||
"pkgmgr.core.credentials.providers.prompt.getpass",
|
||||
return_value="new-valid-token",
|
||||
):
|
||||
# ------------------------------------------------------------------
|
||||
# 5) Validation logic:
|
||||
# - gh token invalid
|
||||
# - keyring token invalid
|
||||
# - prompt token is NOT validated (by design)
|
||||
# ------------------------------------------------------------------
|
||||
def validate_side_effect(
|
||||
provider_kind: str,
|
||||
host: str,
|
||||
token: str,
|
||||
) -> bool:
|
||||
return False # gh + keyring invalid
|
||||
|
||||
with patch(
|
||||
"pkgmgr.core.credentials.resolver.validate_token",
|
||||
side_effect=validate_side_effect,
|
||||
) as validate_mock:
|
||||
result = resolver.get_token(
|
||||
provider_kind="github",
|
||||
host="github.com",
|
||||
)
|
||||
result = resolver.get_token(
|
||||
provider_kind="github",
|
||||
host="github.com",
|
||||
)
|
||||
|
||||
# ----------------------------------------------------------------------
|
||||
# Assertions
|
||||
@@ -93,7 +77,7 @@ class TestTokenResolverIntegration(unittest.TestCase):
|
||||
|
||||
# Keyring must be overwritten with the new token
|
||||
mock_keyring.set_password.assert_called_once()
|
||||
service, username, stored_token = mock_keyring.set_password.call_args.args
|
||||
_service, _username, stored_token = mock_keyring.set_password.call_args.args
|
||||
self.assertEqual(stored_token, "new-valid-token")
|
||||
|
||||
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
#!/usr/bin/env python3
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import unittest
|
||||
|
||||
@@ -7,14 +7,14 @@ import tempfile
|
||||
import types
|
||||
import unittest
|
||||
from contextlib import redirect_stdout
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
from pkgmgr.actions.mirror.setup_cmd import setup_mirrors
|
||||
from pkgmgr.actions.mirror.visibility_cmd import set_mirror_visibility
|
||||
from pkgmgr.core.remote_provisioning.types import RepoSpec
|
||||
|
||||
Repository = Dict[str, Any]
|
||||
Repository = dict[str, Any]
|
||||
|
||||
|
||||
class _FakeRegistry:
|
||||
@@ -45,13 +45,13 @@ class FakeProvider:
|
||||
|
||||
def __init__(self) -> None:
|
||||
# maps (host, owner, name) -> private(bool)
|
||||
self.privacy: Dict[Tuple[str, str, str], bool] = {}
|
||||
self.calls: List[Tuple[str, Any]] = []
|
||||
self.privacy: dict[tuple[str, str, str], bool] = {}
|
||||
self.calls: list[tuple[str, Any]] = []
|
||||
|
||||
def can_handle(self, host: str) -> bool:
|
||||
return True
|
||||
|
||||
def _candidate_hosts(self, host: str) -> List[str]:
|
||||
def _candidate_hosts(self, host: str) -> list[str]:
|
||||
"""
|
||||
Be tolerant against host normalization differences:
|
||||
- may contain scheme (https://...)
|
||||
@@ -75,7 +75,7 @@ class FakeProvider:
|
||||
candidates.append(c.split(":", 1)[0])
|
||||
|
||||
# de-dup
|
||||
out: List[str] = []
|
||||
out: list[str] = []
|
||||
for c in candidates:
|
||||
if c not in out:
|
||||
out.append(c)
|
||||
@@ -94,7 +94,7 @@ class FakeProvider:
|
||||
self.privacy[(spec.host, spec.owner, spec.name)] = bool(spec.private)
|
||||
return types.SimpleNamespace(status="created", message="created", url=None)
|
||||
|
||||
def get_repo_private(self, token: str, spec: RepoSpec) -> Optional[bool]:
|
||||
def get_repo_private(self, token: str, spec: RepoSpec) -> bool | None:
|
||||
self.calls.append(("get_repo_private", (token, spec)))
|
||||
for h in self._candidate_hosts(spec.host):
|
||||
key = (h, spec.owner, spec.name)
|
||||
@@ -113,7 +113,7 @@ class FakeProvider:
|
||||
self.privacy[(spec.host, spec.owner, spec.name)] = bool(private)
|
||||
|
||||
|
||||
def _mk_ctx(*, identifier: str, repo_dir: str, mirrors: Dict[str, str]) -> Any:
|
||||
def _mk_ctx(*, identifier: str, repo_dir: str, mirrors: dict[str, str]) -> Any:
|
||||
return types.SimpleNamespace(
|
||||
identifier=identifier,
|
||||
repo_dir=repo_dir,
|
||||
|
||||
Reference in New Issue
Block a user