Skip to content

Tests Phase 2: httptest-mocked HTTP integration + H1/H2 regression guards#3

Merged
schneik80 merged 1 commit into
mainfrom
tests-phase-2
May 2, 2026
Merged

Tests Phase 2: httptest-mocked HTTP integration + H1/H2 regression guards#3
schneik80 merged 1 commit into
mainfrom
tests-phase-2

Conversation

@schneik80
Copy link
Copy Markdown
Owner

Summary

Second of three planned phases bringing this repo's test suite up. Phase 2 covers the L2 layer per the test plan: in-process httptest.Server fakes for the upstream services FusionDataCLI talks to (APS GraphQL, APS OAuth, Fusion MCP JSON-RPC), exercising every network-bound code path without leaving the process.

Coverage jumps

Package Phase 1 Phase 2 Δ
auth 27.6% 73.9% +46pp
api 13.2% 70.9% +58pp
fusion 27.9% 83.8% +56pp
config 90.6% 90.6%
ui 24.5% 24.5% — (Phase 3)
total 23.9% 38.5% +15pp

ui is unchanged because TUI flow tests are Phase 3 territory. The four core packages now average 79.8%.

What's new

Shared scaffolding:

  • internal/testutil/graphql.goGraphQLServer(t, handler) decodes APS GraphQL POSTs and lets handlers return canned {data, errors} envelopes.
  • internal/testutil/mcp.goNewMCPServer(t, scenario) emulates the Fusion MCP JSON-RPC server (initialize handshake, notifications/initialized, tools/call), with probes for InitCount(), CallCount(tool), and SessionIDsSeen() so retry / session-cache behaviour is testable.

Production refactors (minimal, all constvar so tests can swap):

  • api/client.gographqlEndpoint
  • auth/oauth.goauthEndpoint, tokenEndpoint, authScope
  • auth/callback.gocallbackPort, CallbackURL
  • api/download.gouserHomeDir, nowFunc (for deterministic StepDownloadPath)

Security regression coverage

  • H1: TestWaitForCallback_OAuthError asserts the callback response body contains &lt;script&gt; (HTML-escaped) and not the literal <script>... payload — guarding against anyone removing html.EscapeString from the callback handler.
  • H2: TestDownloadFile_Streams asserts the request to the signed-URL server has an empty Authorization header — guarding against anyone re-introducing the bearer leak.

Notable test-design choices

  • OAuth callback tests bind on an ephemeral port (:0) via the new var-overridable callbackPort/CallbackURL, so they don't collide with anything else on the dev machine.
  • TestInvoke_SessionRetryOn404 drives the production session-cache + 404-on-tools/call → drop-cache → re-handshake retry path through NewMCPServer's probes, asserting the retry actually happened (not just that the second call succeeded).
  • Empty-data GraphQL responses use RawBody: \{}`(nodatakey) rather than{"data":""}— the latter decodes to a 2-bytejson.RawMessagethat doesn't trigger the productionlen(gr.Data) == 0` branch.

Found-but-not-fixed

fusion/mcp.go ActiveHubProjects's payload.Success == false guard (lines 168-170) is dead code: parseToolErrorText in callTool catches the same condition first. Safe to delete or leave as defense-in-depth — not in this PR's scope.

Test plan

  • make check (vet + race + tests) passes locally in <5s
  • Coverage report verified per-package — every package above its Phase 1 baseline
  • H1 + H2 regression tests confirmed firing (mutation-tested by inverting the assertion locally)
  • Confirm test.yml workflow runs green on this PR

🤖 Generated with Claude Code

…ards

Second of three planned phases. Phase 2 covers the L2 layer per the test
plan: in-process httptest.Server fakes for the upstream services
FusionDataCLI talks to (APS GraphQL, APS OAuth, Fusion MCP JSON-RPC),
exercising every network-bound code path without ever leaving the
process.

Coverage jumps:
  auth     27.6% -> 73.9%  (+46pp; OAuth exchange/refresh, callback)
  api      13.2% -> 70.9%  (+58pp; gqlQuery, pagination, details,
                             STEP derivative, file download)
  fusion   27.9% -> 83.8%  (+56pp; MCP init+tools/call, session retry,
                             SSE response parsing, stateless mode)
  config   90.6% -> 90.6%  (unchanged; already covered in Phase 1)
  ui       24.5% -> 24.5%  (unchanged; UI flow tests are Phase 3)
  total    23.9% -> 38.5%

New shared test scaffolding:
  internal/testutil/graphql.go  GraphQLServer helper
  internal/testutil/mcp.go      NewMCPServer helper with InitCount,
                                CallCount, SessionIDsSeen probes

Production refactors (minimal, all const->var so tests can swap):
  api/client.go        graphqlEndpoint
  auth/oauth.go        authEndpoint, tokenEndpoint, authScope
  auth/callback.go     callbackPort, CallbackURL
  api/download.go      userHomeDir, nowFunc (for StepDownloadPath)

Security regression coverage added:
  H1  TestWaitForCallback_OAuthError asserts the response body contains
      `&lt;script&gt;` and not the literal `<script>...` payload, so a
      future change that drops html.EscapeString in the callback handler
      will fail loudly.
  H2  TestDownloadFile_Streams asserts the request to the signed-URL
      server has an empty Authorization header, guarding against
      anyone re-introducing the bearer leak.

Notable test design choices:
- The OAuth callback tests bind the listener on an ephemeral port (`:0`)
  via the new var-overridable callbackPort/CallbackURL, so they can run
  in parallel with anything else on the dev machine and never collide
  with the production 7879.
- The fusion session-retry test (TestInvoke_SessionRetryOn404) drives
  the production session-cache + 404-on-tools/call -> drop-cache ->
  re-handshake retry path through the MCPServer helper, which exposes
  InitCount and SessionIDsSeen so the assertions can prove the retry
  actually happened (not just that the second call succeeded).
- Empty-data GraphQL responses are tested via RawBody: `{}` (no `data`
  key) rather than `{"data":""}`, because the JSON string `""` decodes
  to a 2-byte json.RawMessage that does not trigger the production
  len(gr.Data) == 0 branch.

Found-but-not-fixed (will flag separately):
- fusion/mcp.go ActiveHubProjects's payload.Success==false guard
  (lines 168-170) is dead code: parseToolErrorText in callTool catches
  the same condition first and returns its error, so the
  ActiveHubProjects-level check never fires. Safe to delete or leave
  as defense-in-depth; not in this PR's scope.

All tests run in under 5s locally with -race -count=1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@schneik80 schneik80 merged commit 997cffa into main May 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant