diff --git a/build-mcpb/references/PATTERNS.md b/build-mcpb/references/PATTERNS.md index a8b1f05..b948f99 100644 --- a/build-mcpb/references/PATTERNS.md +++ b/build-mcpb/references/PATTERNS.md @@ -566,77 +566,129 @@ class TestMCPTools: **Key:** FastMCP wraps tool exceptions as `fastmcp.exceptions.ToolError`, not the original type. Always catch `ToolError` in tests. -**Integration tests (`tests-integration/`)** — Requires real API key. Run with `make test-integration`. +**Integration tests (`tests-integration/`)** — Requires real API key in `.env`. Run with `make test-integration`. -`tests-integration/conftest.py` — Gate on env var: +`tests-integration/conftest.py` — The template scaffolds this with an env var gate and client fixture. It loads `.env` automatically via `load_dotenv()`, so the contributor just needs to add their key to `.env` rather than exporting it. + +`tests-integration/test_core_tools.py` — Derive tests from `api_client.py`. Do NOT leave this as a stub or TODO. Write one test class per logical group of methods. + +**Pattern 1 — Read-only method:** +```python +class TestListProjects: + @pytest.mark.asyncio + async def test_list_projects(self, client): + # Chain: get a workspace ID first, then list its projects + workspaces = await client.list_workspaces() + assert len(workspaces) > 0 + workspace_gid = workspaces[0]["gid"] + + result = await client.list_projects(workspace_gid) + assert isinstance(result, dict) + assert "data" in result + print(f"Found {len(result['data'])} project(s)") +``` + +**Pattern 2 — Write method with cleanup:** ```python -def pytest_configure(config): - if not os.environ.get("_API_KEY"): - pytest.exit("ERROR: _API_KEY required for integration tests.") - -@pytest_asyncio.fixture -async def client(api_key: str) -> Client: - client = Client(api_key=api_key) - yield client - await client.close() +class TestContactCRUD: + @pytest.mark.asyncio + async def test_contact_lifecycle(self, client): + contact = None + try: + contact = await client.create_contact( + email=f"test-{int(time.time())}@example.com", + first_name="Integration", + last_name="Test", + ) + assert contact["id"] + + fetched = await client.get_contact(contact["id"]) + assert fetched["id"] == contact["id"] + + updated = await client.update_contact( + contact["id"], first_name="Updated" + ) + assert updated["id"] == contact["id"] + finally: + if contact: + await client.delete_contact(contact["id"]) ``` -`tests-integration/test_core_tools.py` — Tier-skip pattern for plan-gated endpoints: +**Pattern 3 — Tier-gated method:** ```python -async def has_premium_access(client: Client) -> bool: - """Check if the plan supports premium endpoints.""" +async def has_search_access(client, workspace_gid: str) -> bool: try: - await client.premium_method(limit=1) + await client.search_tasks(workspace_gid, text="test", limit=1) return True except APIError as e: - if e.status in (400, 401, 403): + if e.status in (400, 402, 403): return False raise -class TestPremiumFeature: +class TestSearchTasks: @pytest.mark.asyncio - async def test_premium_endpoint(self, client): - if not await has_premium_access(client): - pytest.skip("Premium access not available on current plan") - result = await client.premium_method(limit=5) + async def test_search_tasks(self, client): + workspaces = await client.list_workspaces() + workspace_gid = workspaces[0]["gid"] + + if not await has_search_access(client, workspace_gid): + pytest.skip("Task search requires premium plan") + + result = await client.search_tasks(workspace_gid, text="test", limit=5) assert isinstance(result, list) ``` -**LLM smoke tests (`tests-integration/test_skill_llm.py`)** — Requires `ANTHROPIC_API_KEY`. Run with `make test-llm`. +**Key rules:** +- One test per method or per logical CRUD lifecycle is enough. Don't over-test — you're checking "does the API call work?", not exhaustive coverage. +- Don't test error cases here — unit tests already cover those with mocks. +- Always clean up write operations in `finally` blocks. If no delete method exists, mark resources as completed/archived. +- Use `print()` for visibility — integration tests often run interactively. +- Use timestamped names for created resources (e.g., `f"test-{int(time.time())}@example.com"`) to avoid collisions. + +**LLM smoke tests (`tests-integration/test_skill_llm.py`)** — Requires `ANTHROPIC_API_KEY` in `.env`. Run with `make test-llm`. + +Sends server context (instructions + skill + tools) to Claude Haiku, asserts correct tool selection. The template scaffolds `get_server_context()` and `get_anthropic_client()` — leave those as-is. Replace the commented-out test stub with real tests. + +Write **3–5 tests**, one per key tool. Extract a `call_llm()` helper to avoid repeating the system prompt construction: -Sends server context (instructions + skill + tools) to Claude Haiku, asserts correct tool selection: ```python -async def get_server_context() -> dict: - async with Client(mcp) as client: - init = await client.initialize() - resources = await client.list_resources() - skill_text = "" - for r in resources: - if "skill://" in str(r.uri): - contents = await client.read_resource(str(r.uri)) - skill_text = contents[0].text - tools_list = await client.list_tools() - tools = [{"name": t.name, "description": t.description, "input_schema": t.inputSchema} for t in tools_list] - return {"instructions": init.instructions, "skill": skill_text, "tools": tools} +async def call_llm(prompt: str) -> list: + """Send a prompt to Claude Haiku with full server context, return tool calls.""" + ctx = await get_server_context() + client = get_anthropic_client() + system = ( + f"You are an assistant.\n\n" + f"## Server Instructions\n{ctx['instructions']}\n\n" + f"## Skill Resource\n{ctx['skill']}" + ) + response = client.messages.create( + model="claude-haiku-4-5-20251001", + max_tokens=1024, + system=system, + messages=[{"role": "user", "content": prompt}], + tools=[{"type": "custom", **t} for t in ctx["tools"]], + ) + return [b for b in response.content if b.type == "tool_use"] class TestSkillLLMInvocation: @pytest.mark.asyncio - async def test_query_selects_correct_tool(self): - ctx = await get_server_context() - client = get_anthropic_client() - system = f"You are an assistant.\n\n## Instructions\n{ctx['instructions']}\n\n## Skill\n{ctx['skill']}" - response = client.messages.create( - model="claude-haiku-4-5-20251001", - max_tokens=1024, - system=system, - messages=[{"role": "user", "content": "Your test prompt here"}], - tools=[{"type": "custom", **t} for t in ctx["tools"]], - ) - tool_calls = [b for b in response.content if b.type == "tool_use"] - assert len(tool_calls) > 0 - assert tool_calls[0].name == "expected_tool_name" + async def test_list_projects_selected(self): + tool_calls = await call_llm("Show me all projects in workspace gid_123456") + assert len(tool_calls) > 0, "LLM did not call any tool" + assert tool_calls[0].name == "list_projects" + + @pytest.mark.asyncio + async def test_create_task_selected(self): + tool_calls = await call_llm("Create a task called Review Q3 report in workspace gid_123456") + assert len(tool_calls) > 0, "LLM did not call any tool" + assert tool_calls[0].name == "create_task" ``` +**Key rule — include concrete values for required parameters:** If a tool requires parameters (IDs, coordinates, dates), include concrete values in the prompt — even fake ones. Without them, the LLM will correctly ask for clarification instead of calling the tool, and the test will fail with "LLM did not call any tool." Examples: +- "Show me projects in workspace gid_123456" (not "Show me my projects") +- "What's the weather at lat=51.5, lon=-0.13?" (not "What's the weather in London?") +- "Am I busy tomorrow afternoon?" (not "Am I busy Thursday?" — ambiguous date) + ### Build & Test Commands (Python) ```bash diff --git a/build-mcpb/references/workflow/phase-3-implement-and-verify.md b/build-mcpb/references/workflow/phase-3-implement-and-verify.md index fd7091e..d0454c3 100644 --- a/build-mcpb/references/workflow/phase-3-implement-and-verify.md +++ b/build-mcpb/references/workflow/phase-3-implement-and-verify.md @@ -49,22 +49,17 @@ See `references/PATTERNS.md` → "TypeScript Server Patterns" for complete code Run all checks and fix any issues before proceeding. +### Unit tests and code quality + **Python:** ```bash uv sync --dev make check # format, lint, typecheck, unit tests -make test-integration # real API calls (needs _API_KEY) -make test-llm # LLM smoke tests (needs ANTHROPIC_API_KEY) ``` -The template includes three test layers: -1. **Unit tests** (`tests/`) — Mocked HTTP, FastMCP Client-based tool tests, skill resource tests. Always run. -2. **Integration tests** (`tests-integration/`) — Real API calls with tier-skip helpers for plan-gated endpoints. Run when API key is available. -3. **LLM smoke tests** (`tests-integration/test_skill_llm.py`) — Verify Claude Haiku selects the correct tool given the skill resource. Run when both API key and ANTHROPIC_API_KEY are available. - -At minimum, `make check` must pass (unit tests). Integration and LLM tests are recommended but not blocking for initial release. +Mocked HTTP, FastMCP Client-based tool tests, skill resource tests. The template scaffolds these — fill them in for every tool. `make check` must pass before proceeding. -See `references/PATTERNS.md` → "Test Patterns (Python)" for complete examples including the FastMCP Client pattern, ToolError handling, and tier-skip helpers. +See `references/PATTERNS.md` → "Test Patterns (Python)" for the FastMCP Client pattern, ToolError handling, and mock fixtures. **TypeScript:** ```bash @@ -72,13 +67,64 @@ make check ``` This runs: `format:check` → `lint` → `typecheck` → `test` +### Integration tests + +**Python:** +```bash +make test-integration # real API calls (needs _API_KEY in .env) +``` + +Real API calls against the live service. The template scaffolds `tests-integration/test_core_tools.py` as a stub — you must replace it with real tests. + +**How to write them:** Open `api_client.py` and list every public method (skip `__init__`, `close`, `_request`, `_ensure_session`, and dunder methods). For each method, write a test: + +- **Read methods** (list, get, search): Call with minimal valid parameters, assert the response has the expected shape (list, dict, or model with expected keys). +- **Write methods** (create, update, delete): Create a test resource, verify it, then clean it up in a `finally` block. If no delete method exists, mark the resource as completed/archived and leave a comment. +- **Chained methods:** Some methods need an ID from a prior call (e.g., `list_workspaces` returns a GID needed by `search_tasks`). Chain them — call the list method first, use the first result's ID. +- **Tier-gated methods:** If the API has premium endpoints that may not be available on the user's plan, write a `has__access` helper that probes the endpoint and returns `False` on 401/402/403. Use `pytest.skip()` in the test if access is unavailable. + +See `references/PATTERNS.md` → "Integration Test Patterns (Python)" for concrete examples. + +**How to run them:** + +1. Ask the contributor to add their API key to `.env` (e.g., `ASANA_API_KEY=xxx`). The `.env` file is already in `.gitignore` and `.mcpbignore`. The contributor was asked for this key at the start of the process — they should have it ready. +2. Run `make test-integration`. +3. All tests should pass or skip (for tier-gated features). Fix any failures before proceeding. +4. If the contributor says auth setup is too complex for now (e.g., OAuth flows, multi-step app configuration), proceed — the tests are written and ready to run later. Do not skip writing the tests. + +### LLM smoke tests + +**Python:** +```bash +make test-llm # needs _API_KEY + ANTHROPIC_API_KEY in .env +``` + +Verify Claude Haiku selects the correct tool given the skill resource. Requires both the service API key and `ANTHROPIC_API_KEY`. + +**How to write them:** The template scaffolds `get_server_context()` and `get_anthropic_client()` — leave those as-is. Replace the commented-out test stub with 3–5 real tests, one per key tool. Extract a `call_llm()` helper to avoid repeating the system prompt construction across tests. + +Each test sends a natural language prompt and asserts the LLM selected the expected tool. Include concrete values for any required parameters in the prompt (IDs, coordinates, dates) — without them, the LLM will ask for clarification instead of calling the tool. + +See `references/PATTERNS.md` → "LLM smoke tests" for the `call_llm()` helper pattern and the concrete-identifiers rule. + +**How to run them:** + +1. Ask the contributor to add `ANTHROPIC_API_KEY` to `.env` alongside the service API key. +2. Run `make test-llm`. +3. All tests should pass. If a test fails because the LLM picked the wrong tool, adjust the prompt to be more specific before touching the SKILL.md. +4. If the contributor does not have an `ANTHROPIC_API_KEY`, proceed — the tests are written and ready to run later. Do not skip writing the tests. + ## Gate **Criteria:** - [ ] All tool logic, models, and client code implemented - [ ] Linting passes with no errors - [ ] Type checking passes with no errors -- [ ] All tests pass +- [ ] Unit tests pass (`make check`) +- [ ] Integration tests written with real assertions (not stubs or TODOs) +- [ ] Integration tests pass or skip, if API key is available +- [ ] LLM smoke tests written with real assertions (not stubs or TODOs) +- [ ] LLM smoke tests pass, if `ANTHROPIC_API_KEY` is available **If any criterion fails:** Fix the reported issues and re-run checks.