Skip to content

Organise the tests, split the app by concern, and lint the scripts - #45

Merged
botre merged 2 commits into
masterfrom
chore/tests-modularity-lint
Aug 8, 2026
Merged

Organise the tests, split the app by concern, and lint the scripts#45
botre merged 2 commits into
masterfrom
chore/tests-modularity-lint

Conversation

@botre

@botre botre commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

A pass over tests, structure, linting and prose, plus three fixes the QA tour turned up.

Tests are organised, then filled in

src was one package with application_test.go holding several unrelated
concerns. Tests now sit beside the file they cover, named for the unit under
test with subtests that state a behaviour:

File Covers
endpoint_test.go endpoint IDs and URLs
platform_test.go client IP, header stripping, proxy trust
capture_test.go capture header handling
assets_test.go content-hashed asset URLs
security_test.go the CSP
requestlog_test.go correlation IDs
sockets_test.go the subscriber registry
routes_test.go every route, end to end
logging/logging_test.go redaction, levels, request-ID stamping

The gaps that mattered were the security-critical ones with no coverage at all:
validEndpointID, resolveClientIP, omitHeader, the CSP's environment gate
and the inbound request-ID pattern. Go coverage of src goes from partial to
81%, logging from 0 to 88%.

The Playwright suite grows from 26 to 59 tests, grouped by feature area
(Page / Capture stream / Body rendering / Filtering / Copying / Sending / Tab
indicator) behind a shared tests/support/harness.ts. New ground: the render
window, the connection indicator, query-string display and search, escaped HTML
bodies, HAR omissions, contact-form semantics.

Structure

application.go was 672 lines. It is now one file per subject —
platform, endpoint, capture, api, pages, assets, security,
sockets — and newApplication builds the whole routing surface from
arguments, so the route tests drive real requests without a listening socket.
requireValidEndpoint replaces the validation repeated in six handlers, and
pageMeta replaces the head fields repeated per page.

On the front end, .panel and .panel-flat were byte-identical while a
comment claimed they differed; there is now one surface class. .field-label
and .region-label share one type token, and the three places that re-spelled
that token as a utility string use the class. copy and copyHar share their
flash-and-announce logic.

Lint and types

There was no lint setup, so "zero warnings" was unverifiable. Added a flat
ESLint config covering public/*.js (browser globals, no build step) and
e2e/**/*.ts with type-aware rules, so an implicit any reaching an
assertion is an error. Wired into CI alongside tsc --noEmit and go vet.

Current state, all verified rather than assumed:

  • ESLint errors and warnings: 0
  • eslint-disable: 0
  • @ts-ignore / @ts-expect-error: 0
  • any: 0. One type assertion remains, at the JSON.parse clipboard
    boundary, confined to a single helper and documented there.

Go commands are scoped to ./src/...: node_modules ships a stray Go package
that ./... otherwise builds.

Prose

No misspellings turned up across comments, markdown and UI copy. The comment
tour removed the bug war stories AGENTS.md prohibits (the flicker story, the
click-swallowing header, the select desync) in favour of the constraint each
one protects, corrected har.js's claim that it loads on every page, and
generalised two hyper-specific figures. AGENTS.md gains a code-layout
section for the new file split.

Three fixes from the QA tour

Every response carries its security headers. The static middleware resets
the response when a path resolves to a directory, which silently dropped the
CSP, X-Frame-Options, nosniff, Referrer-Policy and X-Request-Id from
the landing page. Confirmed against a production build of master:

$ curl -sD- -o/dev/null http://localhost:8099/       # master, production
(no security headers at all)
$ curl -sD- -o/dev/null http://localhost:8099/contact
Content-Security-Policy: default-src 'self'; ...

They are now stamped on the way out, where nothing downstream can discard them.

Controls that act on a whole endpoint report the endpoint. The listing is
searched server-side and windowed at 128, so its length is not the endpoint's.
With a search active, the destructive Delete all (N) understated what it
would delete, and the filtered-empty panel said "0 captured on this endpoint"
while three were. The listing now carries a total.

Counted copy reads correctly at one — "Deleted 1 request", not
"1 requests".

QA

Full tour in Chrome plus direct API calls, in development and against the
production container. Landing, contact, endpoint and 404 pages; endpoint
creation; ten capture shapes driven by curl (JSON, XML, text, multipart with a
file, HTML, query strings, every method, infrastructure headers, curl
spoofing); live arrival over WebSocket; search, method filter, the render
window, every copy control, single and bulk delete with confirmation, the send
panel; socket loss and reconnect with the gap-closing refetch. No console
errors. Backend: status codes for malformed endpoint IDs, request-ID echo and
minting, asset hashes tracking content in both modes, the 1 MiB body limit, and
the 125/min production rate limit firing.

Known issue, not fixed here

A request whose body exceeds 1 MiB returns 413 but still stores a capture with
an empty body, beside a Content-Length header saying otherwise — a card
that reads "Body: None" for a payload the client did send. It is pre-existing
and unrelated to this change. I left it alone because a correct fix needs care
around fasthttp's body-read semantics across chunked encoding and
Expect: 100-continue, and getting it wrong would drop legitimate captures.
Worth its own change.

🤖 Generated with Claude Code

botre added 2 commits August 8, 2026 19:42
src was one 672-line file holding routing, capture, platform, asset and
security concerns together, which left most of it reachable only through a
listening socket. It is now one file per subject with its tests beside it, and
newApplication builds the whole routing surface from arguments so tests drive
real requests through it.

Tests are grouped by the unit under test with behaviour-stating subtests, and
the Playwright suite is grouped by feature area behind a shared harness. Go
coverage moves from partial to 81% of src, and the suite covers endpoint-ID
validation, client-IP resolution, header stripping, asset versioning, the CSP
environment gate, request-ID handling and every route.

ESLint now covers the page scripts and the Playwright suite, the latter with
type information so an implicit any is an error. The Go commands are scoped to
./src because node_modules ships a stray Go package.

Three fixes fall out of this:

- Every response now carries its security headers. The static middleware
  resets the response when a path resolves to a directory, which silently
  dropped the CSP, X-Frame-Options, nosniff, Referrer-Policy and X-Request-Id
  from the landing page. They are stamped on the way out, where nothing
  downstream can discard them.
- Controls that act on a whole endpoint report the endpoint. The listing is
  searched server-side and windowed, so its length is not the endpoint's; with
  a search active the destructive "Delete all (N)" understated what it would
  delete, and the filtered-empty panel claimed nothing had been captured. The
  listing now carries a total.
- Counted copy reads correctly at one: "1 request", not "1 requests".

Claude-Session: https://claude.ai/code/session_01XRYoAwHndAhq2w7RvxSGoo
@botre
botre merged commit 5e928ae into master Aug 8, 2026
4 checks passed
@botre
botre deleted the chore/tests-modularity-lint branch August 8, 2026 17:49
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