test(time): characterize serve() through the protocol, per-file coverage

.claude/skills/testing/SKILL.md

@@ -18,16 +18,16 @@ Driving a running server by hand, with the Inspector or an LLM client, is
 
 ⚠️ **The TypeScript and Python harnesses differ.** All four TypeScript servers
 are tested in-process, a real `Client` linked to the server over an in-memory
-transport (#4854). The Python servers' tests call their functions directly,
-and those servers have no coverage gate yet (#4855).
+transport (#4854). Of the Python servers, `time` drives `serve()` in-process;
+`fetch` and `git` call their functions directly. No Python server has a coverage
+gate yet (#4855).
 
 ## Where a test goes
 
 | Server | Directory | File name | Runner |
 | --- | --- | --- | --- |
 | `everything`, `filesystem`, `memory`, `sequentialthinking` | `src/<server>/__tests__/` | `<subject>.test.ts` | vitest, `globals: true` |
-| `fetch`, `git` | `src/<server>/tests/` | `test_<subject>.py` | pytest |
-| `time` | `src/time/test/` | `<subject>_test.py` | pytest |
+| `fetch`, `git`, `time` | `src/<server>/tests/` | `test_<subject>.py` | pytest |
 | Root tooling | `scripts/`, beside the script | `<name>.test.mjs` | `node --test` |
 
 Each vitest config includes only `**/__tests__/**/*.test.ts`, so a test file
@@ -41,9 +41,10 @@ connection harness does (`__tests__/harness.ts` in `everything`,
 | Server | How its tests reach the code |
 | --- | --- |
 | `everything`, `filesystem`, `memory`, `sequentialthinking` | An SDK `Client` connected to the server's `createServer()` over `InMemoryTransport`, in the vitest process, through the server's own harness, plus unit tests of helper modules such as `lib.ts`. `everything` drives its stdio, SSE and Streamable HTTP transports in-process too; the other three have one thin spawn smoke of the built `dist/index.js` |
-| `fetch`, `git`, `time` | Direct calls on the functions in `server.py`, with `unittest.mock` |
+| `fetch`, `git` | Direct calls on the functions in `server.py`, with `unittest.mock` |
+| `time` | A `ClientSession` against `serve()` in-process, over in-memory streams (`tests/test_protocol.py`); direct calls on the helpers in `server.py` (`tests/test_server.py`) |
 
-No Python test opens a `ClientSession` yet.
+Of the Python servers, only `time` opens a `ClientSession` in its tests.
 
 ## Choosing a harness for a new test
 
@@ -165,8 +166,11 @@ like).
 ### Python: `ClientSession` over stdio
 
 Each Python server constructs its `Server` inside `serve()`, so it cannot be
-handed to an in-memory session. A protocol-level test spawns the module and
-talks to it through the SDK's stdio client:
+handed to an in-memory session directly. `time` drives it in-process anyway, by
+patching `stdio_server` to yield the server side of
+`create_client_server_memory_streams()`; see the `serving()` helper in
+`src/time/tests/test_protocol.py`. The other route spawns the module and talks
+to it through the SDK's stdio client:
 
 ```python
 import sys
@@ -196,8 +200,8 @@ def test_get_current_time_over_stdio() -> None:
 and the install there is editable, so this runs your edit with no build step.
 
 How an async test is written differs by server, because the dev dependencies
-do: `fetch` has `pytest-asyncio` with `asyncio_mode = "auto"`, so an
-`async def test_…` just works. `git` and `time` do not have it; wrap the
+do: `fetch` and `time` have `pytest-asyncio` with `asyncio_mode = "auto"`,
+so an `async def test_…` just works. `git` does not have it; wrap the
 coroutine in `anyio.run(...)` as above (`git`'s `serve()` test does), rather
 than adding a dependency for one test.
 
@@ -236,7 +240,7 @@ In Python the same text follows `#`. A bug with no issue yet reads
 | One TypeScript server's coverage gate | `npm run coverage -w src/<server>` (see below) |
 | One TypeScript server's whole chain | `npm run validate -w src/<server>` (format check, lint, typecheck, build, test) |
 | One Python server | `uv run pytest` in `src/<server>` |
-| One Python test | `uv run pytest <path to the test file>::<name>` in `src/<server>` (for example `tests/test_server.py::test_git_checkout_existing_branch`, or `test/time_server_test.py::…` in `time`) |
+| One Python test | `uv run pytest <path to the test file>::<name>` in `src/<server>` (for example `tests/test_server.py::test_git_checkout_existing_branch`) |
 | One Python server's whole chain | `npm run validate:py -- <server>` |
 | Root tooling | `npm run test:scripts` |
 

src/time/.gitignore

@@ -0,0 +1,5 @@
+# Coverage output from the per-server coverage command (see README,
+# "Development"); generated locally and never committed.
+coverage.json
+.coverage
+.coverage.*

src/time/README.md

@@ -274,6 +274,31 @@ npx @modelcontextprotocol/inspector uv run mcp-server-time
 3. "When it's 4 PM in New York, what time is it in London?"
 4. "Convert 9:30 AM Tokyo time to New York time"
 
+## Development
+
+Run the tests from `src/time`. They live in `tests/`: `test_protocol.py` drives
+the server through an MCP `ClientSession` in-process, `test_entrypoints.py`
+covers `main()`, `python -m mcp_server_time` and the console script, and
+`test_server.py` unit-tests the time helpers.
+
+```bash
+cd src/time
+uv run pytest
+```
+
+Coverage is a separate command. It measures line and branch coverage of
+`src/mcp_server_time`, prints the missing lines, and writes `coverage.json`
+(ignored by git):
+
+```bash
+cd src/time
+uv run --frozen pytest --cov --cov-report=term-missing --cov-report=json
+```
+
+Every file must reach 90% of its lines and 90% of its branches. Code that
+cannot be reached is marked `# pragma: no cover  # <reason>`, always with the
+reason, rather than lowering the bar.
+
 ## Build
 
 Docker build:

src/time/pyproject.toml

@@ -39,5 +39,18 @@ dev-dependencies = [
     "freezegun>=1.5.1",
     "pyright>=1.1.389",
     "pytest>=8.3.3",
+    "pytest-asyncio>=1.4.0",
+    "pytest-cov>=7.1.0",
     "ruff>=0.8.1",
 ]
+
+[tool.pytest.ini_options]
+testpaths = ["tests"]
+asyncio_mode = "auto"
+
+[tool.coverage.run]
+branch = true
+source = ["src/mcp_server_time"]
+
+[tool.coverage.report]
+show_missing = true

src/time/src/mcp_server_time/server.py

@@ -208,7 +208,9 @@ async def call_tool(
                         k in arguments
                         for k in ["source_timezone", "time", "target_timezone"]
                     ):
-                        raise ValueError("Missing required arguments")
+                        raise ValueError(  # pragma: no cover  # unreachable: the SDK rejects a missing key against inputSchema first
+                            "Missing required arguments"
+                        )
 
                     result = time_server.convert_time(
                         arguments["source_timezone"],

src/time/tests/test_entrypoints.py

@@ -0,0 +1,116 @@
+# Tests for mcp-server-time's entry points (#4855): `main()` in `__init__.py`,
+# `python -m mcp_server_time` (`__main__.py`), and the `mcp-server-time`
+# console script.
+#
+# `main()` and both `__main__` paths run in-process with `serve` patched out,
+# so coverage measures them and nothing reads real stdin. The console script
+# gets the one thin subprocess smoke test: it is the only way to prove the
+# installed entry point boots and speaks MCP over real stdio.
+
+import os
+import runpy
+import shutil
+import sys
+from pathlib import Path
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from mcp import ClientSession, StdioServerParameters
+from mcp.client.stdio import stdio_client
+
+import mcp_server_time
+
+
+@pytest.mark.parametrize(
+    "argv,expected_local_timezone",
+    [
+        ([], None),
+        (["--local-timezone", "Europe/Paris"], "Europe/Paris"),
+        (["--local-timezone=America/Chicago"], "America/Chicago"),
+    ],
+)
+def test_main_passes_local_timezone_to_serve(
+    argv: list[str], expected_local_timezone: str | None
+) -> None:
+    serve = AsyncMock()
+    with (
+        patch.object(mcp_server_time, "serve", serve),
+        patch.object(sys, "argv", ["mcp-server-time", *argv]),
+    ):
+        mcp_server_time.main()
+    serve.assert_awaited_once_with(expected_local_timezone)
+
+
+def test_main_help_describes_the_server(capsys: pytest.CaptureFixture[str]) -> None:
+    serve = AsyncMock()
+    with (
+        patch.object(mcp_server_time, "serve", serve),
+        patch.object(sys, "argv", ["mcp-server-time", "--help"]),
+        pytest.raises(SystemExit) as exit_info,
+    ):
+        mcp_server_time.main()
+    assert exit_info.value.code == 0
+    out = capsys.readouterr().out
+    assert (
+        "give a model the ability to handle time queries and timezone conversions"
+        in out
+    )
+    assert "--local-timezone" in out
+    serve.assert_not_awaited()
+
+
+def test_main_rejects_unknown_arguments(capsys: pytest.CaptureFixture[str]) -> None:
+    serve = AsyncMock()
+    with (
+        patch.object(mcp_server_time, "serve", serve),
+        patch.object(sys, "argv", ["mcp-server-time", "--bogus"]),
+        pytest.raises(SystemExit) as exit_info,
+    ):
+        mcp_server_time.main()
+    assert exit_info.value.code == 2
+    assert "unrecognized arguments: --bogus" in capsys.readouterr().err
+    serve.assert_not_awaited()
+
+
+def test_python_dash_m_calls_main() -> None:
+    # What `python -m mcp_server_time` executes.
+    main = MagicMock()
+    with patch.object(mcp_server_time, "main", main):
+        runpy.run_module("mcp_server_time", run_name="__main__")
+    main.assert_called_once_with()
+
+
+def test_init_module_run_as_main_calls_main() -> None:
+    # `__init__.py` carries its own `if __name__ == "__main__"` guard. Running
+    # it as a module re-executes it in a fresh namespace, which imports
+    # `serve` from `.server` again, so the patch goes there.
+    serve = AsyncMock()
+    with (
+        patch("mcp_server_time.server.serve", serve),
+        patch.object(sys, "argv", ["mcp-server-time", "--local-timezone", "UTC"]),
+    ):
+        runpy.run_module("mcp_server_time.__init__", run_name="__main__")
+    serve.assert_awaited_once_with("UTC")
+
+
+async def test_console_script_boots_over_stdio() -> None:
+    script = shutil.which("mcp-server-time", path=str(Path(sys.executable).parent))
+    assert script is not None, "the console script is not installed in this venv"
+    params = StdioServerParameters(
+        command=script,
+        args=["--local-timezone", "UTC"],
+        env={**os.environ},
+    )
+    with open(os.devnull, "w") as errlog:
+        async with stdio_client(params, errlog=errlog) as (read, write):
+            async with ClientSession(read, write) as session:
+                init = await session.initialize()
+                tools = await session.list_tools()
+                result = await session.call_tool(
+                    "get_current_time", {"timezone": "UTC"}
+                )
+    assert init.model_dump(by_alias=True, mode="json")["serverInfo"]["name"] == (
+        "mcp-time"
+    )
+    assert [tool.name for tool in tools.tools] == ["get_current_time", "convert_time"]
+    assert result.model_dump(by_alias=True, mode="json")["isError"] is False