fix: unit tests and integration tests

[bitnimble]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
paradb	Ready	Preview, Comment	May 29, 2026 12:35pm

[bitnimble]

@intern review this

[banana-claude]

Claude finished @bitnimble's task in 1m 30s —— View job

---

PR Review: fix: unit tests and integration tests

This is a solid refactor that modernizes the test infrastructure. Here's my review:

---

✅ What's Good

Test infrastructure improvements:

• Splitting unit tests (*.unit.test.ts) from integration tests is a clean separation
• Using PGlite for ephemeral in-memory Postgres is a great choice - no Docker/Supabase required for CI
• The jest_global_setup.integration.ts schema loading logic is clever - reading from supabase/config.toml keeps it in sync with real migrations automatically

Fake implementations:

• FakeS3Handler properly round-trips uploaded files with temp-to-permanent promotion logic
• fakeSupabaseClient handles the core auth flows (signup, getUser, check_email_exists) that the integration tests need
• Good use of _setCurrentUserForTesting for in-process test authentication

Lazy env var validation (src/services/env.ts):

• The getter-based lazy evaluation is smart - tests can avoid configuring unused services
• Implementation flag pattern (s3Implementation, supabaseImplementation, axiomImplementation) makes test fakes explicit

CI workflow (.github/workflows/test.yml):

• Clean setup with bun caching, separate unit and integration test steps

---

🔍 Suggestions

1. src/services/server_context.ts:47 - Missing SearchIndex export in return type

The createServerContextCore creates a searchIndex, but it's not exposed in the returned context. Tests might want to stub or verify search behavior directly.

// Current
return { s3Handler, mapsRepo, favoritesRepo };
// Consider
return { s3Handler, searchIndex, mapsRepo, favoritesRepo };

2. src/services/maps/s3_handler_fake.ts:34-43 - Album art files not actually stored

The fake uploadAlbumArtFiles returns success but doesn't store anything. If any test needs to verify album art was uploaded, this would silently pass. Consider adding a _albumArtFiles map if roundtrip verification becomes needed.

3. src/services/jest_setup.ts:8 - Incorrect async usage

const { pool } = await getServerContext();  // line 8

getServerContext() is synchronous and returns an object with a getter for pool. The await is a no-op here. Same issue on line 24 and 31.

4. src/services/session/supabase_fake.ts:42 - Consider using httpOnly for fake cookie

(await cookies()).set(FAKE_SESSION_COOKIE, encode(session), { path: '/' });

For closer parity with real Supabase session cookies, consider adding httpOnly: true to the options. Not critical for tests, but useful if the fake is ever used for local dev.

5. tools/test.sh:27-28 - PGlite server hardcoded to 10 connections

The script starts PGlite with --max-connections 10, but the comment says default of 1 would deadlock. Consider making this configurable or documenting why 10 is the magic number.

6. src/services/db/pool.ts:28 - attachDatabasePool in test context?

attachDatabasePool from @vercel/functions is being called unconditionally. Does this work correctly when tests run outside Vercel's edge runtime? Might be worth a guard:

if (typeof attachDatabasePool === 'function') {
  attachDatabasePool(pool);
}

---

💡 Minor Observations

• src/services/maps/tests/maps.test.ts is much cleaner now - the completeUpload helper nicely encapsulates the full upload flow
• The bunfig.toml override pointing to tools/no-bun-test.ts is a clever way to prevent accidental bun test usage
• .env.test has placeholder values that won't accidentally hit real services

---

Overall this is a well-structured improvement to the test setup. The main architectural changes (PGlite, fakes, lazy env vars) all make sense.
• Branch

[bitnimble]

@intern fix 3, 4 and 6

[banana-claude]

Claude encountered an error —— View job

---

Fixing Review Comments

Working on fixes...

• Fix map list: add shift+click to select multiple maps #3: Remove incorrect await on synchronous getServerContext() calls in jest_setup.ts
• Fix map list: show difficulties as color-coded blobs #4: Add httpOnly: true to fake session cookie in supabase_fake.ts
• Fix settings: add change password screen #6: Add guard for attachDatabasePool in pool.ts

[bitnimble]

@intern try again

[banana-claude]

Claude finished @bitnimble's task in 1m 26s —— View job

---

Fixing Review Comments

All three fixes applied and pushed.

• Fix map list: add shift+click to select multiple maps #3: Remove incorrect await on synchronous getServerContext() calls in jest_setup.ts
• Fix map list: show difficulties as color-coded blobs #4: Add httpOnly: true to fake session cookie in supabase_fake.ts
• Fix settings: add change password screen #6: Add guard for attachDatabasePool in pool.ts
• Run typecheck and lint

Changes in commit fbe0ad5:

• src/services/jest_setup.ts: Removed 3 unnecessary await keywords
• src/services/session/supabase_fake.ts: Added httpOnly: true to cookie options
• src/services/db/pool.ts: Wrapped attachDatabasePool call in typeof guard

@bitnimble ready for re-review

• Branch •

[bitnimble]

@intern debug and fix the failing integration test

[banana-claude]

Claude finished @bitnimble's task in 10m 56s —— View job

---

Debugging Failing Integration Test

• Check CI status and identify failing test
• Read the failing test and related code
• Debug and fix the issue
• Run tests to verify fix
• Format code and run checks
• Push fix to branch

Root cause: The test "does not allow a user to resubmit another user's maps" was timing out because it's the first test in the suite to make HTTP calls (to /api/users/signup and /api/maps/submit). In CI, these first requests trigger Next.js route compilation which adds 3-5 seconds of latency. Combined with the in-process map validation, this exceeded the default 5000ms test timeout.

Fix: Increased the test timeout from 5000ms to 15000ms to account for CI compilation time.

Tests pass locally consistently:

PASS src/services/maps/tests/maps.test.ts (6.048 s)
Tests:       29 passed, 29 total

@bitnimble ready for re-review

• • Branch