From c575090000debd359075e2bcc5a0f3ea89ac3df7 Mon Sep 17 00:00:00 2001 From: Travis Vasceannie Date: Thu, 2 Oct 2025 05:15:28 -0400 Subject: [PATCH 1/3] fix: adapt claude command detection for platform variants --- hooks/code_quality_guard.py | 43 +++- tests/hooks/conftest.py | 5 +- tests/hooks/test_dynamic_usage.py | 369 +++++++++++++++++++++++++++ tests/hooks/test_helper_functions.py | 123 +++++++-- 4 files changed, 515 insertions(+), 25 deletions(-) create mode 100644 tests/hooks/test_dynamic_usage.py diff --git a/hooks/code_quality_guard.py b/hooks/code_quality_guard.py index f9e464f..a87660d 100644 --- a/hooks/code_quality_guard.py +++ b/hooks/code_quality_guard.py @@ -10,6 +10,7 @@ import json import logging import os import re +import shutil import subprocess import sys import tokenize @@ -418,13 +419,43 @@ def should_skip_file(file_path: str, config: QualityConfig) -> bool: def get_claude_quality_command() -> list[str]: """Return a path-resilient command for invoking claude-quality.""" repo_root = Path(__file__).resolve().parent.parent - venv_python = repo_root / ".venv/bin/python" - if venv_python.exists(): - return [str(venv_python), "-m", "quality.cli.main"] + platform_name = sys.platform + is_windows = platform_name.startswith("win") - venv_cli = repo_root / ".venv/bin/claude-quality" - if venv_cli.exists(): - return [str(venv_cli)] + scripts_dir = "Scripts" if is_windows else "bin" + python_candidates: list[Path] = [] + + primary_python = "python.exe" if is_windows else "python" + python_candidates.append(repo_root / ".venv" / scripts_dir / primary_python) + + secondary_python = "python3.exe" if is_windows else "python3" + if secondary_python != primary_python: + python_candidates.append(repo_root / ".venv" / scripts_dir / secondary_python) + + for python_path in python_candidates: + if python_path.exists(): + return [str(python_path), "-m", "quality.cli.main"] + + cli_candidates: list[Path] = [] + cli_name = "claude-quality.exe" if is_windows else "claude-quality" + cli_candidates.append(repo_root / ".venv" / scripts_dir / cli_name) + + # Some Windows environments may install scripts without the .exe suffix + if is_windows: + cli_candidates.append(repo_root / ".venv" / scripts_dir / "claude-quality") + + for cli_path in cli_candidates: + if cli_path.exists(): + return [str(cli_path)] + + # Fall back to interpreter on PATH, preferring python3 on POSIX + preferred = "python" if is_windows else "python3" + if shutil.which(preferred): + return [preferred, "-m", "quality.cli.main"] + + alternate = "python3" if is_windows else "python" + if shutil.which(alternate): + return [alternate, "-m", "quality.cli.main"] return ["claude-quality"] diff --git a/tests/hooks/conftest.py b/tests/hooks/conftest.py index dec0bd4..ed46461 100644 --- a/tests/hooks/conftest.py +++ b/tests/hooks/conftest.py @@ -121,7 +121,6 @@ def calculate_order_total(orders): def clean_code() -> str: """Sample clean, modern Python code.""" return """ -from typing import List, Optional, Dict from dataclasses import dataclass @@ -132,13 +131,13 @@ class User: active: bool = True -def process_users(users: List[User]) -> Dict[str, int]: +def process_users(users: list[User]) -> dict[str, int]: \"\"\"Process active users and return counts.\"\"\" active_count = sum(1 for user in users if user.active) return {"active": active_count, "total": len(users)} -def find_user(users: List[User], email: str) -> Optional[User]: +def find_user(users: list[User], email: str) -> User | None: \"\"\"Find user by email.\"\"\" return next((u for u in users if u.email == email), None) """ diff --git a/tests/hooks/test_dynamic_usage.py b/tests/hooks/test_dynamic_usage.py new file mode 100644 index 0000000..12a15e8 --- /dev/null +++ b/tests/hooks/test_dynamic_usage.py @@ -0,0 +1,369 @@ +"""Tests for multi-context hook usage across containers, projects, and users.""" + +from __future__ import annotations + +import json +from pathlib import Path +from types import SimpleNamespace +from typing import Iterator +from unittest.mock import patch + +import pytest + +from code_quality_guard import ( + QualityConfig, + posttooluse_hook, + pretooluse_hook, +) + + +@pytest.fixture() +def multi_container_paths(tmp_path: Path) -> dict[str, Path]: + """Create container/project directories used across tests.""" + container_a = tmp_path / "container-a" / "project" / "src" + container_b = tmp_path / "container-b" / "project" / "src" + container_a.mkdir(parents=True) + container_b.mkdir(parents=True) + return {"a": container_a, "b": container_b} + + +def _pre_request( + file_path: Path, + content: str, + *, + container_id: str, + project_id: str, + user_id: str, + platform_name: str = "linux", + runtime_name: str = "python3", +) -> dict[str, object]: + """Build a PreToolUse hook payload with rich metadata.""" + return { + "tool_name": "Write", + "tool_input": { + "file_path": str(file_path), + "content": content, + "metadata": { + "containerId": container_id, + "projectId": project_id, + }, + }, + "metadata": { + "user": {"id": user_id, "role": "developer"}, + "container": {"id": container_id}, + "project": {"id": project_id}, + "platform": {"os": platform_name}, + "runtime": {"name": runtime_name}, + }, + } + + +def _post_request( + file_path: Path, + *, + container_id: str, + project_id: str, + user_id: str, + platform_name: str = "linux", + runtime_name: str = "python3", +) -> dict[str, object]: + """Build a PostToolUse payload mirroring the metadata structure.""" + return { + "tool_name": "Write", + "tool_output": { + "file_path": str(file_path), + "status": "success", + "metadata": { + "containerId": container_id, + "projectId": project_id, + }, + }, + "metadata": { + "user": {"id": user_id, "role": "developer"}, + "container": {"id": container_id}, + "project": {"id": project_id}, + "platform": {"os": platform_name}, + "runtime": {"name": runtime_name}, + }, + } + + +@pytest.mark.parametrize( + ("platform_name", "runtime_name"), + [ + ("linux", "python3"), + ("win32", "python"), + ("darwin", "python3"), + ], +) +def test_pretooluse_handles_platform_metadata( + tmp_path: Path, + platform_name: str, + runtime_name: str, +) -> None: + """Ensure platform/runtime metadata does not change allow decisions.""" + config = QualityConfig() + config.skip_patterns = [] + + file_path = tmp_path / "project" / f"sample_{platform_name}.py" + file_path.parent.mkdir(parents=True, exist_ok=True) + content = "def sample() -> None:\n return None\n" + + with patch("code_quality_guard.analyze_code_quality", return_value={}): + response = pretooluse_hook( + _pre_request( + file_path, + content, + container_id="platform-container", + project_id="platform-project", + user_id="platform-user", + platform_name=platform_name, + runtime_name=runtime_name, + ), + config, + ) + + assert response["permissionDecision"] == "allow" + + +def test_state_tracking_isolation_between_containers(multi_container_paths: dict[str, Path]) -> None: + """State tracking should stay isolated per container/project combination.""" + config = QualityConfig(state_tracking_enabled=True) + config.skip_patterns = [] # Ensure state tracking runs even in pytest temp dirs. + + base_content = """\ + +def alpha(): + return 1 + + +def beta(): + return 2 +""".lstrip() + + container_a = multi_container_paths["a"] + container_b = multi_container_paths["b"] + file_a = container_a / "service.py" + file_b = container_b / "service.py" + + # PreToolUse runs register the pre-state for each container/project pair. + with patch("code_quality_guard.analyze_code_quality", return_value={}): + response_a_pre = pretooluse_hook( + _pre_request( + file_a, + base_content, + container_id="container-a", + project_id="project-alpha", + user_id="user-deny", + ), + config, + ) + response_b_pre = pretooluse_hook( + _pre_request( + file_b, + base_content, + container_id="container-b", + project_id="project-beta", + user_id="user-allow", + ), + config, + ) + + assert response_a_pre["permissionDecision"] == "allow" + assert response_b_pre["permissionDecision"] == "allow" + + # The first container writes fewer functions which should trigger a warning. + file_a.write_text("""\ + +def alpha(): + return 1 +""" + ) + + # The second container preserves the original content. + file_b.write_text(base_content) + + response_a_post = posttooluse_hook( + _post_request( + file_a, + container_id="container-a", + project_id="project-alpha", + user_id="user-deny", + ), + config, + ) + response_b_post = posttooluse_hook( + _post_request( + file_b, + container_id="container-b", + project_id="project-beta", + user_id="user-allow", + ), + config, + ) + + assert response_a_post.get("decision") == "block" + assert "Reduced functions" in response_a_post.get("reason", "") + # Ensure the second container is unaffected by the first one's regression. + assert response_b_post.get("decision") is None + assert "Reduced functions" not in response_b_post.get("reason", "") + + +@pytest.mark.parametrize("project_marker", [".git", "pyproject.toml"]) +def test_cross_file_duplicate_project_root_detection( + tmp_path: Path, + project_marker: str, +) -> None: + """Cross-file duplicate checks should resolve the project root per container.""" + project_root = tmp_path / "workspace" / "container" / "demo-project" + target_dir = project_root / "src" / "package" + target_dir.mkdir(parents=True) + + if project_marker == ".git": + (project_root / ".git").mkdir() + else: + (project_root / project_marker).write_text("{}") + + file_path = target_dir / "module.py" + file_path.write_text("def thing() -> int:\n return 1\n") + + config = QualityConfig(cross_file_check_enabled=True) + + captured: dict[str, list[str]] = {} + + def fake_run( + cmd: list[str], + *, + check: bool, + capture_output: bool, + text: bool, + timeout: int, + ) -> SimpleNamespace: + captured["cmd"] = cmd + return SimpleNamespace(returncode=0, stdout=json.dumps({"duplicates": []})) + + with patch("code_quality_guard.subprocess.run", side_effect=fake_run): + response = posttooluse_hook( + { + "tool_name": "Write", + "tool_output": {"file_path": str(file_path)}, + "metadata": { + "container": {"id": "dynamic-container"}, + "project": {"id": "demo-project"}, + }, + }, + config, + ) + + assert "duplicates" in captured["cmd"] + dup_index = captured["cmd"].index("duplicates") + assert captured["cmd"][dup_index + 1] == str(project_root) + assert "--threshold" in captured["cmd"] + assert response["hookSpecificOutput"]["hookEventName"] == "PostToolUse" + assert response.get("decision") is None + + +def test_main_handles_permission_decisions_for_multiple_users(monkeypatch: pytest.MonkeyPatch) -> None: + """`main` should surface deny/ask decisions for different user contexts.""" + from code_quality_guard import main + + hook_inputs = [ + { + "tool_name": "Write", + "tool_input": { + "file_path": "tenant-one.py", + "content": "print('tenant-one')", + }, + "metadata": {"user": {"id": "user-deny", "role": "viewer"}}, + }, + { + "tool_name": "Write", + "tool_input": { + "file_path": "tenant-two.py", + "content": "print('tenant-two')", + }, + "metadata": {"user": {"id": "user-ask", "role": "contractor"}}, + }, + { + "tool_name": "Write", + "tool_input": { + "file_path": "tenant-three.py", + "content": "print('tenant-three')", + }, + "metadata": {"user": {"id": "user-allow", "role": "admin"}}, + }, + ] + + responses = iter( + [ + { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": "Tenant user-deny lacks write access", + }, + "permissionDecision": "deny", + "reason": "Tenant user-deny lacks write access", + }, + { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "ask", + "permissionDecisionReason": "Tenant user-ask requires approval", + }, + "permissionDecision": "ask", + "reason": "Tenant user-ask requires approval", + }, + { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "allow", + }, + "permissionDecision": "allow", + }, + ] + ) + + input_iter: Iterator[dict[str, object]] = iter(hook_inputs) + + def fake_json_load(_stream: object) -> dict[str, object]: + return next(input_iter) + + def fake_pretooluse(_hook_data: dict[str, object], _config: QualityConfig) -> dict[str, object]: + return next(responses) + + exit_calls: list[tuple[str, int]] = [] + + def fake_exit(reason: str, exit_code: int = 2) -> None: + exit_calls.append((reason, exit_code)) + raise SystemExit(exit_code) + + printed: list[str] = [] + + def fake_print(message: str) -> None: # noqa: D401 - simple passthrough + printed.append(message) + + monkeypatch.setattr("json.load", fake_json_load) + monkeypatch.setattr("code_quality_guard.pretooluse_hook", fake_pretooluse) + monkeypatch.setattr("code_quality_guard._exit_with_reason", fake_exit) + monkeypatch.setattr("builtins.print", fake_print) + + # First tenant should produce a deny decision with exit code 2. + with pytest.raises(SystemExit) as excinfo_one: + main() + assert excinfo_one.value.code == 2 + assert exit_calls[0] == ("Tenant user-deny lacks write access", 2) + + # Second tenant requires approval and should also trigger exit code 2. + with pytest.raises(SystemExit) as excinfo_two: + main() + assert excinfo_two.value.code == 2 + assert exit_calls[1] == ("Tenant user-ask requires approval", 2) + + # Third tenant is allowed and should simply print the response without exiting. + main() + + third_response = json.loads(printed[-1]) + assert third_response["permissionDecision"] == "allow" + assert third_response["hookSpecificOutput"]["hookEventName"] == "PreToolUse" + assert len(exit_calls) == 2 diff --git a/tests/hooks/test_helper_functions.py b/tests/hooks/test_helper_functions.py index 52e5cae..9b7b8a5 100644 --- a/tests/hooks/test_helper_functions.py +++ b/tests/hooks/test_helper_functions.py @@ -2,10 +2,15 @@ import hashlib import json +import shutil +import sys import tempfile from datetime import UTC, datetime +from pathlib import Path from unittest.mock import MagicMock, patch +import pytest + from code_quality_guard import ( QualityConfig, analyze_code_quality, @@ -18,7 +23,6 @@ from code_quality_guard import ( verify_naming_conventions, ) - class TestHelperFunctions: """Test helper functions in the hook.""" @@ -46,25 +50,112 @@ class TestHelperFunctions: should_skip_file("test_file.py", config) is False ) # Default pattern not included - def test_get_claude_quality_command_venv(self): + def test_get_claude_quality_command_venv(self, monkeypatch: pytest.MonkeyPatch): """Prefer python module entrypoint when venv python exists.""" - with patch("pathlib.Path.exists", side_effect=[True]): - cmd = get_claude_quality_command() - assert cmd[0].endswith(".venv/bin/python") - assert cmd[1:] == ["-m", "quality.cli.main"] + repo_root = Path(__file__).resolve().parent.parent.parent + python_path = repo_root / ".venv" / "bin" / "python" + overrides = {str(python_path): True} - def test_get_claude_quality_command_cli_fallback(self): + def fake_exists(path: Path) -> bool: + return overrides.get(str(path), False) + + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setattr(Path, "exists", fake_exists) + monkeypatch.setattr(shutil, "which", lambda _name: None) + + cmd = get_claude_quality_command() + assert cmd == [str(python_path), "-m", "quality.cli.main"] + + def test_get_claude_quality_command_cli_fallback(self, monkeypatch: pytest.MonkeyPatch): """Fallback to claude-quality script when python missing.""" - with patch("pathlib.Path.exists", side_effect=[False, True]): - cmd = get_claude_quality_command() - assert len(cmd) == 1 - assert cmd[0].endswith(".venv/bin/claude-quality") + repo_root = Path(__file__).resolve().parent.parent.parent + overrides = { + str(repo_root / ".venv" / "bin" / "python"): False, + str(repo_root / ".venv" / "bin" / "python3"): False, + str(repo_root / ".venv" / "bin" / "claude-quality"): True, + } - def test_get_claude_quality_command_system(self): - """Fall back to binary on PATH when venv options absent.""" - with patch("pathlib.Path.exists", side_effect=[False, False]): - cmd = get_claude_quality_command() - assert cmd == ["claude-quality"] + def fake_exists(path: Path) -> bool: + return overrides.get(str(path), False) + + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setattr(Path, "exists", fake_exists) + monkeypatch.setattr(shutil, "which", lambda _name: None) + + cmd = get_claude_quality_command() + assert cmd == [str(repo_root / ".venv" / "bin" / "claude-quality")] + + def test_get_claude_quality_command_system_python3(self, monkeypatch: pytest.MonkeyPatch): + """Fall back to python3 on PATH when virtualenv executables absent.""" + + def fake_exists(path: Path) -> bool: # noqa: ARG001 - signature matches Path.exists + return False + + monkeypatch.setattr(sys, "platform", "linux") + monkeypatch.setattr(Path, "exists", fake_exists) + monkeypatch.setattr( + shutil, + "which", + lambda name: "/usr/bin/python3" if name == "python3" else None, + ) + + cmd = get_claude_quality_command() + assert cmd == ["python3", "-m", "quality.cli.main"] + + def test_get_claude_quality_command_windows_paths(self, monkeypatch: pytest.MonkeyPatch): + """Use Windows Scripts directory when python executable exists.""" + repo_root = Path(__file__).resolve().parent.parent.parent + python_path = repo_root / ".venv" / "Scripts" / "python.exe" + overrides = { + str(python_path): True, + str(repo_root / ".venv" / "Scripts" / "python3.exe"): False, + } + + def fake_exists(path: Path) -> bool: + return overrides.get(str(path), False) + + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setattr(Path, "exists", fake_exists) + monkeypatch.setattr(shutil, "which", lambda _name: None) + + cmd = get_claude_quality_command() + assert cmd == [str(python_path), "-m", "quality.cli.main"] + + def test_get_claude_quality_command_windows_python_fallback(self, monkeypatch: pytest.MonkeyPatch): + """Fallback to python on PATH for Windows when scripts missing.""" + + def fake_exists(path: Path) -> bool: # noqa: ARG001 - signature matches Path.exists + return False + + monkeypatch.setattr(sys, "platform", "win32") + monkeypatch.setattr(Path, "exists", fake_exists) + monkeypatch.setattr( + shutil, + "which", + lambda name: "C:/Python/python.exe" if name == "python" else None, + ) + + cmd = get_claude_quality_command() + assert cmd == ["python", "-m", "quality.cli.main"] + + def test_get_claude_quality_command_macos_python3(self, monkeypatch: pytest.MonkeyPatch): + """Prefer python3 binary on macOS when available in virtualenv.""" + repo_root = Path(__file__).resolve().parent.parent.parent + overrides = { + str(repo_root / ".venv" / "bin" / "python"): False, + str(repo_root / ".venv" / "bin" / "python3"): True, + } + + def fake_exists(path: Path) -> bool: + return overrides.get(str(path), False) + + monkeypatch.setattr(sys, "platform", "darwin") + monkeypatch.setattr(Path, "exists", fake_exists) + monkeypatch.setattr(shutil, "which", lambda _name: None) + + cmd = get_claude_quality_command() + expected_path = repo_root / ".venv" / "bin" / "python3" + assert cmd == [str(expected_path), "-m", "quality.cli.main"] def test_store_pre_state(self): """Test storing pre-modification state.""" From 232e1ceaac2fd703e168795b5692842d384b0e4f Mon Sep 17 00:00:00 2001 From: Travis Vasceannie Date: Thu, 2 Oct 2025 05:42:39 -0400 Subject: [PATCH 2/3] fix: streamline claude command resolution --- hooks/code_quality_guard.py | 68 ++++----- tests/hooks/test_dynamic_usage.py | 77 ++++++++++ tests/hooks/test_helper_functions.py | 206 ++++++++++++++++----------- 3 files changed, 236 insertions(+), 115 deletions(-) diff --git a/hooks/code_quality_guard.py b/hooks/code_quality_guard.py index a87660d..3b3adad 100644 --- a/hooks/code_quality_guard.py +++ b/hooks/code_quality_guard.py @@ -416,48 +416,50 @@ def should_skip_file(file_path: str, config: QualityConfig) -> bool: return any(pattern in file_path for pattern in config.skip_patterns) -def get_claude_quality_command() -> list[str]: +def _module_candidate(path: Path) -> tuple[Path, list[str]]: + """Build a module invocation candidate for a python executable.""" + + return path, [str(path), "-m", "quality.cli.main"] + + +def _cli_candidate(path: Path) -> tuple[Path, list[str]]: + """Build a direct CLI invocation candidate.""" + + return path, [str(path)] + + +def get_claude_quality_command(repo_root: Path | None = None) -> list[str]: """Return a path-resilient command for invoking claude-quality.""" - repo_root = Path(__file__).resolve().parent.parent + + repo_root = repo_root or Path(__file__).resolve().parent.parent platform_name = sys.platform is_windows = platform_name.startswith("win") - scripts_dir = "Scripts" if is_windows else "bin" - python_candidates: list[Path] = [] + scripts_dir = repo_root / ".venv" / ("Scripts" if is_windows else "bin") + python_names = ["python.exe", "python3.exe"] if is_windows else ["python", "python3"] + cli_names = ["claude-quality.exe", "claude-quality"] if is_windows else ["claude-quality"] - primary_python = "python.exe" if is_windows else "python" - python_candidates.append(repo_root / ".venv" / scripts_dir / primary_python) + candidates: list[tuple[Path, list[str]]] = [] + for name in python_names: + candidates.append(_module_candidate(scripts_dir / name)) + for name in cli_names: + candidates.append(_cli_candidate(scripts_dir / name)) - secondary_python = "python3.exe" if is_windows else "python3" - if secondary_python != primary_python: - python_candidates.append(repo_root / ".venv" / scripts_dir / secondary_python) + for candidate_path, command in candidates: + if candidate_path.exists(): + return command - for python_path in python_candidates: - if python_path.exists(): - return [str(python_path), "-m", "quality.cli.main"] + interpreter_fallbacks = ["python"] if is_windows else ["python3", "python"] + for interpreter in interpreter_fallbacks: + if shutil.which(interpreter): + return [interpreter, "-m", "quality.cli.main"] - cli_candidates: list[Path] = [] - cli_name = "claude-quality.exe" if is_windows else "claude-quality" - cli_candidates.append(repo_root / ".venv" / scripts_dir / cli_name) + if shutil.which("claude-quality"): + return ["claude-quality"] - # Some Windows environments may install scripts without the .exe suffix - if is_windows: - cli_candidates.append(repo_root / ".venv" / scripts_dir / "claude-quality") - - for cli_path in cli_candidates: - if cli_path.exists(): - return [str(cli_path)] - - # Fall back to interpreter on PATH, preferring python3 on POSIX - preferred = "python" if is_windows else "python3" - if shutil.which(preferred): - return [preferred, "-m", "quality.cli.main"] - - alternate = "python3" if is_windows else "python" - if shutil.which(alternate): - return [alternate, "-m", "quality.cli.main"] - - return ["claude-quality"] + raise RuntimeError( + "'claude-quality' was not found on PATH. Please ensure it is installed and available." + ) def _ensure_tool_installed(tool_name: str) -> bool: diff --git a/tests/hooks/test_dynamic_usage.py b/tests/hooks/test_dynamic_usage.py index 12a15e8..ed53561 100644 --- a/tests/hooks/test_dynamic_usage.py +++ b/tests/hooks/test_dynamic_usage.py @@ -209,6 +209,83 @@ def alpha(): assert "Reduced functions" not in response_b_post.get("reason", "") +def test_state_tracking_id_collision_different_paths(tmp_path: Path) -> None: + """State tracking keys should include the file path when IDs collide.""" + + config = QualityConfig(state_tracking_enabled=True) + config.skip_patterns = [] + + shared_container = "shared-container" + shared_project = "shared-project" + base_content = """\ + +def alpha(): + return 1 + + +def beta(): + return 2 +""".lstrip() + + path_one = tmp_path / "tenant" / "variant-one" / "service.py" + path_two = tmp_path / "tenant" / "variant-two" / "service.py" + path_one.parent.mkdir(parents=True, exist_ok=True) + path_two.parent.mkdir(parents=True, exist_ok=True) + + with patch("code_quality_guard.analyze_code_quality", return_value={}): + pretooluse_hook( + _pre_request( + path_one, + base_content, + container_id=shared_container, + project_id=shared_project, + user_id="collision-user", + ), + config, + ) + pretooluse_hook( + _pre_request( + path_two, + base_content, + container_id=shared_container, + project_id=shared_project, + user_id="collision-user", + ), + config, + ) + + path_one.write_text("""\ + +def alpha(): + return 1 +""".lstrip()) + path_two.write_text(base_content) + + degraded_response = posttooluse_hook( + _post_request( + path_one, + container_id=shared_container, + project_id=shared_project, + user_id="collision-user", + ), + config, + ) + preserved_response = posttooluse_hook( + _post_request( + path_two, + container_id=shared_container, + project_id=shared_project, + user_id="collision-user", + ), + config, + ) + + assert degraded_response.get("decision") == "block" + assert "Reduced functions" in degraded_response.get("reason", "") + assert preserved_response.get("decision") is None + assert "Reduced functions" not in preserved_response.get("reason", "") + + @pytest.mark.parametrize("project_marker", [".git", "pyproject.toml"]) def test_cross_file_duplicate_project_root_detection( tmp_path: Path, diff --git a/tests/hooks/test_helper_functions.py b/tests/hooks/test_helper_functions.py index 9b7b8a5..4413daa 100644 --- a/tests/hooks/test_helper_functions.py +++ b/tests/hooks/test_helper_functions.py @@ -23,6 +23,16 @@ from code_quality_guard import ( verify_naming_conventions, ) + +@pytest.fixture +def set_platform(monkeypatch: pytest.MonkeyPatch): + """Provide a helper to override sys.platform within a test.""" + + def _setter(name: str) -> None: + monkeypatch.setattr(sys, "platform", name) + + return _setter + class TestHelperFunctions: """Test helper functions in the hook.""" @@ -50,112 +60,144 @@ class TestHelperFunctions: should_skip_file("test_file.py", config) is False ) # Default pattern not included - def test_get_claude_quality_command_venv(self, monkeypatch: pytest.MonkeyPatch): - """Prefer python module entrypoint when venv python exists.""" - repo_root = Path(__file__).resolve().parent.parent.parent - python_path = repo_root / ".venv" / "bin" / "python" - overrides = {str(python_path): True} + @staticmethod + def _touch(path: Path) -> Path: + """Create an empty file representing an executable.""" - def fake_exists(path: Path) -> bool: - return overrides.get(str(path), False) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("") + return path - monkeypatch.setattr(sys, "platform", "linux") - monkeypatch.setattr(Path, "exists", fake_exists) + @pytest.mark.parametrize( + ("platform_name", "scripts_dir", "executable_name"), + [ + ("linux", "bin", "python"), + ("darwin", "bin", "python"), + ("win32", "Scripts", "python.exe"), + ], + ) + def test_get_claude_quality_command_prefers_primary_python( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + set_platform, + platform_name: str, + scripts_dir: str, + executable_name: str, + ) -> None: + """Prefer the platform-specific python executable when present in the venv.""" + + set_platform(platform_name) monkeypatch.setattr(shutil, "which", lambda _name: None) - cmd = get_claude_quality_command() + executable = self._touch(tmp_path / ".venv" / scripts_dir / executable_name) + + cmd = get_claude_quality_command(repo_root=tmp_path) + assert cmd == [str(executable), "-m", "quality.cli.main"] + + def test_get_claude_quality_command_python_and_python3(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, set_platform) -> None: + """Prefer python when both python and python3 executables exist in the venv.""" + + set_platform("linux") + monkeypatch.setattr(shutil, "which", lambda _name: None) + + python_path = self._touch(tmp_path / ".venv" / "bin" / "python") + python3_path = self._touch(tmp_path / ".venv" / "bin" / "python3") + + cmd = get_claude_quality_command(repo_root=tmp_path) assert cmd == [str(python_path), "-m", "quality.cli.main"] + assert python3_path.exists() # Sanity check that both executables were present - def test_get_claude_quality_command_cli_fallback(self, monkeypatch: pytest.MonkeyPatch): - """Fallback to claude-quality script when python missing.""" - repo_root = Path(__file__).resolve().parent.parent.parent - overrides = { - str(repo_root / ".venv" / "bin" / "python"): False, - str(repo_root / ".venv" / "bin" / "python3"): False, - str(repo_root / ".venv" / "bin" / "claude-quality"): True, - } + def test_get_claude_quality_command_cli_fallback(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, set_platform) -> None: + """Fallback to claude-quality script when python executables are absent.""" - def fake_exists(path: Path) -> bool: - return overrides.get(str(path), False) - - monkeypatch.setattr(sys, "platform", "linux") - monkeypatch.setattr(Path, "exists", fake_exists) + set_platform("linux") monkeypatch.setattr(shutil, "which", lambda _name: None) - cmd = get_claude_quality_command() - assert cmd == [str(repo_root / ".venv" / "bin" / "claude-quality")] + cli_path = self._touch(tmp_path / ".venv" / "bin" / "claude-quality") - def test_get_claude_quality_command_system_python3(self, monkeypatch: pytest.MonkeyPatch): - """Fall back to python3 on PATH when virtualenv executables absent.""" + cmd = get_claude_quality_command(repo_root=tmp_path) + assert cmd == [str(cli_path)] - def fake_exists(path: Path) -> bool: # noqa: ARG001 - signature matches Path.exists - return False + def test_get_claude_quality_command_windows_cli_without_extension( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + set_platform, + ) -> None: + """Handle Windows environments where the claude-quality script lacks an .exe suffix.""" - monkeypatch.setattr(sys, "platform", "linux") - monkeypatch.setattr(Path, "exists", fake_exists) - monkeypatch.setattr( - shutil, - "which", - lambda name: "/usr/bin/python3" if name == "python3" else None, - ) + set_platform("win32") + monkeypatch.setattr(shutil, "which", lambda _name: None) - cmd = get_claude_quality_command() + cli_path = self._touch(tmp_path / ".venv" / "Scripts" / "claude-quality") + + cmd = get_claude_quality_command(repo_root=tmp_path) + assert cmd == [str(cli_path)] + + def test_get_claude_quality_command_system_python_fallback( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + set_platform, + ) -> None: + """Fall back to python3 on POSIX and python on Windows when no venv executables exist.""" + + set_platform("darwin") + + def fake_which(name: str) -> str | None: + return "/usr/bin/python3" if name == "python3" else None + + monkeypatch.setattr(shutil, "which", fake_which) + + cmd = get_claude_quality_command(repo_root=tmp_path) assert cmd == ["python3", "-m", "quality.cli.main"] - def test_get_claude_quality_command_windows_paths(self, monkeypatch: pytest.MonkeyPatch): - """Use Windows Scripts directory when python executable exists.""" - repo_root = Path(__file__).resolve().parent.parent.parent - python_path = repo_root / ".venv" / "Scripts" / "python.exe" - overrides = { - str(python_path): True, - str(repo_root / ".venv" / "Scripts" / "python3.exe"): False, - } + set_platform("win32") - def fake_exists(path: Path) -> bool: - return overrides.get(str(path), False) + def windows_which(name: str) -> str | None: + return "C:/Python/python.exe" if name == "python" else None - monkeypatch.setattr(sys, "platform", "win32") - monkeypatch.setattr(Path, "exists", fake_exists) - monkeypatch.setattr(shutil, "which", lambda _name: None) + monkeypatch.setattr(shutil, "which", windows_which) - cmd = get_claude_quality_command() - assert cmd == [str(python_path), "-m", "quality.cli.main"] - - def test_get_claude_quality_command_windows_python_fallback(self, monkeypatch: pytest.MonkeyPatch): - """Fallback to python on PATH for Windows when scripts missing.""" - - def fake_exists(path: Path) -> bool: # noqa: ARG001 - signature matches Path.exists - return False - - monkeypatch.setattr(sys, "platform", "win32") - monkeypatch.setattr(Path, "exists", fake_exists) - monkeypatch.setattr( - shutil, - "which", - lambda name: "C:/Python/python.exe" if name == "python" else None, - ) - - cmd = get_claude_quality_command() + cmd = get_claude_quality_command(repo_root=tmp_path) assert cmd == ["python", "-m", "quality.cli.main"] - def test_get_claude_quality_command_macos_python3(self, monkeypatch: pytest.MonkeyPatch): - """Prefer python3 binary on macOS when available in virtualenv.""" - repo_root = Path(__file__).resolve().parent.parent.parent - overrides = { - str(repo_root / ".venv" / "bin" / "python"): False, - str(repo_root / ".venv" / "bin" / "python3"): True, - } + def test_get_claude_quality_command_cli_on_path( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + set_platform, + ) -> None: + """Use claude-quality from PATH when no virtualenv interpreters exist.""" - def fake_exists(path: Path) -> bool: - return overrides.get(str(path), False) + set_platform("linux") - monkeypatch.setattr(sys, "platform", "darwin") - monkeypatch.setattr(Path, "exists", fake_exists) + def fake_which(name: str) -> str | None: + if name == "claude-quality": + return "/usr/local/bin/claude-quality" + return None + + monkeypatch.setattr(shutil, "which", fake_which) + + cmd = get_claude_quality_command(repo_root=tmp_path) + assert cmd == ["claude-quality"] + + def test_get_claude_quality_command_raises_when_missing( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + set_platform, + ) -> None: + """Raise a clear error when no interpreter or CLI can be located.""" + + set_platform("linux") monkeypatch.setattr(shutil, "which", lambda _name: None) - cmd = get_claude_quality_command() - expected_path = repo_root / ".venv" / "bin" / "python3" - assert cmd == [str(expected_path), "-m", "quality.cli.main"] + with pytest.raises(RuntimeError) as excinfo: + get_claude_quality_command(repo_root=tmp_path) + + assert "was not found on PATH" in str(excinfo.value) def test_store_pre_state(self): """Test storing pre-modification state.""" From 44f9d94131e54fa722b0d93f92ca87b3b8cf859d Mon Sep 17 00:00:00 2001 From: Travis Vasceannie Date: Thu, 2 Oct 2025 05:51:36 -0400 Subject: [PATCH 3/3] Update tests/hooks/test_dynamic_usage.py Co-authored-by: qodo-merge-pro[bot] <151058649+qodo-merge-pro[bot]@users.noreply.github.com> --- tests/hooks/test_dynamic_usage.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/hooks/test_dynamic_usage.py b/tests/hooks/test_dynamic_usage.py index ed53561..daa1fca 100644 --- a/tests/hooks/test_dynamic_usage.py +++ b/tests/hooks/test_dynamic_usage.py @@ -232,13 +232,9 @@ def beta(): path_one.parent.mkdir(parents=True, exist_ok=True) path_two.parent.mkdir(parents=True, exist_ok=True) - with patch("code_quality_guard.analyze_code_quality", return_value={}): - pretooluse_hook( - _pre_request( - path_one, - base_content, - container_id=shared_container, - project_id=shared_project, + cmd: list[str], + **kwargs: object, +) -> SimpleNamespace: user_id="collision-user", ), config,