test(mamba-mcp-pg): add comprehensive test coverage#6
Merged
Conversation
Add SAP HANA MCP server to README.md and CLAUDE.md with complete package description, architecture details, and repository structure updates. Also document mamba-mcp-fs utility modules (content.py, errors.py).
- Created internal/docs/codebase-analysis.md with comprehensive analysis of all 4 packages, including architecture overview, critical files, patterns, challenges, and recommendations - Updated CLAUDE.md with Key Patterns to Follow (7 core patterns for server packages), Testing Conventions, and Known Inconsistencies to guide future development - Updated README.md with Architecture section explaining the shared FastMCP + layered tool pattern and tech stack
…mba_mcp_hana Resolve module name asymmetry where mamba-mcp-hana was the only package that didn't follow the 1:1 naming convention (it used mamba_mcp_sap_hana with an extra sap_ prefix). All other packages map directly: mamba-mcp-pg → mamba_mcp_pg, mamba-mcp-fs → mamba_mcp_fs. Changes: - Rename src/mamba_mcp_sap_hana/ → src/mamba_mcp_hana/ - Update pyproject.toml entry point and build config - Update all imports across source and test files - Update CLAUDE.md, README.md, and codebase-analysis.md BREAKING CHANGE: Module import path changed from mamba_mcp_sap_hana to mamba_mcp_hana
Create mamba-mcp-core to extract duplicated code from server packages: - cli.py: validate_env_file, resolve_default_env_file, setup_logging - config.py: module-level env file path state management - errors.py: flat ToolError model + create_tool_error factory with server-specific suggestions_map parameter - fuzzy.py: Levenshtein distance + find_similar_names with scaled threshold (max(2, min(len//2, 5))) — standardizes PG vs HANA approach - transport.py: normalize_transport accepting both "http" and "streamable-http" Includes 51 tests covering all modules. Addresses Challenges #3 (code duplication) and #6 (fuzzy matching inconsistency).
Replace duplicated code across all three server packages with shared imports from mamba-mcp-core. This eliminates ~500 lines of copy-pasted CLI utilities, config state management, error models, and fuzzy matching. Changes per server: - PG: core imports for CLI, config, errors, fuzzy; dict return preserved - FS: core imports for CLI, config, transport; exception hierarchy kept - HANA: core imports for CLI, config, errors, fuzzy, transport; removed redundant resolve_env_file(); ToolError return preserved; suggest_similar wrapper maps max_suggestions→max_results Transport pattern updated to accept both "http" and "streamable-http" in all servers, with normalize_transport() applied before mcp.run(). Test updates: - HANA: removed TestLevenshteinDistance (now in core), removed TestResolveEnvFile (now in core), removed _levenshtein_distance import - All existing tests pass (PG: 66, HANA: 778, Core: 51)
…S pattern Change all 11 HANA tool functions from returning `str` (JSON) to returning `OutputModel | dict[str, Any]` to match the PG/FS convention. Tool changes (4 files, 11 tools): - Return type annotations: `-> str` → `-> SpecificOutput | dict[str, Any]` - Success paths: `result.model_dump_json()` → `result` (return model directly) - Error paths: `error.model_dump_json()` → `error.model_dump()` (return dict) - Added output model imports to each tool file Test changes (4 files): - Removed `json.loads(result)` calls throughout - Success assertions: dict key access → model attribute access - Error assertions: `json.loads(result)["code"]` → `result["code"]` - Added `isinstance` checks for result types - Removed unused `import json` statements All 778 HANA tests pass.
…s, server, and models Add 205 new tests across 4 test files to match HANA/FS test coverage: - test_config.py: DatabaseSettings, ServerSettings, nested settings, env file state - test_errors.py: ErrorCode constants, ERROR_SUGGESTIONS completeness, create_tool_error wrapper - test_server.py: FastMCP instance, AppContext dataclass, app_lifespan lifecycle - test_models.py: All Pydantic models with validation constraints, serialization, edge cases
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test coverage for the mamba-mcp-pg package and migrates all servers (PG, FS, HANA) to use the new mamba-mcp-core shared utilities package. The PR increases PG test count from 66 to 271 tests by adding test files for config, errors, server, and models.
Changes:
- Introduces
mamba-mcp-corepackage with shared CLI, config, error handling, fuzzy matching, and transport utilities - Adds 4 new test files to mamba-mcp-pg with 205 tests covering configuration, errors, server lifespan, and Pydantic models
- Migrates PG, FS, and HANA servers to use core utilities, eliminating ~500 lines of duplicated code
- Updates HANA package naming from
mamba_mcp_sap_hanatomamba_mcp_hanafor consistency
Reviewed changes
Copilot reviewed 65 out of 72 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/mamba-mcp-core/* | New shared utilities package with CLI helpers, config state, error models, fuzzy matching, and transport normalization (51 new tests) |
| packages/mamba-mcp-pg/tests/* | New test files for config (49), errors (21), server (7), models (128 claimed) |
| packages/mamba-mcp-pg/src/* | Migration to use mamba-mcp-core for CLI, config, and error utilities |
| packages/mamba-mcp-hana/* | Package rename and migration to mamba-mcp-core (778 test path updates) |
| packages/mamba-mcp-fs/* | Migration to mamba-mcp-core for CLI and config utilities |
| pyproject.toml, uv.lock | Workspace configuration updates for new core package |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,895 @@ | |||
| """Tests for Pydantic models: schema, relationships, and results.""" | |||
There was a problem hiding this comment.
The test file is missing from the diff. According to the PR description, test_models.py should contain 128 tests covering "All Pydantic I/O models with validation constraints, serialization, edge cases". However, the file is not included in the provided diff, making it impossible to verify the test coverage claims.
| async with app_lifespan(mock_server): | ||
| raise RuntimeError("something broke") | ||
|
|
||
| mock_dispose.assert_called_once_with(mock_engine) |
There was a problem hiding this comment.
This statement is unreachable.
This was referenced Feb 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mamba-mcp-pg, matching HANA/FS coverage levelsNew Test Files
test_config.pytest_errors.pytest_server.pytest_models.pyTest plan