[From PR #1075] Split http_router_test.exs by concern

[claude]

Context

Raised during review of PR #1075 (test(http): cover Origin validation matrix for MCP endpoint, merged 2026-06-03).

Issue

mcp_server/test/ptc_runner_mcp/http_router_test.exs is 902 lines / 39 tests, past the 800-line threshold. It has grown to cover multiple orthogonal concerns exercised through the router dispatch path (Router.call/2 in-process): auth, host validation, origin validation, session dispatch/lifecycle, and telemetry.

Note: the file lives at the top level of ptc_runner_mcp/, not in the ptc_runner_mcp/http/ subdirectory (the original suggestion implied otherwise).

Existing structure to align with

A mcp_server/test/ptc_runner_mcp/http/ subdirectory already exists and holds module-level unit tests (distinct from the router dispatch tests in scope here):

• http/host_test.exs — PtcRunnerMcp.Http.HostTest, unit tests for Host.allowed?/2
• http/origin_test.exs — PtcRunnerMcp.Http.OriginTest, unit tests for Origin.allowed?/2
• http/server_smoke_test.exs — PtcRunnerMcp.Http.ServerSmokeTest, boot/integration smoke test

The router-level tests being split out are integration-style (e.g. "GET /mcp with bad Host returns 403 before 405") and exercise the full dispatch chain, so they are genuinely different from the unit tests above. To avoid name collisions and keep the convention, place the split files in http/ with a router_ prefix.

Suggested split

Move tests into mcp_server/test/ptc_runner_mcp/http/:

New file	Module	Tests (current line refs)
router_auth_test.exs	PtcRunnerMcp.Http.RouterAuthTest	401 challenge (L51), health/ready unauth (L80), missing/bad auth (L85), describe "failed auth rate limiting" block (L696–L793), rate-limit-disabled (L795)
router_host_test.exs	PtcRunnerMcp.Http.RouterHostTest	bad Host → 403 before 405 (L57), hostile Host before POST body (L101), hostile Host on DELETE (L317)
router_origin_test.exs	PtcRunnerMcp.Http.RouterOriginTest	invalid Origin → 403 (L68), missing Origin allowed on loopback (L116), invalid browser Origin rejected (L131)
router_dispatch_test.exs	PtcRunnerMcp.Http.RouterDispatchTest	405 + Allow (L45), content-type (L147), session init/exit/DELETE/drain/crash/permit lifecycle, owner indexing, protocol-version validation (L162–L573), plus the 3 telemetry tests (L575, L617, L653) — keep telemetry here or split into a 5th router_telemetry_test.exs if preferred

Key complication — shared setup

The file has a ~20-line setup block (L23–L43) that every test depends on: it stops/starts SessionRegistry, resolves an HttpConfig, seeds Application.put_env(:ptc_runner_mcp, :http_config, cfg), inits/resets ConcurrencyGate, and registers on_exit cleanup for trace/log/session config. Copy-pasting it into 4 files would create drift.

Recommended approach: extract it into an ExUnit.CaseTemplate (e.g. mcp_server/test/support/http_router_case.ex, module PtcRunnerMcp.Http.RouterCase) that the split files use. No CaseTemplate exists in the repo yet, but test/support/ is already a home for shared helpers (mcp_test_helpers.ex, wait_helpers.ex, etc.). The template should also carry the shared @token, aliases, and any private helpers used across concerns (bad_auth_post/0, initialize_session/0, attach_http_telemetry/0, etc.). Move helpers that are concern-specific into their own file.

Constraints

• Keep async: false on every split file: the tests mutate global Application env and a named registry, so they cannot run concurrently.
• This is a mechanical move — no test logic changes. Test count must stay at 39.
• After the move, delete the original http_router_test.exs.

Acceptance Criteria

• http_router_test.exs is removed; its 39 tests live in the new http/router_*_test.exs files.
• Shared setup lives in one ExUnit.CaseTemplate (PtcRunnerMcp.Http.RouterCase); no duplicated setup blocks.
• Each new file is async: false and under the 800-line threshold (each should be well under).
• cd mcp_server && mix test passes with the same number of tests as before.
• mix precommit is green (format, credo, etc.).

Labels

• tech-debt — mechanical split / refactor, no behavior change
• from-pr-review — surfaced during PR review

Automation State

Field	Value
Status	SUCCESS
PR	#1082
Branch	claude/1076-split-http-router-test
Attempts	1

Details: Split http_router_test.exs (902 lines / 39 tests) into five concern files under http/ (router_auth_test.exs, router_host_test.exs, router_origin_test.exs, router_dispatch_test.exs, router_telemetry_test.exs). Shared setup + conn helpers extracted into a new ExUnit.CaseTemplate PtcRunnerMcp.Http.RouterCase (test/support/http_router_case.ex); no duplicated setup. All files async: false. Original deleted. mix test → 39 router tests, 0 failures; mix precommit green.

Next Steps: Review and merge PR #1082.

[claude]

Issue Review: Split http_router_test.exs by concern

Summary: Verified the file is genuinely oversized (902 lines / 39 tests) and the split is a sound, well-scoped mechanical refactor. Sharpened the issue with the correct file path, a concrete test-to-file mapping, and the one non-mechanical wrinkle.

Pre-Review Checks

• Codebase health: file confirmed at mcp_server/test/ptc_runner_mcp/http_router_test.exs (902 lines), single describe block + many top-level tests.
• Higher-priority blockers: none. PR test(http): cover Origin validation matrix for MCP endpoint #1075 that raised this is merged.
• Dependencies: none — independent refactor, no "Blocked by" needed.

What I changed in the issue body

• Corrected the path: the file is at the top level of ptc_runner_mcp/, not in the http/ subdir.
• Flagged the existing http/ subdir: it already holds host_test.exs / origin_test.exs / server_smoke_test.exs, but those are module-level unit tests (Host.allowed?/2, Origin.allowed?/2). The router tests here are integration-style dispatch tests, so they're distinct. Recommended placing the split files in http/ with a router_ prefix (router_auth_test.exs, router_host_test.exs, …) to follow the convention without name collisions.
• Added a test-to-file mapping for all 39 tests with line refs, including a Telemetry group (L575/L617/L653) that can fold into dispatch or split into a 5th file.
• Called out the shared setup block (L23–L43: registry start/stop, Application.put_env, ConcurrencyGate, trace/log cleanup). Every test depends on it, so copy-pasting across 4 files would drift. Recommended extracting it into an ExUnit.CaseTemplate (PtcRunnerMcp.Http.RouterCase in test/support/) — no CaseTemplate exists yet but test/support/ is the established home for shared helpers.
• Constraints: keep async: false (global env + named registry), preserve all 39 tests, delete the original.
• Added acceptance criteria + mix test / mix precommit verification.

Recommendation: ready-for-implementation. Well-defined, unblocked, mechanical. The only judgment call is the ExUnit.CaseTemplate extraction, which the issue now spells out.

[claude]

@claude Please implement this issue.

[andreasronge]

⏳ Queued for implementation at 2026-06-03T10:37:00Z (enqueued by @claude[bot]). The serialized runner processes one issue at a time to avoid conflicting changes; this issue will be picked up automatically.