Skip to content

test: add a route-level test harness and cover the dashboards API contract #64

Description

@xe-nvdk

Context

Raised by the review of #22. The store, the resolver and the access helper have good direct coverage (cross-org isolation, optimistic concurrency, pruning, credential leakage). The HTTP layer has none — find src/routes -name '*.test.ts' returns nothing.

Every wire-contract decision in the dashboards API is currently unverified by a test:

  • DELETE returns 204 with no body, so a client calling res.json() on success throws
  • error bodies are { error }, never { message } — two shapes in one API is the trap _shared.ts was written to avoid
  • 409 carries currentVersion, which is the only thing that lets a save-conflict dialog offer more than "reload"
  • ?expectedVersion= is required, and is parsed with a strict grammar (1e3 must be rejected, not read as 1000)
  • Content-Type: application/json is required by readBoundedBody
  • 403-vs-404: a request for another org's dashboard must be indistinguishable from one for a uid that does not exist
  • the restore route reads no request body — a body-supplied dashboard_uid must be ignored entirely

These were verified by hand against a running build while building #22, which is not a substitute for a test.

Why it needs a harness decision first

src/lib/server/instance.ts cannot be imported under vitest — it reaches $env/dynamic/private through auth.ts. Route handlers import from $lib/server/* and $app/*, and vitest.config.ts resolves neither $env nor $app.

So this is not "add some tests"; it is choosing an approach:

  • Call the exported handlers directly, constructing a Request and a fake locals. No HTTP, no server boot, fast. Needs $env/$app stubbed in vitest.config.ts aliases.
  • Boot the built server and drive it with fetch. Highest fidelity — it exercises SvelteKit's routing, the CSRF check and adapter-node's body handling, all of which have already produced surprises here. Slower, and needs a seeded scratch database.
  • A mix: handler-level for contract shapes, one booted smoke test for the middleware behaviour.

Whichever is chosen should be documented in CONTRIBUTING.md next to the existing "Running tests" section, because every subsequent route in this epic will follow it.

Acceptance criteria

  • A route test harness exists and is documented
  • The contract list above is covered for the dashboards routes
  • The harness does not require data/launchpad.db (the unit suite already pins LAUNCHPAD_DB_PATH=':memory:' in vitest.config.ts)
  • CI runs them

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dashboardsDashboarding and visualizationenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions