From a96dc494307b59f716379a7c9901fa3ba46b8162 Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:51:43 +0800 Subject: [PATCH 01/12] test: harden package removal checks and portable runner (#275, #276) --- .gitignore | 1 + AGENTS.md | 14 +- tests/run_tests.sh | 303 ++++++++++++++++---------------------- tests/test_package.py | 154 ++++++++++++++++++- tests/test_test_runner.py | 264 +++++++++++++++++++++++++++++++++ 5 files changed, 551 insertions(+), 185 deletions(-) create mode 100644 tests/test_test_runner.py diff --git a/.gitignore b/.gitignore index add18f6..2d84903 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ coverage.xml platform-tests.xml platform-partition.json htmlcov/ +test_reports/ .pytest_cache/ .ruff_cache/ .mypy_cache/ diff --git a/AGENTS.md b/AGENTS.md index d9ac08c..bd5ca9d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,9 +75,17 @@ may point to an existing Chromium executable instead of downloading one. errors. Ownership/teardown guidance lives in [tests](tests/AGENTS.md). - Coverage is measured with branches enabled. The overall floor and independent critical-file floors are documented with [CI](.github/workflows/AGENTS.md). -- `tests/run_tests.sh` assumes a developer-specific Conda path and writes - `test_reports/`; prefer direct pytest. `.pt-snap/`, `.tmp/`, `.worktrees/`, - and `tmp/` are ignored and excluded from root Ruff/Black checks. +- `bash tests/run_tests.sh` is an optional Bash entry point using the active + `python -m pytest`; set `PYTHON=/path/to/python` to choose another executable. + It does not activate environments or install dependencies. Full mode retains + the configured coverage floor and writes ignored `test_reports/` output; + `--reports-dir DIR` changes that destination. `--quick` omits coverage/report + generation and is **not** a substitute for CI's full coverage gate. Use + `--help` for file/keyword/marker selection and `--` for extra pytest arguments. + Pytest's exit status is preserved, including failure and no-tests-collected. + Without Bash, use the direct Python/pytest commands above. `.pt-snap/`, + `.tmp/`, `.worktrees/`, and `tmp/` are ignored and excluded from root + Ruff/Black checks. - `python -m build` writes `dist/` and build intermediates and may download build requirements. See [packaging](src/AGENTS.md) before using it as verification. diff --git a/tests/run_tests.sh b/tests/run_tests.sh index 096d8a4..03fe73d 100755 --- a/tests/run_tests.sh +++ b/tests/run_tests.sh @@ -1,193 +1,142 @@ -#!/bin/bash +#!/usr/bin/env bash -# 测试执行与报告生成脚本 -# 用于运行所有测试并生成详细的测试报告 +# Use the caller's Python environment; never activate or install an environment. +set -eo pipefail -set -e +PROJECT_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +PYTHON="${PYTHON:-python}" +REPORTS_DIR="test_reports" -# 颜色定义 -RED='\033[0;31m' -GREEN='\033[0;32m' -YELLOW='\033[1;33m' -BLUE='\033[0;34m' -NC='\033[0m' # No Color +show_help() { + cat <>> 激活 conda 环境...${NC}" - if [ -f "/Users/test1/liuyekang/miniconda3/etc/profile.d/conda.sh" ]; then - . /Users/test1/liuyekang/miniconda3/etc/profile.d/conda.sh - conda activate openclaw - echo -e "${GREEN}✓ Conda 环境激活成功${NC}" - else - echo -e "${RED}✗ Conda 初始化脚本不存在${NC}" - exit 1 - fi -} +Options: + -h, --help Show this help without running Python + -q, --quick Omit coverage and report generation (not a CI gate) + -f, --file FILE Select a test file/node (repeatable); bare names use tests/ + -k, --keyword EXPR Select tests by pytest keyword expression + -m, --mark EXPR Select tests by pytest marker expression + -x, --stop Stop after the first failure + --reports-dir DIR Coverage output directory (default: test_reports/) + -- Forward remaining arguments unchanged to pytest + +With no --file selection, run tests/. Full mode retains the configured coverage +floor and writes HTML/XML reports under the ignored test_reports/ directory. +Quick mode adds no coverage options and creates no report directory. Arbitrary +pytest options belong after --. Select files/nodes with -f: positional selectors +passed after -- are additive to the default tests/ selection. The pytest exit +code is returned unchanged. -# 创建报告目录 -create_report_dir() { - echo -e "${BLUE}>>> 创建报告目录...${NC}" - mkdir -p "${REPORTS_DIR}/htmlcov" - mkdir -p "${REPORTS_DIR}/xml" - echo -e "${GREEN}✓ 报告目录创建成功${NC}" +Examples: + bash tests/run_tests.sh + bash tests/run_tests.sh --quick -f test_package.py + bash tests/run_tests.sh -m 'not slow and not nightly' -- -q + PYTHON=/path/to/venv/bin/python bash tests/run_tests.sh --quick -- --collect-only +HELP } -# 运行测试 -run_tests() { - local test_pattern="${1:-tests/}" - - echo -e "${BLUE}>>> 运行测试：${test_pattern}${NC}" - echo "========================================" - - pytest "${test_pattern}" \ - --cov=pt_snap_cli \ - --cov-report=html:"${REPORTS_DIR}/htmlcov" \ - --cov-report=xml:"${REPORTS_DIR}/xml/coverage.xml" \ - --cov-report=term-missing \ - -v \ - --tb=short \ - "${@:2}" - - local exit_code=$? - - if [ $exit_code -eq 0 ]; then - echo -e "${GREEN}✓ 所有测试通过${NC}" - else - echo -e "${RED}✗ 部分测试失败${NC}" - fi - - return $exit_code +usage_error() { + printf 'Error: %s\nUse --help for usage.\n' "$1" >&2 + exit 2 } -# 生成报告摘要 -generate_summary() { - echo "" - echo "========================================" - echo -e "${BLUE}>>> 测试报告摘要${NC}" - echo "========================================" - - if [ -f "${REPORTS_DIR}/xml/coverage.xml" ]; then - echo -e "${GREEN}✓ 覆盖率报告已生成${NC}" - echo " - HTML 报告：${REPORTS_DIR}/htmlcov/index.html" - echo " - XML 报告：${REPORTS_DIR}/xml/coverage.xml" +require_value() { + if [[ $# -lt 2 || -z "$2" || "$2" == -* ]]; then + usage_error "$1 requires a value" fi - - echo "" } -# 显示帮助信息 -show_help() { - echo "用法：$0 [选项] [测试模式]" - echo "" - echo "选项:" - echo " -h, --help 显示帮助信息" - echo " -q, --quick 快速模式（不生成覆盖率报告）" - echo " -f, --file FILE 运行指定测试文件" - echo " -k, --keyword KEYWORD 运行匹配关键字的测试" - echo " -m, --mark MARK 运行标记的测试（如 slow）" - echo " -x, --stop 遇到失败立即停止" - echo "" - echo "测试模式:" - echo " all 运行所有测试（默认）" - echo " unit 只运行单元测试" - echo " integration 只运行集成测试" - echo "" - echo "示例:" - echo " $0 # 运行所有测试并生成报告" - echo " $0 -q # 快速运行所有测试" - echo " $0 -f test_cli.py # 运行指定测试文件" - echo " $0 -k test_render # 运行包含关键字的测试" - echo " $0 -m slow # 运行标记为 slow 的测试" - echo " $0 -x # 遇到失败立即停止" +normalize_path() { + # Git Bash accepts native drive/UNC paths when their separators are forward + # slashes. Leave POSIX filenames (which can contain backslashes) unchanged. + case "$1" in + [A-Za-z]:\\*|\\\\*) printf '%s\n' "${1//\\//}" ;; + *) printf '%s\n' "$1" ;; + esac } -# 主函数 -main() { - local quick_mode=false - local pytest_args=() - - # 解析命令行参数 - while [[ $# -gt 0 ]]; do - case $1 in - -h|--help) - show_help - exit 0 - ;; - -q|--quick) - quick_mode=true - shift - ;; - -f|--file) - pytest_args+=("tests/$2") - shift 2 - ;; - -k|--keyword) - pytest_args+=("-k" "$2") - shift 2 - ;; - -m|--mark) - pytest_args+=("-m" "$2") - shift 2 - ;; - -x|--stop) - pytest_args+=("-x") - shift - ;; - all) - shift - ;; - unit|integration) - echo -e "${YELLOW}警告：$1 模式暂未实现，将运行所有测试${NC}" - shift - ;; - *) - echo -e "${RED}未知选项：$1${NC}" - echo "使用 -h 查看帮助" - exit 1 - ;; - esac - done - - # 进入项目目录 - cd "${PROJECT_ROOT}" - - echo "========================================" - echo -e "${BLUE} 测试执行与报告生成脚本${NC}" - echo "========================================" - echo "" - - # 激活环境 - activate_conda - - # 创建报告目录 - create_report_dir - - # 运行测试 - if [ "$quick_mode" = true ]; then - echo -e "${YELLOW}>>> 快速模式：跳过覆盖率报告${NC}" - run_tests "${pytest_args[@]}" - else - echo -e "${BLUE}>>> 完整模式：生成覆盖率报告${NC}" - run_tests "${pytest_args[@]}" - fi - - local test_result=$? - - # 生成报告摘要 - generate_summary - - echo -e "${BLUE}========================================${NC}" - echo -e "${BLUE}测试完成！${NC}" - echo -e "${BLUE}========================================${NC}" - - exit $test_result -} +quick_mode=false +test_paths=() +pytest_args=() +while [[ $# -gt 0 ]]; do + case "$1" in + -h|--help) + show_help + exit 0 + ;; + -q|--quick) + quick_mode=true + shift + ;; + -f|--file) + require_value "$@" + file_path="$(normalize_path "$2")" + case "$file_path" in + /*|[A-Za-z]:/*|tests/*|./tests/*) test_paths+=("$file_path") ;; + *) test_paths+=("tests/$file_path") ;; + esac + shift 2 + ;; + -k|--keyword) + require_value "$@" + pytest_args+=("-k" "$2") + shift 2 + ;; + -m|--mark) + require_value "$@" + pytest_args+=("-m" "$2") + shift 2 + ;; + -x|--stop) + pytest_args+=("-x") + shift + ;; + --reports-dir) + require_value "$@" + REPORTS_DIR="$(normalize_path "$2")" + shift 2 + ;; + all) + shift + ;; + --) + shift + pytest_args+=("$@") + break + ;; + *) + usage_error "unknown option or mode: $1" + ;; + esac +done + +cd "$PROJECT_ROOT" +if ! command -v "$PYTHON" >/dev/null 2>&1; then + printf 'Error: Python executable not found: %s\n' "$PYTHON" >&2 + exit 127 +fi + +if [[ ${#test_paths[@]} -eq 0 ]]; then + test_paths=("tests/") +fi + +coverage_args=() +if [[ "$quick_mode" == false ]]; then + mkdir -p "$REPORTS_DIR/htmlcov" "$REPORTS_DIR/xml" + coverage_args=( + --cov=pt_snap_cli + "--cov-report=html:$REPORTS_DIR/htmlcov" + "--cov-report=xml:$REPORTS_DIR/xml/coverage.xml" + --cov-report=term-missing + ) + printf 'Coverage reports: %s\n' "$REPORTS_DIR" +fi -# 执行主函数 -main "$@" +# exec preserves pytest's success, failure, interruption and collection exit codes. +exec "$PYTHON" -m pytest "${test_paths[@]}" "${coverage_args[@]}" \ + -v --tb=short "${pytest_args[@]}" diff --git a/tests/test_package.py b/tests/test_package.py index 86014e2..58e8633 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -2,15 +2,59 @@ from __future__ import annotations -import importlib +from collections.abc import Iterable +from importlib.machinery import PathFinder from importlib.metadata import distribution, requires, version -from pathlib import Path +from pathlib import Path, PurePosixPath import pytest +import pt_snap_cli from pt_snap_cli import __version__ ROOT = Path(__file__).resolve().parents[1] +REMOVED_PACKAGES = ("mcp", "vendor") + + +def _assert_removed_package(package_root: Path, name: str) -> None: + """Allow empty namespace/cache residue, not restored code or package assets. + + Inspect without importing the removed module: a broken or side-effectful + resurrected module must fail this guard without executing its contents. + Legacy adjacent .pyc files remain importable and must not be ignored. + """ + removed_root = package_root / name + candidates = list(removed_root.rglob("*")) + list(package_root.glob(f"{name}.*")) + if removed_root.is_file(): + candidates.append(removed_root) + remaining = [ + path.relative_to(package_root).as_posix() + for path in candidates + if path.is_file() + and not ( + path.parent.name == "__pycache__" + and path.suffix in {".pyc", ".pyo"} + and "." in path.stem + ) + ] + assert not remaining, f"Removed package content remains: {sorted(remaining)}" + + PathFinder.invalidate_caches() + spec = PathFinder.find_spec(f"pt_snap_cli.{name}", [str(package_root)]) + assert spec is None or spec.loader is None, f"Removed package is importable: {spec}" + + +def _removed_distribution_entries(files: Iterable[str]) -> list[str]: + """A shipped removed package is a regression, even if it contains only cache.""" + removed = [] + for file in files: + parts = PurePosixPath(file).parts + if len(parts) >= 2 and parts[0] == "pt_snap_cli": + if any( + parts[1] == name or parts[1].startswith(f"{name}.") for name in REMOVED_PACKAGES + ): + removed.append(file) + return sorted(removed) def _runtime_requirement_names() -> set[str]: @@ -54,6 +98,106 @@ def test_pyproject_does_not_declare_mcp(self) -> None: assert "pt-snap-mcp" not in text assert "mcp>=" not in text - def test_mcp_package_is_removed(self) -> None: - with pytest.raises(ModuleNotFoundError): - importlib.import_module("pt_snap_cli.mcp") + @pytest.mark.parametrize("name", REMOVED_PACKAGES) + def test_removed_packages_have_no_code_or_assets(self, name: str) -> None: + # Check both the selected import location and this checkout. An editable + # install's RECORD does not enumerate the source tree. + package_roots = {Path(path).resolve() for path in pt_snap_cli.__path__} + package_roots.add(ROOT / "src" / "pt_snap_cli") + for package_root in package_roots: + _assert_removed_package(package_root, name) + + def test_distribution_does_not_ship_removed_packages(self) -> None: + files = distribution("pt-snap-cli").files + assert files is not None, "Installed distribution must have a file manifest" + assert not _removed_distribution_entries(str(file) for file in files) + + +@pytest.mark.parametrize("name", REMOVED_PACKAGES) +@pytest.mark.parametrize("layout", ["absent", "empty", "pycache-only"]) +def test_removed_package_guard_accepts_clean_and_cache_only_trees(tmp_path, name, layout): + package_root = tmp_path / "pt_snap_cli" + package_root.mkdir() + removed_root = package_root / name + if layout != "absent": + removed_root.mkdir() + if layout == "pycache-only": + for relative in ( + "__pycache__/__init__.cpython-313.pyc", + "memsnapdump/__pycache__/reader.cpython-313.pyc", + "memsnapdump/__pycache__/reader.cpython-310.opt-1.pyc", + ): + cache = removed_root / relative + cache.parent.mkdir(parents=True, exist_ok=True) + cache.write_bytes(b"stale cache must not be imported or deleted") + + before = {path: path.read_bytes() for path in package_root.rglob("*") if path.is_file()} + spec = PathFinder.find_spec(f"pt_snap_cli.{name}", [str(package_root)]) + if layout == "absent": + assert spec is None + else: + assert spec is not None and spec.loader is None + assert spec.submodule_search_locations is not None + _assert_removed_package(package_root, name) + assert {path: path.read_bytes() for path in package_root.rglob("*") if path.is_file()} == before + + +@pytest.mark.parametrize("name", REMOVED_PACKAGES) +@pytest.mark.parametrize( + "relative", + [ + "__init__.py", + "server.py", + "memsnapdump/reader.py", + "__init__.pyc", + "memsnapdump/schema.json", + "__pycache__/hidden.py", + "__pycache__/importable.pyc", + ], +) +def test_removed_package_guard_rejects_restored_code_and_assets(tmp_path, name, relative): + package_root = tmp_path / "pt_snap_cli" + restored = package_root / name / relative + restored.parent.mkdir(parents=True) + payload = b"raise RuntimeError('must never execute removed package code')\n" + restored.write_bytes(payload) + + with pytest.raises(AssertionError, match="Removed package content remains"): + _assert_removed_package(package_root, name) + assert restored.read_bytes() == payload + + +@pytest.mark.parametrize("name", REMOVED_PACKAGES) +@pytest.mark.parametrize("suffix", [".py", ".pyc", ".so", ".pyd"]) +def test_removed_package_guard_rejects_flat_module_replacements(tmp_path, name, suffix): + restored = tmp_path / f"{name}{suffix}" + restored.write_bytes(b"removed module") + with pytest.raises(AssertionError, match="Removed package content remains"): + _assert_removed_package(tmp_path, name) + + +@pytest.mark.parametrize("name", REMOVED_PACKAGES) +@pytest.mark.parametrize( + "entry", + [ + "{name}/__init__.py", + "{name}/memsnapdump/reader.py", + "{name}.py", + "{name}/__pycache__/old.pyc", + ], +) +def test_distribution_guard_rejects_rediscovered_removed_content(name, entry): + path = "pt_snap_cli/" + entry.format(name=name) + assert _removed_distribution_entries(["pt_snap_cli/cli.py", path]) == [path] + + +def test_distribution_guard_allows_unrelated_modules_and_metadata(): + assert _removed_distribution_entries( + [ + "pt_snap_cli/cli.py", + "pt_snap_cli/mcp_tools.py", + "pt_snap_cli/vendor_tools/reader.py", + "pt_snap_cli-1.0.dist-info/METADATA", + "pt_snap_cli-1.0.dist-info/licenses/LICENSE", + ] + ) == [] diff --git a/tests/test_test_runner.py b/tests/test_test_runner.py new file mode 100644 index 0000000..44730b8 --- /dev/null +++ b/tests/test_test_runner.py @@ -0,0 +1,264 @@ +"""Exercise the real Bash entry point without starting a nested test suite.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +ROOT = Path(__file__).resolve().parents[1] + + +def _bash() -> str: + if os.name == "nt": + # Prefer Git Bash to Windows' WSL launcher. Git-for-Windows supplies the + # POSIX tools used by the entry point on the Windows compatibility jobs. + git = shutil.which("git") + assert git is not None, "Git for Windows is required to test the Bash entry point" + bash = Path(git).resolve().parents[1] / "bin" / "bash.exe" + assert bash.is_file(), f"Git Bash not found at {bash}" + return str(bash) + bash = shutil.which("bash") + assert bash is not None, "Bash is required to test the Bash entry point" + return bash + + +@pytest.fixture +def shell_runner(tmp_path): + root = tmp_path / "repository with spaces" + (root / "tests").mkdir(parents=True) + script = root / "tests" / "run_tests.sh" + shutil.copyfile(ROOT / "tests" / "run_tests.sh", script) + outside = tmp_path / "outside repository" + outside.mkdir() + recorder = tmp_path / "isolated modules" + recorder.mkdir() + log = tmp_path / "invocation.json" + # Python really dispatches `-m pytest`, but this isolated module records the + # call instead of loading pytest/plugins, collecting tests or generating data. + (recorder / "pytest.py").write_text( + "import json, os, sys\n" + "from pathlib import Path\n" + "Path(os.environ['RUNNER_TEST_LOG']).write_text(json.dumps({\n" + " 'argv': sys.argv[1:], 'cwd': os.getcwd(), 'executable': sys.executable,\n" + "}), encoding='utf-8')\n" + "raise SystemExit(int(os.environ['RUNNER_TEST_EXIT']))\n", + encoding="utf-8", + ) + env = os.environ.copy() + env.update( + PYTHONPATH=str(recorder), + PYTHONUTF8="1", + PATH=str(Path(sys.executable).parent) + os.pathsep + env.get("PATH", ""), + RUNNER_TEST_LOG=str(log), + ) + + def run(*args, exit_code=0, python=Path(sys.executable).as_posix()): + selected_env = dict(env, RUNNER_TEST_EXIT=str(exit_code)) + if python is None: + selected_env.pop("PYTHON", None) + else: + selected_env["PYTHON"] = python + return subprocess.run( + [_bash(), "--noprofile", "--norc", script.as_posix(), *args], + cwd=outside, + env=selected_env, + text=True, + capture_output=True, + check=False, + timeout=30, + ) + + def invocation(): + return json.loads(log.read_text(encoding="utf-8")) + + return SimpleNamespace(root=root, log=log, run=run, invocation=invocation) + + +def test_default_runner_uses_active_python_and_keeps_coverage(shell_runner): + result = shell_runner.run(python=None) + assert result.returncode == 0, result.stdout + result.stderr + call = shell_runner.invocation() + assert Path(call["cwd"]) == shell_runner.root + assert Path(call["executable"]).resolve() == Path(sys.executable).resolve() + assert call["argv"] == [ + "tests/", + "--cov=pt_snap_cli", + "--cov-report=html:test_reports/htmlcov", + "--cov-report=xml:test_reports/xml/coverage.xml", + "--cov-report=term-missing", + "-v", + "--tb=short", + ] + assert (shell_runner.root / "test_reports" / "htmlcov").is_dir() + assert (shell_runner.root / "test_reports" / "xml").is_dir() + + +def test_quick_mode_forwards_selection_and_pytest_arguments_without_reports(shell_runner): + result = shell_runner.run( + "--quick", + "-f", + "test_package.py::TestPackage::test_version", + "--file", + "tests/query/test_builder.py", + "--keyword", + "version or builder", + "-m", + "not slow and not nightly", + "--stop", + "--", + "-q", + "--maxfail=2", + ) + assert result.returncode == 0, result.stdout + result.stderr + assert shell_runner.invocation()["argv"] == [ + "tests/test_package.py::TestPackage::test_version", + "tests/query/test_builder.py", + "-v", + "--tb=short", + "-k", + "version or builder", + "-m", + "not slow and not nightly", + "-x", + "-q", + "--maxfail=2", + ] + assert not (shell_runner.root / "test_reports").exists() + + +def test_short_aliases_and_all_mode_preserve_option_only_invocations(shell_runner): + result = shell_runner.run("-q", "all", "-k", "test_version", "--mark", "not slow", "-x") + assert result.returncode == 0, result.stdout + result.stderr + assert shell_runner.invocation()["argv"] == [ + "tests/", + "-v", + "--tb=short", + "-k", + "test_version", + "-m", + "not slow", + "-x", + ] + assert not (shell_runner.root / "test_reports").exists() + + +def test_explicit_interpreter_and_absolute_test_path_are_respected(shell_runner): + target = shell_runner.root / "tests" / "test with spaces.py" + result = shell_runner.run("--quick", "--file", target.as_posix()) + assert result.returncode == 0, result.stdout + result.stderr + call = shell_runner.invocation() + assert Path(call["executable"]).resolve() == Path(sys.executable).resolve() + assert Path(call["argv"][0]) == target + assert call["argv"][1:] == ["-v", "--tb=short"] + + +def test_custom_report_directory_preserves_spaces_and_repo_relative_paths(shell_runner): + result = shell_runner.run("--reports-dir", ".tmp/reports with spaces") + assert result.returncode == 0, result.stdout + result.stderr + args = shell_runner.invocation()["argv"] + assert "--cov-report=html:.tmp/reports with spaces/htmlcov" in args + assert "--cov-report=xml:.tmp/reports with spaces/xml/coverage.xml" in args + assert (shell_runner.root / ".tmp/reports with spaces/htmlcov").is_dir() + assert (shell_runner.root / ".tmp/reports with spaces/xml").is_dir() + assert not (shell_runner.root / "test_reports").exists() + + +@pytest.mark.parametrize( + ("supplied", "expected"), + [ + (r"C:\example tests\test_case.py", "C:/example tests/test_case.py"), + ("C:/example tests/test_case.py", "C:/example tests/test_case.py"), + (r"\\server\share\test_case.py", "//server/share/test_case.py"), + ("//server/share/test_case.py", "//server/share/test_case.py"), + ("./tests/test_package.py", "./tests/test_package.py"), + ], +) +def test_file_selection_accepts_native_drive_and_unc_path_syntax(shell_runner, supplied, expected): + # These selectors are recorded only; no network share or external file is + # accessed. Windows Bash may translate forward slashes back to native ones. + result = shell_runner.run("--quick", "--file", supplied) + assert result.returncode == 0, result.stdout + result.stderr + assert shell_runner.invocation()["argv"][0].replace("\\", "/") == expected + + +@pytest.mark.parametrize("native_separators", [False, True]) +def test_report_directory_accepts_absolute_temp_paths(shell_runner, tmp_path, native_separators): + report = tmp_path / "absolute reports" + supplied = str(report) if native_separators else report.as_posix() + result = shell_runner.run("--reports-dir", supplied) + assert result.returncode == 0, result.stdout + result.stderr + args = [arg.replace("\\", "/") for arg in shell_runner.invocation()["argv"]] + assert f"--cov-report=html:{report.as_posix()}/htmlcov" in args + assert f"--cov-report=xml:{report.as_posix()}/xml/coverage.xml" in args + assert (report / "htmlcov").is_dir() + assert (report / "xml").is_dir() + assert not (shell_runner.root / "test_reports").exists() + + +def test_quick_mode_does_not_create_custom_reports(shell_runner): + result = shell_runner.run("--quick", "--reports-dir", "custom reports") + assert result.returncode == 0, result.stdout + result.stderr + assert not any(arg.startswith("--cov") for arg in shell_runner.invocation()["argv"]) + assert not (shell_runner.root / "custom reports").exists() + assert not (shell_runner.root / "test_reports").exists() + + +@pytest.mark.parametrize("exit_code", [0, 1, 2, 3, 4, 5]) +@pytest.mark.parametrize("options", [(), ("--quick",)]) +def test_runner_preserves_pytest_exit_codes(shell_runner, exit_code, options): + result = shell_runner.run(*options, exit_code=exit_code) + assert result.returncode == exit_code, result.stdout + result.stderr + assert shell_runner.log.is_file() + + +@pytest.mark.parametrize("option", ["-h", "--help"]) +def test_help_needs_no_python_and_creates_no_reports(shell_runner, option): + result = shell_runner.run(option, python="missing-python-for-runner-test") + assert result.returncode == 0 + assert "PYTHON" in result.stdout and "--quick" in result.stdout + assert "not a CI gate" in result.stdout + assert not shell_runner.log.exists() + assert not (shell_runner.root / "test_reports").exists() + + +@pytest.mark.parametrize( + "args", + [ + ("-f",), + ("--file", ""), + ("-k",), + ("--keyword", "--quick"), + ("-m",), + ("--mark", ""), + ("--reports-dir",), + ("--reports-dir", "--quick"), + ("--unknown",), + ("unit",), + ("integration",), + ], +) +def test_invalid_options_fail_before_invoking_python(shell_runner, args): + result = shell_runner.run(*args) + assert result.returncode == 2 + assert "Error:" in result.stderr + assert not shell_runner.log.exists() + assert not (shell_runner.root / "test_reports").exists() + + +def test_missing_interpreter_is_an_actionable_failure(shell_runner): + result = shell_runner.run(python="missing-python-for-runner-test") + assert result.returncode == 127 + assert "Python executable not found: missing-python-for-runner-test" in result.stderr + assert not shell_runner.log.exists() + assert not (shell_runner.root / "test_reports").exists() + + +def test_default_report_directory_is_ignored(): + assert "test_reports/" in (ROOT / ".gitignore").read_text(encoding="utf-8").splitlines() From f59fe08fc33d6489cb9154809cdf58e341703028 Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:53:20 +0800 Subject: [PATCH 02/12] test: add bounded generated pagination and validation properties (#280) --- pyproject.toml | 1 + tests/core/test_import_metadata_properties.py | 202 ++++++++++++++++++ .../test_snapshot_validation_properties.py | 64 ++++++ tests/query/test_pagination_properties.py | 198 +++++++++++++++++ tests/query/test_template_properties.py | 178 +++++++++++++++ 5 files changed, 643 insertions(+) create mode 100644 tests/core/test_import_metadata_properties.py create mode 100644 tests/core/test_snapshot_validation_properties.py create mode 100644 tests/query/test_pagination_properties.py create mode 100644 tests/query/test_template_properties.py diff --git a/pyproject.toml b/pyproject.toml index f65fa0e..5e777e2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ Issues = "https://github.com/codeasier/pt-snap-cli/issues" dev = [ "pytest>=7.0.0", "pytest-cov>=4.0.0", + "hypothesis>=6.100.0,<7", "playwright>=1.40.0", "black>=23.0.0", "ruff>=0.0.280", diff --git a/tests/core/test_import_metadata_properties.py b/tests/core/test_import_metadata_properties.py new file mode 100644 index 0000000..e904918 --- /dev/null +++ b/tests/core/test_import_metadata_properties.py @@ -0,0 +1,202 @@ +"""Generated import-metadata validation, without deserializing pickle bytes. + +Mappings are trusted test-owned values. SQLite fixtures deliberately omit type +coercion/constraints so malformed stored values reach the actual read validator. +""" + +from __future__ import annotations + +import sqlite3 +from contextlib import closing +from dataclasses import asdict +from datetime import datetime, timezone +from pathlib import Path +from tempfile import TemporaryDirectory + +import pytest +from hypothesis import given, settings +from hypothesis import strategies as st + +from pt_snap_cli.core.import_metadata import ( + IMPORT_FORMAT_VERSION, + METADATA_SCHEMA_VERSION, + ImportMetadataService, +) +from pt_snap_cli.core.import_metadata_contract import metadata_from_mapping + +PROPERTY_SETTINGS = settings( + max_examples=35, derandomize=True, database=None, deadline=None, print_blob=True +) +INTEGER_FIELDS = ( + "metadata_schema_version", + "import_format_version", + "source_size", + "requested_device", +) +STRING_FIELDS = ( + "source_sha256", + "source_name", + "importer_name", + "importer_version", + "completed_at", +) +FIELDS = INTEGER_FIELDS + STRING_FIELDS +TEXT = st.text(alphabet="abcdefghijklmnopqrstuvwxyz0123456789_中文", min_size=1, max_size=24) + + +@st.composite +def _valid_metadata(draw: st.DrawFn) -> dict[str, object]: + return { + "metadata_schema_version": METADATA_SCHEMA_VERSION, + "import_format_version": IMPORT_FORMAT_VERSION, + "source_sha256": draw(st.binary(min_size=32, max_size=32)).hex(), + "source_size": draw(st.integers(0, 2**63 - 1)), + "source_name": draw(TEXT) + ".pkl", + "requested_device": draw(st.one_of(st.none(), st.integers(0, 1024))), + "importer_name": draw(TEXT), + "importer_version": draw(TEXT), + "completed_at": draw( + st.datetimes( + min_value=datetime(2000, 1, 1), + max_value=datetime(2030, 12, 31), + timezones=st.just(timezone.utc), + ) + ).isoformat(), + } + + +def _database(path: Path) -> None: + with closing(sqlite3.connect(path)) as conn, conn: + conn.execute( + "CREATE TABLE dictionary (`table` TEXT, `column` TEXT, `key` TEXT, `value` TEXT)" + ) + conn.execute("CREATE TABLE trace_entry_0 (id INTEGER PRIMARY KEY)") + + +def _store_unvalidated(path: Path, values: dict[str, object]) -> None: + # Field names are fixed test constants, never generated SQL identifiers. + columns = ("id",) + tuple(field for field in FIELDS if field in values) + quoted = ", ".join(f'"{field}"' for field in columns) + placeholders = ", ".join("?" for _ in columns) + with closing(sqlite3.connect(path)) as conn, conn: + conn.execute(f"CREATE TABLE pt_snap_metadata ({quoted})") + conn.execute( + f"INSERT INTO pt_snap_metadata VALUES ({placeholders})", + [1] + [values[field] for field in columns[1:]], + ) + + +def _assert_invalid_read_is_nonmutating(values: dict[str, object]) -> None: + with TemporaryDirectory() as directory: + path = Path(directory) / "invalid.db" + _database(path) + _store_unvalidated(path, values) + before = path.read_bytes() + inspection = ImportMetadataService().inspect(path) + assert inspection.status == "invalid" + assert inspection.reason == "metadata_invalid" + assert inspection.metadata is None + assert path.read_bytes() == before + + +@PROPERTY_SETTINGS +@given(values=_valid_metadata()) +def test_generated_valid_metadata_round_trips_mapping_and_sqlite(values: dict[str, object]) -> None: + metadata = metadata_from_mapping(values) + assert asdict(metadata) == values + with TemporaryDirectory() as directory: + path = Path(directory) / "valid.db" + _database(path) + service = ImportMetadataService() + service.write(path, metadata) + before = path.read_bytes() + inspection = service.inspect(path) + assert inspection.status == "available" + assert inspection.reason is None + assert inspection.metadata == metadata + decision = service.evaluate_cache(path, metadata.source_sha256, metadata.requested_device) + assert decision.reused is True + assert decision.metadata == metadata + assert decision.reason is None + assert path.read_bytes() == before + + +@PROPERTY_SETTINGS +@given(values=_valid_metadata(), missing=st.sets(st.sampled_from(FIELDS), min_size=1)) +def test_generated_missing_metadata_fields_have_controlled_inspection( + values: dict[str, object], missing: set[str] +) -> None: + incomplete = {field: value for field, value in values.items() if field not in missing} + with pytest.raises(KeyError) as error: + metadata_from_mapping(incomplete) + assert error.value.args[0] in missing + _assert_invalid_read_is_nonmutating(incomplete) + + +@pytest.mark.parametrize("field", INTEGER_FIELDS) +@PROPERTY_SETTINGS +@given( + values=_valid_metadata(), + wrong_type=st.one_of( + st.booleans(), + st.floats(min_value=-1000, max_value=1000, allow_nan=False, allow_infinity=False), + st.text(max_size=20), + st.lists(st.integers(-10, 10), max_size=3), + ), +) +def test_generated_integer_metadata_does_not_coerce_wrong_types( + field: str, values: dict[str, object], wrong_type: object +) -> None: + values[field] = wrong_type + # This is the pure mapping contract: in particular bool is not int. SQLite + # binds bool as an integer, so testing this only through SQL would hide it. + with pytest.raises(TypeError, match="Expected integer metadata value"): + metadata_from_mapping(values) + + +@pytest.mark.parametrize("field", INTEGER_FIELDS) +@PROPERTY_SETTINGS +@given(values=_valid_metadata(), magnitude=st.integers(1, 2**63 - 1)) +def test_generated_negative_metadata_numbers_have_controlled_inspection( + field: str, values: dict[str, object], magnitude: int +) -> None: + values[field] = -magnitude + with pytest.raises(ValueError): + metadata_from_mapping(values) + _assert_invalid_read_is_nonmutating(values) + + +@pytest.mark.parametrize("field", STRING_FIELDS) +@PROPERTY_SETTINGS +@given(values=_valid_metadata(), empty=st.sampled_from([None, ""])) +def test_generated_missing_string_values_have_controlled_inspection( + field: str, values: dict[str, object], empty: object +) -> None: + values[field] = empty + with pytest.raises(TypeError, match="Expected non-empty metadata string"): + metadata_from_mapping(values) + _assert_invalid_read_is_nonmutating(values) + + +@PROPERTY_SETTINGS +@given( + values=_valid_metadata(), suffix=st.text(alphabet="0123456789abcdef", min_size=0, max_size=70) +) +def test_generated_malformed_digest_has_controlled_inspection( + values: dict[str, object], suffix: str +) -> None: + values["source_sha256"] = "g" + suffix + with pytest.raises(ValueError, match="lowercase SHA-256 digest"): + metadata_from_mapping(values) + _assert_invalid_read_is_nonmutating(values) + + +@PROPERTY_SETTINGS +@given(values=_valid_metadata(), offset_hours=st.integers(1, 23)) +def test_generated_non_utc_timestamp_has_controlled_inspection( + values: dict[str, object], offset_hours: int +) -> None: + values["completed_at"] = f"2026-01-01T00:00:00+{offset_hours:02d}:00" + with pytest.raises(ValueError, match="completed_at must use UTC"): + metadata_from_mapping(values) + _assert_invalid_read_is_nonmutating(values) diff --git a/tests/core/test_snapshot_validation_properties.py b/tests/core/test_snapshot_validation_properties.py new file mode 100644 index 0000000..18a8602 --- /dev/null +++ b/tests/core/test_snapshot_validation_properties.py @@ -0,0 +1,64 @@ +"""Trusted in-memory numeric metadata rejection through the real import boundary. + +Only the representation-loading seam is replaced: no pickle is created or +unpickled, and conversion, validation, error normalization and cleanup stay real. +Missing keys and arbitrary structural/type errors are a separate runtime gap; +this module covers the existing controlled ValueError conversion contract. +""" + +from __future__ import annotations + +import copy +from pathlib import Path +from tempfile import TemporaryDirectory + +import pytest +from hypothesis import given, settings +from hypothesis import strategies as st + +from pt_snap_cli.core.errors import ImportExecutionError +from pt_snap_cli.core.snapshot_import_backend import SnapshotImportBackend +from pt_snap_cli.snapshot.tools.adaptors import snapshot2db + +PROPERTY_SETTINGS = settings( + max_examples=25, derandomize=True, database=None, deadline=None, print_blob=True +) + + +@pytest.mark.parametrize("field", ["addr", "size", "stream", "device_free", "id"]) +@PROPERTY_SETTINGS +@given(suffix=st.text(alphabet="abcdefghijklmnopqrstuvwxyz0123456789 +-", max_size=40)) +def test_generated_bad_numeric_snapshot_values_reject_without_publication( + field: str, suffix: str +) -> None: + event = {"action": "alloc", "addr": 4096, "size": 1, "stream": 0, "frames": []} + event[field] = f"not-an-integer:{suffix}" + representation = {"segments": [], "device_traces": [[event]]} + before = copy.deepcopy(representation) + + with TemporaryDirectory() as directory, pytest.MonkeyPatch.context() as monkeypatch: + root = Path(directory) + source = root / "trusted-in-memory.pkl" + output = root / "output" + output.mkdir() + backend = SnapshotImportBackend() + destination = backend.target_db_path(source, output) + destination.write_bytes(b"previous database must survive validation failure") + previous = destination.read_bytes() + loads = [] + + def load_representation(path: Path) -> dict[str, object]: + assert path == source + loads.append(path) + return representation + + monkeypatch.setattr(snapshot2db, "load_snapshot_representation", load_representation) + with pytest.raises(ImportExecutionError, match="Snapshot import backend failed") as error: + backend.dump_to_db(source, output, device=0) + + assert isinstance(error.value.__cause__, ValueError) + assert loads == [source] + assert representation == before + assert not source.exists() + assert destination.read_bytes() == previous + assert list(output.iterdir()) == [destination] diff --git a/tests/query/test_pagination_properties.py b/tests/query/test_pagination_properties.py new file mode 100644 index 0000000..b712a9c --- /dev/null +++ b/tests/query/test_pagination_properties.py @@ -0,0 +1,198 @@ +"""Bounded properties for the existing standalone event/block page contract. + +Every generated example owns its database, focus isolation and service lifetime. +The oracle sorts independent input records, not another query's returned rows. +""" + +from __future__ import annotations + +import copy +import sqlite3 +from collections.abc import Iterator +from contextlib import closing, contextmanager +from pathlib import Path +from tempfile import TemporaryDirectory + +import pytest +from hypothesis import example, given, settings +from hypothesis import strategies as st + +from pt_snap_cli.core.query_limits import QueryLimits +from pt_snap_cli.core.query_service import QueryService +from pt_snap_cli.query import registry +from pt_snap_cli.query.config import QueryConfig + +# Deterministic for a fixed Hypothesis version; failing examples are shrunk and +# print a reproduction blob. No example database or machine-speed deadline. +PROPERTY_SETTINGS = settings( + max_examples=40, derandomize=True, database=None, deadline=None, print_blob=True +) +RECORDS = st.lists( + st.tuples(st.integers(-20, 100), st.integers(0, 4)), + unique_by=lambda row: row[0], + max_size=24, +) +LIMITS = st.one_of(st.just(-1), st.integers(0, 30)) +ROW_CAPS = st.one_of(st.none(), st.integers(0, 30)) + + +@contextmanager +def _query_database(records: list[tuple[int, int]]) -> Iterator[tuple[QueryService, Path]]: + with TemporaryDirectory() as directory, pytest.MonkeyPatch.context() as monkeypatch: + root = Path(directory) + monkeypatch.chdir(root) + monkeypatch.setattr(Path, "home", lambda: root) + monkeypatch.delenv("PT_SNAP_DB_PATH", raising=False) + monkeypatch.delenv("PT_SNAP_QUERY_TIMEOUT", raising=False) + # Restore the exact original registry, including other tests' factories. + # Only load the two actual packaged contracts being exercised here. + monkeypatch.setattr(registry, "_registry", copy.deepcopy(registry._registry)) + for name in ("event", "block"): + config = QueryConfig.load_yaml(registry.get_template_dir() / "basic" / f"{name}.yaml") + registry.register_query(config.queries[name]) + + db_path = root / "pages.db" + with closing(sqlite3.connect(db_path)) as conn, conn: + conn.execute( + "CREATE TABLE dictionary (`table` TEXT, `column` TEXT, `key` TEXT, `value` TEXT)" + ) + conn.execute( + "CREATE TABLE trace_entry_0 (id INTEGER PRIMARY KEY, action INTEGER, " + "address INTEGER, size INTEGER, stream INTEGER, allocated INTEGER, " + "active INTEGER, reserved INTEGER, callstack TEXT)" + ) + conn.execute( + "CREATE TABLE block_0 (id INTEGER PRIMARY KEY, address INTEGER, size INTEGER, " + "requestedSize INTEGER, state INTEGER, allocEventId INTEGER, freeEventId INTEGER)" + ) + conn.executemany( + "INSERT INTO trace_entry_0 VALUES (?, 4, 4096, ?, 0, 0, 0, 0, 'frame')", + records, + ) + conn.executemany( + "INSERT INTO block_0 VALUES (?, 4096, ?, ?, 1, ?, -1)", + [(event_id, size, size, event_id) for event_id, size in records], + ) + before = db_path.read_bytes() + with closing(QueryService(query_limits=QueryLimits())) as service: + yield service, db_path + assert db_path.read_bytes() == before + assert not (root / ".pt-snap").exists() + + +@pytest.mark.parametrize("template", ["event", "block"]) +@PROPERTY_SETTINGS +@given( + records=RECORDS, + offset=st.integers(0, 30), + limit=LIMITS, + max_rows=ROW_CAPS, + direction=st.sampled_from(["ASC", "DESC"]), + exact_total=st.booleans(), +) +@example(records=[], offset=0, limit=0, max_rows=None, direction="ASC", exact_total=False) +@example( + records=[(7, 2), (-3, 2), (5, 2), (11, 0)], + offset=1, + limit=3, + max_rows=1, + direction="DESC", + exact_total=True, +) +@example( + records=[(7, 2), (-3, 2)], + offset=0, + limit=0, + max_rows=3, + direction="ASC", + exact_total=True, +) +@example( + records=[(7, 2), (-3, 2)], + offset=4, + limit=-1, + max_rows=0, + direction="ASC", + exact_total=False, +) +def test_generated_pages_match_ordered_slice_and_completeness( + template: str, + records: list[tuple[int, int]], + offset: int, + limit: int, + max_rows: int | None, + direction: str, + exact_total: bool, +) -> None: + ordered = sorted(records, key=lambda row: (row[1], row[0]), reverse=direction == "DESC") + available = ordered[offset:] + # max_rows=0 disables the output cap; a template limit=0 is an empty page. + caps = [limit] if limit >= 0 else [] + if max_rows is not None and max_rows > 0: + caps.append(max_rows) + cap = min(caps) if caps else len(available) + expected = available[:cap] + has_more = len(available) > len(expected) + + with _query_database(records) as (service, db_path): + result = service.execute_query( + template, + db_path=db_path, + device_id=0, + params={"order_by": "size", "order_dir": direction, "offset": offset, "limit": limit}, + max_rows=max_rows, + exact_total=exact_total, + ) + + assert [(row["id"], row["size"]) for row in result.rows] == expected + assert result.returned == len(expected) + assert result.has_more is has_more + assert result.total == (len(records) if exact_total else len(expected)) + assert result.total_is_exact is (exact_total or (not has_more and offset == 0)) + assert result.truncated is ( + has_more or offset > 0 or (exact_total and len(records) > len(expected)) + ) + + +@pytest.mark.parametrize("template", ["event", "block"]) +@PROPERTY_SETTINGS +@given( + records=RECORDS, + page_size=st.integers(1, 12), + max_rows=st.one_of(st.none(), st.integers(1, 12)), + direction=st.sampled_from(["ASC", "DESC"]), +) +@example(records=[(5, 3), (1, 3), (8, 3), (2, 1)], page_size=2, max_rows=1, direction="DESC") +@example(records=[], page_size=1, max_rows=None, direction="ASC") +def test_generated_page_concatenation_matches_unlimited_query( + template: str, + records: list[tuple[int, int]], + page_size: int, + max_rows: int | None, + direction: str, +) -> None: + expected = sorted(records, key=lambda row: (row[1], row[0]), reverse=direction == "DESC") + step = min(page_size, max_rows) if max_rows is not None else page_size + common = {"order_by": "size", "order_dir": direction} + with _query_database(records) as (service, db_path): + whole = service.execute_query(template, params=common, db_path=db_path, device_id=0) + pages = [] + # Arithmetic bounds, not has_more, control termination. Query the empty + # trailing page too, so broken completeness cannot hide missing rows. + for offset in range(0, len(records) + step, step): + page = service.execute_query( + template, + params={**common, "offset": offset, "limit": page_size}, + db_path=db_path, + device_id=0, + max_rows=max_rows, + ) + expected_page = expected[offset : offset + step] + assert [(row["id"], row["size"]) for row in page.rows] == expected_page + assert page.has_more is (offset + len(expected_page) < len(expected)) + pages.extend(page.rows) + assert pages == whole.rows + assert [(row["id"], row["size"]) for row in whole.rows] == expected + assert len({row["id"] for row in pages}) == len(records) + assert whole.total_is_exact is True + assert whole.has_more is whole.truncated is False diff --git a/tests/query/test_template_properties.py b/tests/query/test_template_properties.py new file mode 100644 index 0000000..65f8dfe --- /dev/null +++ b/tests/query/test_template_properties.py @@ -0,0 +1,178 @@ +"""Generated YAML contracts at their existing validation/rejection boundaries. + +The registry promises warning-and-skip for malformed files. The lower-level +QueryConfig loader does not currently normalize arbitrary structural YAML into +one domain error; these tests do not claim that stronger, unsupported contract. +""" + +from __future__ import annotations + +import copy +from pathlib import Path +from tempfile import TemporaryDirectory + +import pytest +import yaml +from hypothesis import given, settings +from hypothesis import strategies as st + +from pt_snap_cli.query import registry +from pt_snap_cli.query.config import QueryConfig + +PROPERTY_SETTINGS = settings( + max_examples=35, derandomize=True, database=None, deadline=None, print_blob=True +) +IDENTIFIERS = st.text(alphabet="abcdefghijklmnopqrstuvwxyz", min_size=1, max_size=16) +VALUES = st.recursive( + st.one_of(st.none(), st.booleans(), st.integers(-100, 100), st.text(max_size=16)), + lambda children: st.one_of( + st.lists(children, max_size=4), st.dictionaries(IDENTIFIERS, children, max_size=4) + ), + max_leaves=8, +) + + +def _load_template(body: dict[str, object]): + return QueryConfig.load_yaml_from_string( + yaml.safe_dump({"queries": {"generated": body}}) + ).queries["generated"] + + +@PROPERTY_SETTINGS +@given( + column=IDENTIFIERS, + choices=st.lists(st.integers(-1000, 1000), min_size=1, max_size=8, unique=True), + version=st.integers(1, 100), + sentinel=st.integers(-1000, 1000), + units=st.sampled_from( + ["bytes", "gib", "percent", "event_id", "count", "address", "flag", "text"] + ), +) +def test_generated_yaml_preserves_schema_and_validates_declared_choices( + column: str, choices: list[int], version: int, sentinel: int, units: str +) -> None: + schema = [{"column": column, "type": "int", "units": units, "sentinel": sentinel}] + template = _load_template( + { + "query": "SELECT {{ selected }} AS value", + "semantics_version": version, + "output_schema": schema, + "parameters": { + "selected": {"type": "int", "choices": choices, "default": choices[0]}, + "required_text": {"type": "str", "required": True}, + }, + } + ) + assert template.name == "generated" + assert template.output_schema == schema + assert template.semantics_version == version + assert template.parameters["selected"].choices == choices + for choice in choices: + assert template.validate_params({"selected": str(choice), "required_text": column}) == { + "selected": choice, + "required_text": column, + } + assert template.validate_params({"required_text": column})["selected"] == choices[0] + with pytest.raises(ValueError, match="Required parameter 'required_text' is missing"): + template.validate_params({}) + with pytest.raises(ValueError, match="'selected' must be one of"): + template.validate_params({"selected": max(choices) + 1, "required_text": column}) + + +@PROPERTY_SETTINGS +@given(unknown=IDENTIFIERS, value=VALUES) +def test_generated_unknown_schema_fields_and_query_parameters_are_rejected( + unknown: str, value: object +) -> None: + key = f"unknown_{unknown}" + with pytest.raises(ValueError, match="unsupported keys") as error: + _load_template( + {"query": "SELECT 1", "output_schema": [{"column": "id", "type": "int", key: value}]} + ) + assert key in str(error.value) + template = _load_template( + {"query": "SELECT 1", "parameters": {"limit": {"type": "int", "default": -1}}} + ) + with pytest.raises(ValueError, match=r"Unknown parameter\(s\)") as error: + template.validate_params({key: value}) + assert key in str(error.value) + + +@pytest.mark.parametrize("missing", ["column", "type"]) +@PROPERTY_SETTINGS +@given(column=IDENTIFIERS) +def test_generated_output_columns_require_both_declared_fields(missing: str, column: str) -> None: + schema = {"column": column, "type": "int"} + del schema[missing] + with pytest.raises(ValueError, match="must declare a non-empty"): + _load_template({"query": "SELECT 1", "output_schema": [schema]}) + + +@pytest.mark.parametrize("field", ["units", "metric_semantics", "scope"]) +@PROPERTY_SETTINGS +@given(value=VALUES) +def test_generated_invalid_enum_metadata_is_rejected(field: str, value: object) -> None: + # A prefix constructs an invalid enum without filtering or discarding cases. + malformed = f"unsupported:{value!r}" if isinstance(value, str) else value + with pytest.raises(ValueError, match=f"{field} must be one of"): + _load_template( + { + "query": "SELECT 1", + "output_schema": [{"column": "id", "type": "int", field: malformed}], + } + ) + + +@PROPERTY_SETTINGS +@given( + value=st.one_of( + st.booleans(), + st.integers(-1000, 0), + st.text(max_size=20), + st.lists(st.integers(-1000, 1000), max_size=4), + ) +) +def test_generated_invalid_semantics_versions_are_rejected(value: object) -> None: + with pytest.raises(ValueError, match="semantics_version.*positive integer"): + _load_template({"query": "SELECT 1", "semantics_version": value}) + + +@pytest.mark.parametrize( + "defect", ["root_sequence", "queries_sequence", "scalar_entry", "parameters_sequence", "syntax"] +) +@PROPERTY_SETTINGS +@given(payload=IDENTIFIERS) +def test_generated_malformed_yaml_is_skipped_without_losing_valid_neighbors( + defect: str, payload: str +) -> None: + malformed = { + "root_sequence": yaml.safe_dump([payload]), + "queries_sequence": yaml.safe_dump({"queries": [payload]}), + "scalar_entry": yaml.safe_dump({"queries": {"invalid": payload}}), + "parameters_sequence": yaml.safe_dump({"queries": {"invalid": {"parameters": [payload]}}}), + "syntax": f"queries: [{payload}", + }[defect] + with TemporaryDirectory() as directory, pytest.MonkeyPatch.context() as monkeypatch: + root = Path(directory) + # A detached registry snapshot keeps generated registrations local to + # this example and restores both normal entries and lazy factories. + monkeypatch.setattr(registry, "_registry", copy.deepcopy(registry._registry)) + invalid_name = f"property_invalid_{payload}" + valid_name = f"property_valid_{payload}" + registry._registry.unregister(invalid_name) + registry._registry.unregister(valid_name) + malformed = malformed.replace("invalid:", f"{invalid_name}:") + bad = root / "a_bad.yaml" + bad.write_text(malformed, encoding="utf-8") + (root / "z_good.yaml").write_text( + yaml.safe_dump({"queries": {valid_name: {"query": "SELECT 1 AS id"}}}), + encoding="utf-8", + ) + with pytest.warns(UserWarning, match="Failed to load query template") as warnings: + registry._load_yaml_templates(root) + assert len(warnings) == 1 + assert str(bad) in str(warnings[0].message) + assert registry.get_query(invalid_name) is None + valid = registry.get_query(valid_name) + assert valid is not None + assert valid.query == "SELECT 1 AS id" From d98c95b97c98326d3ee6fcc7d6fdee257648295f Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:55:06 +0800 Subject: [PATCH 03/12] test: keep runner fixture defaults and package formatting lint-clean --- tests/test_package.py | 21 ++++++++++++--------- tests/test_test_runner.py | 4 +++- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/tests/test_package.py b/tests/test_package.py index 58e8633..edb5e64 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -192,12 +192,15 @@ def test_distribution_guard_rejects_rediscovered_removed_content(name, entry): def test_distribution_guard_allows_unrelated_modules_and_metadata(): - assert _removed_distribution_entries( - [ - "pt_snap_cli/cli.py", - "pt_snap_cli/mcp_tools.py", - "pt_snap_cli/vendor_tools/reader.py", - "pt_snap_cli-1.0.dist-info/METADATA", - "pt_snap_cli-1.0.dist-info/licenses/LICENSE", - ] - ) == [] + assert ( + _removed_distribution_entries( + [ + "pt_snap_cli/cli.py", + "pt_snap_cli/mcp_tools.py", + "pt_snap_cli/vendor_tools/reader.py", + "pt_snap_cli-1.0.dist-info/METADATA", + "pt_snap_cli-1.0.dist-info/licenses/LICENSE", + ] + ) + == [] + ) diff --git a/tests/test_test_runner.py b/tests/test_test_runner.py index 44730b8..768f71e 100644 --- a/tests/test_test_runner.py +++ b/tests/test_test_runner.py @@ -59,7 +59,9 @@ def shell_runner(tmp_path): RUNNER_TEST_LOG=str(log), ) - def run(*args, exit_code=0, python=Path(sys.executable).as_posix()): + current_python = Path(sys.executable).as_posix() + + def run(*args, exit_code=0, python=current_python): selected_env = dict(env, RUNNER_TEST_EXIT=str(exit_code)) if python is None: selected_env.pop("PYTHON", None) From 7210555c33adbbf31faf7faddaa4fc6b6fad6e1d Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:00:39 +0800 Subject: [PATCH 04/12] test: enforce semantic CI gates and visible browser execution (#278, #279) --- .github/workflows/AGENTS.md | 23 +- .github/workflows/platform.yml | 3 +- .github/workflows/test.yml | 3 +- tests/_browser_support.py | 129 ++++++ tests/_ci_contract.py | 408 +++++++++++++++++++ tests/conftest.py | 2 + tests/test_browser_execution_contract.py | 169 ++++++++ tests/test_ci_contract_mutations.py | 414 ++++++++++++++++++++ tests/test_ci_tiers.py | 475 ++++++++++++++++++----- tests/test_memory_tree_browser.py | 172 ++++---- 10 files changed, 1590 insertions(+), 208 deletions(-) create mode 100644 tests/_browser_support.py create mode 100644 tests/_ci_contract.py create mode 100644 tests/test_browser_execution_contract.py create mode 100644 tests/test_ci_contract_mutations.py diff --git a/.github/workflows/AGENTS.md b/.github/workflows/AGENTS.md index ab7d111..91fb4c0 100644 --- a/.github/workflows/AGENTS.md +++ b/.github/workflows/AGENTS.md @@ -24,6 +24,26 @@ Concurrency is keyed by GitHub ref and superseded runs are cancelled. Million-event deep-offset pagination tests are marked `slow`; their small pagination contracts still run across the compatibility matrix. +`tests/test_ci_tiers.py` checks parsed commands, matrix rows, marker truth tables, +conditions, independent file floors and gate ordering, not step display names or +complete shell text. `tests/_ci_contract.py` intentionally supports only a bounded +CI language: simple failure-propagating commands, shell quoting/continuations, +sequential steps or a terminal `&&` chain, and explicit boolean conditions. +Unknown shell control flow, OR/pipes, failure suppression, selector overrides and +unrecognized conditions fail closed. Extend it with positive and negative +mutation tests when introducing new syntax; do not weaken it into substring +checks. Action version bumps, cache configuration, renaming, equivalent selectors +and step splits/merges do not change a gate. + +The Linux browser job installs Chromium and sets `PT_SNAP_REQUIRE_BROWSER=1`. +Browser cases are always collected even if Playwright is absent. Optional matrix +and platform runs report each missing-dependency case as skipped plus an explicit +**not executed** summary; that is not browser-pass evidence. Required mode fails +for missing package/binary, any browser skip, empty/deselected collection or +incomplete execution. `--collect-only` remains a collection diagnostic and is +explicitly reported as not run. Broken installations and launch failures fail in +both modes. Browser results are not added to non-browser platform requirements. + Coverage reporting writes `.coverage`/`coverage.xml`; building writes `dist/` and intermediates. Dependency installation/build isolation may access the network. The [acceptance script](../scripts/clean_wheel_agent_acceptance.py) uses temporary @@ -93,7 +113,8 @@ validation. For non-PR provenance checks, tag runs compare against the prior tag Review `pyproject.toml`, both workflow YAMLs, the provenance/acceptance scripts and `tests/test_release_workflow.py` together when changing execution order. Validate YAML and run `pytest tests/test_governance.py tests/test_release_workflow.py -tests/test_ci_tiers.py tests/test_dataset_acceptance_runner.py tests/test_wheel_audit.py` from +tests/test_ci_tiers.py tests/test_ci_contract_mutations.py +tests/test_browser_execution_contract.py tests/test_dataset_acceptance_runner.py tests/test_wheel_audit.py` from the repository root. Full CI tests the matrix and installed wheel; static tests do not prove a release environment is configured or a publication occurred. diff --git a/.github/workflows/platform.yml b/.github/workflows/platform.yml index 61d62c7..01fe2b1 100644 --- a/.github/workflows/platform.yml +++ b/.github/workflows/platform.yml @@ -56,7 +56,8 @@ jobs: run: python -m pytest tests/test_fixture_provenance.py - name: Run exhaustive non-slow partition # No coverage denominator or scale matrix here. Resource-warning gates - # remain enabled; required Chromium interaction runs in the Linux job. + # remain enabled; browser cases stay collected and explicitly report + # optional skips. Real Chromium execution is required in the Linux job. run: ${{ matrix.command }} - name: Upload platform test results if: always() diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6b7b2be..e65bbe7 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -96,7 +96,8 @@ jobs: - name: Run required Chromium interaction tests env: PT_SNAP_REQUIRE_BROWSER: "1" - run: pytest tests/test_fixture_provenance.py tests/test_memory_tree_browser.py + # Required mode rejects missing dependencies, skips and incomplete execution. + run: pytest -rs tests/test_fixture_provenance.py tests/test_memory_tree_browser.py test: name: Test (Python ${{ matrix.python-version }}) diff --git a/tests/_browser_support.py b/tests/_browser_support.py new file mode 100644 index 0000000..3ff3063 --- /dev/null +++ b/tests/_browser_support.py @@ -0,0 +1,129 @@ +"""Optional-browser fixtures and visible per-test execution accounting. + +Import Playwright only during fixture setup: absent optional dependencies must +never remove browser regression identities from pytest's collection. +""" + +from __future__ import annotations + +import importlib +import os +from pathlib import Path + +import pytest + + +def _unavailable(reason): + message = f"Browser regressions NOT EXECUTED: {reason}" + if os.environ.get("PT_SNAP_REQUIRE_BROWSER") == "1": + pytest.fail(message, pytrace=False) + pytest.skip(message) + + +@pytest.fixture(scope="session") +def playwright_api(): + try: + return importlib.import_module("playwright.sync_api") + except ModuleNotFoundError as exc: + # A broken installation (for example missing greenlet) is not optional. + if exc.name not in {"playwright", "playwright.sync_api"}: + raise + _unavailable("Playwright is not installed; install the .[dev] extra") + + +@pytest.fixture(scope="module") +def chromium_browser(playwright_api): + with playwright_api.sync_playwright() as runtime: + executable = os.environ.get("PT_SNAP_TEST_CHROMIUM") + binary = executable or runtime.chromium.executable_path + if not Path(binary).is_file(): + _unavailable( + "Chromium executable is missing; run python -m playwright install chromium " + "or set PT_SNAP_TEST_CHROMIUM to an existing executable" + ) + options = {"headless": True} + if executable: + options["executable_path"] = executable + try: + browser = runtime.chromium.launch(**options) + except playwright_api.Error as exc: + # Only the explicit preflight above can skip. Crashes, permission + # errors, bad shared libraries and other launch failures always fail. + pytest.fail(f"Cannot launch Chromium for frame navigation tests: {exc}") + try: + yield browser + finally: + browser.close() + + +def browser_expect(locator): + """Assertions run only after browser setup has succeeded.""" + return importlib.import_module("playwright.sync_api").expect(locator) + + +class BrowserSummary: + def __init__(self): + self.nodes = set() + self.outcomes = {} + self.skip_reasons = set() + + @pytest.hookimpl(hookwrapper=True, tryfirst=True) + def pytest_collection_modifyitems(self, items): + self.nodes = {item.nodeid for item in items if item.get_closest_marker("browser")} + yield + + def pytest_runtest_logreport(self, report): + if report.nodeid not in self.nodes: + return + if report.failed: + self.outcomes[report.nodeid] = "failed" + elif self.outcomes.get(report.nodeid) != "failed": + if report.skipped: + self.outcomes[report.nodeid] = "skipped" + reason = ( + report.longrepr[2] if isinstance(report.longrepr, tuple) else report.longrepr + ) + self.skip_reasons.add(str(reason)) + elif report.when == "call": + self.outcomes[report.nodeid] = "passed" + + def pytest_sessionfinish(self, session): + if os.environ.get("PT_SNAP_REQUIRE_BROWSER") != "1" or session.config.option.collectonly: + return + incomplete = not self.nodes or len(self.outcomes) < len(self.nodes) + if (incomplete or "skipped" in self.outcomes.values()) and session.exitstatus in { + pytest.ExitCode.OK, + pytest.ExitCode.NO_TESTS_COLLECTED, + }: + session.exitstatus = pytest.ExitCode.TESTS_FAILED + + def pytest_terminal_summary(self, terminalreporter): + if not self.nodes and os.environ.get("PT_SNAP_REQUIRE_BROWSER") != "1": + return + counts = { + outcome: list(self.outcomes.values()).count(outcome) + for outcome in ("passed", "skipped", "failed") + } + mode = "required" if os.environ.get("PT_SNAP_REQUIRE_BROWSER") == "1" else "optional" + terminalreporter.section(f"Browser regression execution ({mode})") + terminalreporter.write_line( + f"collected={len(self.nodes)} passed={counts['passed']} skipped={counts['skipped']} " + f"failed={counts['failed']} not_run={len(self.nodes) - len(self.outcomes)}" + ) + if not self.nodes or len(self.outcomes) < len(self.nodes): + terminalreporter.write_line( + "Browser regressions NOT EXECUTED: collection/deselection or early termination is not browser execution." + ) + if counts["skipped"]: + terminalreporter.write_line( + "Browser regressions NOT EXECUTED for skipped cases; this is not a browser pass." + ) + for reason in sorted(self.skip_reasons): + terminalreporter.write_line(reason) + + +def pytest_configure(config): + config.addinivalue_line( + "markers", "browser: real Chromium interaction regressions (required in Linux browser CI)" + ) + config.pluginmanager.register(BrowserSummary(), "browser-execution-summary") diff --git a/tests/_ci_contract.py b/tests/_ci_contract.py new file mode 100644 index 0000000..00db830 --- /dev/null +++ b/tests/_ci_contract.py @@ -0,0 +1,408 @@ +"""Small, fail-closed semantic reader for this repository's CI gate language. + +This is deliberately not a general shell or GitHub Actions interpreter. Harmless +formatting/quoting, action versions, boolean rearrangements and sequential step +splits/merges are supported. New shell control flow or expression features need +an explicit contract extension and negative tests, not substring acceptance. +""" + +from __future__ import annotations + +import ast +import itertools +import re +import shlex +from dataclasses import dataclass + +EVENTS = ("pull_request", "push", "workflow_call", "schedule", "workflow_dispatch") +PYTHONS = ("3.10", "3.11", "3.12", "3.13") + + +def require(condition, message): + if not condition: + raise AssertionError(message) + + +def expression(source, values, status="success"): + """Evaluate the supported GHA grammar, including its quoting and precedence. + + GitHub string equality ignores case; quotes escape by doubling apostrophes, + not by Python backslash escapes. Mixed-type loose coercion is unsupported. + Reference: https://docs.github.com/en/actions/reference/workflows-and-actions/expressions + """ + if source is None: + return status == "success" + source = str(source).strip() + if source.startswith("${{") and source.endswith("}}"): + source = source[3:-2].strip() + pattern = re.compile(r"\s*(?:('(?:[^']|'')*')|([\w.-]+)|(&&|\|\||==|!=|!|\(|\)))") + position = 0 + tokens = [] + uses_status = False + while position < len(source): + match = pattern.match(source, position) + require(match is not None, f"Unsupported CI condition: {source}") + literal, name, operator = match.groups() + position = match.end() + if literal is not None: + value = literal[1:-1].replace("''", "'") + require(value.isascii(), "Non-ASCII CI comparison literals need explicit support") + tokens.append(("value", value)) + elif name is not None: + if name in {"always", "success", "failure", "cancelled"}: + call = re.match(r"\s*\(\s*\)", source[position:]) + require(call is not None, "Status predicate must be called") + position += call.end() + uses_status = True + tokens.append(("value", name == "always" or name == status)) + elif name in {"true", "false"}: + tokens.append(("value", name == "true")) + else: + require(name in values, f"Unsupported CI context: {name}") + tokens.append(("value", values[name])) + else: + tokens.append((operator, None)) + index = 0 + + def take(kind): + nonlocal index + if index < len(tokens) and tokens[index][0] == kind: + index += 1 + return True + return False + + def unary(): + nonlocal index + if take("!"): + return not unary() + if take("("): + value = disjunction() + require(take(")"), f"Unclosed CI condition: {source}") + return value + require(index < len(tokens) and tokens[index][0] == "value", f"Expected CI value: {source}") + value = tokens[index][1] + index += 1 + return value + + def comparison(): + left = unary() + operator = tokens[index][0] if index < len(tokens) else None + if operator in {"==", "!="}: + take(operator) + right = unary() + require(type(left) is type(right), "Mixed-type CI coercion is unsupported") + if isinstance(left, str): + require( + left.isascii() and right.isascii(), + "Non-ASCII CI comparison needs explicit support", + ) + left, right = left.lower(), right.lower() + else: + require(isinstance(left, bool), "Only string/bool CI comparisons are supported") + return (left == right) if operator == "==" else (left != right) + return left + + def conjunction(): + value = comparison() + while take("&&"): + right = comparison() + value = right if value else value + return value + + def disjunction(): + value = conjunction() + while take("||"): + right = conjunction() + value = value if value else right + return value + + value = disjunction() + require(index == len(tokens), f"Unsupported CI condition: {source}") + return bool(value) and (uses_status or status == "success") + + +def matrix_rows(matrix): + axes = {key: value for key, value in matrix.items() if key not in {"include", "exclude"}} + require( + all(isinstance(value, list) and value for value in axes.values()), + "Matrix axes must be explicit nonempty lists", + ) + originals = ( + [dict(zip(axes, values, strict=True)) for values in itertools.product(*axes.values())] + if axes + else [] + ) + # GitHub applies excludes before includes, which may add excluded rows back. + originals = [ + row + for row in originals + if not any( + all(row.get(key) == value for key, value in exclusion.items()) + for exclusion in matrix.get("exclude", []) + ) + ] + rows = [dict(row) for row in originals] + for addition in matrix.get("include", []): + matched = False + for index, original in enumerate(originals): + row = rows[index] + if all( + key not in original or original[key] == value for key, value in addition.items() + ): + row.update(addition) + matched = True + if not matched: + rows.append(dict(addition)) + # Includes do not form new Cartesian base combinations. + return rows + + +WHEEL_PROGRAM = """from pathlib import Path; wheels = sorted(Path("dist").glob("*.whl")); assert len(wheels) == 1, wheels; print(wheels[0])""" + + +def _canonical_program(program): + try: + return ast.dump(ast.parse(program)) + except SyntaxError as exc: + raise AssertionError("Invalid wheel-selection program") from exc + + +def command_tokens(script): + """Read failure-propagating simple commands, never search inside run text.""" + script = script.replace("\\\n", " ") + + def wheel(match): + name, invocation = match.groups() + words = shlex.split(invocation) + require( + len(words) == 3 and words[0] in {"python", "python3"} and words[1] == "-c", + "Unsupported wheel selection", + ) + require( + _canonical_program(words[2]) == _canonical_program(WHEEL_PROGRAM), + "Wheel selection must require exactly one built wheel", + ) + return f"__verified_wheel__ {name}" + + script = re.sub(r"(?m)^\s*([A-Z_][A-Z_0-9]*)=\$\((.+)\)\s*$", wheel, script) + # The interpreter path probe is one audited, side-effect-free substitution. + script = script.replace( + "\"$(python -c 'import sys; print(sys.executable)')\"", '"__selected_python__"' + ) + tokens = [] + for line in script.splitlines(): + lexer = shlex.shlex(line, posix=True, punctuation_chars=";&|") + lexer.whitespace_split = True + lexer.commenters = "#" + tokens.extend(lexer) + tokens.append("\n") + commands, current, separators = [], [], [] + for token in tokens + ["\n"]: + if token and all(char in ";&|\n" for char in token): + require( + "|" not in token and token.replace("\n", "") in {"", ";", "&&"}, + "Shell OR/pipes/background/control flow can hide a failed gate", + ) + if current: + commands.append(current) + current = [] + separators.append(token.replace("\n", "") or ";") + else: + current.append(token) + # `a && b; c` suppresses errexit for a. Only a single terminal AND chain is safe. + if "&&" in separators: + require( + all(separator == "&&" for separator in separators[:-1]), + "Mixed AND/sequential shell lists can ignore failures", + ) + result = [] + for words in commands: + if words[0] == "set": + require( + words[1:] in [["-euo", "pipefail"], ["-eu"], ["-e"]], + "Shell failure handling must not be disabled", + ) + continue + if words[0] == "__verified_wheel__": + result.append(tuple(words)) + continue + require( + not any( + any(char in word for char in ("`", ">", "<")) or "$(" in word for word in words + ), + "Unsupported shell substitution/redirection", + ) + if words[0] in {"python", "python3", "python3.13"}: + words = words[2:] if len(words) > 2 and words[1] == "-m" else words[1:] + require( + words + and words[0] + in { + "pytest", + "pip", + "ruff", + "black", + "basedpyright", + "coverage", + "build", + "playwright", + ".github/scripts/audit_wheel.py", + ".github/scripts/clean_wheel_agent_acceptance.py", + ".github/scripts/run_dataset_acceptance.py", + ".github/scripts/run_differential_acceptance.py", + ".github/scripts/check_snapshot_provenance.py", + ".github/scripts/check_critical_coverage.py", + }, + f"Unsupported gate command: {words}", + ) + result.append(tuple(words)) + return result + + +def options(words, valued=(), flags=()): + result, positional = {}, [] + index = 0 + while index < len(words): + word = words[index] + if word.startswith("-"): + key, separator, value = word.partition("=") + require(key in valued or key in flags, f"Unsupported gate option: {word}") + if key in valued and not separator: + index += 1 + require(index < len(words), f"Missing value: {key}") + value = words[index] + if key in flags: + require(not separator, f"Flag does not take a value: {key}") + value = True + result.setdefault(key, []).append(value) + else: + positional.append(word) + index += 1 + return result, positional + + +@dataclass +class Command: + words: tuple[str, ...] + step: dict + index: int + env: dict + + +def commands(job, workflow=None): + require(job.get("continue-on-error", "false") == "false", "Jobs must propagate gate failures") + require(not job.get("needs"), "Independent gates cannot depend on skippable jobs") + defaults = (workflow or {}).get("defaults", {}).get("run", {}) | job.get("defaults", {}).get( + "run", {} + ) + require(not defaults.get("working-directory"), "Gate working directory must be repository root") + result = [] + for step in job["steps"]: + require( + step.get("continue-on-error", "false") == "false", "Steps must propagate gate failures" + ) + if "run" not in step: + continue + require( + step.get("shell", defaults.get("shell", "bash")) == "bash", + "Gate shell must retain bash errexit", + ) + require(not step.get("working-directory"), "Gate working directory must be repository root") + env = (workflow or {}).get("env", {}) | job.get("env", {}) | step.get("env", {}) + require( + not any( + key.startswith(("PYTEST_", "COVERAGE_")) + or key + in { + "BASH_ENV", + "ENV", + "PATH", + "PYTHONPATH", + "PYTHONHOME", + "VIRTUAL_ENV", + "PYTHONOPTIMIZE", + "PYTHONWARNINGS", + } + for key in env + ), + "Environment must not override gate execution", + ) + for words in command_tokens(step["run"]): + result.append(Command(words, step, len(result), env)) + return result + + +def select(commands_, program): + return [command for command in commands_ if command.words[0] == program] + + +def one(commands_, program): + matching = select(commands_, program) + require(len(matching) == 1, f"Expected exactly one {program} gate, got {len(matching)}") + return matching[0] + + +def condition_matches( + job, step, *, versions=PYTHONS, events=EVENTS, predicate, statuses=("success",) +): + for version, event, status in itertools.product(versions, events, statuses): + values = {"matrix.python-version": version, "github.event_name": event} + actual = expression(job.get("if"), values, status) and expression( + step.get("if"), values, status + ) + # Job-level default success does not suppress always() artifact steps in + # an already-running job after a step failure. + if status != "success": + actual = expression(job.get("if"), values, "success") and expression( + step.get("if"), values, status + ) + require( + actual == predicate(version, event, status), + f"Wrong gate condition for Python {version}, {event}, {status}: {step.get('if')}", + ) + + +def marker_truth(source, slow, nightly): + tree = ast.parse(source, mode="eval") + + def evaluate(node): + if isinstance(node, ast.Name): + require(node.id in {"slow", "nightly"}, f"Unexpected marker selector: {node.id}") + return {"slow": slow, "nightly": nightly}[node.id] + if isinstance(node, ast.UnaryOp) and isinstance(node.op, ast.Not): + return not evaluate(node.operand) + if isinstance(node, ast.BoolOp): + values = [evaluate(value) for value in node.values] + return all(values) if isinstance(node.op, ast.And) else any(values) + raise AssertionError(f"Unsupported pytest marker selector: {source}") + + return evaluate(tree.body) + + +def pytest_options(command, expected_marker=None, paths=(), coverage=False): + opts, positional = options( + command.words[1:], + valued=("-m", "--cov-report"), + flags=("--cov", "-q", "-v", "-vv", "-rs", "-ra"), + ) + require( + set(positional) == set(paths) and len(positional) == len(paths), + "Pytest paths must cover the complete intended gate", + ) + require( + opts.get("--cov", []) == ([True] if coverage else []), + "Coverage must run only on the full tier", + ) + if expected_marker is None: + require("-m" not in opts, "Required test paths cannot be marker-filtered") + else: + require(len(opts.get("-m", [])) == 1, "Exactly one pytest marker expression is required") + for slow, nightly in itertools.product((False, True), repeat=2): + require( + marker_truth(opts["-m"][0], slow, nightly) == expected_marker(slow, nightly), + "Pytest marker selector changes tier membership", + ) + require( + set(opts.get("--cov-report", [])) == ({"xml", "term-missing"} if coverage else set()), + "Coverage outputs changed", + ) diff --git a/tests/conftest.py b/tests/conftest.py index 587bc27..11cd944 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -7,6 +7,8 @@ from tests._fixture_provenance import FixtureProvenanceError, verify_snapshot_fixtures +pytest_plugins = ("tests._browser_support",) + def pytest_sessionstart(session: pytest.Session) -> None: """Stop before collection can execute a changed or unreviewed pickle.""" diff --git a/tests/test_browser_execution_contract.py b/tests/test_browser_execution_contract.py new file mode 100644 index 0000000..c9c3cde --- /dev/null +++ b/tests/test_browser_execution_contract.py @@ -0,0 +1,169 @@ +"""Collection and exit-code contracts without requiring a downloaded browser.""" + +from __future__ import annotations + +import ast +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +BROWSER_TEST = ROOT / "tests/test_memory_tree_browser.py" + + +def _browser_nodes(): + return { + node.name + for node in ast.parse(BROWSER_TEST.read_text(encoding="utf-8")).body + if isinstance(node, ast.FunctionDef) and node.name.startswith("test_") + } + + +def _run_unavailable(tmp_path, mode, *, required=False, collect_only=False, extra_args=()): + # Copy the actual regression module, not a stand-in with fewer tests. No + # fixture bytes, browser downloads, or production behavior are exercised. + (tmp_path / "test_browser_copy.py").write_text( + BROWSER_TEST.read_text(encoding="utf-8"), encoding="utf-8" + ) + (tmp_path / "pytest.ini").write_text( + "[pytest]\nmarkers = browser: browser regression\n", encoding="utf-8" + ) + (tmp_path / "conftest.py").write_text( + """ +import importlib +import importlib.abc +import sys +from contextlib import nullcontext +from pathlib import Path +from types import SimpleNamespace + +import pytest + +pytest_plugins = ("tests._browser_support",) + +original = importlib.import_module + +def pytest_configure(config): + binary = Path(__file__).parent / "fake-chromium" + if MODE == "broken_browser": + binary.touch() + class BrowserError(Exception): + pass + def launch(**kwargs): + raise BrowserError("synthetic browser launch failure") + runtime = SimpleNamespace(chromium=SimpleNamespace(executable_path=str(binary), launch=launch)) + api = SimpleNamespace(sync_playwright=lambda: nullcontext(runtime), Error=BrowserError) + def load(name, *args, **kwargs): + if name != "playwright.sync_api": + return original(name, *args, **kwargs) + if MODE in {"missing_package", "broken_package"}: + dependency = "playwright" if MODE == "missing_package" else "greenlet" + raise ModuleNotFoundError("No module named " + dependency, name=dependency) + return api + importlib.import_module = load + if MODE == "missing_package": + class MissingPlaywright(importlib.abc.MetaPathFinder): + def find_spec(self, fullname, path=None, target=None): + if fullname == "playwright" or fullname.startswith("playwright."): + raise ModuleNotFoundError("No module named playwright", name="playwright") + sys.meta_path.insert(0, MissingPlaywright()) + +def pytest_unconfigure(config): + importlib.import_module = original +""".replace("MODE", repr(mode)), + encoding="utf-8", + ) + env = os.environ.copy() + for key in ( + "PT_SNAP_REQUIRE_BROWSER", + "PT_SNAP_TEST_CHROMIUM", + "PYTEST_ADDOPTS", + "PYTEST_PLUGINS", + ): + env.pop(key, None) + env["PYTHONPATH"] = os.pathsep.join((str(ROOT), str(ROOT / "src"))) + env["PYTEST_DISABLE_PLUGIN_AUTOLOAD"] = "1" + if required: + env["PT_SNAP_REQUIRE_BROWSER"] = "1" + args = [sys.executable, "-m", "pytest", "test_browser_copy.py", "-v", "-rs", "--tb=short"] + args.extend(extra_args) + if collect_only: + args.append("--collect-only") + return subprocess.run(args, cwd=tmp_path, env=env, capture_output=True, text=True, timeout=60) + + +@pytest.mark.parametrize("required", [False, True], ids=["optional", "required"]) +def test_browser_collection_keeps_every_real_regression_without_playwright(tmp_path, required): + result = _run_unavailable(tmp_path, "missing_package", required=required, collect_only=True) + nodes = _browser_nodes() + assert len(nodes) >= 12, "Existing browser regressions must not disappear" + assert result.returncode == 0, result.stdout + result.stderr + for node in nodes: + assert node in result.stdout + assert ( + f"collected={len(nodes)} passed=0 skipped=0 failed=0 not_run={len(nodes)}" in result.stdout + ) + + +@pytest.mark.parametrize("mode", ["missing_package", "missing_binary"]) +@pytest.mark.parametrize("required", [False, True], ids=["optional", "required"]) +def test_missing_browser_dependencies_are_visible_and_required_mode_fails(tmp_path, mode, required): + result = _run_unavailable(tmp_path, mode, required=required) + count = len(_browser_nodes()) + assert result.returncode == (1 if required else 0), result.stdout + result.stderr + for node in _browser_nodes(): + assert node in result.stdout + if required: + assert f"collected={count} passed=0 skipped=0 failed={count} not_run=0" in result.stdout + else: + assert f"collected={count} passed=0 skipped={count} failed=0 not_run=0" in result.stdout + assert "Browser regressions NOT EXECUTED" in result.stdout + assert ( + "Playwright is not installed" + if mode == "missing_package" + else "Chromium executable is missing" + ) in result.stdout + + +@pytest.mark.parametrize("mode", ["broken_package", "broken_browser"]) +def test_broken_browser_installations_never_become_optional_skips(tmp_path, mode): + result = _run_unavailable(tmp_path, mode) + count = len(_browser_nodes()) + assert result.returncode == 1, result.stdout + result.stderr + assert f"collected={count} passed=0 skipped=0 failed={count} not_run=0" in result.stdout + assert ( + "greenlet" if mode == "broken_package" else "synthetic browser launch failure" + ) in result.stdout + + +def test_required_mode_rejects_deselected_browser_execution(tmp_path): + result = _run_unavailable( + tmp_path, + "missing_package", + required=True, + extra_args=("-k", "nothing_matches_this_selector"), + ) + count = len(_browser_nodes()) + assert result.returncode == 1, result.stdout + result.stderr + assert f"collected={count} passed=0 skipped=0 failed=0 not_run={count}" in result.stdout + + +def test_required_summary_fails_for_skips_and_incomplete_execution(monkeypatch): + from types import SimpleNamespace + + from tests._browser_support import BrowserSummary + + monkeypatch.setenv("PT_SNAP_REQUIRE_BROWSER", "1") + for nodes, outcomes in ((set(), {}), ({"one"}, {}), ({"one"}, {"one": "skipped"})): + summary = BrowserSummary() + summary.nodes = nodes + summary.outcomes = outcomes + session = SimpleNamespace( + config=SimpleNamespace(option=SimpleNamespace(collectonly=False)), + exitstatus=pytest.ExitCode.OK, + ) + summary.pytest_sessionfinish(session) + assert session.exitstatus == pytest.ExitCode.TESTS_FAILED diff --git a/tests/test_ci_contract_mutations.py b/tests/test_ci_contract_mutations.py new file mode 100644 index 0000000..a579e2a --- /dev/null +++ b/tests/test_ci_contract_mutations.py @@ -0,0 +1,414 @@ +"""Acceptance and adversarial mutations for the bounded CI semantic reader.""" + +from copy import deepcopy + +import pytest + +from tests._ci_contract import command_tokens, commands, expression, matrix_rows +from tests.test_ci_tiers import project, validate_workflow, workflow + + +def _step(config, job, program, contains=None): + matching = [ + command.step + for command in commands(config["jobs"][job], config) + if command.words[0] == program and (contains is None or contains in command.words) + ] + assert matching + return matching[0] + + +def _rewrite(config, job, program, before, after, contains=None): + step = _step(config, job, program, contains) + assert before in step["run"] + step["run"] = step["run"].replace(before, after) + + +def _renamed(config): + for job in config["jobs"].values(): + job["name"] = "A display label, not a contract" + for step in job["steps"]: + step.pop("name", None) + + +def _actions_and_cache(config): + for job in config["jobs"].values(): + for step in job["steps"]: + if "uses" in step: + step["uses"] = step["uses"].split("@")[0] + "@v99" + if step.get("uses", "").startswith("actions/setup-python@"): + step.setdefault("with", {})["cache"] = "pip" + + +def _quotes_and_commands(config): + _rewrite( + config, + "test", + "pytest", + 'pytest -m "not slow and not nightly"', + "python -m pytest -m 'not (nightly or slow)'", + "not slow and not nightly", + ) + _rewrite( + config, + "test", + "pytest", + 'pytest -m "not nightly" --cov --cov-report=xml --cov-report=term-missing', + "python3 -m pytest --cov-report term-missing --cov -m 'not nightly' --cov-report xml", + "--cov", + ) + guard = _step(config, "test", "coverage") + guard["run"] = ( + guard["run"] + .replace("--include='", '--include "') + .replace(".py'", '.py"') + .replace("--fail-under=", "--fail-under ") + ) + + +def _conditions(config): + _step(config, "test", "pytest", "--cov")["if"] = "${{ !(matrix.python-version != '3.13') }}" + _step(config, "test", "pytest", "not slow and not nightly")[ + "if" + ] = "('3.13' != matrix.python-version) && success()" + config["jobs"]["dataset-acceptance"][ + "if" + ] = "${{ github.event_name == 'workflow_dispatch' || ('schedule' == github.event_name) }}" + + +def _uppercase_events(config): + config["jobs"]["dataset-acceptance"][ + "if" + ] = "github.event_name == 'SCHEDULE' || github.event_name == 'WORKFLOW_DISPATCH'" + + +def _excluded_then_restored_matrix(config): + matrix = config["jobs"]["test"]["strategy"]["matrix"] + matrix["exclude"] = [{"python-version": "3.13"}] + matrix["include"] = [{"python-version": "3.13"}] + + +def _split_and_merge(config): + job = config["jobs"]["test"] + guard = _step(config, "test", "coverage") + index = job["steps"].index(guard) + parts = [dict(guard, run=line) for line in guard["run"].splitlines() if line.strip()] + job["steps"][index : index + 1] = parts + # Merge full pytest with the first independent floor; labels are irrelevant. + full = _step(config, "test", "pytest", "--cov") + full["run"] += "\n" + parts[0]["run"] + job["steps"].remove(parts[0]) + + +def _and_chain(config): + guard = _step(config, "test", "coverage") + guard["run"] = " && ".join(guard["run"].splitlines()) + + +def _include_matrix(config): + matrix = config["jobs"]["test"]["strategy"]["matrix"] + versions = matrix.pop("python-version") + matrix["include"] = [{"python-version": version} for version in versions] + + +@pytest.mark.parametrize( + "mutation", + [ + _renamed, + _actions_and_cache, + _quotes_and_commands, + _conditions, + _uppercase_events, + _excluded_then_restored_matrix, + _split_and_merge, + _and_chain, + _include_matrix, + ], + ids=lambda mutation: mutation.__name__.removeprefix("_"), +) +def test_semantically_equivalent_workflow_refactors_are_accepted(mutation): + config = workflow() + mutation(config) + validate_workflow(config, project()) + + +def _weaken(config, case): + job = config["jobs"]["test"] + full = _step(config, "test", "pytest", "--cov") + fast = _step(config, "test", "pytest", "not slow and not nightly") + guard = _step(config, "test", "coverage") + browser = _step(config, "browser", "pytest") + if case == "missing-python": + job["strategy"]["matrix"]["python-version"].remove("3.12") + elif case == "excluded-python": + job["strategy"]["matrix"]["exclude"] = [{"python-version": "3.12"}] + elif case == "extra-python": + job["strategy"]["matrix"]["include"] = [{"python-version": "3.14"}] + elif case == "wrong-matrix-interpreter": + next( + step for step in job["steps"] if step.get("uses", "").startswith("actions/setup-python") + )["with"]["python-version"] = "3.13" + elif case == "filtered-pr-trigger": + config["on"]["pull_request"]["paths"] = ["never-tests/**"] + elif case == "wrong-full-condition": + full["if"] = "matrix.python-version == '3.12'" + elif case == "uppercase-pr-exclusion": + full["if"] += " && github.event_name != 'PULL_REQUEST'" + elif case == "python-escape-condition": + full["if"] += r" && '\x61' == 'a'" + elif case == "wrong-unary-precedence": + fast["if"] = "!matrix.python-version == '3.13'" + elif case == "mixed-type-coercion": + full["if"] += " && (true && github.event_name) == true" + elif case == "wrong-provenance-base": + step = next( + step + for step in config["jobs"]["snapshot-provenance"]["steps"] + if "PR_BODY" in step.get("env", {}) + ) + step["env"]["BASE_SHA"] = "${{ github.event.pull_request.head.sha }}" + elif case == "full-or-bypass": + full["if"] += " || true" + elif case == "skip-full-job": + job["if"] = "github.event_name != 'pull_request'" + elif case == "skippable-dependency": + job["needs"] = "dataset-acceptance" + elif case == "ignored-job-failure": + job["continue-on-error"] = "true" + elif case == "ignored-step-failure": + full["continue-on-error"] = "true" + elif case == "disabled-errexit": + full["run"] = "set +e\n" + full["run"] + elif case == "custom-shell": + full["shell"] = "bash {0}" + elif case == "default-shell": + job["defaults"] = {"run": {"shell": "bash {0}"}} + elif case == "wrong-directory": + full["working-directory"] = "tests/core" + elif case == "hidden-pytest-selector": + full["env"] = {"PYTEST_ADDOPTS": "-k passing_only"} + elif case == "optimized-away-wheel-assert": + job["env"] = {"PYTHONOPTIMIZE": "1"} + elif case == "suppressed-resource-warnings": + full["env"] = {"PYTHONWARNINGS": "ignore"} + elif case == "redirected-pythonpath": + job["env"] = {"PYTHONPATH": "other-source"} + elif case == "redirected-path": + full["env"] = {"PATH": "fake-gate-binaries"} + elif case == "shell-or-true": + full["run"] += " || true" + elif case == "shell-pipe": + full["run"] += " | tee ignored.log" + elif case == "and-list-swallow": + full["run"] += " && pip --version\npip --version" + elif case == "wrong-fast-selector": + fast["run"] = fast["run"].replace("not slow and not nightly", "not slow or not nightly") + elif case == "full-excludes-slow": + full["run"] = full["run"].replace("not nightly", "not slow and not nightly") + elif case == "full-includes-nightly": + full["run"] = full["run"].replace("not nightly", "not nightly or slow") + elif case == "pytest-k-filter": + full["run"] += " -k passing_only" + elif case == "pytest-path-filter": + full["run"] += " tests/core" + elif case == "coverage-threshold": + guard["run"] = guard["run"].replace("--fail-under=90", "--fail-under=89") + elif case == "coverage-combined-selector": + guard["run"] = guard["run"].replace("*/core/snapshot_import_backend.py", "*/core/*.py") + elif case == "coverage-wrong-file": + guard["run"] = guard["run"].replace( + "*/core/snapshot_import_backend.py", "*/core/import_metadata.py" + ) + elif case == "coverage-before-measurement": + job["steps"].remove(guard) + job["steps"].insert(job["steps"].index(full), guard) + elif case == "fixture-after-tests": + fixture = _step(config, "test", "pytest", "tests/test_fixture_provenance.py") + job["steps"].remove(fixture) + job["steps"].append(fixture) + elif case == "optional-browser": + browser["env"]["PT_SNAP_REQUIRE_BROWSER"] = "0" + elif case == "filtered-browser": + browser["run"] += " -k zero_total" + elif case == "missing-browser-install": + install = _step(config, "browser", "playwright") + install["run"] = "pip install -e '.[dev]'" + elif case == "non-linux-browser": + config["jobs"]["browser"]["runs-on"] = "macos-latest" + elif case == "conditional-browser": + browser["if"] = "github.event_name == 'schedule'" + elif case == "nightly-runs-on-pr": + config["jobs"]["dataset-acceptance"].pop("if") + elif case == "nightly-not-manual": + config["jobs"]["dataset-acceptance"]["if"] = "github.event_name == 'schedule'" + elif case == "ambiguous-wheel": + audit = _step(config, "test", ".github/scripts/audit_wheel.py") + audit["run"] = audit["run"].replace( + "assert len(wheels) == 1, wheels", "assert len(wheels) >= 1, wheels" + ) + elif case == "cross-step-wheel-substitution": + acceptance = _step(config, "test", ".github/scripts/clean_wheel_agent_acceptance.py") + acceptance["run"] = "\n".join( + line for line in acceptance["run"].splitlines() if not line.startswith("WHEEL=") + ) + acceptance["env"] = {"WHEEL": "unrelated.whl"} + elif case == "wrong-wheel": + acceptance = _step(config, "test", ".github/scripts/clean_wheel_agent_acceptance.py") + acceptance["run"] = acceptance["run"].replace( + '--wheel "$WHEEL"', '--wheel "dist/unrelated.whl"' + ) + elif case == "missing-sharded-acceptance": + acceptance = _step(config, "test", ".github/scripts/clean_wheel_agent_acceptance.py") + acceptance["run"] = acceptance["run"].replace( + "--events-per-slice 4096", "--events-per-slice 8192" + ) + elif case == "missing-differential-seeds": + acceptance = _step( + config, "dataset-acceptance", ".github/scripts/run_differential_acceptance.py" + ) + acceptance["run"] = acceptance["run"].replace("--seed-count 200", "--seed-count 20") + elif case == "hidden-failed-evidence": + for step in config["jobs"]["dataset-acceptance"]["steps"]: + if step.get("uses", "").startswith("actions/upload-artifact"): + step.pop("if") + elif case == "checkout-main-not-pr": + job["steps"][0]["with"]["ref"] = "main" + else: + raise AssertionError(case) + + +@pytest.mark.parametrize( + "case", + [ + "missing-python", + "excluded-python", + "extra-python", + "wrong-matrix-interpreter", + "filtered-pr-trigger", + "wrong-full-condition", + "full-or-bypass", + "uppercase-pr-exclusion", + "python-escape-condition", + "wrong-unary-precedence", + "mixed-type-coercion", + "wrong-provenance-base", + "skip-full-job", + "skippable-dependency", + "ignored-job-failure", + "ignored-step-failure", + "disabled-errexit", + "custom-shell", + "default-shell", + "wrong-directory", + "hidden-pytest-selector", + "redirected-pythonpath", + "optimized-away-wheel-assert", + "suppressed-resource-warnings", + "redirected-path", + "shell-or-true", + "shell-pipe", + "and-list-swallow", + "wrong-fast-selector", + "full-excludes-slow", + "full-includes-nightly", + "pytest-k-filter", + "pytest-path-filter", + "coverage-threshold", + "coverage-combined-selector", + "coverage-wrong-file", + "coverage-before-measurement", + "fixture-after-tests", + "optional-browser", + "filtered-browser", + "missing-browser-install", + "non-linux-browser", + "conditional-browser", + "nightly-runs-on-pr", + "nightly-not-manual", + "ambiguous-wheel", + "wrong-wheel", + "cross-step-wheel-substitution", + "missing-sharded-acceptance", + "missing-differential-seeds", + "hidden-failed-evidence", + "checkout-main-not-pr", + ], +) +def test_workflow_gate_regressions_are_rejected(case): + config = workflow() + _weaken(config, case) + with pytest.raises(AssertionError): + validate_workflow(config, project()) + + +@pytest.mark.parametrize("field,value", [("branch", False), ("fail_under", 89)]) +def test_coverage_configuration_cannot_weaken(field, value): + settings = deepcopy(project()) + settings["tool"]["coverage"]["run" if field == "branch" else "report"][field] = value + with pytest.raises(AssertionError): + validate_workflow(workflow(), settings) + + +def test_include_matrix_rows_never_become_new_cartesian_originals(): + assert matrix_rows( + { + "python-version": ["3.10"], + "include": [{"python-version": "3.13"}, {"python-version": "3.13", "extra": "second"}], + } + ) == [ + {"python-version": "3.10"}, + {"python-version": "3.13"}, + {"python-version": "3.13", "extra": "second"}, + ] + assert matrix_rows( + { + "python-version": ["3.10", "3.13"], + "include": [{"cache": "pip"}], + "exclude": [{"python-version": "3.10"}], + } + ) == [{"python-version": "3.13", "cache": "pip"}] + + +@pytest.mark.parametrize( + "source", + [ + "contains(github.event_name, 'push')", + "unknown.context == 'yes'", + "success || true", + "github.event_name =~ 'push'", + ], +) +def test_unknown_condition_syntax_is_not_guessed(source): + with pytest.raises(AssertionError): + expression(source, {"github.event_name": "push"}) + + +@pytest.mark.parametrize( + "script", + [ + "pytest || true", + "pytest && pip --version; pip --version", + "if true; then pytest; fi", + "! pytest", + "pytest &", + "pytest > result.txt", + "set +e\npytest", + "exit 0\npytest", + "pytest() { return 0; }; pytest", + ], +) +def test_shell_failure_suppression_and_uninterpreted_control_flow_are_rejected(script): + with pytest.raises(AssertionError): + command_tokens(script) + + +def test_expression_semantics_match_github_not_python_literals_or_precedence(): + assert expression("'PUSH' == 'push'", {}) + assert expression("!'nonempty' == false", {}) + assert expression("'p''ush' != 'push'", {}) + assert not expression(r"'\x61' == 'a'", {}) + assert expression("'p''ush' == 'P''USH'", {}) + with pytest.raises(AssertionError, match="Mixed-type"): + expression("(true && 'push') == true", {}) + with pytest.raises(AssertionError): + expression("\"push\" == 'push'", {}) diff --git a/tests/test_ci_tiers.py b/tests/test_ci_tiers.py index f011586..0be3274 100644 --- a/tests/test_ci_tiers.py +++ b/tests/test_ci_tiers.py @@ -1,114 +1,381 @@ -"""Protect fast/full/nightly boundaries and the existing quality gates.""" +"""Protect CI behavior without coupling gates to human labels or shell quoting.""" from pathlib import Path import yaml +try: + import tomllib +except ModuleNotFoundError: # Python 3.10: also a pytest dependency. + import tomli as tomllib + +from tests._ci_contract import ( + EVENTS, + PYTHONS, + commands, + condition_matches, + matrix_rows, + one, + options, + pytest_options, + require, + select, +) + def workflow(): - return yaml.load(Path(".github/workflows/test.yml").read_text(), Loader=yaml.BaseLoader) - - -def named_steps(job): - return {step["name"]: step for step in job["steps"] if "name" in step} - - -def test_compatibility_matrix_leaves_slow_and_coverage_on_python_313(): - job = workflow()["jobs"]["test"] - assert job["strategy"]["matrix"]["python-version"] == ["3.10", "3.11", "3.12", "3.13"] - steps = named_steps(job) - fast = steps["Run non-slow compatibility tests"] - assert fast["if"] == "matrix.python-version != '3.13'" - assert fast["run"] == 'pytest -m "not slow and not nightly"' - full = steps["Run full PR tests with coverage"] - assert full["if"] == "matrix.python-version == '3.13'" - assert full["run"] == 'pytest -m "not nightly" --cov --cov-report=xml --cov-report=term-missing' - assert ( - 'markers = ["nightly: million-scale acceptance gate (scheduled/manual only)"]' - in Path("pyproject.toml").read_text() - ) - assert "if" not in steps["Verify executable fixture provenance"] - names = list(steps) - assert names.index("Verify executable fixture provenance") < names.index(fast["name"]) - assert names.index("Verify executable fixture provenance") < names.index(full["name"]) - - -def test_full_tier_preserves_independent_coverage_floors_and_packaging(): - steps = named_steps(workflow()["jobs"]["test"]) - guard = steps["Guard critical runtime coverage"] - assert guard["if"] == "matrix.python-version == '3.13'" - for file, floor in ( - ("core/snapshot_import_backend.py", 90), - ("simulate/simulated_caching_allocator.py", 88), - ("simulate/snapshot_mutator.py", 84), - ): - assert f"--include='*/{file}' --fail-under={floor}" in guard["run"] - config = Path("pyproject.toml").read_text() - assert "branch = true" in config and "fail_under = 90" in config - for name in ( - "Build wheel", - "Audit wheel contents against source", - "Clean-wheel Agent acceptance", - ): - assert steps[name]["if"] == "matrix.python-version == '3.13'" - assert steps["Upload coverage report"]["if"] == "matrix.python-version == '3.13' && always()" + return yaml.load( + Path(".github/workflows/test.yml").read_text(encoding="utf-8"), Loader=yaml.BaseLoader + ) -def test_ci_preserves_least_privilege_and_checks_the_published_commit(): - config = workflow() - assert config["permissions"] == {"contents": "read"} - assert "pull_request_target" not in config["on"] - assert config["on"]["pull_request"]["types"] == ["opened", "synchronize", "reopened", "edited"] - assert "workflow_call" in config["on"] - jobs = config["jobs"] - for name in ("snapshot-provenance", "lint", "browser", "test"): - checkout = jobs[name]["steps"][0] - assert checkout["uses"] == "actions/checkout@v4" - assert checkout["with"]["ref"] == "${{ github.event.pull_request.head.sha || github.sha }}" - assert jobs["snapshot-provenance"]["steps"][0]["with"]["fetch-depth"] == "0" - assert ( - "--require-decision" - in named_steps(jobs["snapshot-provenance"])[ - "Require provenance evaluation for snapshot runtime changes" - ]["run"] - ) - browser = named_steps(jobs["browser"])["Run required Chromium interaction tests"] - assert browser["env"]["PT_SNAP_REQUIRE_BROWSER"] == "1" - assert "tests/test_memory_tree_browser.py" in browser["run"] - lint = named_steps(jobs["lint"]) - for name in ("Ruff lint check", "Black format check", "basedPyright check"): - assert name in lint - - -def test_nightly_and_manual_gate_build_and_audit_before_required_acceptance(): - config = workflow() - assert "workflow_dispatch" in config["on"] - assert config["on"]["schedule"] == [{"cron": "17 3 * * *"}] - job = config["jobs"]["dataset-acceptance"] - assert ( - job["if"] == "github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'" - ) - assert job["timeout-minutes"] == "30" - assert job["steps"][0]["with"] == {"fetch-depth": "0", "ref": "${{ github.sha }}"} - steps = named_steps(job) - names = list(steps) - order = [ - "Verify executable fixture provenance", - "Build wheel", - "Audit wheel contents against source", - "Run required installed-wheel dataset acceptance", - "Upload dataset acceptance evidence", +def project(): + return tomllib.loads(Path("pyproject.toml").read_text(encoding="utf-8")) + + +def _action_steps(job, action): + return [step for step in job["steps"] if step.get("uses", "").partition("@")[0] == action] + + +def _full(job, step): + condition_matches(job, step, predicate=lambda version, event, status: version == "3.13") + + +def _all(job, step): + condition_matches(job, step, predicate=lambda version, event, status: True) + + +def _check_wheel_gate(job, all_commands, command, *, nightly=False): + opts, positional = options( + command.words[1:], valued=("--wheel", "--fixture", "--events-per-slice", "--output-dir") + ) + require( + not positional and len(opts.get("--wheel", [])) == 1, + "Wheel gate must consume one verified wheel", + ) + variable = opts["--wheel"][0] + require( + variable.startswith("$") and len(variable) > 1, "Wheel gate must consume the built wheel" + ) + variable = variable[1:].removeprefix("{").removesuffix("}") + selectors = [ + item + for item in all_commands + if item.words == ("__verified_wheel__", variable) + and item.index < command.index + and item.step is command.step ] - assert [names.index(name) for name in order] == sorted(names.index(name) for name in order) - acceptance = steps["Run required installed-wheel dataset acceptance"]["run"] - assert "assert len(wheels) == 1" in acceptance - assert "python .github/scripts/run_dataset_acceptance.py" in acceptance - assert '--wheel "$WHEEL" --output-dir .tmp/dataset-acceptance' in acceptance - for step in job["steps"]: - assert "continue-on-error" not in step - upload = steps["Upload dataset acceptance evidence"] - assert upload["if"] == "always()" - assert upload["with"]["if-no-files-found"] == "error" - assert upload["with"]["include-hidden-files"] == "true" - assert upload["with"]["path"] == ".tmp/dataset-acceptance/" - assert "${{ github.sha }}" in upload["with"]["name"] + require(selectors, "Wheel selection must validate exactly one artifact before use") + if nightly: + _nightly(job, selectors[-1].step) + _nightly(job, command.step) + else: + _full(job, selectors[-1].step) + _full(job, command.step) + return opts + + +def validate_tiers(config, settings): + job = config["jobs"]["test"] + rows = matrix_rows(job["strategy"]["matrix"]) + require( + sorted(row["python-version"] for row in rows) == list(PYTHONS), + "Compatibility matrix must execute each supported Python exactly once", + ) + all_commands = commands(job, config) + pytest = select(all_commands, "pytest") + require(len(pytest) == 3, "Fixture, compatibility and full pytest gates are mandatory") + fixture = [command for command in pytest if "tests/test_fixture_provenance.py" in command.words] + require(len(fixture) == 1, "Missing fixture provenance gate") + fixture = fixture[0] + pytest_options(fixture, paths=("tests/test_fixture_provenance.py",)) + _all(job, fixture.step) + full = [command for command in pytest if "--cov" in command.words] + require(len(full) == 1, "Exactly one full coverage gate is mandatory") + full = full[0] + fast = next(command for command in pytest if command is not full and command is not fixture) + pytest_options(fast, expected_marker=lambda slow, nightly: not slow and not nightly) + pytest_options(full, expected_marker=lambda slow, nightly: not nightly, coverage=True) + condition_matches(job, fast.step, predicate=lambda version, event, status: version != "3.13") + _full(job, full.step) + require( + fixture.index < fast.index and fixture.index < full.index, + "Fixture provenance must precede execution", + ) + require( + any( + marker.partition(":")[0] == "nightly" + for marker in settings["tool"]["pytest"]["ini_options"]["markers"] + ), + "Nightly marker must be registered", + ) + + coverage = settings["tool"]["coverage"] + require(coverage["run"]["branch"] is True, "Branch coverage is mandatory") + require(coverage["report"]["fail_under"] >= 90, "Overall coverage floor must not decrease") + expected = { + "*/core/snapshot_import_backend.py": 90, + "*/simulate/simulated_caching_allocator.py": 88, + "*/simulate/snapshot_mutator.py": 84, + } + reports = select(all_commands, "coverage") + require(len(reports) == len(expected), "Each critical file needs its own coverage floor") + seen = set() + for report in reports: + require(report.words[1] == "report", "Critical coverage must report measured results") + opts, positional = options(report.words[2:], valued=("--include", "--fail-under")) + require( + not positional and set(opts) == {"--include", "--fail-under"}, + "Coverage report must have an independent selector and floor", + ) + require( + all(len(values) == 1 for values in opts.values()), + "Duplicate coverage options are ambiguous", + ) + path = opts["--include"][0] + require( + path in expected and path not in seen, "Wrong/combined critical-file coverage selector" + ) + require( + float(opts["--fail-under"][0]) >= expected[path], + "Critical coverage floor must not decrease", + ) + seen.add(path) + require(full.index < report.index, "Coverage must be measured before its floors") + _full(job, report.step) + + build = one(all_commands, "build") + require(build.words[1:] == ("--wheel",), "Build must produce the wheel") + _full(job, build.step) + audit = one(all_commands, ".github/scripts/audit_wheel.py") + require( + set(_check_wheel_gate(job, all_commands, audit)) == {"--wheel"}, + "Wheel audit arguments changed", + ) + acceptance = one(all_commands, ".github/scripts/clean_wheel_agent_acceptance.py") + opts = _check_wheel_gate(job, all_commands, acceptance) + require( + opts.get("--fixture") == ["tests/fixtures/snapshots/snapshot_with_empty_cache.pkl"] + and opts.get("--events-per-slice") == ["4096"] + and set(opts) == {"--wheel", "--fixture", "--events-per-slice"}, + "Clean-wheel fixture/sharded acceptance is mandatory", + ) + require( + full.index < build.index < audit.index < acceptance.index, + "Wheel acceptance must follow tests, build and audit", + ) + upload = [ + step + for step in _action_steps(job, "actions/upload-artifact") + if step.get("with", {}).get("path") == "coverage.xml" + ] + require(len(upload) == 1, "Coverage evidence must be uploaded") + condition_matches( + job, + upload[0], + predicate=lambda version, event, status: version == "3.13", + statuses=("success", "failure", "cancelled"), + ) + + +def _nightly(job, step, *, always=False): + condition_matches( + job, + step, + predicate=lambda version, event, status: event in {"schedule", "workflow_dispatch"}, + statuses=("success", "failure", "cancelled") if always else ("success",), + ) + + +def validate_nightly(config): + require("workflow_dispatch" in config["on"], "Manual acceptance dispatch is mandatory") + require( + config["on"].get("schedule") == [{"cron": "17 3 * * *"}], + "Nightly acceptance schedule changed", + ) + job = config["jobs"]["dataset-acceptance"] + require(job["timeout-minutes"] == "30", "Dataset acceptance timeout changed") + checkout = _action_steps(job, "actions/checkout") + require( + len(checkout) == 1 and checkout[0]["with"].get("fetch-depth") == "0", + "Dataset acceptance needs full checkout history", + ) + require( + "".join(checkout[0]["with"].get("ref", "").split()) == "${{github.sha}}", + "Dataset acceptance must use the dispatched commit", + ) + setup = _action_steps(job, "actions/setup-python") + require( + len(setup) == 1 and setup[0].get("with", {}).get("python-version") == "3.13", + "Nightly acceptance requires Python 3.13", + ) + _nightly(job, setup[0]) + all_commands = commands(job, config) + for command in all_commands: + _nightly(job, command.step) + fixture = one(all_commands, "pytest") + pytest_options(fixture, paths=("tests/test_fixture_provenance.py",)) + build = one(all_commands, "build") + require(build.words[1:] == ("--wheel",), "Nightly acceptance requires a built wheel") + audit = one(all_commands, ".github/scripts/audit_wheel.py") + require( + set(_check_wheel_gate(job, all_commands, audit, nightly=True)) == {"--wheel"}, + "Nightly wheel audit arguments changed", + ) + acceptance = one(all_commands, ".github/scripts/run_dataset_acceptance.py") + opts = _check_wheel_gate(job, all_commands, acceptance, nightly=True) + require( + opts.get("--output-dir") == [".tmp/dataset-acceptance"] + and set(opts) == {"--wheel", "--output-dir"}, + "Nightly evidence destination changed", + ) + differential = one(all_commands, ".github/scripts/run_differential_acceptance.py") + opts, positional = options(differential.words[1:], valued=("--seed-count", "--output-dir")) + require( + not positional + and opts == {"--seed-count": ["200"], "--output-dir": [".tmp/differential-acceptance"]}, + "Required 200-seed paired acceptance must not weaken", + ) + require( + fixture.index < build.index < audit.index < acceptance.index < differential.index, + "Nightly acceptance order must preserve provenance/build/audit", + ) + uploads = _action_steps(job, "actions/upload-artifact") + for path in (".tmp/dataset-acceptance/", ".tmp/differential-acceptance/*.log"): + matching = [ + step for step in uploads if path in step.get("with", {}).get("path", "").splitlines() + ] + require(len(matching) == 1, f"Missing acceptance evidence upload: {path}") + upload = matching[0] + _nightly(job, upload, always=True) + require( + upload["with"].get("if-no-files-found") == "error" + and upload["with"].get("include-hidden-files") == "true", + "Missing acceptance evidence must fail", + ) + require( + "${{github.sha}}" in "".join(upload["with"].get("name", "").split()), + "Evidence must identify the checkout commit", + ) + require( + job["steps"].index(upload) > job["steps"].index(differential.step), + "Evidence must be uploaded after acceptance", + ) + + +def validate_other_gates(config): + require(config["permissions"] == {"contents": "read"}, "CI permissions must stay read-only") + require( + "pull_request_target" not in config["on"], "Untrusted PRs cannot use privileged trigger" + ) + require( + set(config["on"]["pull_request"]["types"]) + == {"opened", "synchronize", "reopened", "edited"}, + "PR gate triggers changed", + ) + require(set(EVENTS) <= set(config["on"]), "Required CI triggers are missing") + for event in ("push", "pull_request"): + require(config["on"][event].get("branches") == ["main"], "Main push/PR gates are required") + require( + not set(config["on"][event]) & {"paths", "paths-ignore", "branches-ignore"}, + "Gate triggers cannot silently filter changes", + ) + for name in ("snapshot-provenance", "lint", "browser", "test"): + job = config["jobs"][name] + require( + job.get("continue-on-error", "false") == "false", "Job failures must not be ignored" + ) + require(not job.get("needs"), "Independent CI gates cannot depend on skippable jobs") + require( + job.get("permissions", {"contents": "read"}) == {"contents": "read"}, + "Job permissions must stay read-only", + ) + checkout = _action_steps(job, "actions/checkout") + require(len(checkout) == 1, "Exactly one gate checkout is required") + require( + "".join(checkout[0].get("with", {}).get("ref", "").split()) + == "${{github.event.pull_request.head.sha||github.sha}}", + "CI must check the published PR/triggering commit", + ) + _all(job, checkout[0]) + require(job["steps"].index(checkout[0]) == 0, "Gate checkout must precede commands") + if name != "snapshot-provenance": + setup = _action_steps(job, "actions/setup-python") + require(len(setup) == 1, "Every execution tier must select its Python") + version = "".join(setup[0].get("with", {}).get("python-version", "").split()) + require( + version == ("${{matrix.python-version}}" if name == "test" else "3.13"), + "Python setup must match its execution tier", + ) + _all(job, setup[0]) + provenance = config["jobs"]["snapshot-provenance"] + require( + _action_steps(provenance, "actions/checkout")[0]["with"].get("fetch-depth") == "0", + "Provenance requires full history", + ) + # This PR gate is a simple command. The separate non-PR base-resolution + # script is deliberately not interpreted as part of the tier contract. + pr_steps = [step for step in provenance["steps"] if "PR_BODY" in step.get("env", {})] + require(len(pr_steps) == 1, "PR provenance decision gate is required") + pr = one(commands({"steps": pr_steps}, config), ".github/scripts/check_snapshot_provenance.py") + for key, context in (("BASE_SHA", "base.sha"), ("HEAD_SHA", "head.sha"), ("PR_BODY", "body")): + require( + "".join(pr.env.get(key, "").split()) + == "${{github.event.pull_request." + context + "}}", + "PR provenance must compare the actual base/head and decision body", + ) + opts, positional = options( + pr.words[1:], valued=("--base", "--head"), flags=("--require-decision",) + ) + require( + not positional + and opts + == {"--base": ["$BASE_SHA"], "--head": ["$HEAD_SHA"], "--require-decision": [True]}, + "PR provenance decision cannot be bypassed", + ) + condition_matches( + provenance, pr.step, predicate=lambda version, event, status: event == "pull_request" + ) + lint = config["jobs"]["lint"] + lint_commands = commands(lint, config) + for program, arguments in ( + ("ruff", ("check", ".")), + ("black", ("--check", ".")), + ("basedpyright", ("--pythonpath", "__selected_python__")), + ): + command = one(lint_commands, program) + require( + command.words[1:] == arguments, f"{program} must check the intended source/interpreter" + ) + _all(lint, command.step) + browser = config["jobs"]["browser"] + require( + browser["runs-on"].startswith("ubuntu-"), "Real Chromium regression must remain on Linux" + ) + browser_commands = commands(browser, config) + install = one(browser_commands, "playwright") + opts, positional = options(install.words[1:], flags=("--with-deps",)) + require( + positional == ["install", "chromium"] and opts == {"--with-deps": [True]}, + "CI must install real Chromium with OS dependencies", + ) + test = one(browser_commands, "pytest") + pytest_options( + test, paths=("tests/test_fixture_provenance.py", "tests/test_memory_tree_browser.py") + ) + require( + test.env.get("PT_SNAP_REQUIRE_BROWSER") == "1", "Browser absence must fail the required job" + ) + require(install.index < test.index, "Chromium must be installed before required regression") + _all(browser, install.step) + _all(browser, test.step) + + +def validate_workflow(config, settings): + validate_tiers(config, settings) + validate_nightly(config) + validate_other_gates(config) + + +def test_ci_semantic_contract(): + validate_workflow(workflow(), project()) diff --git a/tests/test_memory_tree_browser.py b/tests/test_memory_tree_browser.py index baef121..f9c70bc 100644 --- a/tests/test_memory_tree_browser.py +++ b/tests/test_memory_tree_browser.py @@ -6,8 +6,6 @@ from __future__ import annotations -import importlib -import os import re from collections.abc import Iterator @@ -15,12 +13,9 @@ from pt_snap_cli.core.models import QueryResult from pt_snap_cli.memory_tree_render import render_memory_tree +from tests._browser_support import browser_expect as expect -playwright = ( - importlib.import_module("playwright.sync_api") - if os.environ.get("PT_SNAP_REQUIRE_BROWSER") == "1" - else pytest.importorskip("playwright.sync_api") -) +pytestmark = pytest.mark.browser ROOT_BYTES = 471_498_368 GIANT_BYTES = 283_057_288 @@ -82,27 +77,6 @@ def frame_rows(): return rows -@pytest.fixture(scope="module") -def chromium_browser(): - with playwright.sync_playwright() as runtime: - options = {"headless": True} - executable = os.environ.get("PT_SNAP_TEST_CHROMIUM") - if executable: - options["executable_path"] = executable - try: - browser = runtime.chromium.launch(**options) - except playwright.Error as exc: - # A broken running browser is a test failure, not an optional dependency. - missing = "executable doesn't exist" in str(exc).lower() - if missing and os.environ.get("PT_SNAP_REQUIRE_BROWSER") != "1": - pytest.skip("Chromium is not installed; run python -m playwright install chromium") - pytest.fail(f"Cannot launch Chromium for frame navigation tests: {exc}") - try: - yield browser - finally: - browser.close() - - @pytest.fixture def browser_page(chromium_browser) -> Iterator: context = chromium_browser.new_context(viewport={"width": 1440, "height": 1000}, locale="en-US") @@ -126,7 +100,7 @@ def _render(page, rows): total_is_exact=True, ) page.set_content(render_memory_tree(result, event_id=17, html=True), wait_until="load") - playwright.expect(page.locator("#chart .node").first).to_be_attached() + expect(page.locator("#chart .node").first).to_be_attached() def _button(page, container, node_id): @@ -152,8 +126,8 @@ def _assert_path(locator, *labels): def _assert_selected(page, node_id, *labels): - playwright.expect(page.locator("#chart .node").first).to_have_attribute("data-node-id", node_id) - playwright.expect(page.locator("#detail")).to_contain_text(node_id) + expect(page.locator("#chart .node").first).to_have_attribute("data-node-id", node_id) + expect(page.locator("#detail")).to_contain_text(node_id) _assert_path(page.locator("#focus-path"), *labels) @@ -170,9 +144,9 @@ def _tab_to(page, target): def test_search_keeps_true_proportional_geometry_until_navigation(browser_page, frame_rows): page = browser_page _render(page, frame_rows) - playwright.expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_disabled() - playwright.expect(page.locator("#search-status")).not_to_be_empty() - playwright.expect(page.locator("#search-results button")).to_have_count(0) + expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_disabled() + expect(page.locator("#search-status")).not_to_be_empty() + expect(page.locator("#search-results button")).to_have_count(0) _assert_selected(page, "root", "All active memory") giant_width = float(_rect(page, "frame:giant").get_attribute("width")) @@ -182,7 +156,7 @@ def test_search_keeps_true_proportional_geometry_until_navigation(browser_page, original_geometry = _geometry(page) page.locator("#search").fill("SHARED_FUNCTION") - playwright.expect(page.locator("#search-results button")).to_have_count(2) + expect(page.locator("#search-results button")).to_have_count(2) assert _geometry(page) == original_geometry _assert_selected(page, "root", "All active memory") @@ -201,26 +175,26 @@ def test_search_disambiguates_duplicate_paths_and_supports_native_keyboard( page = browser_page _render(page, frame_rows) page.locator("#search").fill("sHaReD_fUnCtIoN") - playwright.expect(page.locator("#search-results button")).to_have_count(2) - playwright.expect(page.locator("#search-status")).to_contain_text(re.compile(r"\b2\b")) + expect(page.locator("#search-results button")).to_have_count(2) + expect(page.locator("#search-status")).to_contain_text(re.compile(r"\b2\b")) for node_id, branch in ( ("frame:giant/tiny", "giant_branch"), ("frame:other/tiny", "other_branch"), ): button = _button(page, "search-results", node_id) _assert_path(button, "All active memory", branch, "shared_function") - playwright.expect(button).to_contain_text(node_id) - playwright.expect(button).to_contain_text(re.compile(r"40\s+bytes", re.IGNORECASE)) + expect(button).to_contain_text(node_id) + expect(button).to_contain_text(re.compile(r"40\s+bytes", re.IGNORECASE)) target = _button(page, "search-results", "frame:giant/tiny") _tab_to(page, target) page.keyboard.press("Enter") _assert_selected( page, "frame:giant/tiny", "All active memory", "giant_branch", "shared_function" ) - playwright.expect(page.locator("#detail")).to_contain_text("Inclusive: 40 bytes") + expect(page.locator("#detail")).to_contain_text("Inclusive: 40 bytes") # The other same-named frame remains available outside the focused subtree. - playwright.expect(page.locator("#search-results button")).to_have_count(2) + expect(page.locator("#search-results button")).to_have_count(2) target = _button(page, "search-results", "frame:other/tiny") _tab_to(page, target) page.keyboard.press("Space") @@ -234,13 +208,13 @@ def test_node_id_search_is_case_insensitive_and_global_after_focus(browser_page, _render(page, frame_rows) _button(page, "children-list", "frame:giant").click() page.locator("#search").fill("FRAME:OTHER/TINY") - playwright.expect(page.locator("#search-results button")).to_have_count(1) + expect(page.locator("#search-results button")).to_have_count(1) _button(page, "search-results", "frame:other/tiny").click() _assert_selected( page, "frame:other/tiny", "All active memory", "other_branch", "shared_function" ) page.locator("#search").fill("ZERO_LEAF") - playwright.expect(page.locator("#search-results button")).to_have_count(1) + expect(page.locator("#search-results button")).to_have_count(1) _button(page, "search-results", "frame:giant/zero/leaf").click() _assert_selected( page, @@ -250,34 +224,34 @@ def test_node_id_search_is_case_insensitive_and_global_after_focus(browser_page, "zero_parent", "zero_leaf", ) - playwright.expect(page.locator("#detail")).to_contain_text("Inclusive: 0 bytes") + expect(page.locator("#detail")).to_contain_text("Inclusive: 0 bytes") def test_children_reach_tiny_and_zero_frames_and_parent_retraces_path(browser_page, frame_rows): page = browser_page _render(page, frame_rows) - playwright.expect(page.locator("#children-list button")).to_have_count(3) + expect(page.locator("#children-list button")).to_have_count(3) giant = _button(page, "children-list", "frame:giant") _assert_path(giant, "All active memory", "giant_branch") - playwright.expect(giant).to_contain_text("frame:giant") - playwright.expect(giant).to_contain_text("283,057,288 bytes") + expect(giant).to_contain_text("frame:giant") + expect(giant).to_contain_text("283,057,288 bytes") giant.click() - playwright.expect(page.locator("#children-list button")).to_have_count(2) + expect(page.locator("#children-list button")).to_have_count(2) tiny = _button(page, "children-list", "frame:giant/tiny") _assert_path(tiny, "All active memory", "giant_branch", "shared_function") - playwright.expect(tiny).to_contain_text("40 bytes") + expect(tiny).to_contain_text("40 bytes") _tab_to(page, tiny) page.keyboard.press("Enter") _assert_selected( page, "frame:giant/tiny", "All active memory", "giant_branch", "shared_function" ) - playwright.expect(page.locator("#children-list button")).to_have_count(0) - playwright.expect(page.locator("#children-status")).not_to_be_empty() + expect(page.locator("#children-list button")).to_have_count(0) + expect(page.locator("#children-status")).not_to_be_empty() page.get_by_role("button", name="Parent frame", exact=True).click() zero = _button(page, "children-list", "frame:giant/zero") - playwright.expect(zero).to_contain_text("0 bytes") + expect(zero).to_contain_text("0 bytes") _tab_to(page, zero) page.keyboard.press("Space") _assert_selected(page, "frame:giant/zero", "All active memory", "giant_branch", "zero_parent") @@ -290,8 +264,8 @@ def test_children_reach_tiny_and_zero_frames_and_parent_retraces_path(browser_pa "zero_parent", "zero_leaf", ) - playwright.expect(page.locator("#children-list button")).to_have_count(0) - playwright.expect(page.locator("#children-status")).not_to_be_empty() + expect(page.locator("#children-list button")).to_have_count(0) + expect(page.locator("#children-status")).not_to_be_empty() for node_id, labels in ( ("frame:giant/zero", ("All active memory", "giant_branch", "zero_parent")), ("frame:giant", ("All active memory", "giant_branch")), @@ -299,7 +273,7 @@ def test_children_reach_tiny_and_zero_frames_and_parent_retraces_path(browser_pa ): page.get_by_role("button", name="Parent frame", exact=True).click() _assert_selected(page, node_id, *labels) - playwright.expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_disabled() + expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_disabled() def test_reset_is_repeatable_and_preserves_global_search(browser_page, frame_rows): @@ -311,12 +285,10 @@ def test_reset_is_repeatable_and_preserves_global_search(browser_page, frame_row _button(page, "search-results", node_id).click() page.locator("#reset").click() _assert_selected(page, "root", "All active memory") - playwright.expect(page.locator("#search")).to_have_value("shared_function") - playwright.expect(page.locator("#search-results button")).to_have_count(2) - playwright.expect(page.locator("#children-list button")).to_have_count(3) - playwright.expect( - page.get_by_role("button", name="Parent frame", exact=True) - ).to_be_disabled() + expect(page.locator("#search")).to_have_value("shared_function") + expect(page.locator("#search-results button")).to_have_count(2) + expect(page.locator("#children-list button")).to_have_count(3) + expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_disabled() assert _geometry(page) == original_geometry page.locator("#reset").click() _assert_selected(page, "root", "All active memory") @@ -329,14 +301,14 @@ def test_no_results_and_clearing_search_leave_navigation_usable(browser_page, fr _button(page, "children-list", "frame:giant").click() geometry = _geometry(page) page.locator("#search").fill("there is no such frame") - playwright.expect(page.locator("#search-results button")).to_have_count(0) - playwright.expect(page.locator("#search-status")).to_have_text("No matching frames.") + expect(page.locator("#search-results button")).to_have_count(0) + expect(page.locator("#search-status")).to_have_text("No matching frames.") assert _geometry(page) == geometry _assert_selected(page, "frame:giant", "All active memory", "giant_branch") - playwright.expect(page.locator("#children-list button")).to_have_count(2) + expect(page.locator("#children-list button")).to_have_count(2) page.locator("#search").fill("") - playwright.expect(page.locator("#search-status")).to_have_text(instructions) - playwright.expect(page.locator("#search-results button")).to_have_count(0) + expect(page.locator("#search-status")).to_have_text(instructions) + expect(page.locator("#search-results button")).to_have_count(0) _button(page, "children-list", "frame:giant/tiny").click() _assert_selected( page, "frame:giant/tiny", "All active memory", "giant_branch", "shared_function" @@ -347,22 +319,20 @@ def test_unicode_and_html_like_labels_remain_plain_text_everywhere(browser_page, page = browser_page _render(page, frame_rows) root_child = _button(page, "children-list", "frame:malicious") - playwright.expect(root_child).to_contain_text(MALICIOUS_LABEL) + expect(root_child).to_contain_text(MALICIOUS_LABEL) page.locator("#search").fill("中文") - playwright.expect(page.locator("#search-results button")).to_have_count(1) + expect(page.locator("#search-results button")).to_have_count(1) result = _button(page, "search-results", "frame:malicious") - playwright.expect(result).to_contain_text(MALICIOUS_LABEL) + expect(result).to_contain_text(MALICIOUS_LABEL) result.click() _assert_selected(page, "frame:malicious", "All active memory", MALICIOUS_LABEL) - playwright.expect(page.locator("#detail")).to_contain_text(MALICIOUS_LABEL) - playwright.expect(page.locator("#chart text")).to_have_text(MALICIOUS_LABEL) - playwright.expect(page.locator("img")).to_have_count(0) - playwright.expect(page.locator("script")).to_have_count(2) + expect(page.locator("#detail")).to_contain_text(MALICIOUS_LABEL) + expect(page.locator("#chart text")).to_have_text(MALICIOUS_LABEL) + expect(page.locator("img")).to_have_count(0) + expect(page.locator("script")).to_have_count(2) assert page.evaluate("window.__frameInjected === undefined") page.locator("#reset").click() - playwright.expect(_button(page, "children-list", "frame:malicious")).to_contain_text( - MALICIOUS_LABEL - ) + expect(_button(page, "children-list", "frame:malicious")).to_contain_text(MALICIOUS_LABEL) def test_search_does_not_silently_truncate_more_than_100_matches(browser_page): @@ -373,13 +343,13 @@ def test_search_does_not_silently_truncate_more_than_100_matches(browser_page): ) _render(page, rows) page.locator("#search").fill("BATCH FRAME") - playwright.expect(page.locator("#search-results button")).to_have_count(151) - playwright.expect(page.locator("#search-status")).to_contain_text(re.compile(r"\b151\b")) + expect(page.locator("#search-results button")).to_have_count(151) + expect(page.locator("#search-status")).to_contain_text(re.compile(r"\b151\b")) _button(page, "search-results", "batch:150").click() _assert_selected(page, "batch:150", "All active memory", "batch frame 150") - playwright.expect(page.locator("#search-results button")).to_have_count(151) + expect(page.locator("#search-results button")).to_have_count(151) page.locator("#reset").click() - playwright.expect(page.locator("#children-list button")).to_have_count(151) + expect(page.locator("#children-list button")).to_have_count(151) def test_zero_total_tree_can_be_explored_and_reset(browser_page): @@ -395,15 +365,15 @@ def test_zero_total_tree_can_be_explored_and_reset(browser_page): _render(page, rows) original_geometry = _geometry(page) assert float(_rect(page, "empty:branch").get_attribute("width")) == 0 - playwright.expect(page.locator("#children-list button")).to_have_count(2) + expect(page.locator("#children-list button")).to_have_count(2) _button(page, "children-list", "empty:branch").click() leaf = _button(page, "children-list", "empty:leaf") _tab_to(page, leaf) page.keyboard.press("Enter") _assert_selected(page, "empty:leaf", "Empty active memory", "empty branch", "empty leaf") - playwright.expect(page.locator("#detail")).to_contain_text("Inclusive: 0 bytes") - playwright.expect(page.locator("#detail")).to_contain_text("n/a") - playwright.expect(page.locator("#children-status")).not_to_be_empty() + expect(page.locator("#detail")).to_contain_text("Inclusive: 0 bytes") + expect(page.locator("#detail")).to_contain_text("n/a") + expect(page.locator("#children-status")).not_to_be_empty() page.locator("#search").fill("empty sibling") _button(page, "search-results", "empty:sibling").click() _assert_selected(page, "empty:sibling", "Empty active memory", "empty sibling") @@ -418,10 +388,10 @@ def test_existing_svg_click_and_hover_keep_working(browser_page, frame_rows): page = browser_page _render(page, frame_rows) _rect(page, "frame:giant").hover() - playwright.expect(page.locator("#detail")).to_contain_text("283,057,288 bytes") + expect(page.locator("#detail")).to_contain_text("283,057,288 bytes") _rect(page, "frame:giant").click() _assert_selected(page, "frame:giant", "All active memory", "giant_branch") - playwright.expect(page.locator("#children-list button")).to_have_count(2) + expect(page.locator("#children-list button")).to_have_count(2) page.locator("#reset").click() _assert_selected(page, "root", "All active memory") @@ -431,17 +401,17 @@ def test_keyboard_focus_survives_continuous_descent_parent_and_reset(browser_pag _render(page, frame_rows) path = page.locator("#focus-path") _button(page, "children-list", "frame:giant").click() - playwright.expect(path).to_be_focused() + expect(path).to_be_focused() # No focus() helper between navigations: the next Tab reaches the new children. page.keyboard.press("Tab") - playwright.expect(_button(page, "children-list", "frame:giant/tiny")).to_be_focused() + expect(_button(page, "children-list", "frame:giant/tiny")).to_be_focused() page.keyboard.press("Tab") - playwright.expect(_button(page, "children-list", "frame:giant/zero")).to_be_focused() + expect(_button(page, "children-list", "frame:giant/zero")).to_be_focused() page.keyboard.press("Space") - playwright.expect(path).to_be_focused() + expect(path).to_be_focused() page.keyboard.press("Tab") - playwright.expect(_button(page, "children-list", "frame:giant/zero/leaf")).to_be_focused() + expect(_button(page, "children-list", "frame:giant/zero/leaf")).to_be_focused() page.keyboard.press("Enter") _assert_selected( page, @@ -451,25 +421,25 @@ def test_keyboard_focus_survives_continuous_descent_parent_and_reset(browser_pag "zero_parent", "zero_leaf", ) - playwright.expect(path).to_be_focused() + expect(path).to_be_focused() page.keyboard.press("Shift+Tab") - playwright.expect(page.locator("#search")).to_be_focused() + expect(page.locator("#search")).to_be_focused() page.keyboard.press("Shift+Tab") - playwright.expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_focused() + expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_focused() page.keyboard.press("Enter") _assert_selected(page, "frame:giant/zero", "All active memory", "giant_branch", "zero_parent") - playwright.expect(path).to_be_focused() + expect(path).to_be_focused() for _ in range(3): page.keyboard.press("Shift+Tab") - playwright.expect(page.locator("#reset")).to_be_focused() + expect(page.locator("#reset")).to_be_focused() page.keyboard.press("Space") _assert_selected(page, "root", "All active memory") - playwright.expect(path).to_be_focused() - playwright.expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_disabled() + expect(path).to_be_focused() + expect(page.get_by_role("button", name="Parent frame", exact=True)).to_be_disabled() page.keyboard.press("Tab") - playwright.expect(_button(page, "children-list", "frame:giant")).to_be_focused() + expect(_button(page, "children-list", "frame:giant")).to_be_focused() def test_special_allocation_categories_remain_reachable(browser_page): @@ -492,10 +462,10 @@ def test_special_allocation_categories_remain_reachable(browser_page): assert float(_rect(page, node_id).get_attribute("width")) == 0 _button(page, "children-list", node_id).click() _assert_selected(page, node_id, "All active memory", label) - playwright.expect(page.locator("#children-status")).not_to_be_empty() + expect(page.locator("#children-status")).not_to_be_empty() page.locator("#reset").click() page.locator("#search").fill(node_id) - playwright.expect(page.locator("#search-results button")).to_have_count(1) + expect(page.locator("#search-results button")).to_have_count(1) _button(page, "search-results", node_id).click() _assert_selected(page, node_id, "All active memory", label) page.locator("#reset").click() From a45365af3ea7f9a755794eefbf9439e018423682 Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:04:36 +0800 Subject: [PATCH 05/12] test: guard critical publication and rollback coverage independently (#277) --- .github/scripts/check_critical_coverage.py | 84 ++++++ .github/workflows/AGENTS.md | 30 +- .github/workflows/test.yml | 5 +- tests/core/test_critical_coverage_cache.py | 285 ++++++++++++++++++ tests/core/test_critical_coverage_focus.py | 211 +++++++++++++ tests/core/test_critical_coverage_metadata.py | 208 +++++++++++++ tests/core/test_critical_coverage_skills.py | 157 ++++++++++ tests/core/test_critical_coverage_split.py | 193 ++++++++++++ .../test_representation_boundaries.py | 214 +++++++++++++ tests/test_ci_contract_mutations.py | 109 +++++-- tests/test_ci_tiers.py | 67 ++-- tests/test_critical_coverage_gate.py | 122 ++++++++ 12 files changed, 1622 insertions(+), 63 deletions(-) create mode 100644 .github/scripts/check_critical_coverage.py create mode 100644 tests/core/test_critical_coverage_cache.py create mode 100644 tests/core/test_critical_coverage_focus.py create mode 100644 tests/core/test_critical_coverage_metadata.py create mode 100644 tests/core/test_critical_coverage_skills.py create mode 100644 tests/core/test_critical_coverage_split.py create mode 100644 tests/snapshot/test_representation_boundaries.py create mode 100644 tests/test_critical_coverage_gate.py diff --git a/.github/scripts/check_critical_coverage.py b/.github/scripts/check_critical_coverage.py new file mode 100644 index 0000000..249bac2 --- /dev/null +++ b/.github/scripts/check_critical_coverage.py @@ -0,0 +1,84 @@ +"""Fail closed when a critical module loses measured line/branch coverage.""" + +from __future__ import annotations + +import io +import sys +from pathlib import Path + +from coverage import Coverage +from coverage.exceptions import CoverageException + +# These are independent floors, not an average that another module can offset. +# Keep the original three floors unchanged when extending the risk inventory. +CRITICAL_FLOORS = { + "src/pt_snap_cli/core/snapshot_import_backend.py": 90, + "src/pt_snap_cli/snapshot/simulate/simulated_caching_allocator.py": 88, + "src/pt_snap_cli/snapshot/simulate/snapshot_mutator.py": 84, + "src/pt_snap_cli/core/import_metadata_contract.py": 100, + "src/pt_snap_cli/core/import_metadata.py": 90, + "src/pt_snap_cli/core/split_service.py": 90, + "src/pt_snap_cli/core/focus_service.py": 90, + "src/pt_snap_cli/core/skill_service.py": 90, + "src/pt_snap_cli/core/snapshot_cache.py": 90, + "src/pt_snap_cli/snapshot/representation.py": 95, +} + + +class CoverageGuardError(ValueError): + """Coverage evidence is absent, invalid, or below an independent floor.""" + + +def check_coverage(root: Path) -> None: + root = root.resolve() + data_file = root / ".coverage" + if not data_file.is_file(): + raise CoverageGuardError(f"Missing coverage data: {data_file}") + try: + coverage = Coverage(data_file=str(data_file), config_file=str(root / "pyproject.toml")) + coverage.load() + data = coverage.get_data() + if not data.has_arcs(): + raise CoverageGuardError("Critical coverage requires branch data, not line-only data") + measured = {Path(name).resolve() for name in data.measured_files()} + failures = [] + for relative, floor in CRITICAL_FLOORS.items(): + source = root / relative + if not source.is_file(): + failures.append(f"{relative}: missing critical source file") + continue + if source not in measured: + failures.append(f"{relative}: missing measured coverage data") + continue + if not coverage.analysis2(str(source))[1]: + failures.append(f"{relative}: no executable statements") + continue + # report() returns the unrounded line+branch percentage. Never compare + # the rounded terminal percentage, which could hide a regression. + percent = coverage.report( + morfs=[str(source)], + file=io.StringIO(), + ignore_errors=False, + skip_empty=False, + skip_covered=False, + ) + print(f"{relative}: {percent:.4f}% (required {floor}%)") + if percent < floor: + failures.append(f"{relative}: {percent:.4f}% is below {floor}%") + if failures: + raise CoverageGuardError("\n".join(failures)) + except (CoverageException, OSError) as error: + raise CoverageGuardError(f"Cannot validate critical coverage: {error}") from error + + +def main() -> int: + try: + check_coverage(Path(__file__).resolve().parents[2]) + except CoverageGuardError as error: + print(f"Critical coverage gate failed:\n{error}", file=sys.stderr) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/AGENTS.md b/.github/workflows/AGENTS.md index 91fb4c0..9323fc9 100644 --- a/.github/workflows/AGENTS.md +++ b/.github/workflows/AGENTS.md @@ -17,13 +17,41 @@ Concurrency is keyed by GitHub ref and superseded runs are cancelled. | Compatibility matrix | Python 3.10–3.12; fixture provenance first, then `pytest -m "not slow and not nightly"` | | Full PR test tier | Python 3.13; `pytest -m "not nightly"` with branch coverage, preserving the existing slow tests | | Overall coverage | `pyproject.toml` fail-under 90 with branches enabled | -| Critical runtime coverage | Import backend 90; simulated allocator 88; snapshot mutator 84, checked independently | +| Critical runtime coverage | Ten independent line+branch floors, enforced by the fail-closed checker below | | Python 3.13 packaging | Upload coverage, build wheel, run offline clean-wheel skill/CLI acceptance including both sharded formats | | Nightly/manual dataset acceptance | Python 3.13; provenance, wheel build and audit, then `nightly` tests including the required installed-wheel CLI/API test | Million-event deep-offset pagination tests are marked `slow`; their small pagination contracts still run across the compatibility matrix. +### Independent critical-file coverage + +After full Python 3.13 coverage, `check_critical_coverage.py` enforces this complete +inventory. A high average cannot compensate for a single file below its floor. + +| Runtime module (`src/pt_snap_cli/`) | Minimum line + branch coverage | +| --- | --- | +| `core/snapshot_import_backend.py` | 90% | +| `snapshot/simulate/simulated_caching_allocator.py` | 88% | +| `snapshot/simulate/snapshot_mutator.py` | 84% | +| `core/import_metadata_contract.py` | 100% | +| `core/import_metadata.py` | 90% | +| `core/split_service.py` | 90% | +| `core/focus_service.py` | 90% | +| `core/skill_service.py` | 90% | +| `core/snapshot_cache.py` | 90% | +| `snapshot/representation.py` | 95% | + +The checker requires `.coverage`, branch data, every listed source file, measured +coverage for each exact path, and executable statements. Missing/corrupt evidence +fails; percentages are compared before display rounding. It has no selector or +floor overrides. The overall 90% gate remains separate and unchanged. CI contract +mutation tests reject an omitted/conditional checker, alternate coverage commands, +ignored failures, reordered execution, and narrowed source/omit/exclusion settings; +`tests/test_critical_coverage_gate.py` separately protects the exact inventory and +exercises real coverage-data success/failure cases. Raise floors only with actual +CI evidence; do not delete code, add exclusions, or weaken assertions to meet them. + `tests/test_ci_tiers.py` checks parsed commands, matrix rows, marker truth tables, conditions, independent file floors and gate ordering, not step display names or complete shell text. `tests/_ci_contract.py` intentionally supports only a bounded diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e65bbe7..6350d75 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -139,10 +139,7 @@ jobs: - name: Guard critical runtime coverage if: matrix.python-version == '3.13' # Separate floors prevent strong modules from masking a weak boundary. - run: | - python -m coverage report --include='*/core/snapshot_import_backend.py' --fail-under=90 - python -m coverage report --include='*/simulate/simulated_caching_allocator.py' --fail-under=88 - python -m coverage report --include='*/simulate/snapshot_mutator.py' --fail-under=84 + run: python .github/scripts/check_critical_coverage.py - name: Upload coverage report uses: actions/upload-artifact@v4 diff --git a/tests/core/test_critical_coverage_cache.py b/tests/core/test_critical_coverage_cache.py new file mode 100644 index 0000000..3411a30 --- /dev/null +++ b/tests/core/test_critical_coverage_cache.py @@ -0,0 +1,285 @@ +"""Reject malformed cache contracts without repairing or publishing any artifacts.""" + +import copy +import errno +import json +import os +import sqlite3 +from contextlib import closing +from pathlib import Path + +import pytest + +from pt_snap_cli.core import snapshot_cache +from pt_snap_cli.core.snapshot_cache import ( + member_digest, + seal_manifest, + source_hashes, + validate_cache, +) + + +@pytest.fixture +def cache_artifact(tmp_path: Path) -> tuple[Path, dict, str]: + source = tmp_path / "source.pkl" + # Validation hashes but never deserializes this test-owned source. + source.write_bytes(b"test-owned source, deliberately not a pickle") + cache_hash = source_hashes(source)[1] + with closing(sqlite3.connect(tmp_path / "slice.db")) as conn, conn: + conn.executescript( + "CREATE TABLE trace_entry_0(id INTEGER PRIMARY KEY, action INTEGER, " + "address INTEGER, size INTEGER, stream INTEGER, allocated INTEGER, " + "active INTEGER, reserved INTEGER, callstack TEXT);" + "CREATE TABLE block_0(id INTEGER PRIMARY KEY, address INTEGER, size INTEGER, " + "requestedSize INTEGER, state INTEGER, allocEventId INTEGER, freeEventId INTEGER);" + 'CREATE TABLE dictionary("table" TEXT, "column" TEXT, "key" TEXT, "value" TEXT);' + "INSERT INTO trace_entry_0 VALUES(0,4,4096,16,0,16,16,64,'test frame');" + "INSERT INTO block_0 VALUES(0,4096,16,16,1,0,-1);" + "INSERT INTO dictionary VALUES('trace_entry_0','action','4','alloc');" + "INSERT INTO dictionary VALUES('block_0','state','1','active_allocated');" + ) + manifest = { + "schemaVersion": 1, + "status": "complete", + "sourceFile": str(source), + "cacheHash": cache_hash, + "eventsPerSlice": 1, + "devices": { + "0": { + "eventCount": 1, + "sliceCount": 1, + "readySlices": [0], + "slices": [ + { + "index": 0, + "startEventId": 0, + "endEventId": 0, + "file": "slice.db", + "ready": True, + } + ], + } + }, + } + seal_manifest(tmp_path, manifest) + return tmp_path, manifest, cache_hash + + +def _tree_state(directory: Path) -> dict[str, object]: + return { + str(path.relative_to(directory)): ( + ("symlink", str(path.readlink())) + if path.is_symlink() + else ("file", path.read_bytes()) if path.is_file() else ("directory",) + ) + for path in directory.rglob("*") + } + + +def _assert_rejected(artifact: tuple[Path, dict, str], message: str) -> None: + directory, manifest, cache_hash = artifact + (directory / "manifest.json").write_text(json.dumps(manifest), encoding="utf-8") + before = _tree_state(directory) + with pytest.raises(ValueError, match=message): + validate_cache(directory, cache_hash) + assert _tree_state(directory) == before + + +@pytest.mark.parametrize( + ("field", "value", "message"), + [ + ("schemaVersion", 2, "Unsupported snapshot manifest version"), + ("sourceFile", "", "Snapshot is incomplete"), + ("sourceFile", 7, "Snapshot is incomplete"), + ("eventsPerSlice", 0, "Invalid nonnegative integer"), + ("eventsPerSlice", 2**63, "Invalid nonnegative integer"), + ("devices", {}, "Missing snapshot devices"), + ("devices", [], "Missing snapshot devices"), + ("devices", {"01": {}}, "Invalid device ID"), + ("devices", {"0": []}, "Invalid device inventory"), + ("ptSnap", [], "Invalid snapshot extension"), + ("ptSnap", {"identity": []}, "Invalid producer identity"), + ("ptSnap", {"memberHashKind": "future-v2"}, "Unsupported member hash contract"), + ("ptSnap", {"members": []}, "Invalid legacy snapshot member inventory"), + ("ptSnap", {"members": {}}, "Invalid legacy snapshot member inventory"), + ("parserCache", {"version": 2}, "Snapshot parser data changed"), + ], +) +def test_invalid_manifest_contract_preserves_artifacts(cache_artifact, field, value, message): + cache_artifact[1][field] = value + _assert_rejected(cache_artifact, message) + + +@pytest.mark.parametrize( + ("field", "value", "message"), + [ + ("sliceCount", 2, "Incomplete snapshot slice inventory"), + ("slices", [], "Incomplete snapshot slice inventory"), + ("slices", [None], r"slices\[0\]: expected object"), + ("readySlices", [0, 0], "Snapshot slices are not ready"), + ("eventCount", True, "Invalid nonnegative integer"), + ], +) +def test_invalid_device_inventory_preserves_artifacts(cache_artifact, field, value, message): + cache_artifact[1]["devices"]["0"][field] = value + _assert_rejected(cache_artifact, message) + + +@pytest.mark.parametrize( + ("field", "value"), + [("index", 1), ("startEventId", 1), ("endEventId", 1), ("ready", 1)], +) +def test_invalid_slice_range_preserves_artifacts(cache_artifact, field, value): + cache_artifact[1]["devices"]["0"]["slices"][0][field] = value + _assert_rejected(cache_artifact, "Invalid snapshot slice range") + + +def test_duplicate_member_between_devices_preserves_artifacts(cache_artifact): + devices = cache_artifact[1]["devices"] + devices["1"] = copy.deepcopy(devices["0"]) + _assert_rejected(cache_artifact, "Duplicate snapshot member") + + +@pytest.mark.parametrize( + ("relative", "message"), + [ + (None, "Invalid snapshot member path"), + ("", "Invalid snapshot member path"), + ("slice\\db", "Invalid snapshot member path"), + ("./slice.db", "Snapshot member escapes artifact directory"), + ("missing.db", "Missing or aliased snapshot member"), + ], +) +def test_invalid_member_paths_preserve_artifacts(cache_artifact, relative, message): + cache_artifact[1]["devices"]["0"]["slices"][0]["file"] = relative + _assert_rejected(cache_artifact, message) + + +def test_absolute_member_path_is_rejected(cache_artifact): + # A drive-qualified POSIX spelling is also absolute under Windows pathlib. + absolute = (cache_artifact[0] / "outside.db").as_posix() + cache_artifact[1]["devices"]["0"]["slices"][0]["file"] = absolute + _assert_rejected(cache_artifact, "Snapshot member escapes artifact directory") + + +def test_symlink_member_is_rejected_without_touching_target(cache_artifact): + directory, manifest, _ = cache_artifact + (directory / "alias.db").symlink_to(directory / "slice.db") + manifest["devices"]["0"]["slices"][0]["file"] = "alias.db" + _assert_rejected(cache_artifact, "Missing or aliased snapshot member") + + +@pytest.mark.parametrize("suffix", ["-wal", "-shm", "-journal"]) +def test_symlink_sidecar_is_rejected_without_touching_target(cache_artifact, suffix): + directory = cache_artifact[0] + target = directory / "consumer-data" + target.write_bytes(b"must not open or recover this sidecar target") + (directory / f"slice.db{suffix}").symlink_to(target) + _assert_rejected(cache_artifact, "Snapshot has an aliased sidecar") + + +@pytest.mark.parametrize( + ("sql", "message"), + [ + ("DROP TABLE block_0", "Missing parser table: block_0"), + ( + "ALTER TABLE block_0 RENAME TO stored_blocks;" + "CREATE VIEW block_0 AS SELECT * FROM stored_blocks", + "Missing parser table: block_0", + ), + ( + "ALTER TABLE trace_entry_0 RENAME COLUMN callstack TO consumer_stack", + r"Invalid parser column: trace_entry_0\.callstack", + ), + ( + "DROP TABLE block_0;" + "CREATE TABLE block_0(id INTEGER PRIMARY KEY, address INTEGER, size TEXT, " + "requestedSize INTEGER, state INTEGER, allocEventId INTEGER, freeEventId INTEGER)", + r"Invalid parser column: block_0\.size", + ), + ( + "DROP TABLE block_0;" + "CREATE TABLE block_0(id INTEGER, address INTEGER, size INTEGER, " + "requestedSize INTEGER, state INTEGER, allocEventId INTEGER, freeEventId INTEGER)", + r"Missing parser primary key: block_0\.id", + ), + ( + "DROP TABLE block_0;" + "CREATE TABLE block_0(id INTEGER, address INTEGER, size INTEGER, " + "requestedSize INTEGER, state INTEGER, allocEventId INTEGER, freeEventId INTEGER, " + "PRIMARY KEY(id, address))", + r"Missing parser primary key: block_0\.id", + ), + ], +) +def test_invalid_parser_schema_is_not_repaired(cache_artifact, sql, message): + with closing(sqlite3.connect(cache_artifact[0] / "slice.db")) as conn, conn: + conn.executescript(sql) + _assert_rejected(cache_artifact, message) + + +def test_matching_producer_and_parser_identities_validate_read_only(cache_artifact): + directory, manifest, cache_hash = cache_artifact + manifest["parserCache"]["requestedDevice"] = 0 + manifest["ptSnap"] = { + "identity": {"requestedDevice": 0}, + "memberHashKind": "parser-v1", + "members": dict(manifest["parserCache"]["members"]), + } + (directory / "manifest.json").write_text(json.dumps(manifest), encoding="utf-8") + before = _tree_state(directory) + assert validate_cache(directory, cache_hash, device=0) == manifest + assert _tree_state(directory) == before + + +@pytest.mark.parametrize("device", ["01", "0; DROP TABLE block_0"]) +def test_member_digest_rejects_invalid_device_before_database_access(cache_artifact, device): + directory = cache_artifact[0] + before = _tree_state(directory) + with pytest.raises(ValueError, match="Invalid device ID"): + member_digest(directory / "slice.db", device, 0, 0) + assert _tree_state(directory) == before + + +@pytest.mark.parametrize("replace_path", [False, True]) +def test_source_change_during_hashing_is_rejected_and_descriptor_closed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, replace_path: bool +): + source = tmp_path / "source.pkl" + source.write_bytes(b"original test-owned source") + replacement = b"replacement bytes from the concurrent producer" + original_fstat = os.fstat + original_stat = Path.stat + descriptors: list[int] = [] + + def producer_change_at_stat(descriptor: int) -> os.stat_result: + result = original_fstat(descriptor) + descriptors.append(descriptor) + if not replace_path and len(descriptors) == 1: + # Change the open file between the before/after descriptor signatures. + source.write_bytes(replacement) + return result + + def producer_replace_before_path_stat(path: Path, *args, **kwargs) -> os.stat_result: + if path == source: + # Replace after the hashing handle closes, as required on Windows. + # Both descriptor signatures match; only the final path check detects it. + new_source = tmp_path / "replacement.pkl" + new_source.write_bytes(replacement) + new_source.replace(source) + return original_stat(path, *args, **kwargs) + + with monkeypatch.context() as patch: + patch.setattr(snapshot_cache.os, "fstat", producer_change_at_stat) + if replace_path: + patch.setattr(Path, "stat", producer_replace_before_path_stat) + with pytest.raises(ValueError, match="Snapshot source changed while hashing"): + source_hashes(source) + + assert len(descriptors) == 2 + assert descriptors[0] == descriptors[1] + with pytest.raises(OSError) as closed: + original_fstat(descriptors[0]) + assert closed.value.errno == errno.EBADF + assert source.read_bytes() == replacement + assert list(tmp_path.iterdir()) == [source] diff --git a/tests/core/test_critical_coverage_focus.py b/tests/core/test_critical_coverage_focus.py new file mode 100644 index 0000000..35a1bc4 --- /dev/null +++ b/tests/core/test_critical_coverage_focus.py @@ -0,0 +1,211 @@ +"""Focus compensation preserves exact bytes, configuration state, and recovery evidence.""" + +import copy +import os +import sqlite3 +from contextlib import closing +from pathlib import Path + +import pytest + +import pt_snap_cli.core.focus_service as focus_module +from pt_snap_cli.config import Config +from pt_snap_cli.core.errors import ImportExecutionError +from pt_snap_cli.core.focus_service import FocusService + + +@pytest.fixture +def focus(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("PT_SNAP_DB_PATH", raising=False) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + service = FocusService(Config()) + path = service._config.project_focus_path() + path.parent.mkdir() + original = b'{ "db_path" : "previous.db", "extra" : [1,2] }\n' + path.write_bytes(original) + service._config._config = {"nested": {"value": "original"}} + return service, path, original + + +@pytest.fixture +def checkpoint_descriptors(monkeypatch): + descriptors = [] + mkstemp = focus_module.tempfile.mkstemp + + def track(*args, **kwargs): + descriptor, name = mkstemp(*args, **kwargs) + descriptors.append(descriptor) + return descriptor, name + + monkeypatch.setattr(focus_module.tempfile, "mkstemp", track) + yield descriptors + leaked = [] + for descriptor in descriptors: + try: + os.fstat(descriptor) + except OSError: + continue + os.close(descriptor) + leaked.append(descriptor) + assert not leaked, "focus checkpoint descriptors must close on every outcome" + + +def test_short_checkpoint_writes_restore_exact_focus_and_memory( + focus, checkpoint_descriptors, monkeypatch +): + service, path, original = focus + old_state = copy.deepcopy(service._config._config) + write = os.write + writes = [] + + def short_write(descriptor, content): + writes.append(len(content)) + return write(descriptor, content[:3]) + + monkeypatch.setattr(focus_module.os, "write", short_write) + with pytest.raises(RuntimeError, match="focus mutation failed"): + with service.project_focus_transaction(): + path.write_bytes(b"partial focus") + service._config._config["nested"]["value"] = "changed" + raise RuntimeError("focus mutation failed") + assert path.read_bytes() == original + assert service._config._config == old_state + assert len(writes) > 1 + assert len(checkpoint_descriptors) == 1 + assert list(path.parent.iterdir()) == [path] + + +def test_zero_checkpoint_write_fails_before_any_focus_mutation( + focus, checkpoint_descriptors, monkeypatch +): + service, path, original = focus + monkeypatch.setattr(focus_module.os, "write", lambda *args: 0) + with pytest.raises(OSError, match="recovery checkpoint"): + with service.project_focus_transaction(): + pytest.fail("a failed checkpoint must prevent publication") + assert path.read_bytes() == original + assert len(checkpoint_descriptors) == 1 + assert list(path.parent.iterdir()) == [path] + + +def test_checkpoint_permission_fallback_is_verified_and_cleaned( + focus, checkpoint_descriptors, monkeypatch +): + service, path, _ = focus + with monkeypatch.context() as patch: + patch.delattr(focus_module.os, "fchmod", raising=False) + with service.project_focus_transaction(): + path.write_bytes(b"committed focus") + assert path.read_bytes() == b"committed focus" + assert len(checkpoint_descriptors) == 1 + assert list(path.parent.iterdir()) == [path] + + +def test_late_restore_error_does_not_hide_successful_rollback(focus, monkeypatch): + service, path, original = focus + replace = os.replace + + def restore_then_raise(source, destination): + replace(source, destination) + raise OSError("late rename error") + + monkeypatch.setattr(focus_module.os, "replace", restore_then_raise) + with pytest.raises(RuntimeError, match="original focus failure"): + with service.project_focus_transaction(): + path.write_bytes(b"new focus") + raise RuntimeError("original focus failure") + assert path.read_bytes() == original + assert list(path.parent.iterdir()) == [path] + + +def test_unrestored_focus_reports_failure_and_keeps_exact_recovery(focus, monkeypatch): + service, path, original = focus + monkeypatch.setattr(service, "_restore_project_focus", lambda *args: None) + with pytest.raises(ImportExecutionError, match="rollback also failed") as caught: + with service.project_focus_transaction(): + path.write_bytes(b"new focus") + raise RuntimeError("focus failed") + backups = list(path.parent.glob("*.recovery")) + assert len(backups) == 1 + assert backups[0].read_bytes() == original + assert str(backups[0]) in str(caught.value) + assert "prior focus existed: True" in str(caught.value) + assert path.read_bytes() == b"new focus" + assert isinstance(caught.value.__cause__, RuntimeError) + + +def test_cleanup_failure_preserves_checkpoint_without_masking_commit(focus, monkeypatch): + service, path, original = focus + verify = service._verify_checkpoint + committed = False + + def fail_cleanup(checkpoint, identity): + if committed: + raise OSError("cleanup denied") + verify(checkpoint, identity) + + monkeypatch.setattr(FocusService, "_verify_checkpoint", staticmethod(fail_cleanup)) + with service.project_focus_transaction(): + path.write_bytes(b"committed focus") + committed = True + backups = list(path.parent.glob("*.recovery")) + assert len(backups) == 1 + assert backups[0].read_bytes() == original + assert path.read_bytes() == b"committed focus" + + +def test_failure_before_first_focus_write_leaves_no_focus_or_checkpoint(focus): + service, path, _ = focus + path.unlink() + with pytest.raises(RuntimeError, match="before write"): + with service.project_focus_transaction(): + raise RuntimeError("before write") + assert not path.exists() + assert list(path.parent.iterdir()) == [] + + +def test_unsafe_focus_and_substituted_checkpoint_are_not_deleted(focus): + service, path, original = focus + unsafe = path.parent / "directory" + unsafe.mkdir() + with pytest.raises(OSError, match="Unsafe project focus"): + service._safe_focus_path(unsafe) + with pytest.raises(OSError, match="checkpoint substituted"): + service._verify_checkpoint(path, (-1, -1)) + assert unsafe.is_dir() + assert path.read_bytes() == original + + +def test_invalid_focused_schema_keeps_focus_but_has_no_available_devices(focus): + service, path, _ = focus + db = path.parent.parent / "invalid.db" + with closing(sqlite3.connect(db)) as conn, conn: + conn.execute("CREATE TABLE unrelated (value INTEGER)") + service._config.write_project_focus(db) + before = path.read_bytes() + state = service.get_focus() + assert state.db_path == db + assert state.available_devices == [] + assert path.read_bytes() == before + + +def test_global_device_update_keeps_project_focus_unchanged(focus, monkeypatch): + service, path, original = focus + db = path.parent.parent / "snapshot.db" + with closing(sqlite3.connect(db)) as conn, conn: + conn.execute( + 'CREATE TABLE dictionary ("table" TEXT, "column" TEXT, "key" TEXT, "value" TEXT)' + ) + conn.execute("CREATE TABLE trace_entry_0 (id INTEGER PRIMARY KEY)") + conn.execute("CREATE TABLE trace_entry_1 (id INTEGER PRIMARY KEY)") + service.set_global_focus(db, device_id=0) + # An explicit environment focus selects the real DB while retaining the + # unrelated project file; --global still updates only the global selection. + monkeypatch.setenv("PT_SNAP_DB_PATH", str(db)) + state = service.set_device(1, scope="global") + assert state.source == "global" + assert state.device_id == 1 + assert service._config.current_device_id == 1 + assert path.read_bytes() == original diff --git a/tests/core/test_critical_coverage_metadata.py b/tests/core/test_critical_coverage_metadata.py new file mode 100644 index 0000000..39894bb --- /dev/null +++ b/tests/core/test_critical_coverage_metadata.py @@ -0,0 +1,208 @@ +"""Malformed metadata must not become a cache hit or replace valid metadata.""" + +import sqlite3 +from contextlib import closing, contextmanager +from dataclasses import asdict, replace +from types import SimpleNamespace + +import pytest + +import pt_snap_cli.core.import_metadata as metadata_module +from pt_snap_cli.core.errors import ( + DatabaseMissingError, + DatabaseSchemaError, + ImportMetadataError, +) +from pt_snap_cli.core.import_metadata import ImportMetadataService +from pt_snap_cli.core.import_metadata_contract import ( + metadata_from_mapping, + required_int, + required_str, + validate_import_metadata, +) +from pt_snap_cli.core.models import ImportMetadata + + +def metadata(**changes): + original = ImportMetadata( + metadata_module.METADATA_SCHEMA_VERSION, + metadata_module.IMPORT_FORMAT_VERSION, + "a" * 64, + 12, + "snapshot.pkl", + None, + "pt-snap-cli", + "1", + "2026-11-01T00:00:00+00:00", + ) + return replace(original, **changes) + + +def create_database(path): + with closing(sqlite3.connect(path)) as conn, conn: + conn.execute( + 'CREATE TABLE dictionary ("table" TEXT, "column" TEXT, "key" TEXT, "value" TEXT)' + ) + conn.execute("CREATE TABLE trace_entry_0 (id INTEGER PRIMARY KEY)") + return path + + +@pytest.mark.parametrize("value", [True, False, 1.0, "1", None]) +def test_metadata_integer_contract_rejects_coercion(value): + with pytest.raises(TypeError, match="integer"): + required_int(value) + assert required_int(0) == 0 + + +@pytest.mark.parametrize("value", ["", 0, None, b"name"]) +def test_metadata_string_contract_requires_nonempty_text(value): + with pytest.raises(TypeError, match="non-empty"): + required_str(value) + assert required_str("snapshot.pkl") == "snapshot.pkl" + + +@pytest.mark.parametrize( + "changes", + [ + {"metadata_schema_version": 0}, + {"import_format_version": 0}, + {"source_sha256": "A" * 64}, + {"source_sha256": "a" * 63}, + {"source_size": -1}, + {"source_name": ""}, + {"requested_device": -1}, + {"importer_name": ""}, + {"importer_version": ""}, + {"completed_at": "2026-11-01T00:00:00"}, + {"completed_at": "2026-11-01T01:00:00+01:00"}, + {"completed_at": "invalid"}, + ], +) +def test_invalid_values_are_rejected_before_metadata_publication(tmp_path, changes): + path = create_database(tmp_path / "snapshot.db") + service = ImportMetadataService() + original = metadata() + service.write(path, original) + before = path.read_bytes() + invalid = metadata(**changes) + + with pytest.raises(ValueError): + validate_import_metadata(invalid) + with pytest.raises(ImportMetadataError, match="invalid import metadata"): + service.write(path, invalid) + + assert path.read_bytes() == before + assert service.inspect(path).metadata == original + + +@pytest.mark.parametrize("device", [None, 0, 7]) +def test_mapping_preserves_exact_device_and_utc_metadata(device): + original = metadata(requested_device=device) + assert metadata_from_mapping(asdict(original)) == original + + +@pytest.mark.parametrize("field", ["metadata_schema_version", "source_size", "requested_device"]) +def test_mapping_rejects_boolean_integer_fields(field): + row = asdict(metadata()) + row[field] = True + with pytest.raises(TypeError, match="integer"): + metadata_from_mapping(row) + + +@pytest.mark.parametrize("operation", ["hash", "stat"]) +def test_source_io_errors_are_metadata_errors_without_creating_source(tmp_path, operation): + missing = tmp_path / "missing.pkl" + service = ImportMetadataService() + with pytest.raises(ImportMetadataError) as caught: + if operation == "hash": + service.calculate_sha256(missing) + else: + service.build_metadata(missing, "a" * 64, None) + assert isinstance(caught.value.__cause__, FileNotFoundError) + assert not missing.exists() + + +@pytest.mark.parametrize("kind", ["missing", "wrong-schema", "not-sqlite"]) +def test_invalid_databases_never_become_cache_hits(tmp_path, kind): + path = tmp_path / "snapshot.db" + if kind == "wrong-schema": + with closing(sqlite3.connect(path)) as conn, conn: + conn.execute("CREATE TABLE unrelated (value INTEGER)") + elif kind == "not-sqlite": + path.write_bytes(b"not sqlite") + original = path.read_bytes() if path.exists() else None + service = ImportMetadataService() + error = DatabaseMissingError if kind == "missing" else DatabaseSchemaError + with pytest.raises(error): + service.inspect(path) + decision = service.evaluate_cache(path, "a" * 64, None) + assert decision.reused is False + assert decision.reason == ("database_missing" if kind == "missing" else "database_invalid") + assert (path.read_bytes() if path.exists() else None) == original + + +def test_metadata_query_failure_is_invalid_and_closes_reader(tmp_path, monkeypatch): + path = create_database(tmp_path / "snapshot.db") + service = ImportMetadataService() + service.write(path, metadata()) + before = path.read_bytes() + closed = [] + + class BrokenContext: + @contextmanager + def connect(self): + try: + yield self + finally: + closed.append(True) + + def execute(self, *args): + raise sqlite3.DatabaseError("reader failure") + + monkeypatch.setattr(metadata_module, "Context", lambda path: BrokenContext()) + inspection = service.inspect(path) + assert (inspection.status, inspection.reason, inspection.metadata) == ( + "invalid", + "metadata_invalid", + None, + ) + assert closed == [True] + assert path.read_bytes() == before + + +def test_metadata_write_open_failure_is_normalized_without_creating_output(tmp_path): + path = tmp_path / "missing-parent" / "snapshot.db" + with pytest.raises(ImportMetadataError, match="Cannot write import metadata") as caught: + ImportMetadataService().write(path, metadata()) + assert isinstance(caught.value.__cause__, sqlite3.DatabaseError) + assert not path.exists() + assert not path.parent.exists() + + +@pytest.mark.parametrize("raw", [b"not-json", b'{"ptSnap":{}}', None]) +def test_dataset_metadata_corruption_is_invalid_not_reusable(tmp_path, monkeypatch, raw): + manifest = tmp_path / "manifest.json" + if raw is not None: + manifest.write_bytes(raw) + dataset = SimpleNamespace(root=tmp_path, validation=SimpleNamespace(manifest=object())) + monkeypatch.setattr(metadata_module.DatasetResolver, "inspect", lambda self, path: dataset) + + def invalid_owned_cache(path): + raise ValueError("producer metadata corrupt") + + monkeypatch.setattr(metadata_module, "inspect_owned_cache", invalid_owned_cache) + inspection = ImportMetadataService().inspect(tmp_path) + assert (inspection.status, inspection.reason, inspection.metadata) == ( + "invalid", + "metadata_invalid", + None, + ) + assert (manifest.read_bytes() if manifest.exists() else None) == raw + + +def test_unattested_dataset_metadata_is_explicitly_unavailable(tmp_path, monkeypatch): + (tmp_path / "manifest.json").write_bytes(b"{}") + dataset = SimpleNamespace(root=tmp_path, validation=SimpleNamespace(manifest=object())) + monkeypatch.setattr(metadata_module.DatasetResolver, "inspect", lambda self, path: dataset) + inspection = ImportMetadataService().inspect(tmp_path) + assert (inspection.status, inspection.reason) == ("unavailable", "metadata_missing") diff --git a/tests/core/test_critical_coverage_skills.py b/tests/core/test_critical_coverage_skills.py new file mode 100644 index 0000000..1d3d4a4 --- /dev/null +++ b/tests/core/test_critical_coverage_skills.py @@ -0,0 +1,157 @@ +"""Skill publication faults preserve the previous copy or explicit recovery bytes.""" + +from pathlib import Path + +import pytest + +import pt_snap_cli.core.skill_service as skill_module +from pt_snap_cli.core.errors import SkillCatalogError, SkillInstallError +from pt_snap_cli.core.skill_service import SkillService + + +@pytest.fixture +def skills(tmp_path): + catalog = tmp_path / "catalog" + source = catalog / "pt-snap-test" + source.mkdir(parents=True) + (source / "SKILL.md").write_text( + "---\nname: pt-snap-test\ndescription: Test skill\n---\nNew content\n", encoding="utf-8" + ) + service = SkillService( + catalog_dir=catalog, home=tmp_path / "home", cwd=tmp_path / "project", environ={} + ) + return service, source, tmp_path / "installed" / "pt-snap-test" + + +@pytest.mark.parametrize("preexisting", [False, True]) +def test_failed_staging_copy_does_not_publish_or_remove_previous_skill( + skills, monkeypatch, preexisting +): + service, source, dest = skills + if preexisting: + dest.mkdir(parents=True) + (dest / "SKILL.md").write_bytes(b"previous skill") + before = (source / "SKILL.md").read_bytes() + + def fail_copy(source, staging): + staging.mkdir() + (staging / "SKILL.md").write_bytes(b"partial copy") + raise OSError("copy interrupted") + + monkeypatch.setattr(skill_module.shutil, "copytree", fail_copy) + with pytest.raises(SkillInstallError, match="copy interrupted"): + service.install_skills(dest_dir=dest.parent, force=True) + assert (source / "SKILL.md").read_bytes() == before + if preexisting: + assert (dest / "SKILL.md").read_bytes() == b"previous skill" + assert list(dest.parent.iterdir()) == [dest] + else: + assert not dest.exists() + assert list(dest.parent.iterdir()) == [] + + +def test_failed_publish_and_restore_keep_previous_backup(skills, monkeypatch): + service, source, dest = skills + dest.mkdir(parents=True) + (dest / "SKILL.md").write_bytes(b"previous skill") + replace = Path.replace + + def fail_staging_and_restore(path, target): + if path.name.endswith((".pt-snap-staging", ".pt-snap-backup")): + raise OSError("rename denied") + return replace(path, target) + + monkeypatch.setattr(Path, "replace", fail_staging_and_restore) + with pytest.raises(SkillInstallError, match="rename denied"): + service.install_skills(dest_dir=dest.parent, force=True) + backups = list(dest.parent.glob("*.pt-snap-backup")) + assert len(backups) == 1 + assert (backups[0] / "SKILL.md").read_bytes() == b"previous skill" + assert not dest.exists() + assert not list(dest.parent.glob("*.pt-snap-staging")) + assert (source / "SKILL.md").is_file() + + +def test_backup_cleanup_failure_does_not_mask_successful_publication(skills, monkeypatch): + service, source, dest = skills + dest.mkdir(parents=True) + (dest / "SKILL.md").write_bytes(b"previous skill") + remove = skill_module._remove_path + + def fail_backup_cleanup(path): + if path.name.endswith(".pt-snap-backup"): + raise OSError("cleanup denied") + remove(path) + + monkeypatch.setattr(skill_module, "_remove_path", fail_backup_cleanup) + report = service.install_skills(dest_dir=dest.parent, force=True) + assert report.results[0].action == "updated" + assert (dest / "SKILL.md").read_bytes() == (source / "SKILL.md").read_bytes() + backups = list(dest.parent.glob("*.pt-snap-backup")) + assert len(backups) == 1 + assert (backups[0] / "SKILL.md").read_bytes() == b"previous skill" + assert not list(dest.parent.glob("*.pt-snap-staging")) + + +def test_uninstall_oserror_is_normalized_without_touching_other_paths(skills, monkeypatch): + service, _, dest = skills + service.install_skills(dest_dir=dest.parent) + original = (dest / "SKILL.md").read_bytes() + + def denied(path): + assert path == dest + raise PermissionError("delete denied") + + monkeypatch.setattr(skill_module, "_remove_path", denied) + with pytest.raises(SkillInstallError, match="Failed to uninstall") as caught: + service.uninstall_skills(dest_dir=dest.parent) + assert isinstance(caught.value.__cause__, PermissionError) + assert (dest / "SKILL.md").read_bytes() == original + assert list(dest.parent.iterdir()) == [dest] + + +def test_upgrade_current_skill_is_an_explicit_noop(skills): + service, _, dest = skills + service.install_skills(dest_dir=dest.parent) + before = (dest / "SKILL.md").stat() + report = service.upgrade_skills(dest_dir=dest.parent) + after = (dest / "SKILL.md").stat() + assert report.results[0].action == "already_installed" + assert (after.st_ino, after.st_mtime_ns) == (before.st_ino, before.st_mtime_ns) + + +def test_catalog_falls_back_to_package_and_fails_when_no_source_exists(tmp_path, monkeypatch): + packaged = tmp_path / "bundled" + monkeypatch.setattr(skill_module, "_discover_repo_skills_dir", lambda: None) + monkeypatch.setattr(skill_module, "_packaged_skills_dir", lambda: packaged) + assert skill_module.default_catalog_dir({}) == packaged + monkeypatch.setattr(skill_module, "_packaged_skills_dir", lambda: None) + with pytest.raises(SkillCatalogError, match="not available"): + skill_module.default_catalog_dir({}) + + +@pytest.mark.parametrize( + "content", ["plain skill", "---\nname: incomplete", "---\n- list\n---\nbody"] +) +def test_legacy_frontmatter_falls_back_to_directory_name(tmp_path, content): + catalog = tmp_path / "catalog" + skill = catalog / "pt-snap-fallback" + skill.mkdir(parents=True) + (skill / "SKILL.md").write_text(content, encoding="utf-8") + specs = skill_module.list_catalog_skills(catalog) + assert [(spec.name, spec.description) for spec in specs] == [("pt-snap-fallback", "")] + + +def test_unreadable_frontmatter_is_a_catalog_error_without_mutation(skills, monkeypatch): + service, source, dest = skills + read_text = Path.read_text + + def denied(path, *args, **kwargs): + if path == source / "SKILL.md": + raise PermissionError("read denied") + return read_text(path, *args, **kwargs) + + monkeypatch.setattr(Path, "read_text", denied) + with pytest.raises(SkillCatalogError, match="Failed to read skill file"): + service.install_skills(dest_dir=dest.parent) + assert not dest.parent.exists() diff --git a/tests/core/test_critical_coverage_split.py b/tests/core/test_critical_coverage_split.py new file mode 100644 index 0000000..9fc0231 --- /dev/null +++ b/tests/core/test_critical_coverage_split.py @@ -0,0 +1,193 @@ +"""Unsuccessful replay or exclusive publication must leave no partial result.""" + +import errno +import os +import pickle +from pathlib import Path +from types import SimpleNamespace + +import pytest + +import pt_snap_cli.core.split_service as split_module +from pt_snap_cli.core.errors import SplitError +from pt_snap_cli.core.models import SplitOptions +from pt_snap_cli.core.split_service import SplitService + + +@pytest.fixture +def split_input(tmp_path): + source = tmp_path / "snapshot.pkl" + # Test-owned input only. Fault-injected replay below owns the generated + # output; actual allocator validity is covered by the ordinary split suite. + source.write_bytes(pickle.dumps({"segments": [], "device_traces": [[{"id": 0}]]})) + return SplitOptions(snapshot_file=source, output=tmp_path / "split", slices=1) + + +def assert_unpublished(options): + assert not options.output.exists() + assert not list(options.output.parent.glob(".split.pt-snap-*")) + + +def test_missing_device_trace_list_fails_without_staging(split_input): + split_input.snapshot_file.write_bytes(pickle.dumps({"device_traces": {}})) + before = split_input.snapshot_file.read_bytes() + with pytest.raises(SplitError, match="no device trace list") as caught: + SplitService().split(split_input) + assert caught.value.phase == "device" + assert split_input.snapshot_file.read_bytes() == before + assert_unpublished(split_input) + + +def test_stage_creation_failure_is_normalized_without_loading_replay(split_input, monkeypatch): + def denied(*args, **kwargs): + raise PermissionError("staging denied") + + monkeypatch.setattr(split_module.tempfile, "mkdtemp", denied) + monkeypatch.setattr( + split_module, + "replay_snapshot", + lambda *args, **kwargs: pytest.fail("no replay before staging"), + ) + with pytest.raises(SplitError, match="cannot create staging directory") as caught: + SplitService().split(split_input) + assert caught.value.phase == "publication" + assert isinstance(caught.value.__cause__, PermissionError) + assert_unpublished(split_input) + + +@pytest.mark.parametrize( + ("failure", "phase", "message"), + [ + ("source-replay", "load/engine", "replay engine returned false"), + ("empty-output", "load/engine", "engine generated no slices"), + ("bad-filename", "generated-validation", "Unexpected engine output name"), + ("generated-replay", "generated-validation", "replay returned false"), + ], +) +def test_replay_failures_remove_owned_stage_and_preserve_source( + split_input, monkeypatch, failure, phase, message +): + before = split_input.snapshot_file.read_bytes() + calls = [] + + def replay(representation, device, **kwargs): + calls.append("source" if "hooker" in kwargs else "generated") + if "hooker" not in kwargs: + return None, False + if failure in ("bad-filename", "generated-replay"): + stages = list(split_input.output.parent.glob(".split.pt-snap-*")) + assert len(stages) == 1 + raw = stages[0] / ".device-0" + name = "unexpected.pkl" if failure == "bad-filename" else "snapshot_entry_0_0.pkl" + (raw / name).write_bytes(before) + return None, failure != "source-replay" + + monkeypatch.setattr(split_module, "replay_snapshot", replay) + with pytest.raises(SplitError, match=message) as caught: + SplitService().split(split_input) + assert caught.value.phase == phase + assert calls == (["source", "generated"] if failure == "generated-replay" else ["source"]) + assert split_input.snapshot_file.read_bytes() == before + assert_unpublished(split_input) + + +@pytest.mark.parametrize("method", ["_stage_identity", "_verify_stage_identity"]) +def test_non_directory_stage_is_never_accepted_or_deleted(tmp_path, method): + path = tmp_path / "not-a-stage" + path.write_bytes(b"another owner's file") + with pytest.raises(OSError, match="not an owned directory"): + if method == "_stage_identity": + SplitService._stage_identity(path) + else: + SplitService._verify_stage_identity(path, (0, 0, None)) + assert path.read_bytes() == b"another owner's file" + + +def test_descriptor_identity_mismatch_is_rejected_without_deleting_stage(tmp_path): + stage = tmp_path / "stage" + stage.mkdir() + unrelated = tmp_path / "unrelated-file" + unrelated.write_bytes(b"another owner's evidence") + status = stage.lstat() + # An actual mismatched descriptor exercises the held-identity guard on every + # platform, including Windows where ordinary stages do not hold directory fds. + descriptor = os.open(unrelated, os.O_RDONLY) + identity = (status.st_dev, status.st_ino, descriptor) + try: + held = os.fstat(descriptor) + assert (held.st_dev, held.st_ino) != identity[:2] + with pytest.raises(OSError, match="identity changed"): + SplitService._verify_stage_identity(stage, identity) + assert stage.is_dir() + assert unrelated.read_bytes() == b"another owner's evidence" + finally: + SplitService._close_stage_identity(identity) + with pytest.raises(OSError) as closed: + os.fstat(descriptor) + assert closed.value.errno == errno.EBADF + + +def test_closed_stage_descriptor_cleanup_is_idempotent(tmp_path): + identity = SplitService._stage_identity(tmp_path) + SplitService._close_stage_identity(identity) + SplitService._close_stage_identity(identity) + assert tmp_path.is_dir() + + +@pytest.mark.parametrize("platform", ["darwin", "linux"]) +def test_native_publication_errors_preserve_both_paths(tmp_path, monkeypatch, platform): + stage = tmp_path / "stage" + destination = tmp_path / "destination" + stage.mkdir() + destination.mkdir() + (stage / "slice.pkl").write_bytes(b"staged") + (destination / "owner.txt").write_bytes(b"other owner") + + class FailedRename: + def __call__(self, *args): + return -1 + + library = SimpleNamespace(renamex_np=FailedRename(), renameat2=FailedRename()) + monkeypatch.setattr(split_module.sys, "platform", platform) + monkeypatch.setattr(split_module.ctypes, "CDLL", lambda *args, **kwargs: library) + monkeypatch.setattr(split_module.ctypes, "get_errno", lambda: errno.EEXIST) + with pytest.raises(OSError) as caught: + SplitService._publish_directory(stage, destination) + assert caught.value.errno == errno.EEXIST + assert Path(caught.value.filename) == destination + assert (stage / "slice.pkl").read_bytes() == b"staged" + assert (destination / "owner.txt").read_bytes() == b"other owner" + + +@pytest.mark.parametrize("platform", ["darwin", "linux", "unsupported"]) +def test_unavailable_no_replace_primitive_fails_closed(tmp_path, monkeypatch, platform): + stage, destination = tmp_path / "stage", tmp_path / "destination" + stage.mkdir() + monkeypatch.setattr(split_module.sys, "platform", platform) + monkeypatch.setattr(split_module, "os", SimpleNamespace(name="posix")) + monkeypatch.setattr(split_module.ctypes, "CDLL", lambda *args, **kwargs: object()) + with pytest.raises(OSError) as caught: + SplitService._preflight_publication() + assert caught.value.errno == errno.ENOTSUP + if platform != "darwin": + with pytest.raises(OSError) as caught: + SplitService._publish_directory(stage, destination) + assert caught.value.errno == errno.ENOTSUP + assert stage.is_dir() + assert not destination.exists() + + +def test_windows_publication_uses_non_replacing_rename(tmp_path, monkeypatch): + stage, destination = tmp_path / "stage", tmp_path / "destination" + calls = [] + + def rename(source, target): + calls.append((source, target)) + raise FileExistsError(errno.EEXIST, "destination exists", target) + + monkeypatch.setattr(split_module.sys, "platform", "win32") + monkeypatch.setattr(split_module, "os", SimpleNamespace(name="nt", rename=rename)) + SplitService._preflight_publication() + with pytest.raises(FileExistsError): + SplitService._publish_directory(stage, destination) + assert calls == [(stage, destination)] diff --git a/tests/snapshot/test_representation_boundaries.py b/tests/snapshot/test_representation_boundaries.py new file mode 100644 index 0000000..a3d2272 --- /dev/null +++ b/tests/snapshot/test_representation_boundaries.py @@ -0,0 +1,214 @@ +"""Representation dispatch, canonical values and real replay entrypoint contracts.""" + +import copy +import hashlib +import json +from pathlib import Path +from typing import cast + +import pytest + +from pt_snap_cli.snapshot.base import DeviceSnapshot, Segment, TraceEntry +from pt_snap_cli.snapshot.representation import ( + SnapshotFormat, + canonical_snapshot_bytes, + canonical_snapshot_sha256, + canonicalize_snapshot, + load_and_replay_snapshot, + load_snapshot_representation, + replay_snapshot, + save_snapshot_representation, + serialize_snapshot_representation, +) +from pt_snap_cli.snapshot.simulate import AllocatorHooker, SimulateHooker + +from .helpers import assert_valid_snapshot + + +def _representation() -> dict[str, object]: + frames = [{"filename": "test.py", "name": "allocate", "line": 17}] + return { + "segments": [ + { + "address": 4096, + "total_size": 64, + "stream": 0, + "segment_type": "small", + "allocated_size": 0, + "active_size": 0, + "device": 1, + "frames": copy.deepcopy(frames), + "blocks": [], + } + ], + "device_traces": [ + [], + [ + { + "id": 17, + "action": "segment_alloc", + "addr": 4096, + "size": 64, + "stream": 0, + "frames": frames, + } + ], + ], + "metadata": {"label": "内存", "ratio": 1.25, "optional": None}, + } + + +@pytest.mark.parametrize("snapshot_format", ["pickle", "json"]) +@pytest.mark.parametrize("valid_replay", [True, False]) +def test_dispatch_round_trip_and_replay_preserve_source( + tmp_path: Path, snapshot_format: SnapshotFormat, valid_replay: bool +) -> None: + value = _representation() + if not valid_replay: + # The event cannot be undone without its segment: replay must report False. + value["segments"] = [] + original = copy.deepcopy(value) + path = tmp_path / f"snapshot.{snapshot_format}" + encoded = serialize_snapshot_representation(value, snapshot_format) + save_snapshot_representation(value, str(path), snapshot_format) + assert path.read_bytes() == encoded + assert load_snapshot_representation(str(path), snapshot_format) == original + + loaded, runtime, replayed = load_and_replay_snapshot(path, snapshot_format, 1) + + assert loaded == original == value + assert runtime.device == 1 + assert replayed is valid_replay + assert [entry.idx for entry in runtime.device_snapshot.trace_entries] == ( + [] if valid_replay else [17] + ) + assert runtime.device_snapshot.segments == [] + assert runtime.device_snapshot.total_reserved == 0 + assert_valid_snapshot(runtime.device_snapshot) + assert path.read_bytes() == encoded + assert list(tmp_path.iterdir()) == [path] + + +def test_default_dispatch_uses_protocol_four_pickle(tmp_path: Path) -> None: + value = _representation() + path = tmp_path / "snapshot.pkl" + encoded = serialize_snapshot_representation(value) + assert encoded.startswith(b"\x80\x04") + save_snapshot_representation(value, path) + assert path.read_bytes() == encoded + assert load_snapshot_representation(path) == value + + +@pytest.mark.parametrize("exists", [True, False]) +def test_unsupported_formats_do_not_read_or_overwrite_outputs(tmp_path: Path, exists: bool): + path = tmp_path / "snapshot.yaml" + sentinel = b"existing output must be preserved" + if exists: + path.write_bytes(sentinel) + value = _representation() + original = copy.deepcopy(value) + unsupported = cast(SnapshotFormat, "yaml") + + with pytest.raises(ValueError, match="Unsupported snapshot format: yaml"): + load_snapshot_representation(path, unsupported) + with pytest.raises(ValueError, match="Unsupported snapshot format: yaml"): + serialize_snapshot_representation(value, unsupported) + with pytest.raises(ValueError, match="Unsupported snapshot format: yaml"): + save_snapshot_representation(value, path, unsupported) + + assert value == original + assert path.exists() is exists + if exists: + assert path.read_bytes() == sentinel + assert list(tmp_path.iterdir()) == ([path] if exists else []) + + +@pytest.mark.parametrize("value", [None, [], "snapshot", 7, 1.25, True]) +def test_json_requires_object_without_rewriting_source(tmp_path: Path, value: object): + path = tmp_path / "snapshot.json" + original = json.dumps(value).encode() + path.write_bytes(original) + with pytest.raises(ValueError, match=f"actual type: {type(value).__name__}"): + load_snapshot_representation(path, "json") + assert path.read_bytes() == original + assert list(tmp_path.iterdir()) == [path] + + +def test_canonical_finite_values_preserve_sequence_order_and_utf8(): + value = {"z": (None, True, -7, 1.25, -0.0), "a": {"标签": "内存", "order": [3, 1, 2]}} + original = copy.deepcopy(value) + expected = '{"a":{"order":[3,1,2],"标签":"内存"},"z":[null,true,-7,1.25,-0.0]}'.encode() + assert canonical_snapshot_bytes(value) == expected + assert canonical_snapshot_sha256(value) == hashlib.sha256(expected).hexdigest() + assert canonicalize_snapshot(value) == json.loads(expected) + assert value == original + + +@pytest.mark.parametrize("value", [float("inf"), float("-inf")]) +def test_json_nonfinite_values_do_not_replace_previous_output(tmp_path: Path, value: float): + path = tmp_path / "snapshot.json" + path.write_bytes(b"previous output") + with pytest.raises(ValueError, match="non-finite floats"): + save_snapshot_representation({"nested": [value]}, path, "json") + assert path.read_bytes() == b"previous output" + assert list(tmp_path.iterdir()) == [path] + + +class _ReplayObserver(SimulateHooker): + def __init__(self) -> None: + self.calls: list[tuple[str, int, int]] = [] + self.raw_frames: list[bool] = [] + + def pre_undo_event(self, event: TraceEntry, snapshot: DeviceSnapshot) -> bool: + self.calls.append(("pre", event.idx, snapshot.total_reserved)) + self.raw_frames.append(event._raw_frames is not None) + return True + + def post_undo_event(self, event: TraceEntry, snapshot: DeviceSnapshot) -> bool: + self.calls.append(("post", event.idx, snapshot.total_reserved)) + return True + + +class _AllocatorObserver(AllocatorHooker): + def __init__(self) -> None: + self.calls: list[tuple[str, int, int]] = [] + + def pre_replay_unmap_or_free_segment(self, segment: Segment, snapshot: DeviceSnapshot): + self.calls.append(("pre", segment.address, snapshot.total_reserved)) + + def post_replay_unmap_or_free_segment(self, segment: Segment, snapshot: DeviceSnapshot): + self.calls.append(("post", segment.address, snapshot.total_reserved)) + + +@pytest.mark.parametrize( + ("with_event_hook", "with_allocator_hook"), + [(False, False), (True, False), (False, True), (True, True)], +) +@pytest.mark.parametrize("raw_frames", [False, True]) +def test_replay_entrypoint_registers_optional_hooks_before_real_replay( + with_event_hook: bool, with_allocator_hook: bool, raw_frames: bool +) -> None: + value = _representation() + original = copy.deepcopy(value) + event_hook = _ReplayObserver() + allocator_hook = _AllocatorObserver() + + runtime, replayed = replay_snapshot( + value, + 1, + hooker=event_hook if with_event_hook else None, + allocator_hooker=allocator_hook if with_allocator_hook else None, + _raw_frames=raw_frames, + ) + + assert replayed is True + assert runtime.device == 1 + assert event_hook.calls == ([("pre", 17, 64), ("post", 17, 0)] if with_event_hook else []) + assert event_hook.raw_frames == ([raw_frames] if with_event_hook else []) + assert allocator_hook.calls == ( + [("pre", 4096, 64), ("post", 4096, 0)] if with_allocator_hook else [] + ) + assert runtime.device_snapshot.trace_entries == [] + assert runtime.device_snapshot.segments == [] + assert_valid_snapshot(runtime.device_snapshot) + assert value == original diff --git a/tests/test_ci_contract_mutations.py b/tests/test_ci_contract_mutations.py index a579e2a..c1a021f 100644 --- a/tests/test_ci_contract_mutations.py +++ b/tests/test_ci_contract_mutations.py @@ -7,6 +7,8 @@ from tests._ci_contract import command_tokens, commands, expression, matrix_rows from tests.test_ci_tiers import project, validate_workflow, workflow +CRITICAL_CHECKER = ".github/scripts/check_critical_coverage.py" + def _step(config, job, program, contains=None): matching = [ @@ -57,13 +59,8 @@ def _quotes_and_commands(config): "python3 -m pytest --cov-report term-missing --cov -m 'not nightly' --cov-report xml", "--cov", ) - guard = _step(config, "test", "coverage") - guard["run"] = ( - guard["run"] - .replace("--include='", '--include "') - .replace(".py'", '.py"') - .replace("--fail-under=", "--fail-under ") - ) + guard = _step(config, "test", CRITICAL_CHECKER) + guard["run"] = 'python3 ".github/scripts/check_critical_coverage.py"' def _conditions(config): @@ -90,19 +87,23 @@ def _excluded_then_restored_matrix(config): def _split_and_merge(config): job = config["jobs"]["test"] - guard = _step(config, "test", "coverage") - index = job["steps"].index(guard) - parts = [dict(guard, run=line) for line in guard["run"].splitlines() if line.strip()] + build = _step(config, "test", "build") + index = job["steps"].index(build) + parts = [dict(build, run=line) for line in build["run"].splitlines() if line.strip()] + assert len(parts) > 1, "Positive split mutation must split real sequential commands" job["steps"][index : index + 1] = parts - # Merge full pytest with the first independent floor; labels are irrelevant. + # Independently merge full pytest with its checker; labels are irrelevant. + guard = _step(config, "test", CRITICAL_CHECKER) full = _step(config, "test", "pytest", "--cov") - full["run"] += "\n" + parts[0]["run"] - job["steps"].remove(parts[0]) + full["run"] += "\n" + guard["run"] + job["steps"].remove(guard) def _and_chain(config): - guard = _step(config, "test", "coverage") - guard["run"] = " && ".join(guard["run"].splitlines()) + guard = _step(config, "test", CRITICAL_CHECKER) + full = _step(config, "test", "pytest", "--cov") + full["run"] += " && " + guard["run"] + config["jobs"]["test"]["steps"].remove(guard) def _include_matrix(config): @@ -136,7 +137,7 @@ def _weaken(config, case): job = config["jobs"]["test"] full = _step(config, "test", "pytest", "--cov") fast = _step(config, "test", "pytest", "not slow and not nightly") - guard = _step(config, "test", "coverage") + guard = _step(config, "test", CRITICAL_CHECKER) browser = _step(config, "browser", "pytest") if case == "missing-python": job["strategy"]["matrix"]["python-version"].remove("3.12") @@ -211,14 +212,30 @@ def _weaken(config, case): full["run"] += " -k passing_only" elif case == "pytest-path-filter": full["run"] += " tests/core" - elif case == "coverage-threshold": - guard["run"] = guard["run"].replace("--fail-under=90", "--fail-under=89") - elif case == "coverage-combined-selector": - guard["run"] = guard["run"].replace("*/core/snapshot_import_backend.py", "*/core/*.py") - elif case == "coverage-wrong-file": - guard["run"] = guard["run"].replace( - "*/core/snapshot_import_backend.py", "*/core/import_metadata.py" - ) + elif case == "coverage-checker-omitted": + job["steps"].remove(guard) + elif case == "coverage-checker-duplicate": + job["steps"].append(deepcopy(guard)) + elif case == "coverage-checker-conditional": + guard["if"] += " && github.event_name == 'push'" + elif case == "coverage-checker-wrong-python": + guard["if"] = "matrix.python-version == '3.12'" + elif case == "coverage-checker-overrides": + guard["run"] += " --fail-under=0" + elif case == "coverage-checker-positional": + guard["run"] += " other.coverage" + elif case == "coverage-checker-ignored-failure": + guard["continue-on-error"] = "true" + elif case == "coverage-checker-or-true": + guard["run"] += " || true" + elif case == "coverage-checker-and-list-swallow": + guard["run"] += " && pip --version\npip --version" + elif case == "coverage-checker-conditional-shell": + guard["run"] = "if false; then " + guard["run"] + "; fi" + elif case == "coverage-alternate-report": + guard["run"] = "python -m coverage report --fail-under=0" + elif case == "coverage-data-erased": + guard["run"] = "python -m coverage erase\n" + guard["run"] elif case == "coverage-before-measurement": job["steps"].remove(guard) job["steps"].insert(job["steps"].index(full), guard) @@ -313,9 +330,18 @@ def _weaken(config, case): "full-includes-nightly", "pytest-k-filter", "pytest-path-filter", - "coverage-threshold", - "coverage-combined-selector", - "coverage-wrong-file", + "coverage-checker-omitted", + "coverage-checker-duplicate", + "coverage-checker-conditional", + "coverage-checker-wrong-python", + "coverage-checker-overrides", + "coverage-checker-positional", + "coverage-checker-ignored-failure", + "coverage-checker-or-true", + "coverage-checker-and-list-swallow", + "coverage-checker-conditional-shell", + "coverage-alternate-report", + "coverage-data-erased", "coverage-before-measurement", "fixture-after-tests", "optional-browser", @@ -349,6 +375,35 @@ def test_coverage_configuration_cannot_weaken(field, value): validate_workflow(workflow(), settings) +def test_coverage_configuration_order_and_display_precision_do_not_change_gate(): + settings = deepcopy(project()) + coverage = settings["tool"]["coverage"] + coverage["run"]["omit"].reverse() + coverage["report"]["exclude_lines"].reverse() + coverage["report"]["precision"] = 4 + validate_workflow(workflow(), settings) + + +@pytest.mark.parametrize( + ("section", "field", "value"), + [ + ("run", "source", ["pt_snap_cli.snapshot"]), + ("run", "omit", ["*/tests/*", "*/__pycache__/*", "*/core/*"]), + ("run", "include", ["*/snapshot/*"]), + ("report", "include", ["*/snapshot/*"]), + ("report", "omit", ["*/core/*"]), + ("report", "exclude_lines", [".*"]), + ("report", "exclude_also", [".*"]), + ("report", "partial_branches", [".*"]), + ], +) +def test_coverage_selectors_and_exclusions_cannot_hide_runtime(section, field, value): + settings = deepcopy(project()) + settings["tool"]["coverage"][section][field] = value + with pytest.raises(AssertionError): + validate_workflow(workflow(), settings) + + def test_include_matrix_rows_never_become_new_cartesian_originals(): assert matrix_rows( { diff --git a/tests/test_ci_tiers.py b/tests/test_ci_tiers.py index 0be3274..496e94b 100644 --- a/tests/test_ci_tiers.py +++ b/tests/test_ci_tiers.py @@ -112,37 +112,42 @@ def validate_tiers(config, settings): coverage = settings["tool"]["coverage"] require(coverage["run"]["branch"] is True, "Branch coverage is mandatory") - require(coverage["report"]["fail_under"] >= 90, "Overall coverage floor must not decrease") - expected = { - "*/core/snapshot_import_backend.py": 90, - "*/simulate/simulated_caching_allocator.py": 88, - "*/simulate/snapshot_mutator.py": 84, - } - reports = select(all_commands, "coverage") - require(len(reports) == len(expected), "Each critical file needs its own coverage floor") - seen = set() - for report in reports: - require(report.words[1] == "report", "Critical coverage must report measured results") - opts, positional = options(report.words[2:], valued=("--include", "--fail-under")) - require( - not positional and set(opts) == {"--include", "--fail-under"}, - "Coverage report must have an independent selector and floor", - ) - require( - all(len(values) == 1 for values in opts.values()), - "Duplicate coverage options are ambiguous", - ) - path = opts["--include"][0] - require( - path in expected and path not in seen, "Wrong/combined critical-file coverage selector" - ) - require( - float(opts["--fail-under"][0]) >= expected[path], - "Critical coverage floor must not decrease", - ) - seen.add(path) - require(full.index < report.index, "Coverage must be measured before its floors") - _full(job, report.step) + require( + coverage["report"]["fail_under"] >= 90, + "Overall coverage floor must not decrease", + ) + require( + coverage["run"].get("source") == ["pt_snap_cli"], + "Coverage must measure the entire runtime package", + ) + require( + set(coverage["run"].get("omit", [])) == {"*/tests/*", "*/__pycache__/*"}, + "Runtime modules cannot be omitted from coverage", + ) + require( + not coverage["run"].get("include") + and not coverage["report"].get("include") + and not coverage["report"].get("omit"), + "Coverage measurement/report selectors must not narrow the package", + ) + require( + set(coverage["report"].get("exclude_lines", [])) + == {"pragma: no cover", "if __name__ == .__main__.", "if TYPE_CHECKING:"} + and not coverage["report"].get("exclude_also") + and not any(key.startswith("partial_") for key in coverage["report"]), + "Coverage exclusions must not hide runtime statements or branches", + ) + require( + not select(all_commands, "coverage"), + "Alternate coverage commands cannot replace or bypass the critical checker", + ) + guard = one(all_commands, ".github/scripts/check_critical_coverage.py") + require( + len(guard.words) == 1, + "Critical coverage checker must use its complete fixed inventory without overrides", + ) + require(full.index < guard.index, "Coverage must be measured before its floors") + _full(job, guard.step) build = one(all_commands, "build") require(build.words[1:] == ("--wheel",), "Build must produce the wheel") diff --git a/tests/test_critical_coverage_gate.py b/tests/test_critical_coverage_gate.py new file mode 100644 index 0000000..cac9d0c --- /dev/null +++ b/tests/test_critical_coverage_gate.py @@ -0,0 +1,122 @@ +"""Exercise the coverage guard with real coverage databases, without running code.""" + +import importlib.util +from pathlib import Path + +import pytest +from coverage import CoverageData + + +@pytest.fixture +def guard(): + path = Path(".github/scripts/check_critical_coverage.py") + spec = importlib.util.spec_from_file_location("critical_coverage_guard", path) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def write_evidence(guard, root, *, missing=None, uncovered=None, branches=True): + (root / "pyproject.toml").write_text( + "[tool.coverage.run]\nbranch = true\n[tool.coverage.report]\nfail_under = 90\n", + encoding="utf-8", + ) + data = CoverageData(basename=str(root / ".coverage")) + for relative in guard.CRITICAL_FLOORS: + source = root / relative + source.parent.mkdir(parents=True, exist_ok=True) + source.write_text("if flag:\n result = 1\nelse:\n result = 2\n", encoding="utf-8") + if relative == missing: + continue + if branches: + arcs = [(-1, 1), (1, 2), (2, -1)] + if relative != uncovered: + arcs.extend([(1, 4), (4, -1)]) + data.add_arcs({str(source): arcs}) + else: + data.add_lines({str(source): [1, 2, 4]}) + data.write() + + +def test_critical_floor_inventory_preserves_existing_and_adds_risky_modules(guard): + assert guard.CRITICAL_FLOORS == { + "src/pt_snap_cli/core/snapshot_import_backend.py": 90, + "src/pt_snap_cli/snapshot/simulate/simulated_caching_allocator.py": 88, + "src/pt_snap_cli/snapshot/simulate/snapshot_mutator.py": 84, + "src/pt_snap_cli/core/import_metadata_contract.py": 100, + "src/pt_snap_cli/core/import_metadata.py": 90, + "src/pt_snap_cli/core/split_service.py": 90, + "src/pt_snap_cli/core/focus_service.py": 90, + "src/pt_snap_cli/core/skill_service.py": 90, + "src/pt_snap_cli/core/snapshot_cache.py": 90, + "src/pt_snap_cli/snapshot/representation.py": 95, + } + + +def test_guard_accepts_complete_branch_evidence(guard, tmp_path, capsys): + write_evidence(guard, tmp_path) + guard.check_coverage(tmp_path) + output = capsys.readouterr().out + assert output.count("100.0000%") == len(guard.CRITICAL_FLOORS) + + +def test_guard_rejects_missing_data_without_creating_it(guard, tmp_path): + with pytest.raises(guard.CoverageGuardError, match="Missing coverage data"): + guard.check_coverage(tmp_path) + assert not (tmp_path / ".coverage").exists() + + +@pytest.mark.parametrize("failure", ["corrupt", "empty", "line-only"]) +def test_guard_rejects_unusable_evidence(guard, tmp_path, failure): + write_evidence(guard, tmp_path, branches=failure != "line-only") + if failure == "corrupt": + (tmp_path / ".coverage").write_bytes(b"not a coverage database") + elif failure == "empty": + (tmp_path / ".coverage").write_bytes(b"") + with pytest.raises(guard.CoverageGuardError): + guard.check_coverage(tmp_path) + + +@pytest.mark.parametrize("failure", ["unmeasured", "deleted", "no-statements", "below-floor"]) +def test_one_critical_file_cannot_be_hidden_by_other_full_coverage(guard, tmp_path, failure): + relative = "src/pt_snap_cli/core/focus_service.py" + write_evidence( + guard, + tmp_path, + missing=relative if failure == "unmeasured" else None, + uncovered=relative if failure == "below-floor" else None, + ) + if failure == "deleted": + (tmp_path / relative).unlink() + elif failure == "no-statements": + (tmp_path / relative).write_text("# no executable source\n", encoding="utf-8") + with pytest.raises(guard.CoverageGuardError, match="core/focus_service.py"): + guard.check_coverage(tmp_path) + + +def test_guard_compares_unrounded_percentage(guard, tmp_path, monkeypatch): + write_evidence(guard, tmp_path) + real_report = guard.Coverage.report + + def almost_passes(coverage, **kwargs): + if Path(kwargs["morfs"][0]).as_posix().endswith("core/focus_service.py"): + return 89.9999 + return real_report(coverage, **kwargs) + + monkeypatch.setattr(guard.Coverage, "report", almost_passes) + with pytest.raises(guard.CoverageGuardError, match="89.9999% is below 90%"): + guard.check_coverage(tmp_path) + + +@pytest.mark.parametrize("failure", [False, True]) +def test_command_exit_status_reflects_gate_result(guard, monkeypatch, capsys, failure): + def check(root): + assert root == Path(guard.__file__).resolve().parents[2] + if failure: + raise guard.CoverageGuardError("independent floor failed") + + monkeypatch.setattr(guard, "check_coverage", check) + assert guard.main() == (1 if failure else 0) + error = capsys.readouterr().err + assert ("independent floor failed" in error) is failure From 9da523d0417b198c4ee6a21dfa2a40faa526f3f0 Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:08:01 +0800 Subject: [PATCH 06/12] test: resolve the current lazy snapshot loader in each property example --- tests/core/test_snapshot_validation_properties.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/core/test_snapshot_validation_properties.py b/tests/core/test_snapshot_validation_properties.py index 18a8602..175f616 100644 --- a/tests/core/test_snapshot_validation_properties.py +++ b/tests/core/test_snapshot_validation_properties.py @@ -9,6 +9,7 @@ from __future__ import annotations import copy +import importlib from pathlib import Path from tempfile import TemporaryDirectory @@ -18,7 +19,6 @@ from pt_snap_cli.core.errors import ImportExecutionError from pt_snap_cli.core.snapshot_import_backend import SnapshotImportBackend -from pt_snap_cli.snapshot.tools.adaptors import snapshot2db PROPERTY_SETTINGS = settings( max_examples=25, derandomize=True, database=None, deadline=None, print_blob=True @@ -39,6 +39,8 @@ def test_generated_bad_numeric_snapshot_values_reject_without_publication( with TemporaryDirectory() as directory, pytest.MonkeyPatch.context() as monkeypatch: root = Path(directory) source = root / "trusted-in-memory.pkl" + source_bytes = b"inert test placeholder; the representation loader must be replaced" + source.write_bytes(source_bytes) output = root / "output" output.mkdir() backend = SnapshotImportBackend() @@ -52,6 +54,9 @@ def load_representation(path: Path) -> dict[str, object]: loads.append(path) return representation + # The lazy-import test removes this module from sys.modules. Resolve + # the current object here, not a stale reference captured at collection. + snapshot2db = importlib.import_module("pt_snap_cli.snapshot.tools.adaptors.snapshot2db") monkeypatch.setattr(snapshot2db, "load_snapshot_representation", load_representation) with pytest.raises(ImportExecutionError, match="Snapshot import backend failed") as error: backend.dump_to_db(source, output, device=0) @@ -59,6 +64,6 @@ def load_representation(path: Path) -> dict[str, object]: assert isinstance(error.value.__cause__, ValueError) assert loads == [source] assert representation == before - assert not source.exists() + assert source.read_bytes() == source_bytes assert destination.read_bytes() == previous assert list(output.iterdir()) == [destination] From 8432100e682ada528db256b437d91119d84d2943 Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:09:14 +0800 Subject: [PATCH 07/12] test: keep retired-package fixtures independent of historical branding --- tests/test_package.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_package.py b/tests/test_package.py index edb5e64..957d480 100644 --- a/tests/test_package.py +++ b/tests/test_package.py @@ -122,10 +122,12 @@ def test_removed_package_guard_accepts_clean_and_cache_only_trees(tmp_path, name if layout != "absent": removed_root.mkdir() if layout == "pycache-only": + # The nested name is deliberately arbitrary: removal covers every child + # of these namespaces, not a blacklist of historical vendor identities. for relative in ( "__pycache__/__init__.cpython-313.pyc", - "memsnapdump/__pycache__/reader.cpython-313.pyc", - "memsnapdump/__pycache__/reader.cpython-310.opt-1.pyc", + "retired_package/__pycache__/reader.cpython-313.pyc", + "retired_package/__pycache__/reader.cpython-310.opt-1.pyc", ): cache = removed_root / relative cache.parent.mkdir(parents=True, exist_ok=True) @@ -148,9 +150,9 @@ def test_removed_package_guard_accepts_clean_and_cache_only_trees(tmp_path, name [ "__init__.py", "server.py", - "memsnapdump/reader.py", + "retired_package/reader.py", "__init__.pyc", - "memsnapdump/schema.json", + "retired_package/schema.json", "__pycache__/hidden.py", "__pycache__/importable.pyc", ], @@ -181,7 +183,7 @@ def test_removed_package_guard_rejects_flat_module_replacements(tmp_path, name, "entry", [ "{name}/__init__.py", - "{name}/memsnapdump/reader.py", + "{name}/retired_package/reader.py", "{name}.py", "{name}/__pycache__/old.pyc", ], From de04a9e7666fa59facb273c1b4fe1e22d479859a Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 15:28:06 +0800 Subject: [PATCH 08/12] test: close coverage evidence connections on corrupt-data failures --- .github/scripts/check_critical_coverage.py | 12 +++- pyproject.toml | 2 + tests/test_critical_coverage_gate.py | 82 +++++++++++++++++----- 3 files changed, 77 insertions(+), 19 deletions(-) diff --git a/.github/scripts/check_critical_coverage.py b/.github/scripts/check_critical_coverage.py index 249bac2..e9b555f 100644 --- a/.github/scripts/check_critical_coverage.py +++ b/.github/scripts/check_critical_coverage.py @@ -6,7 +6,7 @@ import sys from pathlib import Path -from coverage import Coverage +from coverage import Coverage, CoverageData from coverage.exceptions import CoverageException # These are independent floors, not an average that another module can offset. @@ -31,13 +31,18 @@ class CoverageGuardError(ValueError): def check_coverage(root: Path) -> None: root = root.resolve() + if not callable(getattr(CoverageData, "close", None)): + raise CoverageGuardError("Critical coverage requires coverage>=7.10.3; install .[dev]") data_file = root / ".coverage" if not data_file.is_file(): raise CoverageGuardError(f"Missing coverage data: {data_file}") + data: CoverageData | None = None try: coverage = Coverage(data_file=str(data_file), config_file=str(root / "pyproject.toml")) - coverage.load() + # Own the data before load(): a corrupt SQLite file can fail inside + # coverage.py's connection __enter__, before its normal cleanup runs. data = coverage.get_data() + coverage.load() if not data.has_arcs(): raise CoverageGuardError("Critical coverage requires branch data, not line-only data") measured = {Path(name).resolve() for name in data.measured_files()} @@ -69,6 +74,9 @@ def check_coverage(root: Path) -> None: raise CoverageGuardError("\n".join(failures)) except (CoverageException, OSError) as error: raise CoverageGuardError(f"Cannot validate critical coverage: {error}") from error + finally: + if data is not None: + data.close() def main() -> int: diff --git a/pyproject.toml b/pyproject.toml index f65fa0e..6c6f132 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,8 @@ Issues = "https://github.com/codeasier/pt-snap-cli/issues" dev = [ "pytest>=7.0.0", "pytest-cov>=4.0.0", + # The coverage gate owns/explicitly closes CoverageData, including bad SQLite input. + "coverage[toml]>=7.10.3", "playwright>=1.40.0", "black>=23.0.0", "ruff>=0.0.280", diff --git a/tests/test_critical_coverage_gate.py b/tests/test_critical_coverage_gate.py index cac9d0c..76ccc7c 100644 --- a/tests/test_critical_coverage_gate.py +++ b/tests/test_critical_coverage_gate.py @@ -1,6 +1,8 @@ """Exercise the coverage guard with real coverage databases, without running code.""" import importlib.util +import sqlite3 +from contextlib import closing from pathlib import Path import pytest @@ -17,26 +19,54 @@ def guard(): return module +@pytest.fixture +def coverage_connections(tmp_path, monkeypatch): + connect = sqlite3.connect + connections = [] + + def tracked_connect(database, *args, **kwargs): + connection = connect(database, *args, **kwargs) + if Path(database) == tmp_path / ".coverage": + connections.append(connection) + return connection + + monkeypatch.setattr(sqlite3, "connect", tracked_connect) + try: + yield connections + finally: + # The tests assert closure before teardown; clean up even after a failed + # assertion so a primary lifetime regression does not cause a second leak. + for connection in connections: + connection.close() + + +def assert_connections_closed(connections): + assert connections, "The checker must have inspected real SQLite evidence" + for connection in connections: + with pytest.raises(sqlite3.ProgrammingError, match="closed"): + connection.execute("SELECT 1") + + def write_evidence(guard, root, *, missing=None, uncovered=None, branches=True): (root / "pyproject.toml").write_text( "[tool.coverage.run]\nbranch = true\n[tool.coverage.report]\nfail_under = 90\n", encoding="utf-8", ) - data = CoverageData(basename=str(root / ".coverage")) - for relative in guard.CRITICAL_FLOORS: - source = root / relative - source.parent.mkdir(parents=True, exist_ok=True) - source.write_text("if flag:\n result = 1\nelse:\n result = 2\n", encoding="utf-8") - if relative == missing: - continue - if branches: - arcs = [(-1, 1), (1, 2), (2, -1)] - if relative != uncovered: - arcs.extend([(1, 4), (4, -1)]) - data.add_arcs({str(source): arcs}) - else: - data.add_lines({str(source): [1, 2, 4]}) - data.write() + with closing(CoverageData(basename=str(root / ".coverage"))) as data: + for relative in guard.CRITICAL_FLOORS: + source = root / relative + source.parent.mkdir(parents=True, exist_ok=True) + source.write_text("if flag:\n result = 1\nelse:\n result = 2\n", encoding="utf-8") + if relative == missing: + continue + if branches: + arcs = [(-1, 1), (1, 2), (2, -1)] + if relative != uncovered: + arcs.extend([(1, 4), (4, -1)]) + data.add_arcs({str(source): arcs}) + else: + data.add_lines({str(source): [1, 2, 4]}) + data.write() def test_critical_floor_inventory_preserves_existing_and_adds_risky_modules(guard): @@ -54,11 +84,13 @@ def test_critical_floor_inventory_preserves_existing_and_adds_risky_modules(guar } -def test_guard_accepts_complete_branch_evidence(guard, tmp_path, capsys): +def test_guard_accepts_complete_branch_evidence(guard, tmp_path, capsys, coverage_connections): write_evidence(guard, tmp_path) + read_start = len(coverage_connections) guard.check_coverage(tmp_path) output = capsys.readouterr().out assert output.count("100.0000%") == len(guard.CRITICAL_FLOORS) + assert_connections_closed(coverage_connections[read_start:]) def test_guard_rejects_missing_data_without_creating_it(guard, tmp_path): @@ -67,15 +99,31 @@ def test_guard_rejects_missing_data_without_creating_it(guard, tmp_path): assert not (tmp_path / ".coverage").exists() +def test_guard_rejects_unsupported_coverage_before_opening_evidence(guard, tmp_path, monkeypatch): + write_evidence(guard, tmp_path) + before = (tmp_path / ".coverage").read_bytes() + monkeypatch.setattr(guard, "CoverageData", object) + monkeypatch.setattr( + guard, "Coverage", lambda **kwargs: pytest.fail("Unsupported coverage must not open data") + ) + with pytest.raises(guard.CoverageGuardError, match=r"requires coverage>=7\.10\.3"): + guard.check_coverage(tmp_path) + assert (tmp_path / ".coverage").read_bytes() == before + + @pytest.mark.parametrize("failure", ["corrupt", "empty", "line-only"]) -def test_guard_rejects_unusable_evidence(guard, tmp_path, failure): +def test_guard_rejects_unusable_evidence(guard, tmp_path, failure, coverage_connections): write_evidence(guard, tmp_path, branches=failure != "line-only") if failure == "corrupt": (tmp_path / ".coverage").write_bytes(b"not a coverage database") elif failure == "empty": (tmp_path / ".coverage").write_bytes(b"") + read_start = len(coverage_connections) with pytest.raises(guard.CoverageGuardError): guard.check_coverage(tmp_path) + assert_connections_closed(coverage_connections[read_start:]) + if failure == "corrupt": + assert (tmp_path / ".coverage").read_bytes() == b"not a coverage database" @pytest.mark.parametrize("failure", ["unmeasured", "deleted", "no-statements", "below-floor"]) From 5db7f9866e2b7c4cc7c1630893c511f606ea1fd1 Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:32:37 +0800 Subject: [PATCH 09/12] fix: close failed focus checkpoints before verified cleanup --- src/pt_snap_cli/core/focus_service.py | 33 ++- tests/core/test_focus_checkpoint_cleanup.py | 308 ++++++++++++++++++++ 2 files changed, 330 insertions(+), 11 deletions(-) create mode 100644 tests/core/test_focus_checkpoint_cleanup.py diff --git a/src/pt_snap_cli/core/focus_service.py b/src/pt_snap_cli/core/focus_service.py index f6fd29c..054e959 100644 --- a/src/pt_snap_cli/core/focus_service.py +++ b/src/pt_snap_cli/core/focus_service.py @@ -88,8 +88,11 @@ def _safe_focus_path(path: Path) -> None: def _checkpoint_file(path: Path, content: bytes, mode: int) -> tuple[Path, tuple[int, int]]: fd, name = tempfile.mkstemp(dir=path.parent, prefix=".focus.pt-snap-", suffix=".recovery") backup = Path(name) - identity = os.fstat(fd) + identity: tuple[int, int] | None = None + close_attempted = False try: + info = os.fstat(fd) + identity = (info.st_dev, info.st_ino) remaining = memoryview(content) while remaining: written = os.write(fd, remaining) @@ -99,20 +102,28 @@ def _checkpoint_file(path: Path, content: bytes, mode: int) -> tuple[Path, tuple if hasattr(os, "fchmod"): os.fchmod(fd, mode) else: - FocusService._verify_checkpoint(backup, (identity.st_dev, identity.st_ino)) + FocusService._verify_checkpoint(backup, identity) os.chmod(backup, mode) os.fsync(fd) - FocusService._verify_checkpoint(backup, (identity.st_dev, identity.st_ino)) + FocusService._verify_checkpoint(backup, identity) + close_attempted = True + os.close(fd) except BaseException: - try: - FocusService._verify_checkpoint(backup, (identity.st_dev, identity.st_ino)) - backup.unlink() - except OSError: - pass + # Windows cannot unlink an open checkpoint. Do not retry a failed + # close: the descriptor may already be closed and reused. + if not close_attempted: + try: + os.close(fd) + except OSError: + pass + if identity is not None: + try: + FocusService._verify_checkpoint(backup, identity) + backup.unlink() + except OSError: + pass raise - finally: - os.close(fd) - return backup, (identity.st_dev, identity.st_ino) + return backup, identity @staticmethod def _verify_checkpoint(path: Path, identity: tuple[int, int]) -> None: diff --git a/tests/core/test_focus_checkpoint_cleanup.py b/tests/core/test_focus_checkpoint_cleanup.py new file mode 100644 index 0000000..ed1a106 --- /dev/null +++ b/tests/core/test_focus_checkpoint_cleanup.py @@ -0,0 +1,308 @@ +"""Checkpoint failure cleanup closes owned descriptors before verified deletion.""" + +import copy +import os +from pathlib import Path +from types import SimpleNamespace + +import pytest + +import pt_snap_cli.core.focus_service as focus_module +from pt_snap_cli.config import Config +from pt_snap_cli.core.focus_service import FocusService + + +@pytest.fixture +def focus(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("PT_SNAP_DB_PATH", raising=False) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + service = FocusService(Config()) + path = service._config.project_focus_path() + path.parent.mkdir() + original = b'{ "db_path" : "previous.db", "extra" : [1,2] }\n' + path.write_bytes(original) + service._config._config = {"nested": {"value": "original"}} + return service, path, original + + +@pytest.fixture +def checkpoint_io(monkeypatch): + probe = SimpleNamespace(fd=None, path=None, events=[]) + mkstemp = focus_module.tempfile.mkstemp + close = os.close + fstat = os.fstat + unlink = Path.unlink + verify = FocusService._verify_checkpoint + + def create(*args, **kwargs): + probe.fd, name = mkstemp(*args, **kwargs) + probe.path = Path(name) + return probe.fd, name + + def close_checkpoint(fd): + if fd == probe.fd: + probe.events.append("close") + close(fd) + + def verify_checkpoint(path, identity): + if path == probe.path: + probe.events.append("verify") + verify(path, identity) + + def unlink_checkpoint(path, *args, **kwargs): + if path == probe.path: + # Enforce Windows' close-before-unlink requirement on every runner. + with pytest.raises(OSError): + fstat(probe.fd) + assert probe.events[-1] == "verify" + probe.events.append("unlink") + return unlink(path, *args, **kwargs) + + monkeypatch.setattr(focus_module.tempfile, "mkstemp", create) + monkeypatch.setattr(focus_module.os, "close", close_checkpoint) + monkeypatch.setattr(FocusService, "_verify_checkpoint", staticmethod(verify_checkpoint)) + monkeypatch.setattr(Path, "unlink", unlink_checkpoint) + yield probe + if probe.fd is not None: + try: + fstat(probe.fd) + except OSError: + pass + else: + close(probe.fd) + pytest.fail("checkpoint descriptor leaked") + assert probe.events.count("close") == 1 + + +@pytest.mark.parametrize( + "failure", ["zero", "short-zero", "write", "fchmod", "permissions", "fsync", "verify"] +) +def test_checkpoint_failure_closes_before_cleanup(focus, checkpoint_io, monkeypatch, failure): + service, path, original = focus + state = copy.deepcopy(service._config._config) + error = OSError("injected checkpoint failure") + write = os.write + verify = FocusService._verify_checkpoint + calls = [] + + def fail(*args): + raise error + + def partial_write(fd, content): + calls.append(len(content)) + if failure == "short-zero" and len(calls) == 1: + return write(fd, content[:3]) + return 0 + + def fail_preparation_verify(target, identity): + verify(target, identity) + if "close" not in checkpoint_io.events: + raise error + + if failure in {"zero", "short-zero"}: + monkeypatch.setattr(focus_module.os, "write", partial_write) + elif failure == "permissions": + # Exercise the portable pathname fallback, including real identity checks. + monkeypatch.delattr(focus_module.os, "fchmod", raising=False) + monkeypatch.setattr(focus_module.os, "chmod", fail) + elif failure == "fchmod": + monkeypatch.setattr(focus_module.os, "fchmod", fail, raising=False) + elif failure == "verify": + monkeypatch.setattr( + FocusService, "_verify_checkpoint", staticmethod(fail_preparation_verify) + ) + else: + monkeypatch.setattr(focus_module.os, failure, fail) + + with pytest.raises(OSError) as caught: + with service.project_focus_transaction(): + pytest.fail("checkpoint failure must prevent focus publication") + + if failure in {"zero", "short-zero"}: + assert str(caught.value) == "Could not write focus recovery checkpoint" + assert calls == ( + [len(original)] if failure == "zero" else [len(original), len(original) - 3] + ) + else: + assert caught.value is error + assert checkpoint_io.events[-3:] == ["close", "verify", "unlink"] + assert path.read_bytes() == original + assert service._config._config == state + assert list(path.parent.iterdir()) == [path] + + +@pytest.mark.parametrize("rollback", [False, True], ids=["commit", "rollback"]) +def test_short_writes_keep_successful_transaction_semantics( + focus, checkpoint_io, monkeypatch, rollback +): + service, path, original = focus + state = copy.deepcopy(service._config._config) + write = os.write + writes = [] + error = RuntimeError("focus mutation failed") + + def short_write(fd, content): + writes.append(len(content)) + return write(fd, content[:3]) + + def mutate(): + with service.project_focus_transaction(): + assert checkpoint_io.events[-1] == "close" + assert checkpoint_io.path.read_bytes() == original + path.write_bytes(b"committed focus") + service._config._config["nested"]["value"] = "changed" + if rollback: + raise error + + monkeypatch.setattr(focus_module.os, "write", short_write) + if rollback: + with pytest.raises(RuntimeError) as caught: + mutate() + assert caught.value is error + assert path.read_bytes() == original + assert service._config._config == state + else: + mutate() + assert path.read_bytes() == b"committed focus" + assert service._config._config == {"nested": {"value": "changed"}} + assert checkpoint_io.events[-2:] == ["verify", "unlink"] + assert len(writes) > 1 + assert list(path.parent.iterdir()) == [path] + + +@pytest.mark.parametrize("cleanup_failure", ["close", "unlink", "verify"]) +def test_cleanup_error_keeps_original_checkpoint_error( + focus, checkpoint_io, monkeypatch, cleanup_failure +): + service, path, original = focus + error = OSError("original checkpoint write failure") + close = os.close + verify = FocusService._verify_checkpoint + + def fail_write(*args): + raise error + + def close_then_fail(fd): + close(fd) + raise OSError("late close error") + + def fail_unlink(target, *args, **kwargs): + assert target == checkpoint_io.path + with pytest.raises(OSError): + os.fstat(checkpoint_io.fd) + assert checkpoint_io.events[-2:] == ["close", "verify"] + raise OSError("unlink denied") + + def fail_verify(target, identity): + verify(target, identity) + raise OSError("verification denied") + + monkeypatch.setattr(focus_module.os, "write", fail_write) + if cleanup_failure == "close": + monkeypatch.setattr(focus_module.os, "close", close_then_fail) + elif cleanup_failure == "unlink": + monkeypatch.setattr(Path, "unlink", fail_unlink) + else: + monkeypatch.setattr(FocusService, "_verify_checkpoint", staticmethod(fail_verify)) + + with pytest.raises(OSError) as caught: + with service.project_focus_transaction(): + pytest.fail("checkpoint failure must prevent focus publication") + assert caught.value is error + assert path.read_bytes() == original + assert checkpoint_io.path.exists() is (cleanup_failure != "close") + + +def test_close_failure_after_checkpoint_write_does_not_retry_close( + focus, checkpoint_io, monkeypatch +): + service, path, original = focus + close = os.close + error = OSError("late checkpoint close error") + + def close_then_fail(fd): + close(fd) + raise error + + monkeypatch.setattr(focus_module.os, "close", close_then_fail) + with pytest.raises(OSError) as caught: + with service.project_focus_transaction(): + pytest.fail("checkpoint close failure must prevent focus publication") + assert caught.value is error + assert checkpoint_io.events[-3:] == ["close", "verify", "unlink"] + assert path.read_bytes() == original + assert list(path.parent.iterdir()) == [path] + + +def test_unknown_checkpoint_identity_closes_without_unverified_deletion( + focus, checkpoint_io, monkeypatch +): + service, path, original = focus + error = OSError("checkpoint identity unavailable") + + def fail_fstat(fd): + assert fd == checkpoint_io.fd + raise error + + monkeypatch.setattr(focus_module.os, "fstat", fail_fstat) + with pytest.raises(OSError) as caught: + with service.project_focus_transaction(): + pytest.fail("unverified checkpoint must prevent focus publication") + assert caught.value is error + assert checkpoint_io.events == ["close"] + assert checkpoint_io.path.read_bytes() == b"" + assert path.read_bytes() == original + + +def test_replaced_checkpoint_after_close_is_not_deleted(focus, checkpoint_io, monkeypatch): + service, path, original = focus + close = os.close + replacement = path.parent / "replacement" + replacement.write_bytes(b"independent file") + saved = path.parent / "owned-checkpoint" + error = OSError("original checkpoint write failure") + + def fail_write(*args): + raise error + + def close_then_replace(fd): + close(fd) + # Keep both real files alive, so their distinct identities cannot be reused. + checkpoint_io.path.rename(saved) + replacement.rename(checkpoint_io.path) + + monkeypatch.setattr(focus_module.os, "write", fail_write) + monkeypatch.setattr(focus_module.os, "close", close_then_replace) + with pytest.raises(OSError) as caught: + with service.project_focus_transaction(): + pytest.fail("checkpoint failure must prevent focus publication") + assert caught.value is error + assert checkpoint_io.events == ["close", "verify"] + assert checkpoint_io.path.read_bytes() == b"independent file" + assert saved.read_bytes() == b"" + assert path.read_bytes() == original + + +def test_symlink_metadata_at_cleanup_is_rejected(focus, checkpoint_io, monkeypatch): + service, path, original = focus + is_symlink = Path.is_symlink + error = OSError("original checkpoint write failure") + + def fail_write(*args): + raise error + + def checkpoint_is_symlink(target): + # Inject only the metadata observation; no Windows symlink privilege is needed. + return target == checkpoint_io.path or is_symlink(target) + + monkeypatch.setattr(focus_module.os, "write", fail_write) + monkeypatch.setattr(Path, "is_symlink", checkpoint_is_symlink) + with pytest.raises(OSError) as caught: + with service.project_focus_transaction(): + pytest.fail("checkpoint failure must prevent focus publication") + assert caught.value is error + assert checkpoint_io.events == ["close", "verify"] + assert checkpoint_io.path.read_bytes() == b"" + assert path.read_bytes() == original From 203950ad5a706284d05fd2ee1d9d480c5a00055e Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:50:47 +0800 Subject: [PATCH 10/12] test: scope checkpoint identity failure injection to owned descriptor --- tests/core/test_focus_checkpoint_cleanup.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/core/test_focus_checkpoint_cleanup.py b/tests/core/test_focus_checkpoint_cleanup.py index ed1a106..1e3c6f4 100644 --- a/tests/core/test_focus_checkpoint_cleanup.py +++ b/tests/core/test_focus_checkpoint_cleanup.py @@ -241,15 +241,18 @@ def test_unknown_checkpoint_identity_closes_without_unverified_deletion( ): service, path, original = focus error = OSError("checkpoint identity unavailable") + fstat = os.fstat def fail_fstat(fd): - assert fd == checkpoint_io.fd - raise error + if fd == checkpoint_io.fd: + raise error + return fstat(fd) - monkeypatch.setattr(focus_module.os, "fstat", fail_fstat) - with pytest.raises(OSError) as caught: - with service.project_focus_transaction(): - pytest.fail("unverified checkpoint must prevent focus publication") + with monkeypatch.context() as patch: + patch.setattr(focus_module.os, "fstat", fail_fstat) + with pytest.raises(OSError) as caught: + with service.project_focus_transaction(): + pytest.fail("unverified checkpoint must prevent focus publication") assert caught.value is error assert checkpoint_io.events == ["close"] assert checkpoint_io.path.read_bytes() == b"" From 8df94284b2eb19bc1e6a3442a145d8244d55446d Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 22:53:34 +0800 Subject: [PATCH 11/12] fix: normalize malformed YAML and snapshot input failures --- .../core/snapshot_import_backend.py | 10 +- src/pt_snap_cli/query/config.py | 61 ++- src/pt_snap_cli/snapshot/PROVENANCE.md | 15 + src/pt_snap_cli/snapshot/base/entities.py | 72 ++- .../snapshot/tools/adaptors/snapshot2db.py | 7 + .../test_snapshot_input_errors_properties.py | 434 ++++++++++++++++++ .../query/test_input_structure_properties.py | 215 +++++++++ tests/snapshot/test_entities.py | 8 +- tests/snapshot/test_snapshot2db_runtime.py | 6 +- 9 files changed, 795 insertions(+), 33 deletions(-) create mode 100644 tests/core/test_snapshot_input_errors_properties.py create mode 100644 tests/query/test_input_structure_properties.py diff --git a/src/pt_snap_cli/core/snapshot_import_backend.py b/src/pt_snap_cli/core/snapshot_import_backend.py index 2317622..546caee 100644 --- a/src/pt_snap_cli/core/snapshot_import_backend.py +++ b/src/pt_snap_cli/core/snapshot_import_backend.py @@ -38,7 +38,15 @@ def dump_to_db( ) except UnsafePickleError as exc: raise ImportExecutionError(f"Snapshot pickle rejected: {exc}") from exc - except (OSError, sqlite3.DatabaseError, pickle.UnpicklingError, ValueError) as exc: + except ( + OSError, + sqlite3.DatabaseError, + pickle.UnpicklingError, + ValueError, + OverflowError, + ) as exc: + # SQLite rejects out-of-range integer bindings with OverflowError, + # rather than DatabaseError. Keep the original numeric failure. raise ImportExecutionError(f"Snapshot import backend failed: {exc}") from exc if not ok: diff --git a/src/pt_snap_cli/query/config.py b/src/pt_snap_cli/query/config.py index 6a343e5..4dd4b22 100644 --- a/src/pt_snap_cli/query/config.py +++ b/src/pt_snap_cli/query/config.py @@ -204,8 +204,16 @@ def from_dict(cls, data: dict[str, Any], default_category: str | None = None) -> Returns: QueryTemplate instance. """ + data = _require_mapping(data, "template") + if "name" in data: + _require_name(data["name"], "template") parameters = {} - for name, param_data in data.get("parameters", {}).items(): + parameter_data = _require_mapping(data.get("parameters", {}), "parameters") + for name, param_data in parameter_data.items(): + _require_name(name, "parameter") + param_data = _require_mapping(param_data, f"parameter '{name}'") + if not isinstance(param_data.get("type", "str"), str): + raise ValueError(f"Parameter '{name}' type must be a string") parameters[name] = QueryParameter( name=name, type=param_data.get("type", "str"), @@ -218,6 +226,8 @@ def from_dict(cls, data: dict[str, Any], default_category: str | None = None) -> template_name = data.get("name", "") query_variants = _parse_query_variants(template_name, data.get("query_variants")) explicit_query = data.get("query") + if explicit_query is not None and not isinstance(explicit_query, str): + raise ValueError(f"query for '{template_name}' must be a SQL string") if query_variants: if isinstance(explicit_query, str) and explicit_query.strip(): raise ValueError( @@ -245,6 +255,17 @@ def from_dict(cls, data: dict[str, Any], default_category: str | None = None) -> ) +def _require_mapping(raw: Any, label: str) -> dict[str, Any]: + if not isinstance(raw, dict): + raise ValueError(f"{label} must be a mapping") + return raw + + +def _require_name(raw: Any, label: str) -> None: + if not isinstance(raw, str) or not raw.strip(): + raise ValueError(f"{label} name must be a non-empty string") + + def _parse_semantics_version(template_name: str, raw: Any) -> int | None: if raw is None: return None @@ -408,23 +429,15 @@ def load_yaml(cls, path: str | Path, default_category: str | None = None) -> Que Raises: FileNotFoundError: If file does not exist. yaml.YAMLError: If YAML is invalid. + ValueError: If the configuration structure or template metadata is invalid. """ path = Path(path) if not path.exists(): raise FileNotFoundError(f"Config file not found: {path}") with open(path, encoding="utf-8") as f: - data = yaml.safe_load(f) or {} - - queries = {} - for name, query_data in data.get("queries", {}).items(): - query_data["name"] = name - queries[name] = QueryTemplate.from_dict(query_data, default_category=default_category) - - return cls( - version=data.get("version", "1.0"), - queries=queries, - ) + data = yaml.safe_load(f) + return cls._from_yaml_data(data, default_category=default_category) @classmethod def load_yaml_from_string(cls, content: str) -> QueryConfig: @@ -435,13 +448,29 @@ def load_yaml_from_string(cls, content: str) -> QueryConfig: Returns: QueryConfig instance. + + Raises: + yaml.YAMLError: If YAML is invalid. + ValueError: If the configuration structure or template metadata is invalid. """ - data = yaml.safe_load(content) or {} + return cls._from_yaml_data(yaml.safe_load(content)) + + @classmethod + def _from_yaml_data(cls, raw: Any, default_category: str | None = None) -> QueryConfig: + # An empty YAML document remains an empty configuration. Other falsy + # values ([], false, 0, "") are not mappings and must not silently pass. + data = _require_mapping({} if raw is None else raw, "configuration") + query_definitions = _require_mapping(data.get("queries", {}), "queries") queries = {} - for name, query_data in data.get("queries", {}).items(): - query_data["name"] = name - queries[name] = QueryTemplate.from_dict(query_data) + for name, query_data in query_definitions.items(): + _require_name(name, "query") + query_data = _require_mapping(query_data, f"query '{name}'") + # Do not overwrite caller data or shared YAML aliases with the + # enclosing name. Each query gets its own top-level mapping. + queries[name] = QueryTemplate.from_dict( + {**query_data, "name": name}, default_category=default_category + ) return cls( version=data.get("version", "1.0"), diff --git a/src/pt_snap_cli/snapshot/PROVENANCE.md b/src/pt_snap_cli/snapshot/PROVENANCE.md index 40df201..5ae4c9e 100644 --- a/src/pt_snap_cli/snapshot/PROVENANCE.md +++ b/src/pt_snap_cli/snapshot/PROVENANCE.md @@ -299,3 +299,18 @@ Read-only queries preserve path identity, classify absent/corrupt evidence, and conserve live block, requested-byte and occupancy totals. No upstream code or new executable fixtures were incorporated; source mappings and licenses remain unchanged. This supersedes the unmerged implementation proposed in PR #207. + +## 2026-11-01 — Snapshot input error normalization (issue #280) + +First-party changes in `base/entities.py` validate the mappings, sequences and +required keys consumed by entity construction, finite allocator numeric fields, +and nonempty trace action strings. Failures of existing trace integer conversion +become `ValueError` with the cause preserved. `tools/adaptors/snapshot2db.py` +checks the root/device-trace container before device selection. Numeric strings +in traces, optional defaults, sentinels, tuple containers, event IDs and successful +replay behavior remain unchanged. The core import adapter also normalizes SQLite +integer-binding overflow at its existing domain-error boundary, without imposing +a new representation-wide numeric range. Generated regressions use trusted +in-memory representations behind the loader seam, not new executable pickle +fixtures. No parser/layout, unknown-field policy or upstream-code change is made; +source mappings, attribution and license records are unchanged. diff --git a/src/pt_snap_cli/snapshot/base/entities.py b/src/pt_snap_cli/snapshot/base/entities.py index c057f70..13fea1c 100644 --- a/src/pt_snap_cli/snapshot/base/entities.py +++ b/src/pt_snap_cli/snapshot/base/entities.py @@ -1,7 +1,32 @@ +import math from dataclasses import dataclass, field from typing import Any, Literal, cast +def _validate_mapping(raw: Any, label: str, required: tuple[str, ...] = ()) -> None: + if not isinstance(raw, dict): + raise ValueError(f"{label} must be a mapping") + for name in required: + if name not in raw: + raise ValueError(f"{label} is missing required field '{name}'") + + +def _validate_sequence(raw: Any, label: str) -> None: + # Tuples already work for trusted in-memory representations; retain that + # compatibility rather than requiring a particular serialized container. + if not isinstance(raw, (list, tuple)): + raise ValueError(f"{label} must be a list or tuple") + + +def _validate_numbers(raw: dict[str, Any], label: str, names: tuple[str, ...]) -> None: + for name in names: + value = raw[name] + if not isinstance(value, (int, float)) or ( + isinstance(value, float) and not math.isfinite(value) + ): + raise ValueError(f"{label} field '{name}' must be a finite number") + + class Frame: filename: str = "" line: int = -1 @@ -11,6 +36,7 @@ class Frame: @classmethod def from_dict(cls, frame_dict: dict[str, Any]): + _validate_mapping(frame_dict, "Frame", ("filename", "line", "name")) frame = cls() frame.filename = frame_dict["filename"] frame.line = frame_dict["line"] @@ -39,10 +65,17 @@ def _format_callstack(frames: list[Frame] | list[dict[str, Any]]) -> str: def _validate_raw_frames(frames: list[dict[str, Any]]) -> None: + _validate_sequence(frames, "Frames") for frame in frames: - _ = frame["filename"] - _ = frame["line"] - _ = frame["name"] + _validate_mapping(frame, "Frame", ("filename", "line", "name")) + + +def _trace_integer(trace_dict: dict[str, Any], name: str, default: int) -> int: + """Retain integer conversion while rejecting malformed numeric metadata.""" + try: + return int(trace_dict.get(name, default)) + except (TypeError, ValueError, OverflowError) as exc: + raise ValueError(f"Trace field '{name}' must be integer-compatible") from exc @dataclass @@ -82,19 +115,24 @@ class TraceEntry: @classmethod def from_dict(cls, trace_dict: dict[str, Any], _raw_frames: bool = False): + _validate_mapping(trace_dict, "Trace entry") + action = trace_dict.get("action") + if not isinstance(action, str) or not action.strip(): + raise ValueError("Trace field 'action' must be a non-empty string") frame_dicts = trace_dict.get("frames", []) + _validate_sequence(frame_dicts, "Frames") if _raw_frames: _validate_raw_frames(frame_dicts) trace_entry = cls( - action=trace_dict["action"], - addr=int(trace_dict.get("addr", -1)), - size=int(trace_dict.get("size", 0)), - stream=int(trace_dict.get("stream", 0)), - device_free=int(trace_dict.get("device_free", -1)), + action=action, + addr=_trace_integer(trace_dict, "addr", -1), + size=_trace_integer(trace_dict, "size", 0), + stream=_trace_integer(trace_dict, "stream", 0), + device_free=_trace_integer(trace_dict, "device_free", -1), _origin=trace_dict, frames=[] if _raw_frames else [Frame.from_dict(frame) for frame in frame_dicts], _raw_frames=frame_dicts if _raw_frames else None, - idx=int(trace_dict["id"]) if "id" in trace_dict else -1, + idx=_trace_integer(trace_dict, "id", -1), ) return trace_entry @@ -163,7 +201,12 @@ class Block: @classmethod def from_dict(cls, block_dict: dict[str, Any], _raw_frames: bool = False): + _validate_mapping(block_dict, "Block", ("size", "requested_size", "address", "state")) + _validate_numbers(block_dict, "Block", ("size", "requested_size", "address")) + if not isinstance(block_dict["state"], str): + raise ValueError("Block field 'state' must be a string") frame_dicts = block_dict.get("frames", []) + _validate_sequence(frame_dicts, "Frames") if _raw_frames: _validate_raw_frames(frame_dicts) block = cls( @@ -231,7 +274,12 @@ def from_dict( ignore_inactive_blocks: bool = False, _raw_frames: bool = False, ): + numeric_fields = ("address", "total_size", "stream", "allocated_size", "active_size") + _validate_mapping(segment_dict, "Segment", (*numeric_fields, "segment_type", "blocks")) + _validate_numbers(segment_dict, "Segment", numeric_fields) + _validate_sequence(segment_dict["blocks"], "Segment blocks") frame_dicts = segment_dict.get("frames", []) + _validate_sequence(frame_dicts, "Frames") if _raw_frames: _validate_raw_frames(frame_dicts) segment = cls( @@ -248,6 +296,7 @@ def from_dict( is_expandable=segment_dict.get("is_expandable", False), ) for block in segment_dict["blocks"]: + _validate_mapping(block, "Block", ("state",)) if ignore_inactive_blocks and block["state"] == BlockState.INACTIVE: continue _block = Block.from_dict(block, _raw_frames=_raw_frames) @@ -328,9 +377,13 @@ def from_dict( ignore_inactive_blocks: bool = False, _raw_frames: bool = False, ): + _validate_mapping(snapshot_dict, "Snapshot") segments_dict = snapshot_dict.get("segments", []) device_traces = snapshot_dict.get("device_traces", []) + _validate_sequence(segments_dict, "Snapshot segments") + _validate_sequence(device_traces, "Snapshot device_traces") device_trace_list = device_traces[device] if 0 <= device < len(device_traces) else [] + _validate_sequence(device_trace_list, "Device traces") snapshot = cls() snapshot.segments = [] snapshot.trace_entries = [] @@ -339,6 +392,7 @@ def from_dict( snapshot.total_activated = 0 # 读取dump_snapshot时内存状态 for segment_dict in segments_dict: + _validate_mapping(segment_dict, "Segment") # 当segment原始数据中缺少device字段明确指向其所属设备时，默认其归属为device0 # 此时如果from_dict指定device为0或未指定而缺省为0，则未知归属的device也会纳入分析 if segment_dict.get("device", 0) != device: diff --git a/src/pt_snap_cli/snapshot/tools/adaptors/snapshot2db.py b/src/pt_snap_cli/snapshot/tools/adaptors/snapshot2db.py index 1f3bcad..3f616a8 100644 --- a/src/pt_snap_cli/snapshot/tools/adaptors/snapshot2db.py +++ b/src/pt_snap_cli/snapshot/tools/adaptors/snapshot2db.py @@ -191,8 +191,15 @@ def dump(pickle_file: str | Path, dump_file: str | Path, device: int | None = No except Exception as e: dump_logger.error(f"Failed to load pickle file: {e}") return False + if not isinstance(data, dict): + raise ValueError("Snapshot must be a mapping") # Snapshot format: one list of trace dictionaries per device. + if not isinstance(data.get("device_traces", []), (list, tuple)): + raise ValueError("Snapshot device_traces must be a list or tuple") device_traces = cast(list[list[object]], data.get("device_traces", [])) + for index, traces in enumerate(device_traces): + if not isinstance(traces, (list, tuple)): + raise ValueError(f"Snapshot device_traces[{index}] must be a list or tuple") # 当指定device为空时dump所有记录了跟踪事件的device，否则仅dump指定device need_dump_devices = [device for device in range(len(device_traces)) if device_traces[device]] dump_logger.info(f"Recognized have trace events devices {need_dump_devices}.") diff --git a/tests/core/test_snapshot_input_errors_properties.py b/tests/core/test_snapshot_input_errors_properties.py new file mode 100644 index 0000000..54620d6 --- /dev/null +++ b/tests/core/test_snapshot_input_errors_properties.py @@ -0,0 +1,434 @@ +"""Trace input failures through real replay/SQLite; never deserialize a pickle.""" + +from __future__ import annotations + +import copy +import importlib +import sqlite3 +from contextlib import closing +from pathlib import Path +from tempfile import TemporaryDirectory + +import pytest +from hypothesis import example, given, settings +from hypothesis import strategies as st + +from pt_snap_cli.core.errors import ImportExecutionError +from pt_snap_cli.core.snapshot_import_backend import SnapshotImportBackend +from pt_snap_cli.snapshot.base import DeviceSnapshot, TraceEntry + +PROPERTY_SETTINGS = settings( + max_examples=25, derandomize=True, database=None, deadline=None, print_blob=True +) +SCALARS = st.one_of(st.none(), st.booleans(), st.integers(-100, 100), st.text(max_size=12)) +BAD_NUMBERS = st.one_of( + st.none(), + st.lists(SCALARS, max_size=3), + st.dictionaries(st.text(max_size=8), SCALARS, max_size=3), + st.sampled_from([float("nan"), float("inf"), float("-inf")]), +) +SOURCE_BYTES = b"inert placeholder; only the patched in-memory loader may be used" + + +def _valid_representation() -> dict[str, object]: + return { + "segments": [ + { + "address": 4096, + "total_size": 16, + "stream": 0, + "segment_type": "small", + "allocated_size": 16, + "active_size": 16, + "frames": [], + "blocks": [ + { + "size": 16, + "requested_size": 8, + "address": 4096, + "state": "active_allocated", + "frames": [], + } + ], + } + ], + "device_traces": [ + [ + { + "action": "oom", + "size": 1, + "frames": [{"filename": "generated.py", "line": 1, "name": "run"}], + } + ] + ], + } + + +def _assert_representation_rejected( + representation: object, + cause: type[Exception], + existing: bool, + *, + device: int | None = 0, +) -> Exception: + before = copy.deepcopy(representation) + with TemporaryDirectory() as directory, pytest.MonkeyPatch.context() as monkeypatch: + root = Path(directory) + source = root / "trusted-in-memory.pkl" + source.write_bytes(SOURCE_BYTES) + output = root / "output" + output.mkdir() + backend = SnapshotImportBackend() + destination = backend.target_db_path(source, output) + previous = b"existing destination must remain byte-for-byte unchanged" + if existing: + destination.write_bytes(previous) + loads = [] + + def load_representation(path: Path) -> object: + assert path == source + loads.append(path) + return representation + + # Older lazy-import tests evict snapshot2db from sys.modules. Always + # patch the current module, not a collection-time reference. + snapshot2db = importlib.import_module("pt_snap_cli.snapshot.tools.adaptors.snapshot2db") + monkeypatch.setattr(snapshot2db, "load_snapshot_representation", load_representation) + with pytest.raises(ImportExecutionError, match="Snapshot import backend failed") as error: + backend.dump_to_db(source, output, device=device) + underlying = error.value.__cause__ + assert isinstance(underlying, cause) + assert loads == [source] + assert representation == before + assert source.read_bytes() == SOURCE_BYTES + if existing: + assert destination.read_bytes() == previous + else: + assert not destination.exists() + assert list(output.iterdir()) == ([destination] if existing else []) + return underlying + + +def _assert_rejected(event: object, cause: type[Exception], existing: bool) -> Exception: + return _assert_representation_rejected( + {"segments": [], "device_traces": [[event]]}, cause, existing + ) + + +@PROPERTY_SETTINGS +@given(size=st.integers(0, 1 << 20), existing=st.booleans()) +def test_generated_missing_action_rejects_without_publication(size: int, existing: bool) -> None: + cause = _assert_rejected({"size": size, "frames": []}, ValueError, existing) + assert "Trace field 'action'" in str(cause) + + +@PROPERTY_SETTINGS +@given( + action=st.one_of( + st.none(), + st.booleans(), + st.integers(-(1 << 65), 1 << 65), + st.lists(SCALARS, max_size=3), + st.dictionaries(st.text(max_size=8), SCALARS, max_size=3), + st.sampled_from(["", " ", "\t\n"]), + ), + existing=st.booleans(), +) +def test_generated_malformed_action_rejects_without_publication( + action: object, existing: bool +) -> None: + cause = _assert_rejected({"action": action, "frames": []}, ValueError, existing) + assert "Trace field 'action'" in str(cause) + + +@PROPERTY_SETTINGS +@given(event=st.one_of(SCALARS, st.lists(SCALARS, max_size=3)), existing=st.booleans()) +def test_generated_non_mapping_trace_rejects_without_publication( + event: object, existing: bool +) -> None: + cause = _assert_rejected(event, ValueError, existing) + assert "Trace entry must be a mapping" in str(cause) + + +@pytest.mark.parametrize("field", ["addr", "size", "stream", "device_free", "id"]) +@PROPERTY_SETTINGS +@given(value=BAD_NUMBERS, existing=st.booleans()) +def test_generated_wrong_numeric_types_and_nonfinite_values_retain_conversion_cause( + field: str, value: object, existing: bool +) -> None: + event = {"action": "oom", "size": 1, "frames": [], field: value} + cause = _assert_rejected(event, ValueError, existing) + assert f"Trace field '{field}'" in str(cause) + assert isinstance(cause.__cause__, (TypeError, ValueError, OverflowError)) + + +@pytest.mark.parametrize("field", ["addr", "size", "stream", "id"]) +@PROPERTY_SETTINGS +@given( + extra=st.integers(0, 1 << 128), + negative=st.booleans(), + as_text=st.booleans(), + existing=st.booleans(), +) +def test_generated_sqlite_integer_overflow_is_a_controlled_import_error( + field: str, extra: int, negative: bool, as_text: bool, existing: bool +) -> None: + # The existing SQLite INTEGER boundary, not a new snapshot-wide upper + # bound: device_free is deliberately absent because it is not stored here. + value = -(1 << 63) - 1 - extra if negative else (1 << 63) + extra + event = {"action": "oom", "size": 1, "frames": [], field: str(value) if as_text else value} + _assert_rejected(event, OverflowError, existing) + + +@pytest.mark.parametrize("device", [None, 0]) +@PROPERTY_SETTINGS +@example(size=0, stream=0, event_id=0, as_text=False) +@example(size=0, stream=0, event_id=-1, as_text=True) +@example(size=(1 << 63) - 1, stream=(1 << 63) - 1, event_id=(1 << 63) - 1, as_text=True) +@given( + size=st.integers(0, (1 << 63) - 1), + stream=st.integers(0, (1 << 63) - 1), + event_id=st.integers(0, (1 << 63) - 1), + as_text=st.booleans(), +) +def test_generated_valid_numeric_strings_and_sentinels_still_import( + device: int | None, size: int, stream: int, event_id: int, as_text: bool +) -> None: + event = { + "action": "oom", + "size": str(size) if as_text else size, + "stream": str(stream) if as_text else stream, + "id": str(event_id) if as_text else event_id, + } + # Valid empty list/tuple entries remain empty devices, not malformed data. + representation = {"segments": [], "device_traces": [[event], [], ()]} + before = copy.deepcopy(representation) + trace = TraceEntry.from_dict(event) + assert (trace.addr, trace.device_free, trace.frames) == (-1, -1, []) + assert (trace.size, trace.stream, trace.idx) == (size, stream, event_id) + assert TraceEntry.from_dict({**event, "device_free": str(1 << 128)}).device_free == 1 << 128 + with TemporaryDirectory() as directory, pytest.MonkeyPatch.context() as monkeypatch: + root = Path(directory) + source = root / "trusted-in-memory.pkl" + source.write_bytes(SOURCE_BYTES) + output = root / "output" + loads = [] + + def load_representation(path: Path) -> dict[str, object]: + assert path == source + loads.append(path) + return representation + + snapshot2db = importlib.import_module("pt_snap_cli.snapshot.tools.adaptors.snapshot2db") + monkeypatch.setattr(snapshot2db, "load_snapshot_representation", load_representation) + destination = SnapshotImportBackend().dump_to_db(source, output, device=device) + with closing(sqlite3.connect(destination)) as connection: + assert connection.execute( + "SELECT id, address, size, stream FROM trace_entry_0" + ).fetchall() == [(event_id, -1, size, stream)] + assert loads == [source] + assert representation == before + assert source.read_bytes() == SOURCE_BYTES + assert list(output.iterdir()) == [destination] + + +@pytest.mark.parametrize("error_type", [KeyError, TypeError, AttributeError]) +def test_unrelated_backend_programming_errors_are_not_reclassified(error_type) -> None: + with TemporaryDirectory() as directory, pytest.MonkeyPatch.context() as monkeypatch: + root = Path(directory) + source = root / "trusted-in-memory.pkl" + source.write_bytes(SOURCE_BYTES) + output = root / "output" + output.mkdir() + backend = SnapshotImportBackend() + destination = backend.target_db_path(source, output) + destination.write_bytes(b"previous database") + failure = error_type("unrelated implementation error") + + def fail(**kwargs): + raise failure + + monkeypatch.setattr(backend, "_load_run_dump_to_db", lambda: fail) + with pytest.raises(error_type) as error: + backend.dump_to_db(source, output) + assert error.value is failure + assert destination.read_bytes() == b"previous database" + assert source.read_bytes() == SOURCE_BYTES + assert list(output.iterdir()) == [destination] + + +@pytest.mark.parametrize("location", ["root", "segment", "block", "frame"]) +@PROPERTY_SETTINGS +@given(value=st.one_of(SCALARS, st.lists(SCALARS, max_size=3)), existing=st.booleans()) +def test_generated_non_mapping_snapshot_records_are_controlled( + location: str, value: object, existing: bool +) -> None: + representation = _valid_representation() + if location == "root": + representation = value + elif location == "segment": + representation["segments"][0] = value + elif location == "block": + representation["segments"][0]["blocks"][0] = value + else: + representation["device_traces"][0][0]["frames"][0] = value + cause = _assert_representation_rejected(representation, ValueError, existing) + assert "must be a mapping" in str(cause) + + +@pytest.mark.parametrize( + "location", + ["device_traces", "segments", "blocks", "trace_frames", "segment_frames", "block_frames"], +) +@PROPERTY_SETTINGS +@given( + value=st.one_of(SCALARS, st.dictionaries(st.text(max_size=8), SCALARS, max_size=3)), + existing=st.booleans(), +) +def test_generated_non_sequence_snapshot_containers_are_controlled( + location: str, value: object, existing: bool +) -> None: + representation = _valid_representation() + if location in ("device_traces", "segments"): + representation[location] = value + elif location == "blocks": + representation["segments"][0]["blocks"] = value + elif location == "segment_frames": + representation["segments"][0]["frames"] = value + elif location == "block_frames": + representation["segments"][0]["blocks"][0]["frames"] = value + else: + representation["device_traces"][0][0]["frames"] = value + cause = _assert_representation_rejected(representation, ValueError, existing) + assert "must be a list or tuple" in str(cause) + + +@PROPERTY_SETTINGS +@given(value=SCALARS, existing=st.booleans()) +def test_generated_mapping_instead_of_device_trace_sequence_is_controlled( + value: object, existing: bool +) -> None: + representation = {"segments": [], "device_traces": [{"invalid": value}]} + cause = _assert_representation_rejected(representation, ValueError, existing) + assert "Snapshot device_traces[0] must be a list or tuple" in str(cause) + + +@pytest.mark.parametrize("device", [None, 0]) +@pytest.mark.parametrize("position", [0, 1]) +@PROPERTY_SETTINGS +@example(value=None, existing=False) +@example(value={}, existing=True) +@example(value=False, existing=True) +@example(value=0, existing=False) +@example(value="", existing=False) +@given( + value=st.one_of(SCALARS, st.dictionaries(st.text(max_size=8), SCALARS, max_size=3)), + existing=st.booleans(), +) +def test_generated_invalid_inner_device_traces_reject_before_device_selection( + device: int | None, position: int, value: object, existing: bool +) -> None: + # Truthiness must not let malformed empty values masquerade as an empty + # device, either when dumping every device or selecting device zero. + cause = _assert_representation_rejected( + {"segments": [], "device_traces": [[] for _ in range(position)] + [value]}, + ValueError, + existing, + device=device, + ) + assert f"Snapshot device_traces[{position}] must be a list or tuple" in str(cause) + + +@pytest.mark.parametrize( + ("entity", "missing"), + [("frame", key) for key in ("filename", "line", "name")] + + [("block", key) for key in ("size", "requested_size", "address", "state")] + + [ + ("segment", key) + for key in ( + "address", + "total_size", + "stream", + "allocated_size", + "active_size", + "segment_type", + "blocks", + ) + ], +) +@PROPERTY_SETTINGS +@given(line=st.integers(0, 1000), existing=st.booleans()) +def test_generated_missing_required_snapshot_fields_are_controlled( + entity: str, missing: str, line: int, existing: bool +) -> None: + representation = _valid_representation() + frame = representation["device_traces"][0][0]["frames"][0] + frame["line"] = line + record = { + "frame": frame, + "block": representation["segments"][0]["blocks"][0], + "segment": representation["segments"][0], + }[entity] + del record[missing] + cause = _assert_representation_rejected(representation, ValueError, existing) + assert f"missing required field '{missing}'" in str(cause) + + +@pytest.mark.parametrize( + ("entity", "field"), + [("block", key) for key in ("size", "requested_size", "address")] + + [ + ("segment", key) + for key in ("address", "total_size", "stream", "allocated_size", "active_size") + ], +) +@PROPERTY_SETTINGS +@given(value=st.one_of(BAD_NUMBERS, st.text(max_size=16)), existing=st.booleans()) +def test_generated_non_numeric_allocator_metadata_is_controlled( + entity: str, field: str, value: object, existing: bool +) -> None: + representation = _valid_representation() + record = representation["segments"][0] + if entity == "block": + record = record["blocks"][0] + record[field] = value + cause = _assert_representation_rejected(representation, ValueError, existing) + assert f"field '{field}' must be a finite number" in str(cause) + + +@PROPERTY_SETTINGS +@given(value=st.one_of(st.none(), st.integers(-100, 100), st.lists(SCALARS, max_size=3))) +def test_generated_non_string_block_state_is_controlled(value: object) -> None: + representation = _valid_representation() + representation["segments"][0]["blocks"][0]["state"] = value + cause = _assert_representation_rejected(representation, ValueError, True) + assert "Block field 'state' must be a string" in str(cause) + + +@pytest.mark.parametrize("raw_frames", [False, True]) +@PROPERTY_SETTINGS +@example(size=16.0, use_tuples=False) +@given(size=st.integers(1, 4096), use_tuples=st.booleans()) +def test_generated_valid_entity_containers_preserve_input_and_defaults( + raw_frames: bool, size: int | float, use_tuples: bool +) -> None: + representation = _valid_representation() + segment = representation["segments"][0] + segment.update(total_size=size, allocated_size=size, active_size=size) + segment["blocks"][0].update(size=size, requested_size=size) + if use_tuples: + segment["blocks"] = tuple(segment["blocks"]) + representation["segments"] = tuple(representation["segments"]) + event = representation["device_traces"][0][0] + event["frames"] = tuple(event["frames"]) + representation["device_traces"] = ((event,),) + before = copy.deepcopy(representation) + snapshot = DeviceSnapshot.from_dict(representation, 0, _raw_frames=raw_frames) + assert snapshot.total_reserved == snapshot.total_allocated == snapshot.total_activated == size + assert snapshot.segments[0].blocks[0].size == size + assert snapshot.segments[0].device == 0 + assert snapshot.segments[0].is_expandable is False + trace = snapshot.trace_entries[0] + assert (trace.idx, trace.addr, trace.stream, trace.device_free) == (0, -1, 0, -1) + assert trace.get_callstack() == "generated.py:1 run" + assert representation == before diff --git a/tests/query/test_input_structure_properties.py b/tests/query/test_input_structure_properties.py new file mode 100644 index 0000000..a05d49f --- /dev/null +++ b/tests/query/test_input_structure_properties.py @@ -0,0 +1,215 @@ +"""Bounded loader shape regressions, not a new closed YAML schema.""" + +from __future__ import annotations + +import copy +from pathlib import Path +from tempfile import TemporaryDirectory + +import pytest +import yaml +from hypothesis import given, settings +from hypothesis import strategies as st + +from pt_snap_cli.query import config as config_module +from pt_snap_cli.query.config import QueryConfig, QueryTemplate + +PROPERTY_SETTINGS = settings( + max_examples=25, derandomize=True, database=None, deadline=None, print_blob=True +) +NAMES = st.text(alphabet="abcdefghijklmnopqrstuvwxyz", min_size=1, max_size=16) +SCALARS = st.one_of( + st.none(), st.booleans(), st.integers(-(1 << 65), 1 << 65), st.text(max_size=20) +) +NON_MAPPINGS = st.one_of(SCALARS, st.lists(SCALARS, max_size=4)) +INVALID_NAMES = st.one_of( + st.none(), st.booleans(), st.integers(-(1 << 65), 1 << 65), st.sampled_from(["", " "]) +) + + +def _assert_both_loaders_reject(content: str, label: str) -> None: + with pytest.raises(ValueError, match=f"{label} must be a mapping"): + QueryConfig.load_yaml_from_string(content) + with TemporaryDirectory() as directory: + source = Path(directory) / "template.yaml" + source.write_text(content, encoding="utf-8") + before = source.read_bytes() + with pytest.raises(ValueError, match=f"{label} must be a mapping"): + QueryConfig.load_yaml(source) + assert source.read_bytes() == before + assert list(source.parent.iterdir()) == [source] + + +@PROPERTY_SETTINGS +@given( + value=st.one_of( + st.booleans(), + st.integers(-(1 << 65), 1 << 65), + st.text(max_size=20), + st.lists(SCALARS, max_size=4), + ) +) +def test_generated_non_mapping_roots_reject_including_falsy_values(value: object) -> None: + _assert_both_loaders_reject(yaml.safe_dump(value), "configuration") + + +@pytest.mark.parametrize("location", ["queries", "query", "parameters", "parameter"]) +@PROPERTY_SETTINGS +@given(value=NON_MAPPINGS) +def test_generated_nested_non_mappings_raise_validation_error(location: str, value: object) -> None: + content = { + "queries": {"queries": value}, + "query": {"queries": {"generated": value}}, + "parameters": {"queries": {"generated": {"parameters": value}}}, + "parameter": {"queries": {"generated": {"parameters": {"selected": value}}}}, + }[location] + label = { + "queries": "queries", + "query": "query 'generated'", + "parameters": "parameters", + "parameter": "parameter 'selected'", + }[location] + _assert_both_loaders_reject(yaml.safe_dump(content), label) + + +@pytest.mark.parametrize("location", ["query", "parameter"]) +@PROPERTY_SETTINGS +@given(name=INVALID_NAMES) +def test_generated_invalid_declaration_names_are_rejected(location: str, name: object) -> None: + definitions = ( + {name: {"query": "SELECT 1"}} + if location == "query" + else {"generated": {"query": "SELECT 1", "parameters": {name: {}}}} + ) + content = yaml.safe_dump({"queries": definitions}) + with pytest.raises(ValueError, match=f"{location} name must be a non-empty string"): + QueryConfig.load_yaml_from_string(content) + with TemporaryDirectory() as directory: + source = Path(directory) / "template.yaml" + source.write_text(content, encoding="utf-8") + with pytest.raises(ValueError, match=f"{location} name must be a non-empty string"): + QueryConfig.load_yaml(source) + assert source.read_text(encoding="utf-8") == content + + +@pytest.mark.parametrize("location", ["template", "parameters", "parameter"]) +@PROPERTY_SETTINGS +@given(value=NON_MAPPINGS) +def test_direct_template_parser_uses_the_same_mapping_validation( + location: str, value: object +) -> None: + data = { + "template": value, + "parameters": {"parameters": value}, + "parameter": {"parameters": {"selected": value}}, + }[location] + with pytest.raises(ValueError, match="must be a mapping"): + QueryTemplate.from_dict(data) + + +@PROPERTY_SETTINGS +@given(name=NAMES, value=st.integers(-1000, 1000)) +def test_generated_valid_templates_keep_defaults_names_and_category(name: str, value: int) -> None: + body = { + "description": "generated", + "parameters": {"selected": {"type": "int", "default": value}}, + "query": "SELECT {{ selected }} AS value", + } + content = yaml.safe_dump({"queries": {name: body}}) + from_string = QueryConfig.load_yaml_from_string(content) + with TemporaryDirectory() as directory: + source = Path(directory) / "template.yaml" + source.write_text(content, encoding="utf-8") + from_file = QueryConfig.load_yaml(source) + assert from_file == from_string + categorized = QueryConfig.load_yaml(source, default_category="custom") + assert categorized.queries[name].category == "custom" + assert source.read_text(encoding="utf-8") == content + template = from_string.queries[name] + assert template.name == name + assert template.category == "basic" + assert template.validate_params({}) == {"selected": value} + assert template.output_schema == [] + assert template.query == body["query"] + + +@PROPERTY_SETTINGS +@given(name=NAMES, value=SCALARS) +def test_shared_yaml_alias_data_is_not_mutated_and_unknown_fields_remain_compatible( + name: str, value: object +) -> None: + body = { + "query": "SELECT 1", + "extension": value, + "parameters": {"selected": {"type": "int", "extension": value}}, + } + data = {"queries": {name: body, f"other_{name}": body}, "extension": value} + before = copy.deepcopy(data) + with pytest.MonkeyPatch.context() as monkeypatch: + monkeypatch.setattr(config_module.yaml, "safe_load", lambda stream: data) + config = QueryConfig.load_yaml_from_string("loader result supplied above") + assert data == before + assert data["queries"][name] is data["queries"][f"other_{name}"] + assert config.queries[name].name == name + assert config.queries[f"other_{name}"].name == f"other_{name}" + assert all(template.query == "SELECT 1" for template in config.queries.values()) + + +@pytest.mark.parametrize("content", ["", "# comment only\n", "null", "{}", "queries: {}"]) +def test_empty_configuration_contract_is_preserved(content: str) -> None: + assert QueryConfig.load_yaml_from_string(content) == QueryConfig() + + +@PROPERTY_SETTINGS +@given(name=NAMES) +def test_yaml_syntax_errors_retain_the_documented_parser_error(name: str) -> None: + content = f"queries: [{name}" + with pytest.raises(yaml.YAMLError): + QueryConfig.load_yaml_from_string(content) + with TemporaryDirectory() as directory: + source = Path(directory) / "template.yaml" + source.write_text(content, encoding="utf-8") + with pytest.raises(yaml.YAMLError): + QueryConfig.load_yaml(source) + assert source.read_text(encoding="utf-8") == content + + +@pytest.mark.parametrize("field", ["query", "parameter_type"]) +@PROPERTY_SETTINGS +@given( + value=st.one_of( + st.booleans(), + st.integers(-100, 100), + st.lists(SCALARS, max_size=4), + st.dictionaries(NAMES, SCALARS, max_size=4), + ) +) +def test_generated_invalid_sql_and_parameter_type_metadata_is_rejected( + field: str, value: object +) -> None: + body = {"query": value} if field == "query" else {"parameters": {"selected": {"type": value}}} + content = yaml.safe_dump({"queries": {"generated": body}}) + message = "must be a SQL string" if field == "query" else "type must be a string" + with pytest.raises(ValueError, match=message): + QueryConfig.load_yaml_from_string(content) + with TemporaryDirectory() as directory: + source = Path(directory) / "template.yaml" + source.write_text(content, encoding="utf-8") + with pytest.raises(ValueError, match=message): + QueryConfig.load_yaml(source) + assert source.read_text(encoding="utf-8") == content + + +@PROPERTY_SETTINGS +@given(name=INVALID_NAMES) +def test_direct_template_parser_rejects_invalid_explicit_names(name: object) -> None: + with pytest.raises(ValueError, match="template name must be a non-empty string"): + QueryTemplate.from_dict({"name": name, "query": "SELECT 1"}) + + +def test_null_parameter_type_is_not_a_string_but_null_query_keeps_its_default() -> None: + with pytest.raises(ValueError, match="type must be a string"): + QueryTemplate.from_dict({"parameters": {"selected": {"type": None}}}) + assert QueryTemplate.from_dict({"query": None}).query == "" + legacy = QueryTemplate.from_dict({"parameters": {"selected": {"type": "custom_legacy"}}}) + assert legacy.validate_params({"selected": 42}) == {"selected": "42"} diff --git a/tests/snapshot/test_entities.py b/tests/snapshot/test_entities.py index 6069c29..f9e092a 100644 --- a/tests/snapshot/test_entities.py +++ b/tests/snapshot/test_entities.py @@ -90,7 +90,7 @@ def test_from_dict_loads_oom_without_addr_or_stream(self): self.assertEqual(trace.frames, []) def test_from_dict_requires_action(self): - with self.assertRaises(KeyError): + with self.assertRaisesRegex(ValueError, "Trace field 'action'"): TraceEntry.from_dict({"size": 1024, "frames": []}) def test_get_callstack(self): @@ -149,11 +149,11 @@ def test_raw_frames_preserve_empty_missing_and_incomplete_behavior(self): "", ) incomplete = {**base, "frames": [{"filename": "broken.py", "line": 3}]} - with self.assertRaises(KeyError): + with self.assertRaisesRegex(ValueError, "missing required field 'name'"): TraceEntry.from_dict(incomplete) - with self.assertRaises(KeyError): + with self.assertRaisesRegex(ValueError, "missing required field 'name'"): TraceEntry.from_dict(incomplete, _raw_frames=True) - with self.assertRaises(TypeError): + with self.assertRaisesRegex(ValueError, "Frames must be a list or tuple"): TraceEntry.from_dict({**base, "frames": None}, _raw_frames=True) def test_raw_frames_preserve_positional_constructor_and_synthetic_serialization(self): diff --git a/tests/snapshot/test_snapshot2db_runtime.py b/tests/snapshot/test_snapshot2db_runtime.py index 55bcdf7..4209d55 100644 --- a/tests/snapshot/test_snapshot2db_runtime.py +++ b/tests/snapshot/test_snapshot2db_runtime.py @@ -225,7 +225,7 @@ def pre_undo_event(self, event, current_snapshot): monkeypatch.setattr(snapshot2db.DumpEventHooker, "pre_undo_event", pre_undo_event) - with pytest.raises(KeyError, match="name"): + with pytest.raises(ValueError, match="missing required field 'name'"): snapshot2db.dump(snapshot, database, 0) assert replayed_events == 0 @@ -261,7 +261,7 @@ def test_dump_all_removes_committed_devices_when_later_device_is_malformed(tmp_p with snapshot.open("wb") as stream: pickle.dump(representation, stream) - with pytest.raises(KeyError, match="name"): + with pytest.raises(ValueError, match="missing required field 'name'"): snapshot2db.dump(snapshot, database) assert not database.exists() @@ -291,7 +291,7 @@ def test_failed_dump_preserves_existing_database(tmp_path): connection.execute("INSERT INTO marker VALUES ('preserved')") connection.commit() - with pytest.raises(KeyError, match="name"): + with pytest.raises(ValueError, match="missing required field 'name'"): snapshot2db.dump(snapshot, database, 0) with closing(sqlite3.connect(database)) as connection, connection: From 3fbab2a0f60d3d67a0ae38012cb707cf6e641cde Mon Sep 17 00:00:00 2001 From: codeasier <48463012+codeasier@users.noreply.github.com> Date: Sun, 1 Nov 2026 23:04:22 +0800 Subject: [PATCH 12/12] fix: validate declared block states with precise literal types --- src/pt_snap_cli/snapshot/PROVENANCE.md | 5 ++- src/pt_snap_cli/snapshot/base/entities.py | 21 ++++++++-- .../test_snapshot_input_errors_properties.py | 38 ++++++++++++++++++- 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/src/pt_snap_cli/snapshot/PROVENANCE.md b/src/pt_snap_cli/snapshot/PROVENANCE.md index 5ae4c9e..729618e 100644 --- a/src/pt_snap_cli/snapshot/PROVENANCE.md +++ b/src/pt_snap_cli/snapshot/PROVENANCE.md @@ -304,8 +304,9 @@ unchanged. This supersedes the unmerged implementation proposed in PR #207. First-party changes in `base/entities.py` validate the mappings, sequences and required keys consumed by entity construction, finite allocator numeric fields, -and nonempty trace action strings. Failures of existing trace integer conversion -become `ValueError` with the cause preserved. `tools/adaptors/snapshot2db.py` +the declared three-state block enum, and nonempty trace action strings. Failures +of existing trace integer conversion become `ValueError` with the cause preserved. +`tools/adaptors/snapshot2db.py` checks the root/device-trace container before device selection. Numeric strings in traces, optional defaults, sentinels, tuple containers, event IDs and successful replay behavior remain unchanged. The core import adapter also normalizes SQLite diff --git a/src/pt_snap_cli/snapshot/base/entities.py b/src/pt_snap_cli/snapshot/base/entities.py index 13fea1c..8dac641 100644 --- a/src/pt_snap_cli/snapshot/base/entities.py +++ b/src/pt_snap_cli/snapshot/base/entities.py @@ -176,6 +176,22 @@ class BlockState: INACTIVE = "inactive" +def _parse_block_state( + raw: object, +) -> Literal["active_allocated", "active_pending_free", "inactive"]: + if not isinstance(raw, str): + raise ValueError("Block field 'state' must be a string") + if raw == "active_allocated": + return "active_allocated" + if raw == "active_pending_free": + return "active_pending_free" + if raw == "inactive": + return "inactive" + raise ValueError( + "Block field 'state' must be one of: active_allocated, active_pending_free, inactive" + ) + + @dataclass class Block: # A piece of memory returned from the allocator, or @@ -203,8 +219,7 @@ class Block: def from_dict(cls, block_dict: dict[str, Any], _raw_frames: bool = False): _validate_mapping(block_dict, "Block", ("size", "requested_size", "address", "state")) _validate_numbers(block_dict, "Block", ("size", "requested_size", "address")) - if not isinstance(block_dict["state"], str): - raise ValueError("Block field 'state' must be a string") + state = _parse_block_state(block_dict["state"]) frame_dicts = block_dict.get("frames", []) _validate_sequence(frame_dicts, "Frames") if _raw_frames: @@ -213,7 +228,7 @@ def from_dict(cls, block_dict: dict[str, Any], _raw_frames: bool = False): size=block_dict["size"], requested_size=block_dict["requested_size"], address=block_dict["address"], - state=block_dict["state"], + state=state, frames=[] if _raw_frames else [Frame.from_dict(frame) for frame in frame_dicts], _raw_frames=frame_dicts if _raw_frames else None, ) diff --git a/tests/core/test_snapshot_input_errors_properties.py b/tests/core/test_snapshot_input_errors_properties.py index 54620d6..c183da7 100644 --- a/tests/core/test_snapshot_input_errors_properties.py +++ b/tests/core/test_snapshot_input_errors_properties.py @@ -15,7 +15,7 @@ from pt_snap_cli.core.errors import ImportExecutionError from pt_snap_cli.core.snapshot_import_backend import SnapshotImportBackend -from pt_snap_cli.snapshot.base import DeviceSnapshot, TraceEntry +from pt_snap_cli.snapshot.base import Block, DeviceSnapshot, TraceEntry PROPERTY_SETTINGS = settings( max_examples=25, derandomize=True, database=None, deadline=None, print_blob=True @@ -405,6 +405,42 @@ def test_generated_non_string_block_state_is_controlled(value: object) -> None: assert "Block field 'state' must be a string" in str(cause) +@PROPERTY_SETTINGS +@given( + state=st.one_of( + st.sampled_from(["", "unknown", "99", "active", "ACTIVE_ALLOCATED"]), + st.text(max_size=24).map(lambda value: f"unsupported:{value}"), + ), + existing=st.booleans(), +) +def test_generated_unknown_block_states_are_controlled(state: str, existing: bool) -> None: + representation = _valid_representation() + representation["segments"][0]["blocks"][0]["state"] = state + cause = _assert_representation_rejected(representation, ValueError, existing) + assert "Block field 'state' must be one of:" in str(cause) + + +@pytest.mark.parametrize("state", ["active_allocated", "active_pending_free", "inactive"]) +@pytest.mark.parametrize("raw_frames", [False, True]) +@PROPERTY_SETTINGS +@given(size=st.integers(1, 4096)) +def test_generated_declared_block_states_preserve_eager_and_raw_inputs( + state: str, raw_frames: bool, size: int +) -> None: + block_data = { + "size": size, + "requested_size": size // 2, + "address": 4096, + "state": state, + "frames": [{"filename": "generated.py", "line": 1, "name": "run"}], + } + before = copy.deepcopy(block_data) + block = Block.from_dict(block_data, _raw_frames=raw_frames) + assert block.state == state + assert block.to_dict() == before + assert block_data == before + + @pytest.mark.parametrize("raw_frames", [False, True]) @PROPERTY_SETTINGS @example(size=16.0, use_tuples=False)