feat(tests): module-level integration tests for real route groups (F10) #55

Merged
nrupard merged 2 commits from feat/pms-124-integration-tests into main 2026-06-04 01:50:24 +02:00
Owner

Closes PMS-124. Subtask of PMS-121 (F10).

Background

PMS-121 F10 calls for a tests/ harness covering every real route group, run in CI. Today the repo has zero integration tests at the workspace root (only crates/mokosh-auth/tests/e2e_oidc.rs), and CI runs cargo test --lib only.

The story-level acceptance permits "testcontainers or the dev compose Postgres". This PR takes the #[sqlx::test] + CI service container path, matching the pattern already in use in crates/mokosh-auth/tests/e2e_oidc.rs. testcontainers was rejected as a new heavy dep when an existing in-repo pattern already exists for the same outcome.

Change

Harness (tests/common/mod.rs)

  • boot(pool) builds the PSA router via create_api_router with a stub Google OAuth client, the host crate's LogMailer, and the SSO subsystem deliberately disabled (at_jwt = None, bunyip_verifier = None). It then spawns the router on 127.0.0.1:0 and returns a cookie-aware reqwest client pointed at the assigned URL.
  • seed_admin inserts a super_admin user under the default tenant (id 00000000-0000-0000-0000-000000000001, seeded by migrations/001_initial_schema.sql) with an Argon2-hashed password produced by the host crate's hash_password. It mirrors modules::auth::bootstrap::maybe_bootstrap_admin without touching process env vars (so tests stay parallel-safe).
  • login posts /api/v1/auth/login so the response's Set-Cookie lands in the jar and authenticates subsequent calls.

Tests

  • tests/auth.rs: login -> GET /me reflects the seeded admin.
  • tests/contacts.rs: create company -> list -> get by id -> delete.
  • tests/tenants.rs: GET /tenants for a super_admin contains the seeded default tenant.
  • tests/tickets.rs: seed a company directly, then create ticket -> list -> get -> PUT (title change) -> POST a note.

Host-crate additive change

Database::from_pool(PgPool) so the harness can wrap the per-test pool sqlx hands it. Purely additive; the existing Database::new(url) path is unchanged.

CI (.forgejo/workflows/check.yml)

  • New Postgres 16-alpine service on 5432 with pg_isready health probe.
  • DATABASE_URL=postgres://postgres:postgres@localhost:5432/postgres at job scope.
  • New "Integration tests" step running cargo test --test auth --test contacts --test tenants --test tickets. SQLX_OFFLINE=true stays in effect for compile (uses .sqlx/ cache); runtime sqlx calls in the integration tests still hit the live Postgres service.
  • The existing "Unit tests: cargo test --lib" step stays in place; it covers the lib's pure-unit tests that need no DB.

Out of scope

  • A shared filter.validate()? helper (was the implicit suggestion in the parent story for PMS-123; the existing inline ? pattern is fine).
  • Coverage of every endpoint - the ticket text was explicit about "one #[tokio::test] per route group". Per-endpoint coverage can grow inside each test file later.

Risk notes

  • I do not have local cargo here; src/bins compile-check passed via just check-docker, but the test code (and the new CI Postgres service config) is validated only by CI. Watch the first run.
  • If the Forgejo runner does not expose the Postgres service on localhost, the integration step will time out at pg_isready. Fix would be to switch DATABASE_URL host to the service name (postgres) per Forgejo's container-runner semantics.
Closes PMS-124. Subtask of PMS-121 (F10). ## Background PMS-121 F10 calls for a `tests/` harness covering every real route group, run in CI. Today the repo has zero integration tests at the workspace root (only `crates/mokosh-auth/tests/e2e_oidc.rs`), and CI runs `cargo test --lib` only. The story-level acceptance permits "testcontainers or the dev compose Postgres". This PR takes the `#[sqlx::test]` + CI service container path, matching the pattern already in use in `crates/mokosh-auth/tests/e2e_oidc.rs`. testcontainers was rejected as a new heavy dep when an existing in-repo pattern already exists for the same outcome. ## Change ### Harness (`tests/common/mod.rs`) - `boot(pool)` builds the PSA router via `create_api_router` with a stub Google OAuth client, the host crate's `LogMailer`, and the SSO subsystem deliberately disabled (`at_jwt = None`, `bunyip_verifier = None`). It then spawns the router on `127.0.0.1:0` and returns a cookie-aware reqwest client pointed at the assigned URL. - `seed_admin` inserts a `super_admin` user under the default tenant (id `00000000-0000-0000-0000-000000000001`, seeded by `migrations/001_initial_schema.sql`) with an Argon2-hashed password produced by the host crate's `hash_password`. It mirrors `modules::auth::bootstrap::maybe_bootstrap_admin` without touching process env vars (so tests stay parallel-safe). - `login` posts `/api/v1/auth/login` so the response's `Set-Cookie` lands in the jar and authenticates subsequent calls. ### Tests - `tests/auth.rs`: login -> GET `/me` reflects the seeded admin. - `tests/contacts.rs`: create company -> list -> get by id -> delete. - `tests/tenants.rs`: GET `/tenants` for a super_admin contains the seeded default tenant. - `tests/tickets.rs`: seed a company directly, then create ticket -> list -> get -> PUT (title change) -> POST a note. ### Host-crate additive change `Database::from_pool(PgPool)` so the harness can wrap the per-test pool sqlx hands it. Purely additive; the existing `Database::new(url)` path is unchanged. ### CI (`.forgejo/workflows/check.yml`) - New Postgres 16-alpine service on 5432 with `pg_isready` health probe. - `DATABASE_URL=postgres://postgres:postgres@localhost:5432/postgres` at job scope. - New "Integration tests" step running `cargo test --test auth --test contacts --test tenants --test tickets`. `SQLX_OFFLINE=true` stays in effect for compile (uses `.sqlx/` cache); runtime sqlx calls in the integration tests still hit the live Postgres service. - The existing "Unit tests: cargo test --lib" step stays in place; it covers the lib's pure-unit tests that need no DB. ## Out of scope - A shared `filter.validate()?` helper (was the implicit suggestion in the parent story for PMS-123; the existing inline `?` pattern is fine). - Coverage of every endpoint - the ticket text was explicit about "one `#[tokio::test]` per route group". Per-endpoint coverage can grow inside each test file later. ## Risk notes - I do not have local cargo here; src/bins compile-check passed via `just check-docker`, but the test code (and the new CI Postgres service config) is validated only by CI. Watch the first run. - If the Forgejo runner does not expose the Postgres service on `localhost`, the integration step will time out at `pg_isready`. Fix would be to switch `DATABASE_URL` host to the service name (`postgres`) per Forgejo's container-runner semantics.
feat(tests): module-level integration tests for real route groups (F10)
Some checks failed
Check / fmt + clippy + compile + tests (pull_request) Failing after 26s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m3s
d172c15607
PMS-124. Stand up a `tests/` harness that uses `#[sqlx::test]` to provision a fresh database per test invocation (matching the existing pattern in `crates/mokosh-auth/tests/e2e_oidc.rs`) and exercises the four real PSA route groups end-to-end through reqwest against a real TCP listener.

Harness (`tests/common/mod.rs`):
- `boot(pool)` builds the PSA router via `create_api_router` with a stub Google OAuth client, `LogMailer`, and the SSO subsystem deliberately disabled (`at_jwt = None`, `bunyip_verifier = None`); the tests cover the legacy HS256-cookie auth path that the PSA endpoints still take. Spawns the router on `127.0.0.1:0` and returns the assigned URL plus a cookie-aware reqwest client.
- `seed_admin` inserts a `super_admin` user under the default tenant (id `00000000-0000-0000-0000-000000000001`, seeded by `001_initial_schema.sql`) with an Argon2-hashed password produced by the host crate's `hash_password`, mirroring `modules::auth::bootstrap::maybe_bootstrap_admin` without depending on process env vars.
- `login` posts `/api/v1/auth/login` and lets the cookie jar carry the resulting session cookie into subsequent requests.

Tests:
- `tests/auth.rs`: login -> GET `/me` reflects the seeded admin.
- `tests/contacts.rs`: create company -> list (asserts presence) -> get by id -> delete.
- `tests/tenants.rs`: list `/tenants` for a super_admin contains the seeded default tenant.
- `tests/tickets.rs`: seed a company directly, then create ticket -> list -> get -> PUT (title change) -> POST a note.

Cargo: dev-dep on `reqwest` with the `cookies` feature for cookie-jar handling, and `sqlx` with the `macros` feature for `#[sqlx::test]`.

Host crate: add `Database::from_pool(PgPool)` so the harness can wrap the per-test pool sqlx hands it. Purely additive; the existing `Database::new(url)` path stays unchanged.

CI (`.forgejo/workflows/check.yml`): add a Postgres 16 service on port 5432, set `DATABASE_URL=postgres://postgres:postgres@localhost:5432/postgres`, and add a new "Integration tests" step that runs `cargo test --test auth --test contacts --test tenants --test tickets`. The existing `cargo test --lib` step stays for the unit tests that need no database.

This closes the F10 acceptance criterion on PMS-121 ("A `tests/` harness runs integration tests for all real route groups in CI"). The story-level allowance of "testcontainers or the dev compose Postgres" is taken via `#[sqlx::test]` plus a CI service container, which keeps the test toolchain consistent with the auth crate's existing `e2e_oidc.rs`.

#PMS-124

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
fix(tests): authenticate via Authorization: Bearer, not a cookie jar
Some checks failed
Check / fmt + clippy + compile + tests (pull_request) Failing after 23s
Build OCI container / Build and push mokosh-api image (push) Successful in 3m2s
Create release / Create release from merged PR (pull_request) Has been skipped
806312fb3e
Self-review of #55 caught that the legacy `AuthMiddleware` reads `Authorization: Bearer <token>` from the request headers (`src/modules/auth/middleware.rs:123-126`) - the `/login` handler returns the JWT in the `LoginResponse` JSON body, NOT as a `Set-Cookie`. The first iteration's reqwest cookie jar approach would have failed `/me` with 401 on the very first integration run.

Replace the cookie-jar setup with bearer-token plumbing:

- `common::login` now parses the `access_token` field out of the login JSON body and returns it as a `String`. The caller stores it and attaches it via reqwest's `.bearer_auth(&token)` builder on every subsequent request.
- All four tests (`auth`, `contacts`, `tenants`, `tickets`) thread the token through their HTTP calls. `tests/auth.rs` now asserts that `/me` succeeds when the token is attached.
- Drop the `cookies` feature from the dev-dep reqwest declaration; feature unification would have leaked it into the main build for no purpose.
- The `cookie_secure` argument to `create_api_router` is still passed as `false` in the test boot - cookies aren't on the test path at all, but the value is recorded as part of the route construction.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

#PMS-124
nrupard deleted branch feat/pms-124-integration-tests 2026-06-04 01:50:24 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/mokosh-server!55
No description provided.