mirror of
https://github.com/usestrix/strix.git
synced 2026-09-24 00:51:20 +00:00
test: remove Unix-only and developer-specific assumptions from the suite
A full run on Windows 11 failed 31 tests, almost all because a fixture assumed POSIX behaviour or read the developer's own Git configuration rather than because Strix misbehaved. That noise hides real regressions. - Resolve git with `shutil.which` instead of `/usr/bin/env git`, and skip the module when no git is present. This single line accounted for 19 of the 31 failures. - Isolate every git subprocess from `GIT_CONFIG_GLOBAL`/`GIT_CONFIG_SYSTEM` and supply a commit identity, so a global `core.excludesFile`, `commit.gpgSign`, or `core.hooksPath` cannot change what a test sees. - Create control-character filenames through a fixture that skips where the filesystem rejects them, instead of failing to build the fixture. - Choose the protected system directory for the platform in the mount policy test. `check_mountable_dir` already handles both families, so on Windows this now exercises the real policy instead of failing early. - Assert secret-file permissions through a fixture that keeps the POSIX 0o600 check exact and, on Windows, skips with the reason stated rather than weakening the assertion. The device-identity test is split so its identity contract still runs everywhere. - Pin mypy to `platform = "linux"` so type checking resolves the same APIs as CI and the container target. This removes the six `fcntl.flock`/`os.getuid`/`os.getgid` attribute errors reported on Windows without adding ignores that `warn_unused_ignores` would then flag on Linux. Windows now reports 12 failures, all owned elsewhere: #1258 (7-8, intermittent), #648/#652, #1288, and #1285. Refs #1259 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
95e085eb6c
commit
a3d60a72a0
8 changed files with 125 additions and 12 deletions
|
|
@ -104,6 +104,7 @@ path = "scripts/tui_sidecar_hook.py"
|
|||
|
||||
[tool.mypy]
|
||||
python_version = "3.12"
|
||||
platform = "linux"
|
||||
strict = true
|
||||
strict_optional = true
|
||||
warn_redundant_casts = true
|
||||
|
|
|
|||
|
|
@ -2,9 +2,17 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate_mcp_config(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path_factory: pytest.TempPathFactory
|
||||
|
|
@ -51,3 +59,82 @@ def _isolate_wallet_config(monkeypatch: pytest.MonkeyPatch) -> None:
|
|||
"""
|
||||
for name in ("MPPX_ACCOUNT", "MPPX_STRIPE_SECRET_KEY", "MPPX_STRIPE_PAYMENT_METHOD"):
|
||||
monkeypatch.delenv(name, raising=False)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _isolate_git_config(
|
||||
monkeypatch: pytest.MonkeyPatch, tmp_path_factory: pytest.TempPathFactory
|
||||
) -> None:
|
||||
"""Run every `git` subprocess against empty global and system config.
|
||||
|
||||
Several tests build a throwaway repository and assert on what Git reports
|
||||
about it. Git layers the developer's own configuration under that, so a
|
||||
global `core.excludesFile` silently removes files from the source
|
||||
selection tests, and `commit.gpgSign` or a `core.hooksPath` can fail the
|
||||
commit outright. Point both config layers at a path that does not exist
|
||||
and supply the identity the commits need, so the repository a test sees is
|
||||
the one it created.
|
||||
"""
|
||||
empty = tmp_path_factory.mktemp("git-isolation") / "absent.gitconfig"
|
||||
monkeypatch.setenv("GIT_CONFIG_GLOBAL", str(empty))
|
||||
monkeypatch.setenv("GIT_CONFIG_SYSTEM", str(empty))
|
||||
monkeypatch.setenv("GIT_CONFIG_NOSYSTEM", "1")
|
||||
monkeypatch.setenv("GIT_AUTHOR_NAME", "Strix Tests")
|
||||
monkeypatch.setenv("GIT_AUTHOR_EMAIL", "tests@example.com")
|
||||
monkeypatch.setenv("GIT_COMMITTER_NAME", "Strix Tests")
|
||||
monkeypatch.setenv("GIT_COMMITTER_EMAIL", "tests@example.com")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def write_control_character_file() -> Callable[[Path, str, str], Path]:
|
||||
"""Create a file whose *name* carries terminal control characters, or skip.
|
||||
|
||||
A few tests prove that Strix never echoes a filename back to the terminal
|
||||
with its escape sequences intact, so they need a hostile name on disk to
|
||||
have something to render. Windows rejects those characters in a filename
|
||||
outright, so the file cannot be created and the injection the test guards
|
||||
against is not reachable on that platform. Skip with the real `OSError`
|
||||
rather than assert, so the POSIX contract keeps being exercised where it
|
||||
applies and Windows does not report a product failure it does not have.
|
||||
"""
|
||||
|
||||
def _write(directory: Path, name: str, content: str = "{}") -> Path:
|
||||
path = directory / name
|
||||
try:
|
||||
path.write_text(content, encoding="utf-8")
|
||||
except OSError as exc: # pragma: no cover - platform dependent
|
||||
pytest.skip(f"this filesystem rejects control characters in filenames: {exc}")
|
||||
return path
|
||||
|
||||
return _write
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def assert_secret_file_permissions() -> Callable[[Path], None]:
|
||||
"""Assert a secret file is owner-only, or skip where that cannot hold.
|
||||
|
||||
`write_secret_text` opens secret files with `SECRET_FILE_MODE` (0o600).
|
||||
Windows does not implement POSIX mode bits: `os.open` maps the mode down to
|
||||
a single read-only flag, so `stat()` reports 0o666 no matter what was
|
||||
requested, and confidentiality comes from the ACL on the user profile
|
||||
instead. Asserting 0o600 there would fail forever without saying anything
|
||||
about whether the file is actually protected.
|
||||
|
||||
Keep the POSIX assertion exact, and on Windows skip with that reason
|
||||
spelled out, so the mode check is never quietly downgraded to something
|
||||
weaker that still looks green.
|
||||
"""
|
||||
|
||||
def _assert(path: Path) -> None:
|
||||
assert path.is_file(), f"{path} was not written"
|
||||
if os.name == "nt": # pragma: no cover - platform dependent
|
||||
pytest.skip(
|
||||
"Windows ignores POSIX mode bits, so a secret file always reports "
|
||||
"0o666; its confidentiality comes from the user-profile ACL, which "
|
||||
"this test does not assert"
|
||||
)
|
||||
assert path.stat().st_mode & 0o777 == 0o600, (
|
||||
f"{path} is {path.stat().st_mode & 0o777:#o}, expected 0o600"
|
||||
)
|
||||
|
||||
return _assert
|
||||
|
|
|
|||
|
|
@ -282,11 +282,11 @@ def test_human_rendering_neutralizes_osc_and_csi_control_sequences() -> None:
|
|||
|
||||
|
||||
def test_source_prompt_shows_paths_and_literal_confirmation(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, write_control_character_file: Any
|
||||
) -> None:
|
||||
(tmp_path / "app.py").write_text("print('ok')\n", encoding="utf-8")
|
||||
dangerous_name = "visible\x1b]52;c;copied\x07\x1b[2J.py"
|
||||
(tmp_path / dangerous_name).write_text("print('safe')\n", encoding="utf-8")
|
||||
write_control_character_file(tmp_path, dangerous_name, "print('safe')\n")
|
||||
output = io.StringIO()
|
||||
console = Console(file=output, width=100)
|
||||
prompts: list[tuple[str, bool]] = []
|
||||
|
|
|
|||
|
|
@ -163,4 +163,12 @@ def test_cli_device_identity_is_stable_and_privacy_safe(
|
|||
second = platform_identity.read_or_create_identity(device_name=" Build laptop ")
|
||||
assert second["client_instance_id"] == first["client_instance_id"]
|
||||
assert second["device_name"] == "Build laptop"
|
||||
assert path.stat().st_mode & 0o777 == 0o600
|
||||
|
||||
|
||||
def test_cli_identity_file_is_owner_only(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, assert_secret_file_permissions: Any
|
||||
) -> None:
|
||||
path = tmp_path / "cli-identity.json"
|
||||
monkeypatch.setattr(platform_identity, "IDENTITY_PATH", path)
|
||||
platform_identity.read_or_create_identity()
|
||||
assert_secret_file_permissions(path)
|
||||
|
|
|
|||
|
|
@ -128,12 +128,12 @@ def test_filesystem_completion_for_source_output_and_data(tmp_path: Any, monkeyp
|
|||
|
||||
|
||||
def test_filesystem_completion_omits_terminal_control_names(
|
||||
tmp_path: Any, monkeypatch: Any, capsys: Any
|
||||
tmp_path: Any, monkeypatch: Any, capsys: Any, write_control_character_file: Any
|
||||
) -> None:
|
||||
monkeypatch.chdir(tmp_path)
|
||||
(tmp_path / "safe.json").write_text("{}", encoding="utf-8")
|
||||
(tmp_path / "unsafe\nname.json").write_text("{}", encoding="utf-8")
|
||||
(tmp_path / "unsafe\x1b]52;c;payload\x07.json").write_text("{}", encoding="utf-8")
|
||||
write_control_character_file(tmp_path, "unsafe\nname.json")
|
||||
write_control_character_file(tmp_path, "unsafe\x1b]52;c;payload\x07.json")
|
||||
|
||||
words = ["cloud", "scans", "start", "--data", "@"]
|
||||
assert completion_candidates(words) == ["@safe.json"]
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import os
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
|
|
@ -177,8 +178,19 @@ def test_check_mountable_dir_accepts_a_project_under_the_home_root(
|
|||
|
||||
|
||||
def test_infer_target_type_applies_the_mount_policy() -> None:
|
||||
# `infer_target_type` only reaches the mount policy for a path that exists,
|
||||
# so pick a protected system directory this platform actually has. The
|
||||
# policy itself already knows about both families (`_FORBIDDEN_MOUNT_TREES`
|
||||
# and `_FORBIDDEN_WINDOWS_TREE_NAMES`); only the fixture was Unix-only.
|
||||
candidates = (
|
||||
[Path(Path.cwd().anchor) / "Windows"] if os.name == "nt" else [Path("/etc"), Path("/usr")]
|
||||
)
|
||||
system_dir = next((p for p in candidates if p.is_dir()), None)
|
||||
if system_dir is None:
|
||||
pytest.skip("no protected system directory on this platform")
|
||||
|
||||
with pytest.raises(ValueError, match="Refusing to mount"):
|
||||
infer_target_type("/etc")
|
||||
infer_target_type(str(system_dir))
|
||||
|
||||
|
||||
def test_read_target_list_file_strips_blank_lines(tmp_path: Path) -> None:
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import shutil
|
||||
import subprocess
|
||||
from typing import TYPE_CHECKING
|
||||
|
||||
|
|
@ -46,8 +47,14 @@ self-scoped information leak. Low: verbose errors.
|
|||
"""
|
||||
|
||||
|
||||
_GIT = shutil.which("git")
|
||||
|
||||
pytestmark = pytest.mark.skipif(_GIT is None, reason="these tests need a git executable")
|
||||
|
||||
|
||||
def _git(repo: Path, *args: str) -> None:
|
||||
subprocess.run(["/usr/bin/env", "git", *args], cwd=repo, check=True) # noqa: S603
|
||||
assert _GIT is not None
|
||||
subprocess.run([_GIT, *args], cwd=repo, check=True) # noqa: S603
|
||||
|
||||
|
||||
def _make_repo(tmp_path: Path, name: str = "repo") -> Path:
|
||||
|
|
|
|||
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
from __future__ import annotations
|
||||
|
||||
import stat
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
|
|
@ -86,10 +85,9 @@ def test_is_verified_accepts_epoch_expiry() -> None:
|
|||
assert auth.is_verified() is True
|
||||
|
||||
|
||||
def test_write_auth_is_0600() -> None:
|
||||
def test_write_auth_is_0600(assert_secret_file_permissions: Any) -> None:
|
||||
auth.write_auth(email="a@b.com", token="t", verified_at="") # nosec B106
|
||||
mode = stat.S_IMODE(auth.AUTH_PATH.stat().st_mode)
|
||||
assert mode == 0o600
|
||||
assert_secret_file_permissions(auth.AUTH_PATH)
|
||||
|
||||
|
||||
def test_read_auth_rejects_incomplete_record() -> None:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue