diff --git a/AGENTS.md b/AGENTS.md index 2cb3682..c8ebca3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -400,6 +400,11 @@ Prioritize testing: Avoid tests that depend heavily on implementation details. +Keep scenario-specific data local. Extract stable objects or configuration +repeated across test files into typed factories or fixtures under the relevant +application's `src/test/fixtures/` directory; prefer override-based factories +for domain variants and do not share fixtures across unrelated boundaries. + Do not weaken, remove, or rewrite a legitimate failing test merely to make a change pass. diff --git a/apps/api/src/auth/google/google-oidc.routes.test.ts b/apps/api/src/auth/google/google-oidc.routes.test.ts index 4b7804c..14a65f6 100644 --- a/apps/api/src/auth/google/google-oidc.routes.test.ts +++ b/apps/api/src/auth/google/google-oidc.routes.test.ts @@ -12,21 +12,9 @@ import { validateReturnTo, } from './google-oidc.routes.js'; import { createSessionMiddleware } from '../session/session.middleware.js'; +import { testSessionConfiguration } from '../../test/fixtures/session-configuration.fixture.js'; const redirectUri = 'http://localhost:3000/auth/google/callback'; -const sessionConfiguration = { - SESSION_SECRET: 'test-session-secret-at-least-32-characters', - SESSION_DURATION_SECONDS: 604800, - SESSION_COOKIE: { - name: 'goforlift.sid', - httpOnly: true, - secure: false, - sameSite: 'lax' as const, - path: '/', - maxAgeMs: 604800000, - }, -}; - type TestContextOptions = { exchangeAuthorizationCode?: ExchangeAuthorizationCode; now?: () => number; @@ -49,7 +37,7 @@ function createTestContext(options: TestContextOptions = {}) { 'google-client-secret', ); const app = express(); - app.use(createSessionMiddleware(store, sessionConfiguration)); + app.use(createSessionMiddleware(store, testSessionConfiguration)); app.use( '/auth', createGoogleOidcRouter({ diff --git a/apps/api/src/auth/guards/authorization.middleware.test.ts b/apps/api/src/auth/guards/authorization.middleware.test.ts index cf6e478..1a3fc7e 100644 --- a/apps/api/src/auth/guards/authorization.middleware.test.ts +++ b/apps/api/src/auth/guards/authorization.middleware.test.ts @@ -9,31 +9,22 @@ import { establishAuthenticatedTestSession, withAuthenticatedTestUser, } from '../../test/authenticated-session.test-helper.js'; +import { testSessionConfiguration } from '../../test/fixtures/session-configuration.fixture.js'; import { getAuthenticatedUserId, requireAuthentication, } from './authentication.middleware.js'; import { requireAuthorization } from './authorization.middleware.js'; -const sessionConfiguration = { - SESSION_SECRET: 'test-session-secret-at-least-32-characters', - SESSION_DURATION_SECONDS: 604800, - SESSION_COOKIE: { - name: 'goforlift.sid', - httpOnly: true, - secure: false, - sameSite: 'lax' as const, - path: '/', - maxAgeMs: 604800000, - }, -}; - function createProtectedTestApp( userOwnsResource: (resourceId: string, userId: string) => Promise, ) { const app = express(); app.use( - createSessionMiddleware(new session.MemoryStore(), sessionConfiguration), + createSessionMiddleware( + new session.MemoryStore(), + testSessionConfiguration, + ), ); app.use(establishAuthenticatedTestSession()); app.get( diff --git a/apps/api/src/auth/session/session.middleware.test.ts b/apps/api/src/auth/session/session.middleware.test.ts index b616388..410406a 100644 --- a/apps/api/src/auth/session/session.middleware.test.ts +++ b/apps/api/src/auth/session/session.middleware.test.ts @@ -4,24 +4,12 @@ import session from 'express-session'; import request from 'supertest'; import { describe, expect, it } from 'vitest'; +import { testSessionConfiguration } from '../../test/fixtures/session-configuration.fixture.js'; import { createSessionMiddleware } from './session.middleware.js'; -const sessionConfiguration = { - SESSION_SECRET: 'test-session-secret-at-least-32-characters', - SESSION_DURATION_SECONDS: 604800, - SESSION_COOKIE: { - name: 'goforlift.sid', - httpOnly: true, - secure: false, - sameSite: 'lax' as const, - path: '/', - maxAgeMs: 604800000, - }, -}; - function createSessionTestApp(store: session.MemoryStore) { const app = express(); - app.use(createSessionMiddleware(store, sessionConfiguration)); + app.use(createSessionMiddleware(store, testSessionConfiguration)); app.post('/session', (request, response) => { request.session.userId = 'user-123'; diff --git a/apps/api/src/auth/session/session.routes.test.ts b/apps/api/src/auth/session/session.routes.test.ts index c637f62..706b02e 100644 --- a/apps/api/src/auth/session/session.routes.test.ts +++ b/apps/api/src/auth/session/session.routes.test.ts @@ -5,29 +5,15 @@ import request from 'supertest'; import { describe, expect, it, vi } from 'vitest'; import { z } from 'zod'; +import { createPublicUser } from '../../test/fixtures/public-user.fixture.js'; +import { testSessionConfiguration } from '../../test/fixtures/session-configuration.fixture.js'; import { csrfErrorHandler, csrfProtection } from '../csrf/csrf.middleware.js'; import { createSessionMiddleware } from './session.middleware.js'; import { createSessionRouter } from './session.routes.js'; -const sessionConfiguration = { - SESSION_SECRET: 'test-session-secret-at-least-32-characters', - SESSION_DURATION_SECONDS: 604800, - SESSION_COOKIE: { - name: 'goforlift.sid', - httpOnly: true, - secure: false, - sameSite: 'lax' as const, - path: '/', - maxAgeMs: 604800000, - }, -}; - -const publicUser = { - id: '26d34dc0-8e4c-4bd0-9e3b-7b839b44e486', - email: 'lifter@example.com', - displayName: 'Go For Lifter', +const publicUser = createPublicUser({ avatarUrl: 'https://images.example.com/avatar.png', -}; +}); const csrfTokenResponseSchema = z.object({ csrfToken: z.string() }); @@ -36,7 +22,7 @@ function createTestContext( ) { const store = new session.MemoryStore(); const app = express(); - app.use(createSessionMiddleware(store, sessionConfiguration)); + app.use(createSessionMiddleware(store, testSessionConfiguration)); app.use((request, _response, next) => { const testUserId = request.header('x-test-user-id'); if (testUserId) { @@ -48,8 +34,8 @@ function createTestContext( app.use( '/auth', createSessionRouter({ - cookieName: sessionConfiguration.SESSION_COOKIE.name, - cookiePath: sessionConfiguration.SESSION_COOKIE.path, + cookieName: testSessionConfiguration.SESSION_COOKIE.name, + cookiePath: testSessionConfiguration.SESSION_COOKIE.path, findPublicUser, }), ); diff --git a/apps/api/src/auth/user/current-user.service.test.ts b/apps/api/src/auth/user/current-user.service.test.ts index 4c9e8a1..f2be08b 100644 --- a/apps/api/src/auth/user/current-user.service.test.ts +++ b/apps/api/src/auth/user/current-user.service.test.ts @@ -3,16 +3,12 @@ import type { NodePgDatabase } from 'drizzle-orm/node-postgres'; import { describe, expect, it, vi } from 'vitest'; import { users } from '../../db/schema/index.js'; +import { createPublicUser } from '../../test/fixtures/public-user.fixture.js'; import { findPublicUserById } from './current-user.service.js'; describe('findPublicUserById', () => { it('returns the public user projection', async () => { - const publicUser = { - id: '26d34dc0-8e4c-4bd0-9e3b-7b839b44e486', - email: 'lifter@example.com', - displayName: 'Go For Lifter', - avatarUrl: null, - }; + const publicUser = createPublicUser(); const limit = vi.fn().mockResolvedValue([publicUser]); const where = vi.fn(() => ({ limit })); const from = vi.fn(() => ({ where })); diff --git a/apps/api/src/test/fixtures/public-user.fixture.ts b/apps/api/src/test/fixtures/public-user.fixture.ts new file mode 100644 index 0000000..94bab12 --- /dev/null +++ b/apps/api/src/test/fixtures/public-user.fixture.ts @@ -0,0 +1,14 @@ +/** Creates typed public-user API test data with optional scenario overrides. */ +import type { PublicUser } from '@goforlift/contracts'; + +export function createPublicUser( + overrides: Partial = {}, +): PublicUser { + return { + id: '26d34dc0-8e4c-4bd0-9e3b-7b839b44e486', + email: 'lifter@example.com', + displayName: 'Go For Lifter', + avatarUrl: null, + ...overrides, + }; +} diff --git a/apps/api/src/test/fixtures/session-configuration.fixture.ts b/apps/api/src/test/fixtures/session-configuration.fixture.ts new file mode 100644 index 0000000..717e415 --- /dev/null +++ b/apps/api/src/test/fixtures/session-configuration.fixture.ts @@ -0,0 +1,18 @@ +/** Provides the shared secure session configuration used by API tests. */ +import type { Env } from '../../config/env-schema.js'; + +export const testSessionConfiguration = { + SESSION_SECRET: 'test-session-secret-at-least-32-characters', + SESSION_DURATION_SECONDS: 604800, + SESSION_COOKIE: { + name: 'goforlift.sid', + httpOnly: true, + secure: false, + sameSite: 'lax', + path: '/', + maxAgeMs: 604800000, + }, +} satisfies Pick< + Env, + 'SESSION_COOKIE' | 'SESSION_DURATION_SECONDS' | 'SESSION_SECRET' +>; diff --git a/apps/web/src/features/auth/auth.api.test.ts b/apps/web/src/features/auth/auth.api.test.ts index 0dd3da8..880a70f 100644 --- a/apps/web/src/features/auth/auth.api.test.ts +++ b/apps/web/src/features/auth/auth.api.test.ts @@ -1,6 +1,7 @@ /** Verifies credential inclusion, response validation, CSRF attachment, and 401 behavior. */ import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createPublicUser } from '../../test/fixtures/public-user.fixture'; import { AuthApiError, fetchCurrentUser, @@ -8,12 +9,7 @@ import { requestWithCsrf, } from './auth.api'; -const publicUser = { - id: '26d34dc0-8e4c-4bd0-9e3b-7b839b44e486', - email: 'lifter@example.com', - displayName: 'Go For Lifter', - avatarUrl: null, -}; +const publicUser = createPublicUser(); afterEach(() => vi.unstubAllGlobals()); diff --git a/apps/web/src/features/auth/current-user.query.test.tsx b/apps/web/src/features/auth/current-user.query.test.tsx index 2552c53..20b2db8 100644 --- a/apps/web/src/features/auth/current-user.query.test.tsx +++ b/apps/web/src/features/auth/current-user.query.test.tsx @@ -4,14 +4,10 @@ import { renderHook, waitFor } from '@testing-library/react'; import type { PropsWithChildren } from 'react'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createPublicUser } from '../../test/fixtures/public-user.fixture'; import { useCurrentUser } from './current-user.query'; -const publicUser = { - id: '26d34dc0-8e4c-4bd0-9e3b-7b839b44e486', - email: 'lifter@example.com', - displayName: 'Go For Lifter', - avatarUrl: null, -}; +const publicUser = createPublicUser(); afterEach(() => vi.unstubAllGlobals()); diff --git a/apps/web/src/features/dashboard/dashboard-access-boundary.test.tsx b/apps/web/src/features/dashboard/dashboard-access-boundary.test.tsx index 67e3227..cd279f2 100644 --- a/apps/web/src/features/dashboard/dashboard-access-boundary.test.tsx +++ b/apps/web/src/features/dashboard/dashboard-access-boundary.test.tsx @@ -11,15 +11,11 @@ import { import { cleanup, render, screen } from '@testing-library/react'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createPublicUser } from '../../test/fixtures/public-user.fixture'; import { startGuestSession } from '../auth/guest-session'; import { DashboardAccessBoundary } from './dashboard-access-boundary'; -const publicUser = { - id: '26d34dc0-8e4c-4bd0-9e3b-7b839b44e486', - email: 'lifter@example.com', - displayName: 'Go For Lifter', - avatarUrl: null, -}; +const publicUser = createPublicUser(); afterEach(() => { cleanup(); diff --git a/apps/web/src/test/fixtures/public-user.fixture.ts b/apps/web/src/test/fixtures/public-user.fixture.ts new file mode 100644 index 0000000..d28d8c1 --- /dev/null +++ b/apps/web/src/test/fixtures/public-user.fixture.ts @@ -0,0 +1,14 @@ +// Creates typed public-user test data with optional scenario-specific overrides. +import type { PublicUser } from '@goforlift/contracts'; + +export function createPublicUser( + overrides: Partial = {}, +): PublicUser { + return { + id: '26d34dc0-8e4c-4bd0-9e3b-7b839b44e486', + email: 'lifter@example.com', + displayName: 'Go For Lifter', + avatarUrl: null, + ...overrides, + }; +}