Skip to content

test: support server-side unit testing#2485

Open
michaelhthomas wants to merge 6 commits intoseerr-team:developfrom
michaelhthomas:feat/server-tests
Open

test: support server-side unit testing#2485
michaelhthomas wants to merge 6 commits intoseerr-team:developfrom
michaelhthomas:feat/server-tests

Conversation

@michaelhthomas
Copy link
Contributor

@michaelhthomas michaelhthomas commented Feb 17, 2026

Description

This PR adds support for server-side unit testing using Jest and Supertest (Express testing). It also adds an example test for the auth endpoints, which ended up catching a few issues. Those issues are also resolved here.

Tests for the auth endpoints were generated by Claude Code. All other code is my own.

How Has This Been Tested?

Tests pass!

Screenshots / Logs (if applicable)

asciicast

Checklist:

  • I have read and followed the contribution guidelines.
  • Disclosed any use of AI (see our policy)
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • Successful build pnpm build
  • Translation keys pnpm i18n:extract
  • Database migration (if required)

Summary by CodeRabbit

  • Tests

    • Added a comprehensive authentication test suite (login, logout, reset flows), test utilities, seeding helpers, a Jest config, and a CI job to run unit tests.
  • Bug Fixes

    • Improved password recovery timestamp precision and migration to handle datetime.
    • Ensured password is filtered from user responses.
    • Made password reset/save operations more reliable so resets take effect immediately.

@coderabbitai
Copy link

coderabbitai bot commented Feb 17, 2026

📝 Walkthrough

Walkthrough

Adds a test suite and CI job, Jest TypeScript config, test DB seeding/reset utilities, extensive authentication integration tests, awaits persisted saves in auth routes, and migrates user.recoveryLinkExpirationDate from date to datetime with corresponding Postgres and SQLite migrations.

Changes

Cohort / File(s) Summary
CI & Test deps
.github/workflows/ci.yml, package.json
Adds a dedicated "Unit Tests" CI job and introduces Jest/ts-jest/supertest and related types; adds a test npm script.
Jest config
server/jest.config.cts
New Jest TypeScript configuration using ts-jest preset and path mapping from tsconfig paths.
Data source (test mode)
server/datasource.ts
Introduces an in-memory SQLite testConfig and returns it when NODE_ENV === 'test'.
User entity changes
server/entity/User.ts
Includes password in filteredFields and changes recoveryLinkExpirationDate decorator from @Column({ type: 'date' }) to @DbAwareColumn({ type: 'datetime' }).
Migrations
server/migration/postgres/...1771337333450-RecoveryLinkExpirationDateTime.ts, server/migration/sqlite/...1771337037917-RecoveryLinkExpirationDateTime.ts
Adds Postgres migration to alter column to TIMESTAMP WITH TIME ZONE and SQLite migration that recreates the table to add a datetime recoveryLinkExpirationDate (with reversible up/down).
Auth routes adjustments
server/routes/auth.ts
Converts two synchronous repository.save calls to awaited saves to ensure persistence in reset-password flows.
Auth integration tests
server/routes/auth.test.ts
Adds a comprehensive integration test suite for /auth endpoints (me, local login, logout, reset-password flows), with mocked email and session handling.
Test DB utilities & wiring
server/utils/seedTestDb.ts, server/test/db.ts, server/scripts/prepareTestDb.ts
New seed/reset utilities (seedTestDb, resetTestDb) and a setupTestDb helper; prepareTestDb simplified to delegate to seeding utilities and preserve settings copy.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through code to plant a test,

Seeds of users, migrations dressed.
Jest and CI now hum a tune,
Password resets beneath the moon.
Hop, validate — the rabbits approve! 🥕✨

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'test: support server-side unit testing' directly and clearly describes the main objective of the PR, which is to add server-side unit testing support using Jest and Supertest.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@michaelhthomas michaelhthomas marked this pull request as ready for review February 17, 2026 15:32
@michaelhthomas michaelhthomas requested a review from a team as a code owner February 17, 2026 15:32
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
server/utils/seedTestDb.ts (1)

65-76: Guard against re-initializing the DataSource.

dataSource.initialize() throws if called twice in the same process (e.g., watch mode or repeated setup). Consider using isInitialized to make this helper idempotent.

♻️ Suggested change
-  const dbConnection = await dataSource.initialize();
+  const dbConnection = dataSource.isInitialized
+    ? dataSource
+    : await dataSource.initialize();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/utils/seedTestDb.ts` around lines 65 - 76, The helper calls
dataSource.initialize() unconditionally which throws if the DataSource is
already initialized; make seedTestDb idempotent by checking
dataSource.isInitialized and only calling dataSource.initialize() when false,
otherwise reuse the existing dataSource instance for dbConnection (e.g., let
dbConnection = dataSource.isInitialized ? dataSource : await
dataSource.initialize()); keep the rest of the logic (dropDatabase,
runMigrations/synchronize) the same so operations run against the reused
connection.
server/routes/auth.test.ts (1)

28-32: Avoid hard‑coded the session secret in tests.

The test hardcodes 'test-secret', while the production code in server/index.ts properly uses settings.clientId. Consider using a dynamic secret per test run to prevent accidental reuse:

🔧 Suggested update
+import { randomBytes } from 'crypto';
 import { getRepository } from '@server/datasource';
 import { User } from '@server/entity/User';
 import { getSettings } from '@server/lib/settings';
 import { checkUser } from '@server/middleware/auth';
 import { setupTestDb } from '@server/test/db';
 import type { Express } from 'express';
 import express from 'express';
 import session from 'express-session';
 import request from 'supertest';
 import authRoutes from './auth';

 function createApp() {
   const app = express();
   app.use(express.json());
   app.use(
     session({
-      secret: 'test-secret',
+      secret: process.env.TEST_SESSION_SECRET ?? randomBytes(32).toString('hex'),
       resave: false,
       saveUninitialized: false,
     })
   );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/routes/auth.test.ts` around lines 28 - 32, The test currently
hardcodes the session secret in the session({ secret: 'test-secret', ... })
call; replace this static secret with a dynamic/test-safe value: derive the
secret from the existing application test settings or environment (e.g., reuse
settings.clientId or generate a random string per test run) and inject it into
the session configuration used by the tests (locate the session(...) call in
auth.test.ts and update the secret parameter to read from the test settings or a
helper that produces a per-run secret).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/datasource.ts`:
- Around line 41-46: The test DataSourceOptions object testConfig currently sets
synchronize: true and dropSchema: true which causes initialize() to always
wipe/recreate the schema and prevents seedTestDb({ preserveDb: true }) and
withMigrations flows from working correctly; change testConfig to set
synchronize: false and dropSchema: false and let seedTestDb (and its preserveDb
/ withMigrations logic) control whether the schema is dropped/seeded/migrated,
keeping other options (e.g., logging) unchanged and ensuring initialize() no
longer forcibly resets the DB.

---

Nitpick comments:
In `@server/routes/auth.test.ts`:
- Around line 28-32: The test currently hardcodes the session secret in the
session({ secret: 'test-secret', ... }) call; replace this static secret with a
dynamic/test-safe value: derive the secret from the existing application test
settings or environment (e.g., reuse settings.clientId or generate a random
string per test run) and inject it into the session configuration used by the
tests (locate the session(...) call in auth.test.ts and update the secret
parameter to read from the test settings or a helper that produces a per-run
secret).

In `@server/utils/seedTestDb.ts`:
- Around line 65-76: The helper calls dataSource.initialize() unconditionally
which throws if the DataSource is already initialized; make seedTestDb
idempotent by checking dataSource.isInitialized and only calling
dataSource.initialize() when false, otherwise reuse the existing dataSource
instance for dbConnection (e.g., let dbConnection = dataSource.isInitialized ?
dataSource : await dataSource.initialize()); keep the rest of the logic
(dropDatabase, runMigrations/synchronize) the same so operations run against the
reused connection.

TypeORM represents the 'date' column type (date only) as a string, not a JS date object. This was
causing  the reset password expiration check to never activate, since it compares the date (which is
a string) with new Date(), which is always truthy.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
server/routes/auth.test.ts (1)

60-70: Consider isolating settings mutations.

The authenticatedAgent helper mutates settings.main.localLogin directly. Since getSettings() returns a singleton, these mutations persist across tests. While this works when tests run sequentially, it could cause flaky behavior if Jest runs tests in parallel or in unexpected order.

Consider wrapping settings mutations in a try/finally or using Jest's beforeEach/afterEach to ensure cleanup:

Example pattern for settings cleanup
// In tests that modify settings:
let originalLocalLogin: boolean;

beforeEach(() => {
  const settings = getSettings();
  originalLocalLogin = settings.main.localLogin;
});

afterEach(() => {
  const settings = getSettings();
  settings.main.localLogin = originalLocalLogin;
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/routes/auth.test.ts` around lines 60 - 70, The helper
authenticatedAgent currently mutates the singleton returned by getSettings() by
setting settings.main.localLogin = true and never restoring it; change this so
the original value is preserved and restored (either wrap the mutation in a
try/finally inside authenticatedAgent or capture/restore in the test suite using
beforeEach/afterEach). Specifically, read and save the original boolean from
settings.main.localLogin, set it to true for the authentication attempt, then
restore the saved value in a finally block (or restore it in afterEach) to avoid
leaking state across tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/scripts/prepareTestDb.ts`:
- Line 18: The top-level call to prepareDb() can reject and cause an unhandled
promise rejection; wrap or replace the bare call so rejections are handled: call
prepareDb() and attach a .catch(err => { console.error('prepareDb failed', err);
process.exit(1); }) (or use an async IIFE with try/catch and process.exit(1) on
error) to ensure errors are logged and the process exits non-zero; reference the
prepareDb() invocation in server/scripts/prepareTestDb.ts.

---

Nitpick comments:
In `@server/routes/auth.test.ts`:
- Around line 60-70: The helper authenticatedAgent currently mutates the
singleton returned by getSettings() by setting settings.main.localLogin = true
and never restoring it; change this so the original value is preserved and
restored (either wrap the mutation in a try/finally inside authenticatedAgent or
capture/restore in the test suite using beforeEach/afterEach). Specifically,
read and save the original boolean from settings.main.localLogin, set it to true
for the authentication attempt, then restore the saved value in a finally block
(or restore it in afterEach) to avoid leaking state across tests.

@michaelhthomas michaelhthomas changed the title feat: support server-side unit testing test: support server-side unit testing Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant