diff --git a/tests/test_agent_architecture.py b/tests/agents/test_agent_architecture.py similarity index 91% rename from tests/test_agent_architecture.py rename to tests/agents/test_agent_architecture.py index 6c1bacca..ca1e380c 100644 --- a/tests/test_agent_architecture.py +++ b/tests/agents/test_agent_architecture.py @@ -22,7 +22,7 @@ from pathlib import Path import pytest # Add apps/backend directory to path for imports -sys.path.insert(0, str(Path(__file__).parent.parent / "apps" / "backend")) +sys.path.insert(0, str(Path(__file__).parent.parent.parent / "apps" / "backend")) class TestNoExternalParallelism: @@ -31,7 +31,7 @@ class TestNoExternalParallelism: def test_no_coordinator_module(self): """No external coordinator module should exist.""" coordinator_path = ( - Path(__file__).parent.parent / "apps" / "backend" / "coordinator.py" + Path(__file__).parent.parent.parent / "apps" / "backend" / "coordinator.py" ) assert not coordinator_path.exists(), ( "coordinator.py should not exist. Parallel orchestration is handled " @@ -41,7 +41,7 @@ class TestNoExternalParallelism: def test_no_task_tool_module(self): """No task_tool wrapper module should exist.""" task_tool_path = ( - Path(__file__).parent.parent / "apps" / "backend" / "task_tool.py" + Path(__file__).parent.parent.parent / "apps" / "backend" / "task_tool.py" ) assert not task_tool_path.exists(), ( "task_tool.py should not exist. The agent spawns subagents directly " @@ -51,7 +51,7 @@ class TestNoExternalParallelism: def test_no_subtask_worker_config(self): """No external subtask worker agent config should exist.""" worker_config = ( - Path(__file__).parent.parent / ".claude" / "agents" / "subtask-worker.md" + Path(__file__).parent.parent.parent / ".claude" / "agents" / "subtask-worker.md" ) assert not worker_config.exists(), ( "subtask-worker.md should not exist. Subagents use Claude Code's " @@ -64,7 +64,7 @@ class TestCLIInterface: def test_no_parallel_flag(self): """CLI should not have --parallel argument.""" - run_py_path = Path(__file__).parent.parent / "apps" / "backend" / "run.py" + run_py_path = Path(__file__).parent.parent.parent / "apps" / "backend" / "run.py" content = run_py_path.read_text(encoding="utf-8") # Check that --parallel is not defined as an argument @@ -79,7 +79,7 @@ class TestCLIInterface: def test_no_parallel_examples_in_docs(self): """CLI documentation should not mention parallel mode.""" - run_py_path = Path(__file__).parent.parent / "apps" / "backend" / "run.py" + run_py_path = Path(__file__).parent.parent.parent / "apps" / "backend" / "run.py" content = run_py_path.read_text(encoding="utf-8") # The docstring should not have --parallel examples @@ -132,7 +132,7 @@ class TestAgentPrompt: def test_mentions_subagents(self): """Agent prompt mentions subagent capability.""" coder_prompt_path = ( - Path(__file__).parent.parent / "apps" / "backend" / "prompts" / "coder.md" + Path(__file__).parent.parent.parent / "apps" / "backend" / "prompts" / "coder.md" ) content = coder_prompt_path.read_text(encoding="utf-8") @@ -143,7 +143,7 @@ class TestAgentPrompt: def test_mentions_parallel_capability(self): """Agent prompt mentions parallel/concurrent capability.""" coder_prompt_path = ( - Path(__file__).parent.parent / "apps" / "backend" / "prompts" / "coder.md" + Path(__file__).parent.parent.parent / "apps" / "backend" / "prompts" / "coder.md" ) content = coder_prompt_path.read_text(encoding="utf-8") @@ -170,7 +170,7 @@ class TestModuleIntegrity: def test_run_module_valid_syntax(self): """Run module has valid Python syntax.""" - run_py_path = Path(__file__).parent.parent / "apps" / "backend" / "run.py" + run_py_path = Path(__file__).parent.parent.parent / "apps" / "backend" / "run.py" content = run_py_path.read_text(encoding="utf-8") try: @@ -181,7 +181,7 @@ class TestModuleIntegrity: def test_no_coordinator_imports(self): """Core modules don't import coordinator.""" for filename in ["run.py", "core/agent.py"]: - filepath = Path(__file__).parent.parent / "apps" / "backend" / filename + filepath = Path(__file__).parent.parent.parent / "apps" / "backend" / filename content = filepath.read_text(encoding="utf-8") assert "from coordinator import" not in content, ( @@ -194,7 +194,7 @@ class TestModuleIntegrity: def test_no_task_tool_imports(self): """Core modules don't import task_tool.""" for filename in ["run.py", "core/agent.py"]: - filepath = Path(__file__).parent.parent / "apps" / "backend" / filename + filepath = Path(__file__).parent.parent.parent / "apps" / "backend" / filename content = filepath.read_text(encoding="utf-8") assert "from task_tool import" not in content, ( @@ -210,7 +210,7 @@ class TestProjectDocumentation: def test_no_parallel_cli_documented(self): """CLAUDE.md doesn't document --parallel flag.""" - claude_md_path = Path(__file__).parent.parent / "CLAUDE.md" + claude_md_path = Path(__file__).parent.parent.parent / "CLAUDE.md" content = claude_md_path.read_text(encoding="utf-8") assert "--parallel 2" not in content, ( @@ -219,7 +219,7 @@ class TestProjectDocumentation: def test_subagent_architecture_documented(self): """CLAUDE.md documents subagent-based architecture.""" - claude_md_path = Path(__file__).parent.parent / "CLAUDE.md" + claude_md_path = Path(__file__).parent.parent.parent / "CLAUDE.md" content = claude_md_path.read_text(encoding="utf-8") has_subagent = "subagent" in content.lower() @@ -334,7 +334,7 @@ class TestSubtaskTerminology: def test_progress_uses_subtask_terminology(self): """Progress module uses subtask terminology.""" progress_path = ( - Path(__file__).parent.parent / "apps" / "backend" / "core" / "progress.py" + Path(__file__).parent.parent.parent / "apps" / "backend" / "core" / "progress.py" ) content = progress_path.read_text(encoding="utf-8") diff --git a/tests/test_agent_configs.py b/tests/agents/test_agent_configs.py similarity index 99% rename from tests/test_agent_configs.py rename to tests/agents/test_agent_configs.py index 8e9e2832..761e1c4a 100644 --- a/tests/test_agent_configs.py +++ b/tests/agents/test_agent_configs.py @@ -14,7 +14,7 @@ import sys from pathlib import Path # Add backend to path -backend_path = Path(__file__).parent.parent / "apps" / "backend" +backend_path = Path(__file__).parent.parent.parent / "apps" / "backend" sys.path.insert(0, str(backend_path)) diff --git a/tests/test_agent_flow.py b/tests/agents/test_agent_flow.py similarity index 97% rename from tests/test_agent_flow.py rename to tests/agents/test_agent_flow.py index a43a4463..cb7973fb 100644 --- a/tests/test_agent_flow.py +++ b/tests/agents/test_agent_flow.py @@ -12,7 +12,6 @@ Tests for planner→coder→QA state transitions including: Note: Uses temp_git_repo fixture from conftest.py for proper git isolation. """ -import asyncio import json import subprocess import sys @@ -22,7 +21,7 @@ from unittest.mock import AsyncMock, patch import pytest # Add parent directory to path for imports -sys.path.insert(0, str(Path(__file__).parent.parent / "apps" / "backend")) +sys.path.insert(0, str(Path(__file__).parent.parent.parent / "apps" / "backend")) # ============================================================================= @@ -190,7 +189,7 @@ class TestPlannerToCoderTransition: class TestPostSessionProcessing: """Tests for post_session_processing function.""" - def test_completed_subtask_records_success(self, test_env): + async def test_completed_subtask_records_success(self, test_env): """Test that completed subtask is recorded as successful.""" from recovery import RecoveryManager from agents.session import post_session_processing @@ -212,20 +211,16 @@ class TestPostSessionProcessing: mock_insights.return_value = {"file_insights": [], "patterns_discovered": []} mock_memory.return_value = (True, "file") - # Run async function using asyncio.run() - async def run_test(): - return await post_session_processing( - spec_dir=spec_dir, - project_dir=project_dir, - subtask_id="subtask-1", - session_num=1, - commit_before=commit_before, - commit_count_before=1, - recovery_manager=recovery_manager, - linear_enabled=False, - ) - - result = asyncio.run(run_test()) + result = await post_session_processing( + spec_dir=spec_dir, + project_dir=project_dir, + subtask_id="subtask-1", + session_num=1, + commit_before=commit_before, + commit_count_before=1, + recovery_manager=recovery_manager, + linear_enabled=False, + ) assert result is True, "Completed subtask should return True" @@ -235,7 +230,7 @@ class TestPostSessionProcessing: assert history["attempts"][0]["success"] is True, "Attempt should be successful" assert history["status"] == "completed", "Status should be completed" - def test_in_progress_subtask_records_failure(self, test_env): + async def test_in_progress_subtask_records_failure(self, test_env): """Test that in_progress subtask is recorded as incomplete.""" from recovery import RecoveryManager from agents.session import post_session_processing @@ -258,20 +253,16 @@ class TestPostSessionProcessing: mock_insights.return_value = {"file_insights": [], "patterns_discovered": []} mock_memory.return_value = (True, "file") - # Run async function using asyncio.run() - async def run_test(): - return await post_session_processing( - spec_dir=spec_dir, - project_dir=project_dir, - subtask_id="subtask-1", - session_num=1, - commit_before=commit_before, - commit_count_before=1, - recovery_manager=recovery_manager, - linear_enabled=False, - ) - - result = asyncio.run(run_test()) + result = await post_session_processing( + spec_dir=spec_dir, + project_dir=project_dir, + subtask_id="subtask-1", + session_num=1, + commit_before=commit_before, + commit_count_before=1, + recovery_manager=recovery_manager, + linear_enabled=False, + ) assert result is False, "In-progress subtask should return False" @@ -280,7 +271,7 @@ class TestPostSessionProcessing: assert len(history["attempts"]) == 1, "Should have 1 attempt" assert history["attempts"][0]["success"] is False, "Attempt should be unsuccessful" - def test_pending_subtask_records_failure(self, test_env): + async def test_pending_subtask_records_failure(self, test_env): """Test that pending (no progress) subtask is recorded as failure.""" from recovery import RecoveryManager from agents.session import post_session_processing @@ -301,20 +292,16 @@ class TestPostSessionProcessing: mock_insights.return_value = {"file_insights": [], "patterns_discovered": []} mock_memory.return_value = (True, "file") - # Run async function using asyncio.run() - async def run_test(): - return await post_session_processing( - spec_dir=spec_dir, - project_dir=project_dir, - subtask_id="subtask-1", - session_num=1, - commit_before=commit_before, - commit_count_before=1, - recovery_manager=recovery_manager, - linear_enabled=False, - ) - - result = asyncio.run(run_test()) + result = await post_session_processing( + spec_dir=spec_dir, + project_dir=project_dir, + subtask_id="subtask-1", + session_num=1, + commit_before=commit_before, + commit_count_before=1, + recovery_manager=recovery_manager, + linear_enabled=False, + ) assert result is False, "Pending subtask should return False" diff --git a/tests/conftest.py b/tests/conftest.py index 58e98fd7..af3c7cfa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -66,6 +66,13 @@ _POTENTIALLY_MOCKED_MODULES = [ 'review', 'validate_spec', 'graphiti_providers', + 'agents.memory_manager', + 'agents.base', + 'core.error_utils', + 'security.tool_input_validator', + 'debug', + 'prompts_pkg', + 'prompts_pkg.project_context', ] # Store original module references at import time (before any mocking) @@ -113,6 +120,8 @@ def pytest_runtest_setup(item): 'test_spec_pipeline': {'claude_code_sdk', 'claude_code_sdk.types', 'init', 'client', 'review', 'task_logger', 'ui', 'validate_spec'}, 'test_spec_complexity': {'claude_code_sdk', 'claude_code_sdk.types', 'claude_agent_sdk', 'claude_agent_sdk.types'}, 'test_spec_phases': {'claude_code_sdk', 'claude_code_sdk.types', 'claude_agent_sdk', 'graphiti_providers', 'validate_spec', 'client'}, + 'test_qa_fixer': {'claude_agent_sdk', 'ui', 'progress', 'task_logger', 'linear_updater', 'client', 'agents.memory_manager', 'agents.base', 'core.error_utils', 'security.tool_input_validator', 'debug'}, + 'test_qa_reviewer': {'claude_agent_sdk', 'ui', 'progress', 'task_logger', 'linear_updater', 'client', 'agents.memory_manager', 'agents.base', 'core.error_utils', 'security.tool_input_validator', 'debug', 'prompts_pkg', 'prompts_pkg.project_context'}, } # Get the mocks that the current test module needs to preserve @@ -157,8 +166,6 @@ def pytest_runtest_setup(item): pass - - # ============================================================================= # DIRECTORY FIXTURES # ============================================================================= diff --git a/tests/qa_test_helpers.py b/tests/qa_test_helpers.py new file mode 100644 index 00000000..0d54cafc --- /dev/null +++ b/tests/qa_test_helpers.py @@ -0,0 +1,376 @@ +#!/usr/bin/env python3 +""" +Shared QA Test Helpers +====================== + +Consolidates duplicated mock setup and utilities for test_qa_fixer.py and test_qa_reviewer.py. + +This module provides: +- AsyncIteratorMock: Async iterator mock for receive_response +- ReceiveResponseMock: Smart wrapper supporting both .set_messages() and .return_value +- setup_qa_mocks(): Module-level mock setup +- cleanup_qa_mocks(): Module-level cleanup +- reset_qa_mocks(): Reset shared mocks to default state +- get_mock_*(): Accessor functions for mock objects +- Mock response creation helpers +- Shared pytest fixtures +""" + +import sys +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock + +# Add apps/backend to path for imports +sys.path.insert(0, str(Path(__file__).parent.parent / "apps" / "backend")) + + +# ============================================================================= +# ASYNC ITERATOR MOCKS +# ============================================================================= + +class AsyncIteratorMock: + """Async iterator mock that yields stored messages and acts as async context manager.""" + + def __init__(self): + self._messages = [] + self._index = 0 + + def __aiter__(self): + return self + + async def __anext__(self): + if self._index >= len(self._messages): + raise StopAsyncIteration + msg = self._messages[self._index] + self._index += 1 + return msg + + async def __aenter__(self): + return self + + async def __aexit__(self, exc_type, exc_val, exc_tb): + return False + + def set_messages(self, messages): + self._messages = messages + self._index = 0 + + +class ReceiveResponseMock: + """Mock for receive_response that supports both .set_messages() and .return_value assignment.""" + + def __init__(self): + self._iterator = AsyncIteratorMock() + self.called = False # MagicMock compatibility + + def __call__(self, *args, **kwargs): + self.called = True + return self._iterator + + @property + def return_value(self): + return self._iterator + + @return_value.setter + def return_value(self, value): + # When tests do mock_client.receive_response.return_value = list, + # we set the messages on the iterator + self._iterator.set_messages(value) + + +# ============================================================================= +# MODULE-LEVEL MOCKS +# ============================================================================= + +# Store original modules for cleanup +_original_modules = {} +_mocked_module_names = [ + 'claude_agent_sdk', + 'ui', + 'progress', + 'task_logger', + 'linear_updater', + 'client', + 'prompts_pkg', + 'prompts_pkg.project_context', + 'agents.memory_manager', + 'agents.base', + 'core.error_utils', + 'security.tool_input_validator', + 'debug', +] + +# Mock objects (initialized by setup_qa_mocks) +_mock_state = { + 'sdk': None, + 'prompts_pkg': None, + 'project_context': None, + 'memory_manager': None, + 'agents_base': None, + 'error_utils': None, + 'validator': None, + 'debug': None, + 'ui': None, + 'progress': None, + 'task_logger': None, + 'linear': None, + 'client_module': None, + 'setup_done': False, + 'include_prompts_pkg': False, # Track what config was used +} + + +def get_mock_error_utils(): + """Get the mock_error_utils object after setup.""" + return _mock_state['error_utils'] + + +def get_mock_memory_manager(): + """Get the mock_memory_manager object after setup.""" + return _mock_state['memory_manager'] + + +def setup_qa_mocks(include_prompts_pkg: bool = False): + """Set up module-level mocks for QA tests. + + Args: + include_prompts_pkg: If True, mock prompts_pkg (needed for reviewer, not fixer) + + Call this at module level before importing from qa modules. + """ + # Guard against redundant setup when called with same parameters + # But allow prompts_pkg to be added if a later call needs it + if _mock_state['setup_done']: + # If prompts_pkg is already set up OR current call doesn't need it, skip + if _mock_state['include_prompts_pkg'] or not include_prompts_pkg: + return + # Otherwise, we need to add prompts_pkg to existing setup + # Fall through to only set up prompts_pkg below + + # If setup is done but we need to add prompts_pkg, only do that part + if _mock_state['setup_done'] and include_prompts_pkg and not _mock_state['include_prompts_pkg']: + # Save originals before mocking + for name in ['prompts_pkg', 'prompts_pkg.project_context']: + if name in sys.modules and name not in _original_modules: + _original_modules[name] = sys.modules[name] + + # Only set up prompts_pkg + mock_prompts_pkg = MagicMock() + mock_prompts_pkg.get_qa_reviewer_prompt = MagicMock(return_value="Test QA prompt") + sys.modules['prompts_pkg'] = mock_prompts_pkg + _mock_state['prompts_pkg'] = mock_prompts_pkg + mock_project_context = MagicMock() + mock_prompts_pkg.project_context = mock_project_context + sys.modules['prompts_pkg.project_context'] = mock_project_context + _mock_state['project_context'] = mock_project_context + _mock_state['include_prompts_pkg'] = True + return + + # Save originals for each module individually before mocking + # This handles multiple setup calls with different parameters + for name in _mocked_module_names: + if name in sys.modules and name not in _original_modules: + _original_modules[name] = sys.modules[name] + + # Mock claude_agent_sdk FIRST + mock_sdk = MagicMock() + mock_sdk.ClaudeSDKClient = MagicMock() + mock_sdk.ClaudeAgentOptions = MagicMock() + mock_sdk.ClaudeCodeOptions = MagicMock() + sys.modules['claude_agent_sdk'] = mock_sdk + _mock_state['sdk'] = mock_sdk + + # Mock prompts_pkg if needed + if include_prompts_pkg: + mock_prompts_pkg = MagicMock() + mock_prompts_pkg.get_qa_reviewer_prompt = MagicMock(return_value="Test QA prompt") + sys.modules['prompts_pkg'] = mock_prompts_pkg + _mock_state['prompts_pkg'] = mock_prompts_pkg + # Also mock prompts_pkg.project_context for imports in core/client.py + mock_project_context = MagicMock() + mock_prompts_pkg.project_context = mock_project_context + sys.modules['prompts_pkg.project_context'] = mock_project_context + _mock_state['project_context'] = mock_project_context + + # Mock agents.memory_manager + mock_memory_manager = MagicMock() + mock_memory_manager.get_graphiti_context = AsyncMock(return_value=None) + mock_memory_manager.save_session_memory = AsyncMock(return_value=None) + sys.modules['agents.memory_manager'] = mock_memory_manager + _mock_state['memory_manager'] = mock_memory_manager + + # Mock agents.base + mock_agents_base = MagicMock() + mock_agents_base.sanitize_error_message = lambda x: x + sys.modules['agents.base'] = mock_agents_base + _mock_state['agents_base'] = mock_agents_base + + # Mock core.error_utils + mock_error_utils = MagicMock() + mock_error_utils.is_rate_limit_error = MagicMock(return_value=False) + mock_error_utils.is_tool_concurrency_error = MagicMock(return_value=False) + sys.modules['core.error_utils'] = mock_error_utils + _mock_state['error_utils'] = mock_error_utils + + # Mock security.tool_input_validator + mock_validator = MagicMock() + mock_validator.get_safe_tool_input = lambda block: getattr(block, 'input', {}) + sys.modules['security.tool_input_validator'] = mock_validator + _mock_state['validator'] = mock_validator + + # Mock debug + mock_debug = MagicMock() + sys.modules['debug'] = mock_debug + _mock_state['debug'] = mock_debug + + # Mock UI module + mock_ui = MagicMock() + sys.modules['ui'] = mock_ui + _mock_state['ui'] = mock_ui + + # Mock progress module + mock_progress = MagicMock() + sys.modules['progress'] = mock_progress + _mock_state['progress'] = mock_progress + + # Mock task_logger + mock_task_logger = MagicMock() + mock_task_logger.LogPhase = MagicMock() + mock_task_logger.LogEntryType = MagicMock() + mock_task_logger.get_task_logger = MagicMock(return_value=None) + sys.modules['task_logger'] = mock_task_logger + _mock_state['task_logger'] = mock_task_logger + + # Mock linear_updater + mock_linear = MagicMock() + sys.modules['linear_updater'] = mock_linear + _mock_state['linear'] = mock_linear + + # Mock client - create a factory that returns properly configured clients + def _create_mock_client(): + """Factory function that creates a properly configured mock client.""" + client = MagicMock() + client.query = AsyncMock() + client.receive_response = ReceiveResponseMock() + return client + + mock_client_module = MagicMock() + mock_client_module.create_client = _create_mock_client + sys.modules['client'] = mock_client_module + _mock_state['client_module'] = mock_client_module + _mock_state['setup_done'] = True + _mock_state['include_prompts_pkg'] = include_prompts_pkg + + +def cleanup_qa_mocks(): + """Restore original modules after tests complete. + + Call this in a module-scoped autouse fixture. + """ + for name in _mocked_module_names: + if name in _original_modules: + sys.modules[name] = _original_modules[name] + elif name in sys.modules: + del sys.modules[name] + _mock_state['setup_done'] = False + _mock_state['include_prompts_pkg'] = False + # Note: We do NOT clear _original_modules here because: + # 1. Multiple test modules may call cleanup, and clearing would break subsequent cleanups + # 2. The 'if name not in _original_modules' guard in setup_qa_mocks prevents stale state + # 3. Originals are saved per-module, so different setups can coexist + + +def reset_qa_mocks(): + """Reset shared mocks to default state. + + Call this before and after each test to ensure isolation. + """ + mock_error_utils = _mock_state.get('error_utils') + mock_memory_manager = _mock_state.get('memory_manager') + + if mock_error_utils is not None: + mock_error_utils.is_rate_limit_error.return_value = False + mock_error_utils.is_tool_concurrency_error.return_value = False + if mock_memory_manager is not None: + mock_memory_manager.get_graphiti_context.reset_mock() + mock_memory_manager.save_session_memory.reset_mock() + + +# ============================================================================= +# MOCK RESPONSE HELPERS +# ============================================================================= + +def create_mock_response(text: str = "Session complete."): + """Create a standard mock assistant+user message pair. + + Args: + text: Text content for the AssistantMessage's TextBlock + + Returns: + List of mock messages [AssistantMessage, UserMessage] + """ + msg1 = MagicMock() + msg1.__class__.__name__ = "AssistantMessage" + text_block = MagicMock() + text_block.__class__.__name__ = "TextBlock" + text_block.text = text + msg1.content = [text_block] + + msg2 = MagicMock() + msg2.__class__.__name__ = "UserMessage" + msg2.content = [] + + return [msg1, msg2] + + +def create_mock_fixed_response(): + """Create mock response for fixed QA. + + Returns: + List of mock messages [AssistantMessage with 'Fixes applied successfully.', UserMessage] + """ + return create_mock_response("Fixes applied successfully.") + + +def create_mock_tool_use_response(tool_name: str = "Bash", tool_input: dict = None): + """Create mock response with tool use. + + Args: + tool_name: Name of the tool being used + tool_input: Input dict for the tool + + Returns: + List of mock messages [AssistantMessage with ToolUseBlock, UserMessage] + """ + if tool_input is None: + tool_input = {"command": "echo test"} + + msg1 = MagicMock() + msg1.__class__.__name__ = "AssistantMessage" + tool_block = MagicMock() + tool_block.__class__.__name__ = "ToolUseBlock" + tool_block.name = tool_name + tool_block.input = tool_input + msg1.content = [tool_block] + + msg2 = MagicMock() + msg2.__class__.__name__ = "UserMessage" + msg2.content = [] + + return [msg1, msg2] + + +# ============================================================================= +# FIXTURE HELPERS +# ============================================================================= + +def create_mock_client(): + """Create a mock Claude SDK client for use in fixtures. + + Returns: + MagicMock configured as a Claude SDK client + """ + client = MagicMock() + client.query = AsyncMock() + client.receive_response = ReceiveResponseMock() + return client diff --git a/tests/test_ci_discovery.py b/tests/test_ci_discovery.py index 5acc2a7a..ee02f919 100644 --- a/tests/test_ci_discovery.py +++ b/tests/test_ci_discovery.py @@ -13,6 +13,7 @@ Tests cover: import json import tempfile from pathlib import Path +from unittest.mock import patch import pytest @@ -630,9 +631,10 @@ class TestEdgeCases: """Test handling of non-existent directory.""" fake_dir = Path("/nonexistent/path") - # Should not raise - result = discovery.discover(fake_dir) - assert result is None + # Should not raise - mock exists to avoid permission error + with patch.object(Path, 'exists', return_value=False): + result = discovery.discover(fake_dir) + assert result is None def test_ci_priority_github_first(self, discovery, temp_dir): """Test that GitHub Actions takes priority.""" diff --git a/tests/test_prompt_generator.py b/tests/test_prompt_generator.py index a0489846..1294c19a 100644 --- a/tests/test_prompt_generator.py +++ b/tests/test_prompt_generator.py @@ -66,16 +66,21 @@ class TestDetectWorktreeIsolation: 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") - is_worktree, forbidden = detect_worktree_isolation(project_dir) + # 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 + 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.""" diff --git a/tests/test_qa_fixer.py b/tests/test_qa_fixer.py new file mode 100644 index 00000000..39c08c0f --- /dev/null +++ b/tests/test_qa_fixer.py @@ -0,0 +1,497 @@ +#!/usr/bin/env python3 +""" +Tests for QA Fixer Agent Session +================================ + +Tests the qa/fixer.py module functionality including: +- load_qa_fixer_prompt function +- run_qa_fixer_session function +- QA fixer session execution flow +- Error handling and edge cases +- Memory integration hooks +""" + +import shutil +import tempfile +from pathlib import Path +from unittest.mock import AsyncMock, patch + +import pytest + +# ============================================================================= +# MOCK SETUP - Must happen before ANY imports from auto-claude +# ============================================================================= + +# Import shared mock helpers +from tests.qa_test_helpers import ( + setup_qa_mocks, + cleanup_qa_mocks, + reset_qa_mocks, + create_mock_response, + create_mock_fixed_response, + create_mock_tool_use_response, + create_mock_client, +) + +# Set up mocks (no prompts_pkg needed for fixer) +setup_qa_mocks(include_prompts_pkg=False) + +# Import after mocks are set up +from qa.fixer import load_qa_fixer_prompt, run_qa_fixer_session +from qa.criteria import save_implementation_plan + + +# ============================================================================= +# FIXTURES +# ============================================================================= + + +@pytest.fixture(scope="module", autouse=True) +def cleanup_mocked_modules(): + """Restore original modules after all tests in this module complete.""" + yield + cleanup_qa_mocks() + + +@pytest.fixture +def spec_dir(temp_dir): + """Create a spec directory with basic structure.""" + spec = temp_dir / "spec" + spec.mkdir() + return spec + + +@pytest.fixture +def project_dir(temp_dir): + """Create a project directory.""" + project = temp_dir / "project" + project.mkdir() + return project + + +@pytest.fixture +def mock_client(): + """Create a mock Claude SDK client.""" + return create_mock_client() + + +@pytest.fixture(autouse=True, scope='function') +def reset_shared_mocks_before_test(): + """Reset shared module-level mocks before and after each test.""" + reset_qa_mocks() + yield + reset_qa_mocks() + + +# ============================================================================= +# MOCK RESPONSE HELPERS (fixer-specific) +# ============================================================================= + +def _create_mock_response(text: str = "Fixer session complete."): + """Create a standard mock assistant+user message pair.""" + return create_mock_response(text) + + +def _create_mock_fixed_response(): + """Create mock response for fixed QA.""" + return create_mock_fixed_response() + + +def _create_mock_tool_use_response(): + """Create mock response with tool use blocks.""" + return create_mock_tool_use_response("Edit", {"file_path": "/test/file.py"}) + + +@pytest.fixture +def fix_request_file(spec_dir): + """Create a QA_FIX_REQUEST.md file.""" + fix_request = spec_dir / "QA_FIX_REQUEST.md" + fix_request.write_text("# Fix Request\n\nFix the following issues:\n- Issue 1\n- Issue 2") + return fix_request + + +# ============================================================================= +# TEST CLASSES +# ============================================================================= + + +class TestLoadQAFixerPrompt: + """Tests for load_qa_fixer_prompt function.""" + + def test_load_prompt_success(self, spec_dir, monkeypatch): + """Test successful prompt loading.""" + # Create prompts directory in temp location + prompts_dir = spec_dir / "prompts" + prompts_dir.mkdir(parents=True, exist_ok=True) + + prompt_file = prompts_dir / "qa_fixer.md" + prompt_content = "# QA Fixer Prompt\n\nFix the issues..." + prompt_file.write_text(prompt_content) + + # Patch QA_PROMPTS_DIR to point to temp directory + import qa.fixer as qa_fixer_module + monkeypatch.setattr(qa_fixer_module, "QA_PROMPTS_DIR", prompts_dir) + + result = load_qa_fixer_prompt() + + assert result == prompt_content + + def test_load_prompt_file_not_found(self, monkeypatch): + """Test FileNotFoundError when prompt file doesn't exist.""" + # Create an empty temp directory with no qa_fixer.md + empty_dir = Path(tempfile.mkdtemp()) + + try: + # Patch QA_PROMPTS_DIR to point to empty directory + import qa.fixer as qa_fixer_module + monkeypatch.setattr(qa_fixer_module, "QA_PROMPTS_DIR", empty_dir) + + with pytest.raises(FileNotFoundError): + load_qa_fixer_prompt() + finally: + # Clean up temp directory + shutil.rmtree(empty_dir) + + +class TestRunQAFixerSessionFixed: + """Tests for run_qa_fixer_session returning fixed status.""" + + async def test_fixed_status(self, mock_client, spec_dir, fix_request_file): + """Test that fixed status is returned when ready_for_qa_revalidation is True.""" + # Setup implementation plan with ready_for_qa_revalidation + plan = { + "feature": "Test", + "qa_signoff": { + "status": "fixes_applied", + "ready_for_qa_revalidation": True, + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_fixed_response()) + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + assert result[0] == "fixed" + assert len(result[1]) > 0 # Response text + assert result[2] == {} # No error info + + async def test_fixed_status_with_project_dir(self, mock_client, spec_dir, project_dir): + """Test session with explicit project_dir parameter.""" + # Create fix request file + fix_request = spec_dir / "QA_FIX_REQUEST.md" + fix_request.write_text("# Fix Request\n\nFix issues") + + # Setup implementation plan + plan = { + "feature": "Test", + "qa_signoff": { + "status": "fixes_applied", + "ready_for_qa_revalidation": True, + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_fixed_response()) + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False, + project_dir=project_dir + ) + + assert result[0] == "fixed" + + +class TestRunQAFixerSessionError: + """Tests for run_qa_fixer_session error handling.""" + + async def test_error_missing_fix_request(self, mock_client, spec_dir): + """Test error when QA_FIX_REQUEST.md is missing.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Don't create QA_FIX_REQUEST.md + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + assert result[0] == "error" + assert "not found" in result[1].lower() + assert result[2]["type"] == "other" + assert result[2]["exception_type"] == "FileNotFoundError" + + async def test_exception_handling(self, mock_client, spec_dir, fix_request_file): + """Test exception handling during fixer session.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client to raise exception + mock_client.query.side_effect = Exception("Test exception") + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + assert result[0] == "error" + assert "Test exception" in result[1] or "test exception" in result[1].lower() + assert result[2]["type"] == "other" + assert result[2]["exception_type"] == "Exception" + + +class TestRunQAFixerSessionParameters: + """Tests for run_qa_fixer_session parameter handling.""" + + async def test_verbose_mode(self, mock_client, spec_dir, fix_request_file): + """Test session with verbose mode enabled.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_response()) + + await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + verbose=True + ) + + # Verify query was called + assert mock_client.query.called + + async def test_fix_session_number(self, mock_client, spec_dir, fix_request_file): + """Test session with different fix_session numbers.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_response()) + + await run_qa_fixer_session( + mock_client, + spec_dir, + fix_session=3, + verbose=False + ) + + # Verify query was called + assert mock_client.query.called + + +class TestRunQAFixerSessionIntegration: + """Integration tests for QA fixer session.""" + + async def test_full_session_flow(self, mock_client, spec_dir, fix_request_file): + """Test complete session flow from start to finish.""" + # Setup implementation plan + plan = { + "feature": "Test Feature", + "qa_signoff": { + "status": "fixes_applied", + "ready_for_qa_revalidation": True, + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_response("Applying fixes...")) + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + fix_session=1, + verbose=False + ) + + assert result[0] == "fixed" + assert mock_client.query.called + assert mock_client.receive_response.called + + +class TestMemoryIntegration: + """Tests for memory integration in QA fixer.""" + + async def test_memory_context_retrieval(self, mock_client, spec_dir, fix_request_file): + """Test that memory context is retrieved during session.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_response()) + + # Patch where the function is used (in qa.fixer module) + with patch('qa.fixer.get_graphiti_context', new_callable=AsyncMock) as mock_get_context: + mock_get_context.return_value = "Past fix patterns: check imports" + + await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + # Verify memory context was retrieved + assert mock_get_context.called + + async def test_memory_save_on_fixed(self, mock_client, spec_dir, fix_request_file): + """Test that session memory is saved when fixes are applied.""" + # Setup implementation plan + plan = { + "feature": "Test", + "qa_signoff": { + "status": "fixes_applied", + "ready_for_qa_revalidation": True, + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_fixed_response()) + + # Patch where the function is used + with patch('qa.fixer.get_graphiti_context', new_callable=AsyncMock, return_value=None), \ + patch('qa.fixer.save_session_memory', new_callable=AsyncMock) as mock_save: + + await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + # Verify memory was saved + assert mock_save.called + + +class TestErrorDetection: + """Tests for error type detection in QA fixer.""" + + async def test_rate_limit_error_detection(self, mock_client, spec_dir, fix_request_file): + """Test that rate limit errors are properly detected.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client to raise exception + mock_client.query.side_effect = Exception("Rate limit exceeded") + + # Patch where the functions are used (qa.fixer) not where they're defined + with patch('qa.fixer.is_rate_limit_error', return_value=True), \ + patch('qa.fixer.is_tool_concurrency_error', return_value=False): + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + assert result[0] == "error" + assert result[2]["type"] == "rate_limit" + + async def test_tool_concurrency_error_detection(self, mock_client, spec_dir, fix_request_file): + """Test that tool concurrency errors are properly detected.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client to raise exception + mock_client.query.side_effect = Exception("Tool concurrency limit") + + # Patch where the functions are used (qa.fixer) not where they're defined + with patch('qa.fixer.is_tool_concurrency_error', return_value=True), \ + patch('qa.fixer.is_rate_limit_error', return_value=False), \ + patch('qa.fixer.get_graphiti_context', new_callable=AsyncMock, return_value=None): + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + assert result[0] == "error" + assert result[2]["type"] == "tool_concurrency" + + +class TestStatusNotUpdated: + """Tests for when fixer doesn't update status.""" + + async def test_fixed_assumed_when_status_not_updated(self, mock_client, spec_dir, fix_request_file): + """Test that fixed is assumed even when status not updated.""" + # Setup implementation plan without ready_for_qa_revalidation + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_response()) + + # Patch where the function is used + with patch('qa.fixer.get_graphiti_context', new_callable=AsyncMock, return_value=None), \ + patch('qa.fixer.save_session_memory', new_callable=AsyncMock) as mock_save: + + result = await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + # Should still return "fixed" even though status wasn't updated + assert result[0] == "fixed" + # Memory should still be saved + assert mock_save.called + + +class TestToolUseHandling: + """Tests for tool use handling in QA fixer.""" + + async def test_tool_use_blocks(self, mock_client, spec_dir, fix_request_file): + """Test that tool use blocks are handled correctly.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses with tool use + mock_client.query.return_value = None + mock_client.receive_response.return_value.set_messages(_create_mock_tool_use_response()) + + await run_qa_fixer_session( + mock_client, + spec_dir, + 1, + False + ) + + # Verify query was called + assert mock_client.query.called diff --git a/tests/test_qa_reviewer.py b/tests/test_qa_reviewer.py new file mode 100644 index 00000000..7c4bd27a --- /dev/null +++ b/tests/test_qa_reviewer.py @@ -0,0 +1,506 @@ +#!/usr/bin/env python3 +""" +Tests for QA Reviewer Agent Session +=================================== + +Tests the qa/reviewer.py module functionality including: +- run_qa_agent_session function +- QA session execution flow +- Error handling and edge cases +- Memory integration hooks +""" + +from datetime import datetime, timezone +from unittest.mock import AsyncMock, patch + +import pytest + +# ============================================================================= +# MOCK SETUP - Must happen before ANY imports from auto-claude +# ============================================================================= + +# Import shared mock helpers +from tests.qa_test_helpers import ( + setup_qa_mocks, + cleanup_qa_mocks, + reset_qa_mocks, + create_mock_response, + create_mock_client, +) + +# Set up mocks (reviewer needs prompts_pkg) +setup_qa_mocks(include_prompts_pkg=True) + +# Import after mocks are set up +from qa.reviewer import run_qa_agent_session +from qa.criteria import save_implementation_plan + + +# ============================================================================= +# FIXTURES +# ============================================================================= + + +@pytest.fixture(scope="module", autouse=True) +def cleanup_mocked_modules(): + """Restore original modules after all tests in this module complete.""" + yield + cleanup_qa_mocks() + + +@pytest.fixture +def spec_dir(temp_dir): + """Create a spec directory with basic structure.""" + spec = temp_dir / "spec" + spec.mkdir() + return spec + + +@pytest.fixture +def project_dir(temp_dir): + """Create a project directory.""" + project = temp_dir / "project" + project.mkdir() + return project + + +@pytest.fixture +def mock_client(): + """Create a mock Claude SDK client.""" + return create_mock_client() + + +@pytest.fixture(autouse=True, scope='function') +def reset_shared_mocks_before_test(): + """Reset shared module-level mocks before and after each test.""" + reset_qa_mocks() + yield + reset_qa_mocks() + + +# ============================================================================= +# MOCK RESPONSE HELPERS (reviewer-specific) +# ============================================================================= + +def _create_approved_response(): + """Create mock response for approved QA.""" + return create_mock_response("QA approved - all criteria met.") + + +def _create_rejected_response(): + """Create mock response for rejected QA.""" + return create_mock_response("QA rejected - found issues.") + + +def _create_no_signoff_response(): + """Create mock response where agent doesn't update signoff.""" + return create_mock_response("QA review complete.") + + +def _create_tool_use_response(): + """Create mock response with tool use blocks.""" + msg1, msg2 = create_mock_response("Checking files...") + # Add tool use block to first message + from unittest.mock import MagicMock + tool_block = MagicMock() + tool_block.__class__.__name__ = "ToolUseBlock" + tool_block.name = "Read" + tool_block.input = {"file_path": "/test/file.py"} + msg1.content.append(tool_block) + + return [msg1, msg2] + + +# ============================================================================= +# TEST CLASSES +# ============================================================================= + + +class TestRunQAAgentSessionApproved: + """Tests for run_qa_agent_session returning approved status.""" + + async def test_approved_status(self, mock_client, spec_dir, project_dir): + """Test that approved status is returned correctly.""" + # Setup implementation plan with approved status + plan = { + "feature": "Test", + "qa_signoff": { + "status": "approved", + "qa_session": 1, + "timestamp": datetime.now(timezone.utc).isoformat(), + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_approved_response() + + result = await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + assert result[0] == "approved" + assert len(result[1]) > 0 # Response text + assert result[2] == {} # No error info + + +class TestRunQAAgentSessionRejected: + """Tests for run_qa_agent_session returning rejected status.""" + + async def test_rejected_status(self, mock_client, spec_dir, project_dir): + """Test that rejected status is returned correctly.""" + # Setup implementation plan with rejected status + plan = { + "feature": "Test", + "qa_signoff": { + "status": "rejected", + "qa_session": 1, + "timestamp": datetime.now(timezone.utc).isoformat(), + "issues_found": [ + {"title": "Test failure", "type": "unit_test"}, + ] + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_rejected_response() + + result = await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + assert result[0] == "rejected" + assert len(result[1]) > 0 # Response text + assert result[2] == {} # No error info + + +class TestRunQAAgentSessionError: + """Tests for run_qa_agent_session error handling.""" + + async def test_error_status_no_signoff(self, mock_client, spec_dir, project_dir): + """Test error status when agent doesn't update signoff.""" + # Setup implementation plan without qa_signoff + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses - agent doesn't update signoff + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_no_signoff_response() + + result = await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + assert result[0] == "error" + assert "did not update" in result[1].lower() + assert result[2]["type"] == "other" + + async def test_exception_handling(self, mock_client, spec_dir, project_dir): + """Test exception handling during QA session.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client to raise exception + mock_client.query.side_effect = Exception("Test exception") + + result = await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + assert result[0] == "error" + assert "Test exception" in result[1] or "test exception" in result[1].lower() + assert result[2]["type"] == "other" + assert result[2]["exception_type"] == "Exception" + + +class TestRunQAAgentSessionParameters: + """Tests for run_qa_agent_session parameter handling.""" + + async def test_with_previous_error(self, mock_client, spec_dir, project_dir): + """Test session with previous error context.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + previous_error = { + "error_type": "missing_implementation_plan_update", + "error_message": "Test error", + "consecutive_errors": 2, + } + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_no_signoff_response() + + await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False, + previous_error=previous_error + ) + + # Verify query was called (it should include error context) + assert mock_client.query.called + + async def test_verbose_mode(self, mock_client, spec_dir, project_dir): + """Test session with verbose mode enabled.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_no_signoff_response() + + await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + verbose=True + ) + + # Verify query was called + assert mock_client.query.called + + +class TestRunQAAgentSessionIntegration: + """Integration tests for QA reviewer session.""" + + async def test_full_session_flow(self, mock_client, spec_dir, project_dir): + """Test complete session flow from start to finish.""" + # Setup implementation plan + plan = { + "feature": "Test Feature", + "qa_signoff": { + "status": "approved", + "qa_session": 1, + "timestamp": datetime.now(timezone.utc).isoformat(), + "tests_passed": {"unit": True, "integration": True}, + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_approved_response() + + result = await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + qa_session=1, + max_iterations=50, + verbose=False + ) + + assert result[0] == "approved" + assert mock_client.query.called + assert mock_client.receive_response.called + + +class TestMemoryIntegration: + """Tests for memory integration in QA reviewer.""" + + async def test_memory_context_retrieval(self, mock_client, spec_dir, project_dir): + """Test that memory context is retrieved during session.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_no_signoff_response() + + # Patch where the function is used (in qa.reviewer module) + with patch('qa.reviewer.get_graphiti_context', new_callable=AsyncMock) as mock_get_context: + mock_get_context.return_value = "Past QA insights: check for edge cases" + + await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + # Verify memory context was retrieved + assert mock_get_context.called + + async def test_memory_save_on_approved(self, mock_client, spec_dir, project_dir): + """Test that session memory is saved on approval.""" + # Setup implementation plan with approved status + plan = { + "feature": "Test", + "qa_signoff": { + "status": "approved", + "qa_session": 1, + "timestamp": datetime.now(timezone.utc).isoformat(), + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_approved_response() + + # Patch where the functions are used + with patch('qa.reviewer.get_graphiti_context', new_callable=AsyncMock, return_value=None), \ + patch('qa.reviewer.save_session_memory', new_callable=AsyncMock) as mock_save: + + await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + # Verify memory was saved + assert mock_save.called + + async def test_memory_save_on_rejected(self, mock_client, spec_dir, project_dir): + """Test that session memory is saved on rejection with issues.""" + # Setup implementation plan with rejected status + plan = { + "feature": "Test", + "qa_signoff": { + "status": "rejected", + "qa_session": 1, + "timestamp": datetime.now(timezone.utc).isoformat(), + "issues_found": [ + {"title": "Test failure", "type": "unit_test"}, + ] + } + } + save_implementation_plan(spec_dir, plan) + + # Mock client responses + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_rejected_response() + + # Patch where the functions are used + with patch('qa.reviewer.get_graphiti_context', new_callable=AsyncMock, return_value=None), \ + patch('qa.reviewer.save_session_memory', new_callable=AsyncMock) as mock_save: + + await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + # Verify memory was saved with issues + assert mock_save.called + + +class TestErrorDetection: + """Tests for error type detection in QA reviewer.""" + + async def test_rate_limit_error_detection(self, mock_client, spec_dir, project_dir): + """Test that rate limit errors are properly detected.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client to raise exception + mock_client.query.side_effect = Exception("Rate limit exceeded") + + # Patch where the functions are used (qa.reviewer) not where they're defined + with patch('qa.reviewer.is_rate_limit_error', return_value=True), \ + patch('qa.reviewer.is_tool_concurrency_error', return_value=False): + + result = await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + assert result[0] == "error" + assert result[2]["type"] == "rate_limit" + + async def test_tool_concurrency_error_detection(self, mock_client, spec_dir, project_dir): + """Test that tool concurrency errors are properly detected.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client to raise exception + mock_client.query.side_effect = Exception("Tool concurrency limit") + + # Patch where the functions are used + with patch('qa.reviewer.is_tool_concurrency_error', return_value=True), \ + patch('qa.reviewer.is_rate_limit_error', return_value=False): + + result = await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + assert result[0] == "error" + assert result[2]["type"] == "tool_concurrency" + + +class TestToolUseHandling: + """Tests for tool use handling in QA reviewer.""" + + async def test_tool_use_blocks(self, mock_client, spec_dir, project_dir): + """Test that tool use blocks are handled correctly.""" + # Setup implementation plan + plan = {"feature": "Test"} + save_implementation_plan(spec_dir, plan) + + # Mock client responses with tool use + mock_client.query.return_value = None + mock_client.receive_response.return_value = _create_tool_use_response() + + await run_qa_agent_session( + mock_client, + project_dir, + spec_dir, + 1, + 50, + False + ) + + # Verify query was called + assert mock_client.query.called diff --git a/tests/test_security_scanner.py b/tests/test_security_scanner.py index 6b73443a..9420a31f 100644 --- a/tests/test_security_scanner.py +++ b/tests/test_security_scanner.py @@ -384,9 +384,10 @@ class TestEdgeCases: """Test handling of non-existent directory.""" fake_dir = Path("/nonexistent/path") - # Should not crash, may have errors - result = scanner.scan(fake_dir) - assert isinstance(result, SecurityScanResult) + # Should not crash, may have errors - mock exists to avoid permission error + with patch.object(Path, 'exists', return_value=False): + result = scanner.scan(fake_dir) + assert isinstance(result, SecurityScanResult) def test_scan_specific_files(self, scanner, python_project): """Test scanning specific files only.""" diff --git a/tests/test_service_orchestrator.py b/tests/test_service_orchestrator.py index 73dac1a0..78227ce0 100644 --- a/tests/test_service_orchestrator.py +++ b/tests/test_service_orchestrator.py @@ -12,6 +12,7 @@ Tests cover: import json import tempfile from pathlib import Path +from unittest.mock import patch import pytest @@ -439,9 +440,10 @@ class TestEdgeCases: """Test handling of non-existent directory.""" fake_dir = Path("/nonexistent/path") - # Should not crash - orchestrator = ServiceOrchestrator(fake_dir) - assert orchestrator.is_multi_service() is False + # Should not crash - mock exists to avoid permission error + with patch.object(Path, 'exists', return_value=False): + orchestrator = ServiceOrchestrator(fake_dir) + assert orchestrator.is_multi_service() is False def test_empty_compose_file(self, temp_dir): """Test handling of empty compose file.""" diff --git a/tests/test_spec_validate_pkg_validators_context_validator.py b/tests/test_spec_validate_pkg_validators_context_validator.py new file mode 100644 index 00000000..07b89200 --- /dev/null +++ b/tests/test_spec_validate_pkg_validators_context_validator.py @@ -0,0 +1,460 @@ +#!/usr/bin/env python3 +""" +Tests for spec/validate_pkg/validators/context_validator.py +============================================================ + +Tests for ContextValidator class covering: +- File existence checks +- JSON parsing validation +- Required field validation +- Recommended field warnings +- ValidationResult return values +""" + +import json +from pathlib import Path + + +class TestContextValidatorInit: + """Tests for ContextValidator initialization.""" + + def test_initialization_with_path(self, spec_dir: Path): + """ContextValidator initializes with spec_dir path.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + validator = ContextValidator(spec_dir) + + assert validator.spec_dir == spec_dir + assert isinstance(validator.spec_dir, Path) + + def test_converts_string_to_path(self, spec_dir: Path): + """ContextValidator converts string path to Path object.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + validator = ContextValidator(str(spec_dir)) + + assert isinstance(validator.spec_dir, Path) + assert validator.spec_dir == spec_dir + + +class TestValidateFileNotFound: + """Tests for validate() when context.json does not exist.""" + + def test_returns_error_when_file_missing(self, spec_dir: Path): + """Should return ValidationResult with error when context.json missing.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert result.checkpoint == "context" + assert any("not found" in err.lower() for err in result.errors) + assert len(result.fixes) > 0 + + def test_error_message_includes_filename(self, spec_dir: Path): + """Error message should mention context.json.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert "context.json" in result.errors[0] + + def test_fix_suggests_command(self, spec_dir: Path): + """Suggested fix should include the context.py command.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert any("auto-claude/context.py" in fix for fix in result.fixes) + assert any("--output context.json" in fix for fix in result.fixes) + + +class TestValidateInvalidJson: + """Tests for validate() with invalid JSON content.""" + + def test_returns_error_for_invalid_json(self, spec_dir: Path): + """Should return error when context.json has invalid JSON.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text("{invalid json content", encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert result.checkpoint == "context" + assert any("invalid json" in err.lower() for err in result.errors) + + def test_error_includes_json_parse_message(self, spec_dir: Path): + """Error message should include JSON parsing error details.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text('{"unclosed": true', encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Error message should mention the JSON decode error + assert any("json" in err.lower() for err in result.errors) + + def test_fix_suggests_regenerate(self, spec_dir: Path): + """Suggested fix should mention regenerating context.json.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text("{bad}", encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert any("regenerate" in fix.lower() or "fix" in fix.lower() for fix in result.fixes) + + +class TestValidateMissingRequiredFields: + """Tests for validate() with missing required fields.""" + + def test_error_when_task_description_missing(self, spec_dir: Path): + """Should error when required field 'task_description' is missing.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text('{"other_field": "value"}', encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert any("task_description" in err for err in result.errors) + + def test_error_for_all_required_fields_missing(self, spec_dir: Path): + """Should list all missing required fields.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + from spec.validate_pkg.schemas import CONTEXT_SCHEMA + + context_file = spec_dir / "context.json" + context_file.write_text("{}", encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Check that all required fields are mentioned in errors + required_fields = CONTEXT_SCHEMA["required_fields"] + for field in required_fields: + assert any(field in err for err in result.errors), f"Field {field} not in errors" + + def test_fixes_suggest_adding_missing_fields(self, spec_dir: Path): + """Suggested fixes should include adding missing fields.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text('{"created_at": "2024-01-01"}', encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Fixes should suggest adding task_description + assert any("task_description" in fix for fix in result.fixes) + + def test_valid_when_all_required_fields_present(self, spec_dir: Path): + """Should pass validation when all required fields exist.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = {"task_description": "Add user authentication"} + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert len(result.errors) == 0 + + +class TestValidateRecommendedFields: + """Tests for validate() recommended field warnings.""" + + def test_warns_when_files_to_modify_missing(self, spec_dir: Path): + """Should warn when 'files_to_modify' is missing.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = {"task_description": "Test task"} + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Missing recommended field should be a warning, not error + assert any("files_to_modify" in warn for warn in result.warnings) + assert all("files_to_modify" not in err for err in result.errors) + + def test_warns_when_files_to_reference_missing(self, spec_dir: Path): + """Should warn when 'files_to_reference' is missing.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = {"task_description": "Test task"} + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert any("files_to_reference" in warn for warn in result.warnings) + + def test_warns_when_scoped_services_missing(self, spec_dir: Path): + """Should warn when 'scoped_services' is missing.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = {"task_description": "Test task"} + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert any("scoped_services" in warn for warn in result.warnings) + + def test_warns_for_empty_recommended_fields(self, spec_dir: Path): + """Should warn when recommended fields exist but are empty.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = { + "task_description": "Test task", + "files_to_modify": [], + "files_to_reference": None, + } + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Empty fields should trigger warnings + assert any("files_to_modify" in warn for warn in result.warnings) + + def test_no_warnings_when_recommended_fields_present(self, spec_dir: Path): + """Should not warn when all recommended fields are present.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = { + "task_description": "Test task", + "files_to_modify": ["src/auth.py"], + "files_to_reference": ["src/user.py"], + "scoped_services": ["backend"], + } + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Check that no warnings for these fields exist + assert not any("files_to_modify" in warn for warn in result.warnings) + assert not any("files_to_reference" in warn for warn in result.warnings) + assert not any("scoped_services" in warn for warn in result.warnings) + + +class TestValidateValidContext: + """Tests for validate() with valid context.json.""" + + def test_returns_valid_for_minimal_context(self, spec_dir: Path): + """Should return valid result with minimal required fields.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = {"task_description": "Implement OAuth login"} + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert result.checkpoint == "context" + assert len(result.errors) == 0 + # Warnings for missing recommended fields are expected + + def test_returns_valid_with_all_fields(self, spec_dir: Path): + """Should return valid result with all fields present.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = { + "task_description": "Add OAuth", + "scoped_services": ["backend", "frontend"], + "files_to_modify": ["src/auth.py"], + "files_to_reference": ["src/user.py"], + "patterns": ["singleton pattern"], + "service_contexts": {"backend": "FastAPI app"}, + "created_at": "2024-01-15T10:00:00Z", + } + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert len(result.errors) == 0 + assert len(result.warnings) == 0 + + +class TestValidationResultStructure: + """Tests for ValidationResult structure and fields.""" + + def test_result_has_all_fields(self, spec_dir: Path): + """ValidationResult should have all expected fields.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text('{"task_description": "Test"}', encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Check all fields exist + assert hasattr(result, "valid") + assert hasattr(result, "checkpoint") + assert hasattr(result, "errors") + assert hasattr(result, "warnings") + assert hasattr(result, "fixes") + + def test_checkpoint_is_context(self, spec_dir: Path): + """Checkpoint field should always be 'context'.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text('{"task_description": "Test"}', encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.checkpoint == "context" + + def test_fixes_only_on_invalid(self, spec_dir: Path): + """Fixes should only be present when validation fails.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + # Valid case - no fixes needed + context_file = spec_dir / "context.json" + context_file.write_text('{"task_description": "Test"}', encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert len(result.fixes) == 0 + + def test_lists_are_initialized(self, spec_dir: Path): + """Errors, warnings, and fixes should always be lists.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text('{"task_description": "Test"}', encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert isinstance(result.errors, list) + assert isinstance(result.warnings, list) + assert isinstance(result.fixes, list) + + +class TestEdgeCases: + """Tests for edge cases and boundary conditions.""" + + def test_handles_unicode_in_context(self, spec_dir: Path): + """Should handle unicode characters in context.json.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_data = { + "task_description": "添加用户认证", + } + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + def test_handles_large_context_file(self, spec_dir: Path): + """Should handle large context.json files.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + # Create a large context with many files + context_data = { + "task_description": "Large refactoring", + "files_to_modify": [f"src/file{i}.py" for i in range(1000)], + "files_to_reference": [f"lib/file{i}.py" for i in range(500)], + } + + context_file = spec_dir / "context.json" + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + def test_handles_empty_context_object(self, spec_dir: Path): + """Should handle empty JSON object.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_file = spec_dir / "context.json" + context_file.write_text("{}", encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert any("task_description" in err for err in result.errors) + + def test_handles_nested_json_structure(self, spec_dir: Path): + """Should handle nested JSON objects.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_data = { + "task_description": "Complex task", + "service_contexts": { + "backend": { + "framework": "FastAPI", + "version": "0.100.0", + "config": {"debug": True, "port": 8000}, + } + }, + "patterns": [ + {"name": "singleton", "description": "Single instance"}, + {"name": "factory", "description": "Object creation"}, + ], + } + + context_file = spec_dir / "context.json" + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + def test_handles_extra_fields(self, spec_dir: Path): + """Should allow extra fields not in schema.""" + from spec.validate_pkg.validators.context_validator import ContextValidator + + context_data = { + "task_description": "Test task", + "custom_field": "custom value", + "another_extra": 123, + } + + context_file = spec_dir / "context.json" + context_file.write_text(json.dumps(context_data), encoding="utf-8") + + validator = ContextValidator(spec_dir) + result = validator.validate() + + # Extra fields should not cause validation errors + assert result.valid is True diff --git a/tests/test_spec_validate_pkg_validators_prereqs_validator.py b/tests/test_spec_validate_pkg_validators_prereqs_validator.py new file mode 100644 index 00000000..1b25e7de --- /dev/null +++ b/tests/test_spec_validate_pkg_validators_prereqs_validator.py @@ -0,0 +1,368 @@ +#!/usr/bin/env python3 +""" +Tests for spec/validate_pkg/validators/prereqs_validator.py +=========================================================== + +Tests for PrereqsValidator class covering: +- Spec directory existence checks +- project_index.json existence checks +- Auto-claude level fallback checks +- ValidationResult return values +""" + +import json +from pathlib import Path + +import pytest + + +# ============================================================================= +# HELPER FUNCTIONS +# ============================================================================= + +def clean_project_index_files(spec_dir: Path) -> None: + """Remove project_index.json files that may interfere with tests. + + Cleans up both: + - spec_dir / "project_index.json" + - spec_dir.parent.parent / "project_index.json" (auto-claude level) + + This prevents test isolation issues when tests share the same temp_dir parent. + """ + # Clean spec_dir level + spec_index = spec_dir / "project_index.json" + if spec_index.exists(): + spec_index.unlink() + + # Clean auto-claude level (two levels up from spec_dir) + auto_build_index = spec_dir.parent.parent / "project_index.json" + if auto_build_index.exists(): + auto_build_index.unlink() + + +class TestPrereqsValidatorInit: + """Tests for PrereqsValidator initialization.""" + + def test_initialization_with_path(self, spec_dir: Path): + """PrereqsValidator initializes with spec_dir path.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + validator = PrereqsValidator(spec_dir) + + assert validator.spec_dir == spec_dir + assert isinstance(validator.spec_dir, Path) + + def test_converts_string_to_path(self, spec_dir: Path): + """PrereqsValidator converts string path to Path object.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + validator = PrereqsValidator(str(spec_dir)) + + assert isinstance(validator.spec_dir, Path) + assert validator.spec_dir == spec_dir + + +class TestValidateSpecDirMissing: + """Tests for validate() when spec directory does not exist.""" + + def test_returns_error_when_spec_dir_missing(self, temp_dir: Path): + """Should return error when spec directory does not exist.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + non_existent_dir = temp_dir / "nonexistent" / "spec" + validator = PrereqsValidator(non_existent_dir) + result = validator.validate() + + assert result.valid is False + assert result.checkpoint == "prereqs" + assert len(result.errors) > 0 + assert any("does not exist" in err.lower() for err in result.errors) + + def test_error_includes_directory_path(self, temp_dir: Path): + """Error message should include the directory path.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + non_existent_dir = temp_dir / "missing" / "spec" + validator = PrereqsValidator(non_existent_dir) + result = validator.validate() + + error_msg = result.errors[0] + assert str(non_existent_dir) in error_msg + + def test_fix_suggests_mkdir_command(self, temp_dir: Path): + """Suggested fix should include mkdir -p command.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + non_existent_dir = temp_dir / "new" / "spec" + validator = PrereqsValidator(non_existent_dir) + result = validator.validate() + + assert any("mkdir" in fix.lower() for fix in result.fixes) + assert any("-p" in fix for fix in result.fixes) + + +class TestValidateProjectIndexMissing: + """Tests for validate() when project_index.json is missing.""" + + def test_returns_error_when_project_index_missing(self, spec_dir: Path): + """Should return error when project_index.json does not exist.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + clean_project_index_files(spec_dir) + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert any("project_index.json" in err for err in result.errors) + + def test_error_when_no_auto_claude_index(self, spec_dir: Path): + """Should error when project_index.json missing at both levels.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + clean_project_index_files(spec_dir) + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert not result.warnings # No warning if no auto-claude fallback exists + + def test_fix_suggests_running_analyzer(self, spec_dir: Path): + """Suggested fix should suggest running analyzer.py.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + clean_project_index_files(spec_dir) + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert any("analyzer.py" in fix for fix in result.fixes) + assert any("auto-claude" in fix for fix in result.fixes) + + +class TestValidateAutoClaudeFallback: + """Tests for validate() with auto-claude level project_index.json.""" + + def test_warns_when_auto_claude_index_exists(self, spec_dir: Path): + """Should warn when project_index.json exists at auto-claude/ level.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + # The validator checks spec_dir.parent.parent for the auto-claude index + # Create project_index.json at the correct level (two levels up from spec_dir) + auto_build_index = spec_dir.parent.parent / "project_index.json" + auto_build_index.parent.mkdir(parents=True, exist_ok=True) + auto_build_index.write_text('{"project_type": "single"}', encoding="utf-8") + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + # When auto-claude index exists but spec_dir index doesn't, it's valid with a warning + assert result.valid is True # Valid because warning path, not error path + assert len(result.warnings) > 0 + assert any("auto-claude" in warn or "spec folder" in warn for warn in result.warnings) + + def test_fix_suggests_copy_command(self, spec_dir: Path): + """Suggested fix should include cp command when auto-claude index exists.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + # Create project_index.json at the auto-claude level (two levels up) + auto_build_index = spec_dir.parent.parent / "project_index.json" + auto_build_index.parent.mkdir(parents=True, exist_ok=True) + auto_build_index.write_text('{"project_type": "monorepo"}', encoding="utf-8") + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert any("cp" in fix for fix in result.fixes) + assert any(str(auto_build_index) in fix for fix in result.fixes) + + def test_no_warning_when_auto_claude_index_missing(self, spec_dir: Path): + """Should not warn when auto-claude level index also missing.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + clean_project_index_files(spec_dir) + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + # Should be invalid since no index exists anywhere + assert result.valid is False + assert not any("auto-claude" in warn for warn in result.warnings) + assert any("not found" in err for err in result.errors) + + +class TestValidateValidPrereqs: + """Tests for validate() with valid prerequisites.""" + + def test_returns_valid_when_project_index_exists(self, spec_dir: Path): + """Should return valid when project_index.json exists in spec dir.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + project_index = spec_dir / "project_index.json" + project_index.write_text('{"project_type": "single"}', encoding="utf-8") + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert result.checkpoint == "prereqs" + assert len(result.errors) == 0 + + def test_valid_with_valid_project_index_content(self, spec_dir: Path): + """Should be valid with properly structured project_index.json.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + project_index = spec_dir / "project_index.json" + project_index.write_text(json.dumps({ + "project_type": "monorepo", + "services": { + "backend": {"path": "backend", "language": "python"}, + "frontend": {"path": "frontend", "language": "typescript"}, + }, + "file_count": 150, + }), encoding="utf-8") + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + +class TestValidationResultStructure: + """Tests for ValidationResult structure.""" + + def test_result_has_all_fields(self, spec_dir: Path): + """ValidationResult should have all expected fields.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert hasattr(result, "valid") + assert hasattr(result, "checkpoint") + assert hasattr(result, "errors") + assert hasattr(result, "warnings") + assert hasattr(result, "fixes") + + def test_checkpoint_is_prereqs(self, spec_dir: Path): + """Checkpoint field should always be 'prereqs'.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert result.checkpoint == "prereqs" + + def test_lists_are_initialized(self, spec_dir: Path): + """Errors, warnings, and fixes should always be lists.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + assert isinstance(result.errors, list) + assert isinstance(result.warnings, list) + assert isinstance(result.fixes, list) + + +class TestEdgeCases: + """Tests for edge cases and boundary conditions.""" + + def test_handles_relative_paths(self, temp_dir: Path, monkeypatch): + """Should handle relative path arguments.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + # Create spec directory + spec_path = temp_dir / "spec" + spec_path.mkdir() + + # Use relative path with monkeypatch for safe directory change + relative_path = "spec" + monkeypatch.chdir(temp_dir) + validator = PrereqsValidator(relative_path) + result = validator.validate() + + # Should work (will be invalid since no project_index.json) + assert result.checkpoint == "prereqs" + + def test_handles_symlink_to_directory(self, temp_dir: Path): + """Should handle symlinks to directories.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + # Create actual spec directory + actual_spec = temp_dir / "actual_spec" + actual_spec.mkdir() + + # Create symlink + import os + link_spec = temp_dir / "link_spec" + try: + os.symlink(actual_spec, link_spec) + except OSError: + # Symlinks may not be supported on all systems + pytest.skip("Symlinks not supported") + + validator = PrereqsValidator(link_spec) + result = validator.validate() + + # Should handle the symlinked directory + assert result.checkpoint == "prereqs" + + def test_multiple_validations_independent(self, spec_dir: Path): + """Multiple validations should be independent.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + clean_project_index_files(spec_dir) + + validator1 = PrereqsValidator(spec_dir) + result1 = validator1.validate() + + # Create project_index.json between validations + project_index = spec_dir / "project_index.json" + project_index.write_text('{"project_type": "single"}', encoding="utf-8") + + validator2 = PrereqsValidator(spec_dir) + result2 = validator2.validate() + + # First result should be invalid (no index existed at validation time) + assert result1.valid is False + # Second result should be valid (index now exists) + assert result2.valid is True + + def test_handles_empty_project_index(self, spec_dir: Path): + """Should handle empty project_index.json file.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + + project_index = spec_dir / "project_index.json" + project_index.write_text("{}", encoding="utf-8") + + validator = PrereqsValidator(spec_dir) + result = validator.validate() + + # Should be valid since file exists (content validation not required) + assert result.valid is True + + +class TestPrereqsValidatorIntegration: + """Integration tests with other validators.""" + + def test_works_with_context_validator(self, spec_dir: Path): + """Should work correctly when used with ContextValidator.""" + from spec.validate_pkg.validators.prereqs_validator import PrereqsValidator + from spec.validate_pkg.validators.context_validator import ContextValidator + + # Create project_index.json + project_index = spec_dir / "project_index.json" + project_index.write_text('{"project_type": "single"}', encoding="utf-8") + + prereq_validator = PrereqsValidator(spec_dir) + prereq_result = prereq_validator.validate() + + context_validator = ContextValidator(spec_dir) + context_result = context_validator.validate() + + # Prereqs should be valid + assert prereq_result.valid is True + # Context should be invalid (no context.json) + assert context_result.valid is False diff --git a/tests/test_spec_validate_pkg_validators_spec_document_validator.py b/tests/test_spec_validate_pkg_validators_spec_document_validator.py new file mode 100644 index 00000000..73cbfd19 --- /dev/null +++ b/tests/test_spec_validate_pkg_validators_spec_document_validator.py @@ -0,0 +1,486 @@ +#!/usr/bin/env python3 +""" +Tests for spec/validate_pkg/validators/spec_document_validator.py +================================================================= + +Tests for SpecDocumentValidator class covering: +- File existence checks +- Required section validation +- Recommended section warnings +- Content length validation +- ValidationResult return values +""" + +from pathlib import Path + + +class TestSpecDocumentValidatorInit: + """Tests for SpecDocumentValidator initialization.""" + + def test_initialization_with_path(self, spec_dir: Path): + """SpecDocumentValidator initializes with spec_dir path.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + validator = SpecDocumentValidator(spec_dir) + + assert validator.spec_dir == spec_dir + assert isinstance(validator.spec_dir, Path) + + def test_converts_string_to_path(self, spec_dir: Path): + """SpecDocumentValidator converts string path to Path object.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + validator = SpecDocumentValidator(str(spec_dir)) + + assert isinstance(validator.spec_dir, Path) + assert validator.spec_dir == spec_dir + + +class TestValidateFileNotFound: + """Tests for validate() when spec.md does not exist.""" + + def test_returns_error_when_file_missing(self, spec_dir: Path): + """Should return ValidationResult with error when spec.md missing.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert result.checkpoint == "spec" + assert any("not found" in err.lower() or "spec.md" in err.lower() for err in result.errors) + + def test_error_message_includes_filename(self, spec_dir: Path): + """Error message should mention spec.md.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert "spec.md" in result.errors[0] + + def test_fix_suggests_creation(self, spec_dir: Path): + """Suggested fix should mention creating spec.md.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert any("create" in fix.lower() for fix in result.fixes) + + +class TestValidateRequiredSections: + """Tests for validate() with missing required sections.""" + + def test_error_when_overview_missing(self, spec_dir: Path): + """Should error when required section 'Overview' is missing.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + spec_file.write_text("# Other Section\n\nContent here.\n", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert any("overview" in err.lower() for err in result.errors) + + def test_error_for_all_required_sections_missing(self, spec_dir: Path): + """Should list all missing required sections.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + from spec.validate_pkg.schemas import SPEC_REQUIRED_SECTIONS + + spec_file = spec_dir / "spec.md" + spec_file.write_text("# Other\n\nContent.\n", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Check that all required sections are mentioned in errors + for section in SPEC_REQUIRED_SECTIONS: + assert any(section.lower() in err.lower() for err in result.errors), \ + f"Section {section} not in errors" + + def test_accepts_hash_hash_format(self, spec_dir: Path): + """Should accept ## Section format (double hash).""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\nContent\n\n## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nScope\n\n## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert len(result.errors) == 0 + + def test_accepts_single_hash_format(self, spec_dir: Path): + """Should accept # Section format (single hash).""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "# Overview\n\nContent\n\n# Workflow Type\n\nFeature\n\n" + content += "# Task Scope\n\nScope\n\n# Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + def test_case_insensitive_section_matching(self, spec_dir: Path): + """Should match sections case-insensitively.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## OVERVIEW\n\nContent\n\n## workflow type\n\nFeature\n\n" + content += "## task scope\n\nScope\n\n## success criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + def test_fixes_suggest_adding_sections(self, spec_dir: Path): + """Suggested fixes should include adding missing sections.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + spec_file.write_text("# Other\n\nContent.\n", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Fixes should suggest adding sections + assert any("##" in fix for fix in result.fixes) + + +class TestValidateRecommendedSections: + """Tests for validate() with recommended sections.""" + + def test_warns_when_files_to_modify_missing(self, spec_dir: Path): + """Should warn when 'Files to Modify' section is missing.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\nContent\n\n## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nScope\n\n## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Missing recommended section should be a warning, not error + assert any("files to modify" in warn.lower() for warn in result.warnings) + + def test_warns_for_multiple_missing_recommended(self, spec_dir: Path): + """Should warn for all missing recommended sections.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\nContent\n\n## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nScope\n\n## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Should have warnings for missing recommended sections + assert len(result.warnings) > 0 + + def test_no_warnings_with_all_recommended(self, spec_dir: Path): + """Should not warn when all recommended sections present.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + from spec.validate_pkg.schemas import SPEC_RECOMMENDED_SECTIONS + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\nThis is a comprehensive overview of the feature that we are building.\n\n" + content += "## Workflow Type\n\nFeature implementation workflow with multiple phases.\n\n" + content += "## Task Scope\n\nThe scope includes backend API changes and database updates.\n\n" + content += "## Success Criteria\n\nAll tests pass and the feature works as expected.\n\n" + + # Add all recommended sections with substantial content + for section in SPEC_RECOMMENDED_SECTIONS: + content += f"## {section}\n\nThis section contains detailed information about {section.lower()}. " + content += "We need to ensure that all requirements are properly documented and reviewed.\n\n" + + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert len(result.warnings) == 0 + + +class TestValidateContentLength: + """Tests for content length validation.""" + + def test_warns_when_content_too_short(self, spec_dir: Path): + """Should warn when spec.md is less than 500 characters.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\nShort.\n\n## Workflow Type\n\nX\n\n" + content += "## Task Scope\n\nY\n\n## Success Criteria\n\nZ\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert any("too short" in warn.lower() for warn in result.warnings) + + def test_no_warning_for_adequate_length(self, spec_dir: Path): + """Should not warn when spec.md has adequate length.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + # Create content longer than 500 characters + content = "## Overview\n\n" + "X" * 600 + "\n\n" + content += "## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nScope\n\n" + content += "## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert not any("too short" in warn.lower() for warn in result.warnings) + + def test_content_check_counts_all_characters(self, spec_dir: Path): + """Content length check should count all characters including whitespace.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + # Create content exactly over 500 characters with mixed content + content = "## Overview\n\n" + "A" * 480 + "\n\n" + content += "## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nScope\n\n" + content += "## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Should not have length warning + assert not any("too short" in warn.lower() for warn in result.warnings) + + +class TestValidateValidSpec: + """Tests for validate() with valid spec.md.""" + + def test_returns_valid_for_minimal_spec(self, spec_dir: Path): + """Should return valid with minimal required sections.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\nImplement feature.\n\n## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nAdd user auth.\n\n## Success Criteria\n\nTests pass.\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert result.checkpoint == "spec" + # May have warnings about recommended sections or length + + def test_returns_valid_with_comprehensive_spec(self, spec_dir: Path): + """Should return valid with comprehensive spec document.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + from spec.validate_pkg.schemas import SPEC_REQUIRED_SECTIONS, SPEC_RECOMMENDED_SECTIONS + + spec_file = spec_dir / "spec.md" + content = "" + + # Add all required sections + for section in SPEC_REQUIRED_SECTIONS: + content += f"## {section}\n\nDetailed content for {section}.\n\n" + + # Add all recommended sections + for section in SPEC_RECOMMENDED_SECTIONS: + content += f"## {section}\n\nDetails about {section}.\n\n" + + # Add more content to avoid length warning + content += "Additional implementation details..." * 50 + + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + assert len(result.errors) == 0 + assert len(result.warnings) == 0 + + +class TestValidationResultStructure: + """Tests for ValidationResult structure.""" + + def test_result_has_all_fields(self, spec_dir: Path): + """ValidationResult should have all expected fields.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + spec_file.write_text("## Overview\n\nContent\n", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert hasattr(result, "valid") + assert hasattr(result, "checkpoint") + assert hasattr(result, "errors") + assert hasattr(result, "warnings") + assert hasattr(result, "fixes") + + def test_checkpoint_is_spec(self, spec_dir: Path): + """Checkpoint field should always be 'spec'.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + spec_file.write_text("## Overview\n\nContent\n", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.checkpoint == "spec" + + def test_lists_are_initialized(self, spec_dir: Path): + """Errors, warnings, and fixes should always be lists.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + spec_file.write_text("## Overview\n\nContent\n", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert isinstance(result.errors, list) + assert isinstance(result.warnings, list) + assert isinstance(result.fixes, list) + + +class TestEdgeCases: + """Tests for edge cases and boundary conditions.""" + + def test_handles_unicode_in_spec(self, spec_dir: Path): + """Should handle unicode characters in spec.md.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\n添加用户认证功能\n\n## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\n范围\n\n## Success Criteria\n\n完成\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + def test_handles_extra_whitespace(self, spec_dir: Path): + """Should handle extra whitespace in sections.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview \n\nContent\n\n## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nScope\n\n## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Should still match despite extra whitespace + assert result.valid is True + + def test_handles_mixed_heading_levels(self, spec_dir: Path): + """Should handle spec with various heading levels.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview\n\nContent\n\n### Subsection\n\nDetails\n\n" + content += "## Workflow Type\n\nFeature\n\n## Task Scope\n\nScope\n\n" + content += "## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is True + + def test_section_pattern_excludes_subsections(self, spec_dir: Path): + """Should not match subsections (###) as main sections.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + # Only has subsections, not main sections + content = "### Overview\n\nContent\n\n### Workflow Type\n\nFeature\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Should be invalid - ### doesn't count as ## or # + assert result.valid is False + + def test_handles_empty_spec_file(self, spec_dir: Path): + """Should handle empty spec.md file.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + spec_file.write_text("", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + # Should warn about being too short + assert any("too short" in warn.lower() for warn in result.warnings) + + def test_handles_spec_with_only_whitespace(self, spec_dir: Path): + """Should handle spec.md with only whitespace.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + spec_file.write_text(" \n\n \n", encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + assert result.valid is False + assert any("too short" in warn.lower() for warn in result.warnings) + + +class TestSectionMatching: + """Tests for section heading pattern matching.""" + + def test_matches_section_with_trailing_colon(self, spec_dir: Path): + """Should match sections with trailing colon.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview:\n\nContent\n\n## Workflow Type:\n\nFeature\n\n" + content += "## Task Scope:\n\nScope\n\n## Success Criteria:\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Should match despite trailing colon + assert result.valid is True + + def test_matches_section_with_special_chars(self, spec_dir: Path): + """Should match sections with special characters.""" + from spec.validate_pkg.validators.spec_document_validator import SpecDocumentValidator + + spec_file = spec_dir / "spec.md" + content = "## Overview (v2.0)\n\nContent\n\n## Workflow Type\n\nFeature\n\n" + content += "## Task Scope\n\nScope\n\n## Success Criteria\n\nDone\n" + spec_file.write_text(content, encoding="utf-8") + + validator = SpecDocumentValidator(spec_dir) + result = validator.validate() + + # Should still match + assert result.valid is True diff --git a/tests/test_validation_strategy.py b/tests/test_validation_strategy.py index a8a6174f..a0d152e4 100644 --- a/tests/test_validation_strategy.py +++ b/tests/test_validation_strategy.py @@ -631,11 +631,17 @@ class TestEdgeCases: def test_nonexistent_directory(self, builder): """Test handling of non-existent directory.""" + from unittest.mock import patch + fake_dir = Path("/nonexistent/path") - # Should not crash, returns unknown - strategy = builder.build_strategy(fake_dir, fake_dir, "medium") - assert strategy.project_type == "unknown" + # Mock multiple Path methods to avoid permission errors on nonexistent paths + with patch.object(Path, 'exists', return_value=False), \ + patch.object(Path, 'is_dir', return_value=False), \ + patch.object(Path, 'glob', return_value=[]): + # Should not crash, returns unknown + strategy = builder.build_strategy(fake_dir, fake_dir, "medium") + assert strategy.project_type == "unknown" def test_empty_risk_level_defaults_medium(self, builder, temp_dir): """Test that None risk level defaults to medium."""