# Task 5: Test Reorganization
## Overview
**Priority**: Low
**Goal**: Reorganize test suite into a maintainable structure
**Target**: Logical test organization with shared fixtures and clear separation
**Status**: ✅ COMPLETE
## Outstanding Tasks
- [x] Create new directory structure (tests/unit, tests/integration, tests/fixtures, tests/utils)
- [x] Set up conftest.py with shared fixtures
- [x] Create test utilities and helpers
- [x] Create shared fixtures
- [x] Migrate remaining unit tests from old structure
- [x] Migrate remaining integration tests from old structure
- [x] Create end-to-end test structure
- [x] Remove old test files once migration is complete
- [x] Update test documentation
- [x] Verify all tests pass in new structure
## Current Problem
The current test structure has several issues:
- 12 test files with some overlap and inconsistent patterns
- Mixed unit, integration, and end-to-end tests in same files
- No shared fixtures or test utilities
- Inconsistent naming and organization
- Difficult to run specific test categories
- Some tests are tightly coupled to implementation details
This makes the test suite:
- Hard to navigate and understand
- Difficult to maintain as code evolves
- Challenging to run specific test subsets
- Prone to duplication and inconsistency
## Current Test Files
```
tests/
├── test_enhanced_mcp_tools.py
├── test_environment_variable_solution.py
├── test_git_integration.py
├── test_git_mcp_tools.py
├── test_gitpython_complete.py
├── test_gitpython_service.py
├── test_integration.py
├── test_mcp_server_problem_demonstration.py
├── test_mcp_tools.py
├── test_repository_targeting.py
├── test_signoff_enforcement.py
├── test_signoff_integration.py
├── verify_commitizen_git_apis.py
└── commit_helper_mcp/
├── test_commitizen_server.py
├── test_commitizen_service.py
└── test_plugin_adapters.py
```
## Target Structure
```
tests/
├── conftest.py # Shared fixtures and configuration
├── unit/ # Unit tests (isolated components)
│ ├── __init__.py
│ ├── services/
│ │ ├── __init__.py
│ │ ├── test_commitizen_core.py
│ │ ├── test_gitpython_core.py
│ │ ├── test_commit_orchestrator.py
│ │ ├── test_repository_manager.py
│ │ └── test_validation_service.py
│ ├── server/
│ │ ├── __init__.py
│ │ ├── test_message_tools.py
│ │ ├── test_git_tools.py
│ │ ├── test_workflow_tools.py
│ │ ├── test_enhanced_tools.py
│ │ └── test_resources.py
│ ├── config/
│ │ ├── __init__.py
│ │ ├── test_settings.py
│ │ ├── test_repository_config.py
│ │ └── test_plugin_config.py
│ ├── errors/
│ │ ├── __init__.py
│ │ ├── test_exceptions.py
│ │ ├── test_handlers.py
│ │ └── test_responses.py
│ └── test_plugin_adapters.py
├── integration/ # Integration tests (component interactions)
│ ├── __init__.py
│ ├── git_operations/
│ │ ├── __init__.py
│ │ ├── test_git_service_integration.py
│ │ ├── test_commit_workflows.py
│ │ └── test_repository_targeting.py
│ ├── mcp_protocol/
│ │ ├── __init__.py
│ │ ├── test_mcp_tools_integration.py
│ │ ├── test_mcp_resources_integration.py
│ │ └── test_server_lifecycle.py
│ └── end_to_end/
│ ├── __init__.py
│ ├── test_complete_workflows.py
│ ├── test_multi_repository.py
│ └── test_error_scenarios.py
├── fixtures/ # Shared test fixtures
│ ├── __init__.py
│ ├── git_repositories.py
│ ├── commitizen_configs.py
│ ├── mock_services.py
│ └── test_data.py
└── utils/ # Test utilities
├── __init__.py
├── assertions.py
├── helpers.py
└── mocks.py
```
## Implementation Steps
### Step 1: Create Test Configuration
**File**: `tests/conftest.py`
**Responsibilities**:
- Global pytest configuration
- Shared fixtures for all tests
- Test environment setup
- Common test utilities
```python
"""
Global Test Configuration
Provides shared fixtures, configuration, and utilities for all tests
in the Commit Helper MCP test suite.
"""
import pytest
import tempfile
import shutil
from pathlib import Path
from typing import Dict, Any, Generator
from unittest.mock import Mock, MagicMock
# Import fixtures from fixtures package
from .fixtures.git_repositories import *
from .fixtures.commitizen_configs import *
from .fixtures.mock_services import *
from .fixtures.test_data import *
@pytest.fixture(scope="session")
def test_data_dir() -> Path:
"""Get path to test data directory."""
return Path(__file__).parent / "fixtures" / "data"
@pytest.fixture
def temp_dir() -> Generator[Path, None, None]:
"""Create temporary directory for test isolation."""
with tempfile.TemporaryDirectory() as tmp_dir:
yield Path(tmp_dir)
@pytest.fixture
def mock_logger():
"""Mock logger for testing logging behavior."""
return Mock()
@pytest.fixture(autouse=True)
def reset_global_state():
"""Reset global state between tests."""
# Reset any global singletons or caches
yield
# Cleanup after test
# Pytest configuration
def pytest_configure(config):
"""Configure pytest with custom markers."""
config.addinivalue_line(
"markers", "unit: mark test as a unit test"
)
config.addinivalue_line(
"markers", "integration: mark test as an integration test"
)
config.addinivalue_line(
"markers", "e2e: mark test as an end-to-end test"
)
config.addinivalue_line(
"markers", "slow: mark test as slow running"
)
config.addinivalue_line(
"markers", "git: mark test as requiring git operations"
)
def pytest_collection_modifyitems(config, items):
"""Modify test collection to add markers based on location."""
for item in items:
# Add markers based on test file location
if "unit/" in str(item.fspath):
item.add_marker(pytest.mark.unit)
elif "integration/" in str(item.fspath):
item.add_marker(pytest.mark.integration)
elif "end_to_end/" in str(item.fspath):
item.add_marker(pytest.mark.e2e)
# Add git marker for git-related tests
if any(keyword in str(item.fspath) for keyword in ["git", "repository"]):
item.add_marker(pytest.mark.git)
```
### Step 2: Create Shared Fixtures
**File**: `tests/fixtures/__init__.py`
```python
"""
Shared Test Fixtures
This package contains reusable fixtures for testing the Commit Helper MCP.
"""
from .git_repositories import *
from .commitizen_configs import *
from .mock_services import *
from .test_data import *
__all__ = [
# Git repository fixtures
'temp_git_repo',
'git_repo_with_commits',
'git_repo_with_staged_files',
# Configuration fixtures
'conventional_commits_config',
'custom_plugin_config',
'invalid_config',
# Service mocks
'mock_commitizen_service',
'mock_git_service',
'mock_mcp_server',
# Test data
'sample_commit_messages',
'sample_git_status',
'sample_mcp_responses'
]
```
**File**: `tests/fixtures/git_repositories.py`
```python
"""
Git Repository Fixtures
Provides fixtures for creating and managing test git repositories.
"""
import pytest
import subprocess
from pathlib import Path
from typing import Generator
from git import Repo
@pytest.fixture
def temp_git_repo(temp_dir) -> Generator[Repo, None, None]:
"""Create a temporary git repository."""
repo = Repo.init(temp_dir)
# Configure git user for tests
with repo.config_writer() as config:
config.set_value("user", "name", "Test User")
config.set_value("user", "email", "test@example.com")
yield repo
@pytest.fixture
def git_repo_with_commits(temp_git_repo) -> Repo:
"""Create git repository with some initial commits."""
repo = temp_git_repo
# Create initial commit
test_file = Path(repo.working_dir) / "README.md"
test_file.write_text("# Test Repository\n")
repo.index.add([str(test_file)])
repo.index.commit("Initial commit")
# Create second commit
test_file.write_text("# Test Repository\n\nSome content\n")
repo.index.add([str(test_file)])
repo.index.commit("Add content to README")
return repo
@pytest.fixture
def git_repo_with_staged_files(temp_git_repo) -> Repo:
"""Create git repository with staged files ready for commit."""
repo = temp_git_repo
# Create and stage files
test_file = Path(repo.working_dir) / "test.py"
test_file.write_text("print('Hello, World!')\n")
repo.index.add([str(test_file)])
return repo
```
**File**: `tests/fixtures/mock_services.py`
```python
"""
Mock Service Fixtures
Provides mock implementations of services for testing.
"""
import pytest
from unittest.mock import Mock, MagicMock
from typing import Dict, Any
@pytest.fixture
def mock_commitizen_service():
"""Mock CommitzenService for testing."""
service = Mock()
service.generate_message.return_value = "feat: add new feature"
service.validate_message.return_value = True
service.get_commit_types.return_value = [
{"name": "feat", "value": "feat"},
{"name": "fix", "value": "fix"}
]
service.git_enabled = True
return service
@pytest.fixture
def mock_git_service():
"""Mock GitService for testing."""
service = Mock()
service.is_git_project.return_value = True
service.is_staging_clean.return_value = False
service.get_staged_files.return_value = ["test.py"]
service.preview_commit.return_value = {
"success": True,
"staged_files": ["test.py"],
"would_execute": True
}
return service
@pytest.fixture
def mock_mcp_server():
"""Mock MCP server for testing."""
server = Mock()
server._tools = {}
server._resources = {}
return server
```
### Step 3: Reorganize Unit Tests
**Example**: `tests/unit/services/test_commitizen_core.py`
```python
"""
Unit Tests for CommitzenCore Service
Tests the core Commitizen functionality in isolation.
"""
import pytest
from unittest.mock import Mock, patch
from src.commit_helper_mcp.services.commitizen_core import CommitzenCore
class TestCommitzenCore:
"""Test cases for CommitzenCore service."""
@pytest.fixture
def commitizen_core(self, temp_dir):
"""Create CommitzenCore instance for testing."""
return CommitzenCore(repo_path=str(temp_dir))
def test_generate_message_success(self, commitizen_core):
"""Test successful message generation."""
answers = {
"type": "feat",
"subject": "add new feature",
"body": "Detailed description"
}
message = commitizen_core.generate_message(answers)
assert message.startswith("feat:")
assert "add new feature" in message
def test_validate_message_valid(self, commitizen_core):
"""Test validation of valid commit message."""
message = "feat: add new feature"
is_valid = commitizen_core.validate_message(message)
assert is_valid is True
def test_validate_message_invalid(self, commitizen_core):
"""Test validation of invalid commit message."""
message = "invalid message format"
is_valid = commitizen_core.validate_message(message)
assert is_valid is False
def test_get_commit_types(self, commitizen_core):
"""Test getting available commit types."""
commit_types = commitizen_core.get_commit_types()
assert isinstance(commit_types, list)
assert len(commit_types) > 0
assert all("name" in ct and "value" in ct for ct in commit_types)
```
### Step 4: Reorganize Integration Tests
**Example**: `tests/integration/git_operations/test_commit_workflows.py`
```python
"""
Integration Tests for Commit Workflows
Tests the complete commit workflow integration between services.
"""
import pytest
from src.commit_helper_mcp.services.commit_orchestrator import CommitOrchestrator
class TestCommitWorkflows:
"""Test cases for complete commit workflows."""
@pytest.fixture
def orchestrator(self, mock_commitizen_service, mock_git_service):
"""Create CommitOrchestrator for testing."""
return CommitOrchestrator(mock_commitizen_service, mock_git_service)
def test_preview_commit_workflow(self, orchestrator):
"""Test complete preview workflow."""
message = "feat: add new feature"
result = orchestrator.preview_commit_operation(message)
assert result["success"] is True
assert result["message"] == message
assert "staged_files" in result
def test_execute_commit_workflow(self, orchestrator):
"""Test complete execution workflow."""
message = "feat: add new feature"
result = orchestrator.execute_commit_operation(
message,
force_execute=True
)
assert result["success"] is True
assert result["executed"] is True
```
### Step 5: Create Test Utilities
**File**: `tests/utils/assertions.py`
```python
"""
Custom Test Assertions
Provides domain-specific assertions for testing.
"""
from typing import Dict, Any, List
def assert_valid_commit_message(message: str):
"""Assert that a commit message follows conventional format."""
assert isinstance(message, str)
assert len(message) > 0
assert ":" in message
# Basic conventional commit format check
parts = message.split(":", 1)
assert len(parts) == 2
assert len(parts[0].strip()) > 0
assert len(parts[1].strip()) > 0
def assert_mcp_tool_response(response: Dict[str, Any]):
"""Assert that response follows MCP tool format."""
assert isinstance(response, dict)
# Should have either success=True or error field
if "success" in response:
assert isinstance(response["success"], bool)
else:
assert "error" in response
def assert_git_status_response(response: Dict[str, Any]):
"""Assert that git status response is valid."""
assert_mcp_tool_response(response)
if response.get("success"):
assert "staged_files" in response
assert isinstance(response["staged_files"], list)
assert "staged_count" in response
assert isinstance(response["staged_count"], int)
def assert_error_response(response: Dict[str, Any]):
"""Assert that error response has proper structure."""
assert isinstance(response, dict)
assert response.get("success") is False
assert "error" in response
assert isinstance(response["error"], str)
assert len(response["error"]) > 0
```
### Step 6: Migration Plan
#### Phase 5.1: Create New Structure
1. Create new directory structure
2. Set up conftest.py and shared fixtures
3. Create test utilities and helpers
#### Phase 5.2: Migrate Unit Tests
1. Extract unit tests from existing files
2. Organize by component (services, server, config, errors)
3. Update imports and fixtures
#### Phase 5.3: Migrate Integration Tests
1. Identify integration test scenarios
2. Organize by functional area
3. Create end-to-end test scenarios
#### Phase 5.4: Update Test Configuration
1. Update pytest configuration
2. Create test running scripts
3. Update CI/CD configuration
#### Phase 5.5: Cleanup
1. Remove old test files
2. Update documentation
3. Verify all tests pass
## Test Categories and Markers
### Pytest Markers
- `@pytest.mark.unit`: Unit tests (fast, isolated)
- `@pytest.mark.integration`: Integration tests (moderate speed)
- `@pytest.mark.e2e`: End-to-end tests (slower)
- `@pytest.mark.slow`: Tests that take significant time
- `@pytest.mark.git`: Tests requiring git operations
### Running Specific Test Categories
```bash
# Run only unit tests
pytest -m unit
# Run only integration tests
pytest -m integration
# Run all tests except slow ones
pytest -m "not slow"
# Run git-related tests
pytest -m git
# Run tests for specific component
pytest tests/unit/services/
# Run with coverage
pytest --cov=src/commit_helper_mcp
```
## Validation Criteria
### Success Metrics
- [x] All tests organized into logical categories
- [x] Shared fixtures eliminate duplication
- [x] Test utilities provide consistent assertions
- [x] Easy to run specific test subsets
- [x] Clear separation between unit/integration/e2e tests
- [x] All existing tests migrated and passing
- [x] Test coverage maintained or improved
### Test Organization Goals
- [x] No test file over 500 lines
- [x] Clear naming conventions
- [x] Consistent test structure
- [x] Minimal test interdependencies
- [x] Fast unit test execution (< 30 seconds)
- [x] Reasonable integration test time (< 2 minutes)
## Testing Plan
### Migration Testing
- Verify all existing tests are migrated
- Ensure no tests are lost in migration
- Validate test coverage is maintained
- Check that all test scenarios still work
### New Structure Testing
- Test that pytest markers work correctly
- Verify shared fixtures function properly
- Ensure test utilities work as expected
- Validate test isolation
## Dependencies
### Prerequisites
- All previous tasks completed
- Understanding of pytest best practices
- Familiarity with test organization patterns
### Affected Files
- All existing test files (migration)
- CI/CD configuration (test commands)
- Documentation (test running instructions)
## Estimated Effort
**Time Estimate**: 15 minutes
**Complexity**: Medium
**Risk Level**: Low (test reorganization, no functional changes)
## Next Steps
After completing this task:
1. Update CI/CD pipelines to use new test structure
2. Update documentation with new test organization
3. Create test writing guidelines for future development
4. Consider adding performance benchmarks
5. Set up test coverage reporting and monitoring
## Completion Notes
The test reorganization task has been successfully completed:
1. All tests have been migrated to the new structure
2. All tests are passing in the new structure
3. Old test files have been removed
4. The cleanup script has been removed as it's no longer needed
5. Documentation has been updated to reflect the new structure
## Benefits
### Immediate Benefits
- **Maintainability**: Easier to find and update tests
- **Performance**: Faster test execution with better organization
- **Reliability**: Reduced test flakiness with better isolation
- **Developer Experience**: Easier to write and run tests
### Long-term Benefits
- **Scalability**: Easy to add new tests as codebase grows
- **Team Development**: Clear patterns for test organization
- **Quality**: Better test coverage and consistency
- **CI/CD**: More efficient test execution in pipelines
