fix(tests): make green mean green — repair test:security and env-only baseline failures - #2338
Conversation
… baseline failures test:security had 6 known-failing suites on every clean checkout, and four env-only local failures forced agents to memorize a failure baseline. This eliminates all of them so a red check always means a regression. test:security (now 51/51): - Retire "Login Route"/"Signup Route"/"Mobile Login" — their test files were deleted with password auth in PR #861; repoint to the surfaces that replaced them (magic-link, passkey, passkey signup, mobile OAuth exchange/refresh) using directory-prefix filters so new tests are picked up automatically. - Route "Session Service"/"Device Auth Utilities"/"Permissions" through the DB-aware test:db config (they are deliberately excluded from the default lib vitest config, so plain `test` found no files and exited 1). - Make the script self-provision the dockerized test DB (reuse a running or stopped shared container, start it via compose otherwise, migrate), so a clean checkout goes green without ambient setup. Env-only failures: - processor magika suites: Node >= 23 removed util.isNullOrUndefined, which tfjs-node 4.x still calls — restore the predicate in a vitest setup shim so the real model runs on every Node version (the model assets were never missing). - admin-role-version/activity-tools: fail fast with an actionable "test DB not provisioned" message via a shared ensureTestDb() probe instead of a cryptic drizzle "Failed query" wall. - grouping.test.ts: pin TZ=UTC (fixtures are UTC but grouping compares local calendar days; the midnight test flipped west of UTC) and restore the original TZ afterwards. - MachineFileTree.test.tsx: gone — deleted with the machines-world teardown (623e632); nothing to fix. - test-with-db.sh: tolerate the fixed-name test container being owned by another checkout's compose project (reuse instead of name-conflicting). Drive-by: remove the dead changelog:generate script (scripts/changelog/ was deleted in #1044) plus the now-unused tsx root devDependency, and update CLAUDE.md accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XnzUuAixdTJ8xDpP5S92Ts
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Summary
"Green means green": eliminates every known-failing suite and env-only local failure that agents previously had to memorize as a baseline, so a red check always means a real regression.
test:security — was 44/50 with 6 permanently red, now 51/51
@pagespace/lib's default vitest config (DB-backed), so the script's plaintestinvocation found no files and exited 1. They now run through thetest:dbconfig against the dockerized test DB.pagespace-postgres-testcontainer, waits for readiness, runs migrations, and exportsDATABASE_URL/NODE_ENV=test— a fresh clone with Docker running goes 51/51 with no ambient setup.Env-only failures (the memorized local-failure list)
util.isNullOrUndefined, which@tensorflow/tfjs-node4.x still calls. A vitest setup shim (apps/processor/src/test/setup.ts) restores the predicate; the real magika model now loads and all 22 previously-failing tests pass on modern Node.ensureTestDb()probe (apps/web/src/test/ensure-test-db.ts) that fails fast with "test DB is not provisioned — run scripts/test-with-db.sh or docker compose -f docker-compose.test.yml up -d" instead of a wall of drizzleFailed queryerrors when run outside the DB harness.shouldShowDateDividermidnight case): fixtures are UTC but the code compares local calendar days, so the suite flipped in any timezone west of UTC. PinnedTZ=UTCin the suite (restored afterwards) and documented why.docker compose upwhen another checkout/worktree created it (compose scopes ownership by project dir). Now reuses the existing container by name and leaves it running on exit in that case, sobun run testis green from any clean worktree.Drive-by
changelog:generateroot script (scripts/changelog/was deleted in chore: remove stale docs/ and prune cross-references #1044; the script was a guaranteed MODULE_NOT_FOUND) plus the now-unused roottsxdevDependency, and updated CLAUDE.md's command list.Gate results (run sequentially in this worktree)
bun run lintbun run typecheckbun run knip:check[ok] knip: 4 issue(s), all within baseline (4)bun run testbun run test:securityNote: local machine hit a disk-full incident mid-verification (Docker crash + ENOSPC build failures); after cleanup all gates were re-run to completion as listed above.
🤖 Generated with Claude Code
https://claude.ai/code/session_01XnzUuAixdTJ8xDpP5S92Ts