Browser test suite: strategy review and follow-ups

[irees]

Captured while reviewing the overall structure of test/browser/ after adding the #239 report-frequency suite. The bones of the suite are sound — fixture-based, text/label-first locators, shared helpers — but upkeep discipline and scaffolding have lagged. This issue gathers the observations so they can be tackled incrementally.

Inventory (at time of writing)

• smoke.test.ts — 3 tests, static page-load
• query.test.ts — 6 tests, run-a-query happy path (3 stale, see below)
• filters.test.ts — 4 tests, filter interactions & count deltas
• choropleth.test.ts — 7 tests, aggregation overlay UI
• report-frequency.test.ts — 15 tests, Revise calculations for frequency/visits calculations in tabular reports #239 report columns (new)
• map-legend.test.ts — 2 tests, Revise calculations for frequency/visits calculations in tabular reports #239 mode-aware legend (new)
• helpers.ts — shared utilities

Every file boots off testdata/gtfs/calact_tlserver.dump via a local tlserver + the downtown Portland bbox.

Strengths to preserve

• One fixture, real protocol. Every test exercises the real proxy → real GraphQL → real renderer. No mocks, hermetic, reproducible.
• Text/label-first locators. getByRole, getByLabel, getByText dominate; CSS selectors appear only where needed. Ages well against markup churn.
• Helpers encode UI semantics. openFilterSubtab, resetFilters, ensureFilterPanelOpen, enableAllDay/disableAllDay capture non-obvious behaviors (e.g. the outer Filter tab being a toggle) so each new test author doesn't rediscover them.
• Shape-not-magnitude assertions for time-sensitive values. The Revise calculations for frequency/visits calculations in tabular reports #239 file asserts MM:SS / HH:MM / non-negative integer — exact arithmetic lives in Vitest against synthetic fixtures. Good split.

Weaknesses

1. No documented independence convention. Files are mixed: query.test.ts is strictly sequential, filters.test.ts is strictly independent, choropleth.test.ts drifts, the new Revise calculations for frequency/visits calculations in tabular reports #239 files are independent. Readers have to figure out per-file which style is in play.

2. query.test.ts has stale tests. "Showing fixed-route service by:", the Route radio, and Individual results no longer exist in the UI. The 3 tests asserting on them silently failed pre-auth-fix (because the scenario fetch 401'd first); now that the data-loading hook works, the staleness is visible. They actively erode trust.

3. Per-file scenario load is the dominant cost. Six files × ~30s scenario boot = ~3 minutes before any assertions, with workers: 1 and no cross-file sharing.

4. waitForScenarioLoad hooks on a toast. The "Browsing query data loaded successfully" notification is a UX side effect, not a functional completion signal. If the toast is removed or renamed, every integration test breaks silently.

5. Big holes in surface coverage.

   • CSV and GeoJSON download (the feature the tool ships)
   • URL share / permalink round-trip (scenario config is in the URL)
   • Filter ↔ report propagation (does a filter change re-render the report tables?)
   • Flex services — untested UI-side
   • WSDOT / VisionEval analyses — ignored
   • Auth-gated production mode — only the requireLogin=false path runs

6. Fixture data is undocumented. Tests lean on the Portland bbox having "a mix of modes and agencies", but what's actually in calact_tlserver.dump? Which agencies, which route types, what date range, any cross-midnight service? When a test fails, debugging is guesswork.

7. No visible CI integration. pnpm test:browser is in package.json but no GitHub Actions workflow runs it. If these tests only run when a developer remembers to, that's most of why query.test.ts rotted.

8. Mixed test-name register. query.test.ts names are task-level ("map shows legend and share button"); the Revise calculations for frequency/visits calculations in tabular reports #239 file uses mechanism-level ("Routes (all-day) Slowest Frequency tooltip preserves the cross-service-day exclusion"). Both defensible, but inconsistency hurts triage.

9. Locator class coupling. .cal-report .cat-tabs couples tests to both an app class and a Catenary class; renames in either repo break tests.

10. No visual/screenshot regression. Pure DOM/text assertions. The choropleth magnitude shift called out in the Revise calculations for frequency/visits calculations in tabular reports #239 plan (values ~5–30× larger) is exactly the kind of thing a screenshot diff would catch.

Prioritized follow-ups

Small

• Fix or delete the 3 stale query.test.ts assertions ("Showing fixed-route service by:", Route radio, Individual results).
• Add test/browser/README.md documenting the independence convention and the shape-not-magnitude philosophy.
• Add a functional-signal fallback to waitForScenarioLoad alongside the toast check (e.g. .cal-report-table presence, non-zero route count).
• Write a short testdata/gtfs/FIXTURE.md enumerating the Portland dump's known properties — agencies, modes, date range, anything tests depend on.

Medium

• CI workflow that boots tlserver from the dump and runs pnpm test:browser on PRs.
• Playwright test fixture (test.extend) that shares a loaded scenario across tests in a worker — cuts suite time ~6× for files that don't mutate state.
• One test per download path (CSV + GeoJSON), verifying content.
• URL share / reload round-trip test.

Large

• Smoke vs. full split: test:browser:smoke (no scenario load, asserts labels/DOM only) vs. test:browser (full). Keeps PR runs under a minute.
• Screenshot regression for map/legend surfaces where textual assertions are awkward.
• Coverage for WSDOT / VisionEval flows.

Not in scope for this issue

• Mock-based testing (MSW / GraphQL mocking). Worth exploring eventually but a much bigger investment than the follow-ups above.
• Replacing Playwright. The framework isn't the problem.

Context: analysis produced during #328 (revise frequency calculations) after the suite was expanded with #239 coverage. #333 (merged) fixed the underlying 401-on-unauth issue that had been papering over the query.test.ts staleness.