StillKnotKnown
ed93df698b
* fix: add mock reset fixtures and resolve async iterator mock issues - Add pytest_runtest_setup and pytest_runtest_teardown hooks to reset shared module-level mocks between tests - Add module-specific mock reset fixtures for test_qa_fixer and test_qa_reviewer to prevent test interference - Fix async iterator mock for receive_response to properly return an AsyncIteratorMock instance - Update test_qa_fixer.py and test_qa_reviewer.py with proper mock setup for isolated test execution * docs(agents): add CLAUDE.md documentation for agents module Documents the agents module architecture including: - Module components (coder, planner, session, memory_manager, base) - Single-agent architecture without external parallelism - Subagent architecture clarification * Revert "docs(agents): add CLAUDE.md documentation for agents module" This reverts commit bf1ddd7da08f2f34352d11a5d823da981f1a98bb. * chore: update gitignore to allow agents/tests/ * fix(tests): resolve mock isolation and path permission issues - Fix test_tool_concurrency_error_detection by patching where functions are used (qa.fixer) instead of where they're defined - Add Path.exists/is_dir/glob mocks to avoid permission errors on nonexistent directories in test_validation_strategy.py - Add helper function clean_project_index_files() to reduce code duplication in prereqs_validator tests - Add comprehensive tests for spec validation validators (context, prereqs, spec_document) - Fix similar mock/path issues in test_qa_reviewer.py, test_service_orchestrator.py, test_ci_discovery.py, test_prompt_generator.py, test_security_scanner.py All 2103 tests now pass. * fix(tests): remove unused imports and fix double assignment - Remove unused 'patch' import from validator test files - Remove unused 'pytest' import where not needed - Fix double assignment typo in test_error_message_includes_filename * fix(tests): move agents tests to tests/agents/ directory - Move test_agent_architecture.py, test_agent_configs.py, and test_agent_flow.py from apps/backend/agents/tests/ to tests/agents/ - Fix path resolution to work from new location - Remove gitignore exception for agents/tests/ (no longer needed) This resolves the issue where tests were not included in the PR because they were in an untracked location. * fix(tests): simplify conftest.py mock management - Remove redundant pytest_runtest_teardown and pytest_runtest_call hooks (autouse fixtures in test files already handle mock reset) - Add prompts_pkg.project_context to potentially mocked modules list - Remove prompts_pkg from test_qa_fixer entry (not used there) This reduces maintenance burden by having mock reset in one place. * refactor(tests): consolidate duplicate mock setup into shared helper - Create tests/qa_test_helpers.py with shared mock infrastructure: - AsyncIteratorMock and ReceiveResponseMock classes - setup_qa_mocks(), cleanup_qa_mocks(), reset_qa_mocks() functions - Mock response creation helpers - Accessor functions for mock objects - Refactor test_qa_fixer.py to use shared helpers - Reduces ~80 lines of duplicated code per test file - Fixes potential mock binding issues by using accessor functions This addresses code quality issues identified in PR review: - Duplicate mock setup between test_qa_fixer.py and test_qa_reviewer.py - Duplicated _AsyncIteratorMock class across files * refactor(tests): consolidate test_qa_reviewer.py with shared helpers - Refactor test_qa_reviewer.py to use shared qa_test_helpers - Remove ~170 lines of duplicated mock setup and helper functions - Fix unused imports in test_qa_fixer.py (json, sys, MagicMock, etc.) - Fix rate limit error detection tests to patch where functions are used - Consolidate duplicated _create_*_response helper methods to module level Addresses CodeQL warnings about unused imports and reduces code duplication between test_qa_fixer.py and test_qa_reviewer.py. * fix(tests): remove unused Path import from test_qa_reviewer.py * fix(tests): address all PR review findings PR Review Fixes: - Remove unused create_mock_qa_approved_response/rejected_response functions - Guard against overwriting _original_modules on second setup_qa_mocks() call - Clear _original_modules in cleanup_qa_mocks() to prevent stale state - Add prompts_pkg.project_context to test_qa_reviewer preserved_mocks in conftest - Convert asyncio.run() pattern to native async tests in test_agent_flow.py - Remove redundant @pytest.mark.asyncio decorators (asyncio_mode=auto) - Remove unused pytest import from qa_test_helpers.py - Fix structural duplication by keeping fixtures in test files Code Quality: - Removed ~100 lines of duplicated/unused code - Consistent async test patterns across all QA test files - Proper mock state management to prevent test pollution * fix(tests): save original modules individually in setup_qa_mocks The boolean guard `setup_done` prevented saving original modules on subsequent calls with different parameters. When setup_qa_mocks was called first with include_prompts_pkg=False, then with True, the prompts_pkg modules were never saved to _original_modules. During cleanup, these unsaved modules were deleted from sys.modules instead of being restored, causing ModuleNotFoundError in subsequent tests. Now checks each module individually before mocking, ensuring all originals are saved across multiple setup calls. * fix(tests): address all PR review findings including low priority - Fix path in test_no_subtask_worker_config (parent.parent.parent) - Add guard to prevent double setup in setup_qa_mocks() - Don't clear _original_modules in cleanup to fix multi-module cleanup * fix(tests): address PR review follow-up findings - Fix module-level mock setup ordering dependency: now tracks include_prompts_pkg config and allows incremental setup when test_qa_fixer.py (False) is imported before test_qa_reviewer.py (True) - Remove unused asyncio import from test_agent_flow.py - Replace os.chdir() with monkeypatch.chdir() in prereqs validator tests for safe parallel test execution --------- Co-authored-by: StillKnotKnown <stillknotknown@users.noreply.github.com>
255 lines
11 KiB
Python
255 lines
11 KiB
Python
"""
|
|
Tests for prompt_generator module functions.
|
|
|
|
Tests for worktree detection and environment context generation.
|
|
"""
|
|
|
|
from pathlib import Path
|
|
|
|
# Note: sys.path manipulation is handled by conftest.py line 46
|
|
from prompts_pkg.prompt_generator import (
|
|
detect_worktree_isolation,
|
|
generate_environment_context,
|
|
)
|
|
|
|
|
|
def normalize_path(path_str: str) -> str:
|
|
"""Normalize path string for cross-platform comparison."""
|
|
# Convert to lowercase and replace backslashes with forward slashes
|
|
return path_str.lower().replace("\\", "/")
|
|
|
|
|
|
class TestDetectWorktreeIsolation:
|
|
"""Tests for detect_worktree_isolation function."""
|
|
|
|
def test_new_worktree_unix_path(self):
|
|
"""Test detection of new worktree location on Unix-style path."""
|
|
# New worktree: /project/.auto-claude/worktrees/tasks/spec-name/
|
|
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature")
|
|
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is True
|
|
assert forbidden is not None
|
|
# On Windows, paths get resolved with drive letter, so check for key parts
|
|
norm_forbidden = normalize_path(str(forbidden))
|
|
assert "opt/dev/project" in norm_forbidden
|
|
assert ".auto-claude" not in norm_forbidden
|
|
|
|
def test_new_worktree_windows_path(self):
|
|
"""Test detection of new worktree location on Windows."""
|
|
# Windows path with backslashes
|
|
project_dir = Path("E:/projects/x/.auto-claude/worktrees/tasks/009-audit")
|
|
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is True
|
|
assert forbidden is not None
|
|
# Check the essential parts
|
|
norm_forbidden = normalize_path(str(forbidden))
|
|
assert "projects" in norm_forbidden and "x" in norm_forbidden
|
|
assert ".auto-claude" not in norm_forbidden
|
|
|
|
def test_legacy_worktree_unix_path(self):
|
|
"""Test detection of legacy worktree location on Unix-style path."""
|
|
# Legacy worktree: /project/.worktrees/spec-name/
|
|
project_dir = Path("/opt/dev/project/.worktrees/001-feature")
|
|
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is True
|
|
assert forbidden is not None
|
|
# Check for key parts
|
|
norm_forbidden = normalize_path(str(forbidden))
|
|
assert "opt/dev/project" in norm_forbidden
|
|
assert ".worktrees" not in norm_forbidden
|
|
|
|
def test_legacy_worktree_windows_path(self):
|
|
"""Test detection of legacy worktree location on Windows."""
|
|
from unittest.mock import patch
|
|
|
|
project_dir = Path("C:/projects/x/.worktrees/009-audit")
|
|
|
|
# Mock resolve() to return a fixed path on Windows-style paths
|
|
# since resolve() on Linux would prepend current working directory
|
|
with patch.object(Path, 'resolve', return_value=Path("C:/projects/x/.worktrees/009-audit")):
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is True
|
|
assert forbidden is not None
|
|
# Check the essential parts
|
|
norm_forbidden = normalize_path(str(forbidden))
|
|
assert "projects" in norm_forbidden
|
|
assert ".worktrees" not in norm_forbidden
|
|
|
|
def test_pr_worktree_unix_path(self):
|
|
"""Test detection of PR review worktree location on Unix-style path."""
|
|
# PR worktree: /project/.auto-claude/github/pr/worktrees/123/
|
|
project_dir = Path("/opt/dev/project/.auto-claude/github/pr/worktrees/123")
|
|
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is True
|
|
assert forbidden is not None
|
|
# Check for key parts
|
|
norm_forbidden = normalize_path(str(forbidden))
|
|
assert "opt/dev/project" in norm_forbidden
|
|
assert ".auto-claude" not in norm_forbidden
|
|
|
|
def test_pr_worktree_windows_path(self):
|
|
"""Test detection of PR review worktree location on Windows."""
|
|
project_dir = Path("E:/projects/auto-claude/.auto-claude/github/pr/worktrees/1528")
|
|
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is True
|
|
assert forbidden is not None
|
|
# The forbidden path should be E:/projects/auto-claude (the project folder)
|
|
# Note: project folder itself is named "auto-claude", so check for that
|
|
norm_forbidden = normalize_path(str(forbidden))
|
|
assert "projects/auto-claude" in norm_forbidden # project folder name
|
|
assert "github/pr/worktrees" not in norm_forbidden
|
|
|
|
def test_not_in_worktree(self):
|
|
"""Test when not in a worktree (direct mode)."""
|
|
# Direct mode: /project/
|
|
project_dir = Path("/opt/dev/project")
|
|
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is False
|
|
assert forbidden is None
|
|
|
|
def test_deeply_nested_worktree(self):
|
|
"""Test worktree detection with deeply nested project directory."""
|
|
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/009-very-long-spec-name-for-testing")
|
|
|
|
is_worktree, forbidden = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is True
|
|
assert forbidden is not None
|
|
# Check for key parts
|
|
norm_forbidden = normalize_path(str(forbidden))
|
|
assert "opt/dev/project" in norm_forbidden
|
|
assert ".auto-claude" not in norm_forbidden
|
|
|
|
def test_regular_auto_claude_dir(self):
|
|
"""Test that regular .auto-claude dir is NOT detected as worktree."""
|
|
# Just having .auto-claude in path doesn't make it a worktree
|
|
project_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature")
|
|
|
|
is_worktree, parent_path = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is False
|
|
assert parent_path is None
|
|
|
|
def test_empty_or_root_path(self):
|
|
"""Test edge case with minimal paths."""
|
|
# Root path
|
|
project_dir = Path("/")
|
|
|
|
is_worktree, parent_path = detect_worktree_isolation(project_dir)
|
|
|
|
assert is_worktree is False
|
|
assert parent_path is None
|
|
|
|
|
|
class TestGenerateEnvironmentContext:
|
|
"""Tests for generate_environment_context function."""
|
|
|
|
def test_context_includes_worktree_warning(self):
|
|
"""Test that worktree isolation warning is included when in worktree."""
|
|
spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature")
|
|
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature")
|
|
|
|
context = generate_environment_context(project_dir, spec_dir)
|
|
|
|
# Verify worktree warning is present
|
|
assert "ISOLATED WORKTREE - CRITICAL" in context
|
|
assert "FORBIDDEN PATH:" in context
|
|
# Check that some form of the parent path is shown
|
|
assert "opt" in context.lower() and "project" in context.lower()
|
|
|
|
def test_context_no_worktree_warning_in_direct_mode(self):
|
|
"""Test that worktree warning is NOT included in direct mode."""
|
|
spec_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature")
|
|
project_dir = Path("/opt/dev/project")
|
|
|
|
context = generate_environment_context(project_dir, spec_dir)
|
|
|
|
# Verify worktree warning is NOT present
|
|
assert "ISOLATED WORKTREE - CRITICAL" not in context
|
|
assert "FORBIDDEN PATH:" not in context
|
|
|
|
def test_context_includes_basic_environment(self):
|
|
"""Test that basic environment information is always included."""
|
|
spec_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature")
|
|
project_dir = Path("/opt/dev/project")
|
|
|
|
context = generate_environment_context(project_dir, spec_dir)
|
|
|
|
# Verify basic sections
|
|
assert "## YOUR ENVIRONMENT" in context
|
|
assert "**Working Directory:**" in context
|
|
assert "**Spec Location:**" in context
|
|
assert "implementation_plan.json" in context
|
|
|
|
def test_context_windows_worktree(self):
|
|
"""Test worktree warning with Windows paths (from ticket ACS-394)."""
|
|
# This is the exact scenario from the bug report
|
|
spec_dir = Path(
|
|
"E:/projects/x/.auto-claude/worktrees/tasks/009-audit"
|
|
"/.auto-claude/specs/009-audit"
|
|
)
|
|
project_dir = Path(
|
|
"E:/projects/x/.auto-claude/worktrees/tasks/009-audit"
|
|
)
|
|
|
|
context = generate_environment_context(project_dir, spec_dir)
|
|
|
|
# Verify worktree warning includes the Windows path
|
|
# Note: Path resolution on Windows converts forward slashes to backslashes
|
|
assert "ISOLATED WORKTREE - CRITICAL" in context
|
|
# The forbidden path should be the parent project
|
|
assert "FORBIDDEN PATH:" in context
|
|
|
|
def test_context_forbidden_path_examples(self):
|
|
"""Test that forbidden path is shown and rules are included."""
|
|
spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature")
|
|
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature")
|
|
|
|
context = generate_environment_context(project_dir, spec_dir)
|
|
|
|
# Verify forbidden parent path is shown
|
|
assert "FORBIDDEN PATH:" in context
|
|
# Check that some form of the parent path is shown (cross-platform)
|
|
assert "opt" in context.lower() and "project" in context.lower()
|
|
|
|
# Verify Rules section exists
|
|
assert "### Rules:" in context
|
|
assert "**NEVER**" in context # Explicit prohibition
|
|
|
|
# Verify Why This Matters section explains consequences
|
|
assert "### Why This Matters:" in context
|
|
assert "Git commits made in the parent project go to the WRONG branch" in context
|
|
|
|
def test_context_includes_isolation_mode_indicator(self):
|
|
"""Test that Isolation Mode indicator is shown when in worktree."""
|
|
spec_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature/.auto-claude/specs/001-feature")
|
|
project_dir = Path("/opt/dev/project/.auto-claude/worktrees/tasks/001-feature")
|
|
|
|
context = generate_environment_context(project_dir, spec_dir)
|
|
|
|
# Verify Isolation Mode indicator is present
|
|
assert "**Isolation Mode:** WORKTREE" in context
|
|
|
|
def test_context_no_isolation_mode_in_direct_mode(self):
|
|
"""Test that Isolation Mode indicator is NOT shown in direct mode."""
|
|
spec_dir = Path("/opt/dev/project/.auto-claude/specs/001-feature")
|
|
project_dir = Path("/opt/dev/project")
|
|
|
|
context = generate_environment_context(project_dir, spec_dir)
|
|
|
|
# Verify Isolation Mode is not present
|
|
assert "**Isolation Mode:**" not in context
|