From 54b53360493408c382d8be1f97d5ccf0a383769c Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Fri, 4 Sep 2026 22:03:51 +0000 Subject: [PATCH 1/6] refactor(test): remove redundant vi.clearAllMocks and add .vitest to gitignore Vitest restoreAllMocks config handles mock cleanup automatically. Remove manual vi.clearAllMocks() calls and unused imports across test files. Co-Authored-By: Claude Opus 4.6 Claude-Session: https://claude.ai/code/session_01LpBu3jvGWLNEeyWVL2PQZT --- .gitignore | 3 + .../services/atcoder_verification.test.ts | 1 - src/features/auth/server/auth.test.ts | 5 +- src/features/auth/server/session.test.ts | 1 - .../auth/services/admin_access.test.ts | 5 +- .../auth/services/credentials.test.ts | 5 +- .../auth/services/session_guards.test.ts | 5 +- .../votes/services/vote_grade.test.ts | 5 +- .../votes/services/vote_statistics.test.ts | 5 +- .../services/workbook_placements/crud.test.ts | 60 +++++++++++-------- .../workbooks/services/workbooks.test.ts | 5 +- src/lib/services/tags.test.ts | 5 +- src/routes/problems/page_server.test.ts | 1 - src/routes/votes/page_server.test.ts | 1 - src/test/lib/services/tasks.test.ts | 5 +- src/test/lib/services/users.test.ts | 11 ++-- src/test/lib/utils/auth_forms.test.ts | 2 - 17 files changed, 51 insertions(+), 74 deletions(-) diff --git a/.gitignore b/.gitignore index 3b24d23ad..e0c033cfa 100644 --- a/.gitignore +++ b/.gitignore @@ -155,5 +155,8 @@ vite.config.ts.timestamp-* # Prisma prisma/.fabbrica +# Vitest +.vitest/ + # Directory for playwright test results test-results diff --git a/src/features/account/services/atcoder_verification.test.ts b/src/features/account/services/atcoder_verification.test.ts index 1f63c872d..a758a7b34 100644 --- a/src/features/account/services/atcoder_verification.test.ts +++ b/src/features/account/services/atcoder_verification.test.ts @@ -118,7 +118,6 @@ function mockFetch(body: unknown, ok = true): void { // --------------------------------------------------------------------------- beforeEach(() => { - vi.clearAllMocks(); vi.stubEnv('CONFIRM_API_URL', SAMPLE_API_URL); }); diff --git a/src/features/auth/server/auth.test.ts b/src/features/auth/server/auth.test.ts index 768775131..bc6a515eb 100644 --- a/src/features/auth/server/auth.test.ts +++ b/src/features/auth/server/auth.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import type { RequestEvent } from '@sveltejs/kit'; import { Roles } from '@prisma/client'; @@ -37,9 +37,6 @@ const createMockEvent = (cookieValue?: string) => { }; }; -beforeEach(() => { - vi.clearAllMocks(); -}); describe('createAuthRequest', () => { describe('validate', () => { diff --git a/src/features/auth/server/session.test.ts b/src/features/auth/server/session.test.ts index 09ad8679c..f67ae1211 100644 --- a/src/features/auth/server/session.test.ts +++ b/src/features/auth/server/session.test.ts @@ -56,7 +56,6 @@ const buildPrismaError = (code: string, message: string) => new Prisma.PrismaClientKnownRequestError(message, { code, clientVersion: '5.0.0' }); beforeEach(() => { - vi.clearAllMocks(); vi.useFakeTimers(); vi.setSystemTime(NOW); }); diff --git a/src/features/auth/services/admin_access.test.ts b/src/features/auth/services/admin_access.test.ts index 056bf528b..670cc51bd 100644 --- a/src/features/auth/services/admin_access.test.ts +++ b/src/features/auth/services/admin_access.test.ts @@ -1,4 +1,4 @@ -import { expect, test, describe, vi, afterEach } from 'vitest'; +import { expect, test, describe, vi } from 'vitest'; vi.mock('@sveltejs/kit', () => { const redirectImpl = (status: number, location: string) => { @@ -23,9 +23,6 @@ vi.mock('$lib/services/users', () => ({ getUser: vi.fn(), })); -afterEach(() => { - vi.clearAllMocks(); -}); import * as userService from '$lib/services/users'; import { Roles } from '$lib/types/user'; diff --git a/src/features/auth/services/credentials.test.ts b/src/features/auth/services/credentials.test.ts index 35e909937..4c985e298 100644 --- a/src/features/auth/services/credentials.test.ts +++ b/src/features/auth/services/credentials.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { Prisma } from '@prisma/client'; @@ -42,9 +42,6 @@ const HASHED_PASSWORD = 's2:0123456789abcdef:' + 'a'.repeat(128); const buildPrismaError = (code: string, message: string) => new Prisma.PrismaClientKnownRequestError(message, { code, clientVersion: '5.0.0' }); -beforeEach(() => { - vi.clearAllMocks(); -}); describe('registerUser', () => { describe('successful case', () => { diff --git a/src/features/auth/services/session_guards.test.ts b/src/features/auth/services/session_guards.test.ts index 96bcbe23f..676714050 100644 --- a/src/features/auth/services/session_guards.test.ts +++ b/src/features/auth/services/session_guards.test.ts @@ -1,4 +1,4 @@ -import { expect, test, describe, vi, afterEach } from 'vitest'; +import { expect, test, describe, vi } from 'vitest'; vi.mock('@sveltejs/kit', () => { const redirectImpl = (status: number, location: string) => { @@ -12,9 +12,6 @@ vi.mock('@sveltejs/kit', () => { return { redirect: vi.fn(redirectImpl) }; }); -afterEach(() => { - vi.clearAllMocks(); -}); import { ensureSessionOrRedirect, getLoggedInUser } from './session_guards'; diff --git a/src/features/votes/services/vote_grade.test.ts b/src/features/votes/services/vote_grade.test.ts index 397c965c3..c3deec418 100644 --- a/src/features/votes/services/vote_grade.test.ts +++ b/src/features/votes/services/vote_grade.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, vi, beforeEach } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { TaskGrade } from '@prisma/client'; @@ -32,9 +32,6 @@ vi.mock('$lib/server/database', () => ({ import prisma from '$lib/server/database'; import { invalidateVoteCaches } from '$features/votes/server/cache'; -beforeEach(() => { - vi.clearAllMocks(); -}); // --------------------------------------------------------------------------- // Type aliases diff --git a/src/features/votes/services/vote_statistics.test.ts b/src/features/votes/services/vote_statistics.test.ts index 729dddeb2..4c888f5b0 100644 --- a/src/features/votes/services/vote_statistics.test.ts +++ b/src/features/votes/services/vote_statistics.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, vi, beforeEach } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { TaskGrade } from '@prisma/client'; @@ -37,9 +37,6 @@ vi.mock('$features/votes/server/cache', () => ({ import prisma from '$lib/server/database'; -beforeEach(() => { - vi.clearAllMocks(); -}); // --------------------------------------------------------------------------- // Type aliases diff --git a/src/features/workbooks/services/workbook_placements/crud.test.ts b/src/features/workbooks/services/workbook_placements/crud.test.ts index 11ae00e10..45edaf36c 100644 --- a/src/features/workbooks/services/workbook_placements/crud.test.ts +++ b/src/features/workbooks/services/workbook_placements/crud.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, vi, beforeEach } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { TaskGrade } from '$lib/types/task'; import { WorkBookType } from '$features/workbooks/types/workbook'; @@ -42,31 +42,44 @@ vi.mock('$lib/server/database', () => ({ import prisma from '$lib/server/database'; -beforeEach(() => { - vi.clearAllMocks(); -}); - function mockFindMany(placements: WorkBookPlacements) { vi.mocked(prisma.workBookPlacement.findMany).mockResolvedValue( placements as unknown as Awaited>, ); } -function mockPlacementFindManyOnce(placements: WorkBookPlacements) { - vi.mocked(prisma.workBookPlacement.findMany).mockResolvedValueOnce( - placements as unknown as Awaited>, - ); -} +function mockUnplacedWorkbooks( + curriculum: { id: number }[], + solution: { id: number }[], +) { + const mock = vi.mocked(prisma.workBook.findMany); -function mockWorkBookFindManyOnce(result: { id: number }[]) { - vi.mocked(prisma.workBook.findMany).mockResolvedValueOnce( - result as unknown as Awaited>, - ); + vi.when(mock) + .calledWith( + expect.objectContaining({ + where: expect.objectContaining({ workBookType: WorkBookType.CURRICULUM }), + }), + ) + .thenResolve( + curriculum as unknown as Awaited>, + ); + + vi.when(mock) + .calledWith( + expect.objectContaining({ + where: expect.objectContaining({ workBookType: WorkBookType.SOLUTION }), + }), + ) + .thenResolve( + solution as unknown as Awaited>, + ); } describe('getWorkbooksWithPlacements', () => { test('returns workbooks of type CURRICULUM and SOLUTION with their placements', async () => { - mockWorkBookFindManyOnce(workbooksWithPlacements); + vi.mocked(prisma.workBook.findMany).mockResolvedValue( + workbooksWithPlacements as unknown as Awaited>, + ); const result = await getWorkbooksWithPlacements(); @@ -184,8 +197,7 @@ describe('updateWorkBookPlacements', () => { describe('createInitialPlacements', () => { test('does nothing when all workbooks are already placed', async () => { - mockWorkBookFindManyOnce([]); // unplaced CURRICULUM - mockWorkBookFindManyOnce([]); // unplaced SOLUTION + mockUnplacedWorkbooks([], []); await createInitialPlacements(); @@ -195,8 +207,7 @@ describe('createInitialPlacements', () => { test('creates placements for unplaced curriculum and solution workbooks', async () => { // unplacedCurriculumRows: 2 workbooks → 2 curriculum placements // unplacedSolutionWorkbooks: 2 workbooks → 2 solution placements (PENDING) - mockWorkBookFindManyOnce(unplacedCurriculumRows); - mockWorkBookFindManyOnce(unplacedSolutionWorkbooks); + mockUnplacedWorkbooks(unplacedCurriculumRows, unplacedSolutionWorkbooks); vi.mocked(prisma.workBookPlacement.createMany).mockResolvedValue({ count: 4 }); await createInitialPlacements(); @@ -207,8 +218,7 @@ describe('createInitialPlacements', () => { }); test('calls createMany with skipDuplicates to tolerate concurrent double-submit', async () => { - mockWorkBookFindManyOnce(unplacedCurriculumRows); - mockWorkBookFindManyOnce(unplacedSolutionWorkbooks); + mockUnplacedWorkbooks(unplacedCurriculumRows, unplacedSolutionWorkbooks); vi.mocked(prisma.workBookPlacement.createMany).mockResolvedValue({ count: 4 }); await createInitialPlacements(); @@ -220,7 +230,7 @@ describe('createInitialPlacements', () => { describe('validateAndUpdatePlacements', () => { test('returns null and calls upsert when all updates are valid', async () => { - mockPlacementFindManyOnce([curriculumPlacementRow]); + mockFindMany([curriculumPlacementRow]); vi.mocked(prisma.$transaction).mockResolvedValue([]); const result = await validateAndUpdatePlacements([ @@ -232,7 +242,7 @@ describe('validateAndUpdatePlacements', () => { }); test('returns error when placement id does not exist', async () => { - mockPlacementFindManyOnce([]); + mockFindMany([]); const result = await validateAndUpdatePlacements([ { id: 999, priority: 1, taskGrade: null, solutionCategory: SolutionCategory.GRAPH }, @@ -243,7 +253,7 @@ describe('validateAndUpdatePlacements', () => { }); test('returns error for CURRICULUM → SOLUTION cross-type movement', async () => { - mockPlacementFindManyOnce([curriculumPlacementRow]); + mockFindMany([curriculumPlacementRow]); const result = await validateAndUpdatePlacements([ { id: 1, priority: 1, taskGrade: null, solutionCategory: SolutionCategory.GRAPH }, @@ -254,7 +264,7 @@ describe('validateAndUpdatePlacements', () => { }); test('returns error for SOLUTION → CURRICULUM cross-type movement', async () => { - mockPlacementFindManyOnce([solutionPlacementRow]); + mockFindMany([solutionPlacementRow]); const result = await validateAndUpdatePlacements([ { id: 101, priority: 1, taskGrade: TaskGrade.Q10, solutionCategory: null }, diff --git a/src/features/workbooks/services/workbooks.test.ts b/src/features/workbooks/services/workbooks.test.ts index eebdc1f41..2f1b3f8a7 100644 --- a/src/features/workbooks/services/workbooks.test.ts +++ b/src/features/workbooks/services/workbooks.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, vi, beforeEach } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { getWorkBook, @@ -49,9 +49,6 @@ vi.mock('$features/workbooks/server/cache', () => ({ import prisma from '$lib/server/database'; import * as usersCrud from '$lib/services/users'; -beforeEach(() => { - vi.clearAllMocks(); -}); function prepareWorkBook(overrides: Partial = {}): WorkBook { return { diff --git a/src/lib/services/tags.test.ts b/src/lib/services/tags.test.ts index 716430ea6..96308927b 100644 --- a/src/lib/services/tags.test.ts +++ b/src/lib/services/tags.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { getTag } from '$lib/services/tags'; @@ -15,9 +15,6 @@ import db from '$lib/server/database'; describe('getTag', () => { const mockDb = db as unknown as { tag: { findUnique: ReturnType } }; - beforeEach(() => { - vi.clearAllMocks(); - }); describe('successful case', () => { test('returns tag when tag exists', async () => { diff --git a/src/routes/problems/page_server.test.ts b/src/routes/problems/page_server.test.ts index f69682584..5394a8e30 100644 --- a/src/routes/problems/page_server.test.ts +++ b/src/routes/problems/page_server.test.ts @@ -53,7 +53,6 @@ const LOGGED_IN_SESSION: MockSession = { }; beforeEach(() => { - vi.clearAllMocks(); mockGetTaskResults.mockResolvedValue([]); mockGetTasksWithTagIds.mockResolvedValue([]); mockGetVoteGradeStatistics.mockResolvedValue(new Map()); diff --git a/src/routes/votes/page_server.test.ts b/src/routes/votes/page_server.test.ts index b7d9a1767..640265409 100644 --- a/src/routes/votes/page_server.test.ts +++ b/src/routes/votes/page_server.test.ts @@ -29,7 +29,6 @@ const LOGGED_IN_SESSION: MockSession = { }; beforeEach(() => { - vi.clearAllMocks(); mockGetAllTasksWithVoteInfo.mockResolvedValue([]); }); diff --git a/src/test/lib/services/tasks.test.ts b/src/test/lib/services/tasks.test.ts index e9e65012e..250bb9e0f 100644 --- a/src/test/lib/services/tasks.test.ts +++ b/src/test/lib/services/tasks.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, vi } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { Prisma } from '@prisma/client'; @@ -29,9 +29,6 @@ import { invalidateVoteCaches } from '$features/votes/server/cache'; describe('updateTask', () => { const mockDb = db as unknown as { task: { update: ReturnType } }; - beforeEach(() => { - vi.clearAllMocks(); - }); describe('successful case', () => { test('returns undefined when task is updated successfully', async () => { diff --git a/src/test/lib/services/users.test.ts b/src/test/lib/services/users.test.ts index d7e91b58f..d741f5bb2 100644 --- a/src/test/lib/services/users.test.ts +++ b/src/test/lib/services/users.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, vi, beforeEach } from 'vitest'; +import { describe, test, expect, vi } from 'vitest'; import { Roles, type User } from '@prisma/client'; vi.mock('$lib/server/database', () => ({ @@ -61,24 +61,21 @@ function prepareUser(overrides: Partial = {}): UserWithAccount // --------------------------------------------------------------------------- function mockFindUnique(value: UserWithAccount | null): void { - vi.mocked(db.user.findUnique).mockResolvedValueOnce(value); + vi.mocked(db.user.findUnique).mockResolvedValue(value); } function mockDelete(value: UserWithAccount): void { - vi.mocked(db.user.delete).mockResolvedValueOnce(value); + vi.mocked(db.user.delete).mockResolvedValue(value); } function mockDeleteError(message: string = 'not found'): void { - vi.mocked(db.user.delete).mockRejectedValueOnce(new Error(message)); + vi.mocked(db.user.delete).mockRejectedValue(new Error(message)); } // --------------------------------------------------------------------------- // Setup // --------------------------------------------------------------------------- -beforeEach(() => { - vi.clearAllMocks(); -}); describe('getUser', () => { describe('successful case', () => { diff --git a/src/test/lib/utils/auth_forms.test.ts b/src/test/lib/utils/auth_forms.test.ts index 1cc277acf..305646209 100644 --- a/src/test/lib/utils/auth_forms.test.ts +++ b/src/test/lib/utils/auth_forms.test.ts @@ -77,8 +77,6 @@ const createMockLocals = (hasSession: boolean = false) => describe('auth_forms', () => { beforeEach(() => { - vi.clearAllMocks(); - // Mock console methods vi.stubGlobal('console', { ...console, From 43193bfe97737ad082c698bfded924a1019e15cd Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Fri, 4 Sep 2026 22:05:24 +0000 Subject: [PATCH 2/6] style(test): remove leftover blank lines after clearAllMocks cleanup Co-Authored-By: Claude Opus 4.6 Claude-Session: https://claude.ai/code/session_01LpBu3jvGWLNEeyWVL2PQZT --- src/features/auth/server/auth.test.ts | 1 - src/features/auth/services/admin_access.test.ts | 1 - src/features/auth/services/credentials.test.ts | 1 - src/features/auth/services/session_guards.test.ts | 1 - src/features/votes/services/vote_grade.test.ts | 1 - src/features/votes/services/vote_statistics.test.ts | 1 - .../services/workbook_placements/crud.test.ts | 13 +++---------- src/features/workbooks/services/workbooks.test.ts | 1 - src/lib/services/tags.test.ts | 1 - src/test/lib/services/tasks.test.ts | 1 - src/test/lib/services/users.test.ts | 1 - 11 files changed, 3 insertions(+), 20 deletions(-) diff --git a/src/features/auth/server/auth.test.ts b/src/features/auth/server/auth.test.ts index bc6a515eb..dfaa4fefb 100644 --- a/src/features/auth/server/auth.test.ts +++ b/src/features/auth/server/auth.test.ts @@ -37,7 +37,6 @@ const createMockEvent = (cookieValue?: string) => { }; }; - describe('createAuthRequest', () => { describe('validate', () => { test('returns null and skips validateSession when no cookie is present', async () => { diff --git a/src/features/auth/services/admin_access.test.ts b/src/features/auth/services/admin_access.test.ts index 670cc51bd..c10a26b88 100644 --- a/src/features/auth/services/admin_access.test.ts +++ b/src/features/auth/services/admin_access.test.ts @@ -23,7 +23,6 @@ vi.mock('$lib/services/users', () => ({ getUser: vi.fn(), })); - import * as userService from '$lib/services/users'; import { Roles } from '$lib/types/user'; import { validateAdminAccess, validateAdminAccessForApi } from './admin_access'; diff --git a/src/features/auth/services/credentials.test.ts b/src/features/auth/services/credentials.test.ts index 4c985e298..4988afa96 100644 --- a/src/features/auth/services/credentials.test.ts +++ b/src/features/auth/services/credentials.test.ts @@ -42,7 +42,6 @@ const HASHED_PASSWORD = 's2:0123456789abcdef:' + 'a'.repeat(128); const buildPrismaError = (code: string, message: string) => new Prisma.PrismaClientKnownRequestError(message, { code, clientVersion: '5.0.0' }); - describe('registerUser', () => { describe('successful case', () => { test('creates the user and key in a single transaction and returns the new user id', async () => { diff --git a/src/features/auth/services/session_guards.test.ts b/src/features/auth/services/session_guards.test.ts index 676714050..b0e28f781 100644 --- a/src/features/auth/services/session_guards.test.ts +++ b/src/features/auth/services/session_guards.test.ts @@ -12,7 +12,6 @@ vi.mock('@sveltejs/kit', () => { return { redirect: vi.fn(redirectImpl) }; }); - import { ensureSessionOrRedirect, getLoggedInUser } from './session_guards'; const createMockLocalsWithValidSession = (user = { id: 'test-user', name: 'Test User' }) => diff --git a/src/features/votes/services/vote_grade.test.ts b/src/features/votes/services/vote_grade.test.ts index c3deec418..6734225bf 100644 --- a/src/features/votes/services/vote_grade.test.ts +++ b/src/features/votes/services/vote_grade.test.ts @@ -32,7 +32,6 @@ vi.mock('$lib/server/database', () => ({ import prisma from '$lib/server/database'; import { invalidateVoteCaches } from '$features/votes/server/cache'; - // --------------------------------------------------------------------------- // Type aliases // --------------------------------------------------------------------------- diff --git a/src/features/votes/services/vote_statistics.test.ts b/src/features/votes/services/vote_statistics.test.ts index 4c888f5b0..577994230 100644 --- a/src/features/votes/services/vote_statistics.test.ts +++ b/src/features/votes/services/vote_statistics.test.ts @@ -37,7 +37,6 @@ vi.mock('$features/votes/server/cache', () => ({ import prisma from '$lib/server/database'; - // --------------------------------------------------------------------------- // Type aliases // --------------------------------------------------------------------------- diff --git a/src/features/workbooks/services/workbook_placements/crud.test.ts b/src/features/workbooks/services/workbook_placements/crud.test.ts index 45edaf36c..6d69008cc 100644 --- a/src/features/workbooks/services/workbook_placements/crud.test.ts +++ b/src/features/workbooks/services/workbook_placements/crud.test.ts @@ -48,10 +48,7 @@ function mockFindMany(placements: WorkBookPlacements) { ); } -function mockUnplacedWorkbooks( - curriculum: { id: number }[], - solution: { id: number }[], -) { +function mockUnplacedWorkbooks(curriculum: { id: number }[], solution: { id: number }[]) { const mock = vi.mocked(prisma.workBook.findMany); vi.when(mock) @@ -60,9 +57,7 @@ function mockUnplacedWorkbooks( where: expect.objectContaining({ workBookType: WorkBookType.CURRICULUM }), }), ) - .thenResolve( - curriculum as unknown as Awaited>, - ); + .thenResolve(curriculum as unknown as Awaited>); vi.when(mock) .calledWith( @@ -70,9 +65,7 @@ function mockUnplacedWorkbooks( where: expect.objectContaining({ workBookType: WorkBookType.SOLUTION }), }), ) - .thenResolve( - solution as unknown as Awaited>, - ); + .thenResolve(solution as unknown as Awaited>); } describe('getWorkbooksWithPlacements', () => { diff --git a/src/features/workbooks/services/workbooks.test.ts b/src/features/workbooks/services/workbooks.test.ts index 2f1b3f8a7..0f1a7bede 100644 --- a/src/features/workbooks/services/workbooks.test.ts +++ b/src/features/workbooks/services/workbooks.test.ts @@ -49,7 +49,6 @@ vi.mock('$features/workbooks/server/cache', () => ({ import prisma from '$lib/server/database'; import * as usersCrud from '$lib/services/users'; - function prepareWorkBook(overrides: Partial = {}): WorkBook { return { id: 1, diff --git a/src/lib/services/tags.test.ts b/src/lib/services/tags.test.ts index 96308927b..d4f4e26ef 100644 --- a/src/lib/services/tags.test.ts +++ b/src/lib/services/tags.test.ts @@ -15,7 +15,6 @@ import db from '$lib/server/database'; describe('getTag', () => { const mockDb = db as unknown as { tag: { findUnique: ReturnType } }; - describe('successful case', () => { test('returns tag when tag exists', async () => { const tag = { id: '1', name: 'DP', is_official: true, is_published: true }; diff --git a/src/test/lib/services/tasks.test.ts b/src/test/lib/services/tasks.test.ts index 250bb9e0f..4cb6c67ea 100644 --- a/src/test/lib/services/tasks.test.ts +++ b/src/test/lib/services/tasks.test.ts @@ -29,7 +29,6 @@ import { invalidateVoteCaches } from '$features/votes/server/cache'; describe('updateTask', () => { const mockDb = db as unknown as { task: { update: ReturnType } }; - describe('successful case', () => { test('returns undefined when task is updated successfully', async () => { mockDb.task.update.mockResolvedValue({ diff --git a/src/test/lib/services/users.test.ts b/src/test/lib/services/users.test.ts index d741f5bb2..ac09732b8 100644 --- a/src/test/lib/services/users.test.ts +++ b/src/test/lib/services/users.test.ts @@ -76,7 +76,6 @@ function mockDeleteError(message: string = 'not found'): void { // Setup // --------------------------------------------------------------------------- - describe('getUser', () => { describe('successful case', () => { test('returns user with atCoderAccount when found', async () => { From 6f6a197454138b7adc549d293b71ab6df71a9eaa Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Fri, 4 Sep 2026 22:07:39 +0000 Subject: [PATCH 3/6] docs(rules): add Vitest v5 mock cleanup and vi.when() guidance Co-Authored-By: Claude Opus 4.6 Claude-Session: https://claude.ai/code/session_01LpBu3jvGWLNEeyWVL2PQZT --- .claude/rules/testing.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index ee415a835..2f560f9bf 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -115,6 +115,11 @@ describe('validate', () => { - Verify fixture alignment: test name "tie-break" must exercise that code path - Shared fixtures at `describe` scope avoid duplication (DRY + auto-sync) +### Mock Cleanup (Vitest v5) + +- `clearMocks: true` is the v5 default — **never add `vi.clearAllMocks()`**; use `mockResolvedValue` (not `Once`) since auto-clear handles reset +- `restoreMocks` is still `false` — `vi.restoreAllMocks()` remains needed for `vi.spyOn` + ### Service Layer Mocking Mock Prisma with `vi.mock('$lib/server/database', ...)` — no real DB mutations. Use helpers: @@ -124,6 +129,14 @@ const mockFindUnique = (data) => db.task.findUnique.mockResolvedValue(data); const mockFindMany = (data) => db.task.findMany.mockResolvedValue(data); ``` +When the same mock is called with different arguments in one test, use `vi.when()` instead of `mockResolvedValueOnce` chains: + +```typescript +vi.when(vi.mocked(prisma.workBook.findMany)) + .calledWith(expect.objectContaining({ where: expect.objectContaining({ workBookType: WorkBookType.CURRICULUM }) })) + .thenResolve(curriculumRows); +``` + ### Cache Module Tests Prevent timer leaks and test isolation: From db14f18e51d62407106eded78826ab76469d3269 Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Fri, 4 Sep 2026 22:09:50 +0000 Subject: [PATCH 4/6] style(rules): format vi.when() code example in testing rules Co-Authored-By: Claude Opus 4.6 Claude-Session: https://claude.ai/code/session_01LpBu3jvGWLNEeyWVL2PQZT --- .claude/rules/testing.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 2f560f9bf..5fd18df61 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -133,7 +133,11 @@ When the same mock is called with different arguments in one test, use `vi.when( ```typescript vi.when(vi.mocked(prisma.workBook.findMany)) - .calledWith(expect.objectContaining({ where: expect.objectContaining({ workBookType: WorkBookType.CURRICULUM }) })) + .calledWith( + expect.objectContaining({ + where: expect.objectContaining({ workBookType: WorkBookType.CURRICULUM }), + }), + ) .thenResolve(curriculumRows); ``` From 0f19ff42c828b08a8d75a7a037da68d7f76548c4 Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Fri, 4 Sep 2026 22:14:58 +0000 Subject: [PATCH 5/6] docs(rules): add guard clause reachability rule and simplify testing guide Add rule for ensuring guard clauses don't create unreachable code. Condense testing.md by removing verbose examples while preserving all essential patterns and conventions. Co-Authored-By: Claude Opus 4.6 Claude-Session: https://claude.ai/code/session_01LpBu3jvGWLNEeyWVL2PQZT --- .claude/rules/coding-style.md | 4 + .claude/rules/testing.md | 251 +++++++++------------------------- 2 files changed, 65 insertions(+), 190 deletions(-) diff --git a/.claude/rules/coding-style.md b/.claude/rules/coding-style.md index ca6c1ceee..527b7253e 100644 --- a/.claude/rules/coding-style.md +++ b/.claude/rules/coding-style.md @@ -45,6 +45,10 @@ const grade = isValidTaskGrade(data.grade) ? data.grade : null; Extract to `src/lib/utils/` with adjacent tests. +## Guard Clause Reachability + +Ensure guard clauses don't make later code unreachable. If an early return covers all remaining cases, delete the dead code below it rather than leaving it. + ## Dead Code: Three-Condition Rule Delete function only if: (1) zero callers, (2) replacement exists, (3) dependent fields also deleted. diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 5fd18df61..01ee0a917 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -9,94 +9,39 @@ paths: # Testing -## Core Principles +## Principles -- **Tests ship with implementation**: same commit, feature not done until tests pass -- **English only**: describe expected behavior (e.g., `'returns empty array when workbooks is empty'`) -- **Test integrity**: never weaken assertions to make tests pass; fix implementation instead -- **Unused imports**: signal missing tests, not dead code—add the test case first -- **TDD exceptions**: skip test-first for exploratory spikes, type-only changes, and config files with no branching logic; write tests before implementation for all service/util/store code +- **Tests ship with implementation** — same commit; feature not done until tests pass. Never defer tests for non-trivial logic +- **English only** — describe expected behavior (e.g., `'returns empty array when workbooks is empty'`) +- **Test integrity** — never weaken assertions to make tests pass; fix implementation instead +- **Unused imports** — signal missing tests, not dead code — add the test case first +- **TDD exceptions** — skip test-first for exploratory spikes, type-only changes, and config files with no branching logic +- **Component testing** — extract logic to `utils/`; omit component Vitest if template-only **and** E2E covers rendering paths +- **Coverage** — cover happy path, error cases, and domain edge cases (empty arrays, null, enum extremes). Treat low coverage as a signal to review, not a target -## Test Types +## File Layout & Environment | Type | Tool | Location | Command | | ---- | ---------- | ------------------------- | ---------------- | | Unit | Vitest | `src/test/` or co-located | `pnpm test:unit` | | E2E | Playwright | `e2e/` | `pnpm test:e2e` | -E2E files: **must** use `.spec.ts` extension (`.test.ts` not detected). +- E2E files **must** use `.spec.ts` (`.test.ts` not detected) +- Route unit tests: name `page_server.test.ts`, never `+page.server.test.ts` (SvelteKit reserves `+` prefix) +- Both centralized (`src/test/`) and co-located (`src/features/`, `src/lib/`) test locations are supported — see `vite.config.ts` `include` -Route unit tests: `src/routes/**/*.test.ts` is included by `vite.config.ts`. **Never use `+` as a filename prefix** — SvelteKit reserves it and `pnpm check` will error. Name route test files `page_server.test.ts`, not `+page.server.test.ts`. +**Environment:** Default `node`. Only opt in to jsdom (`// @vitest-environment jsdom` at file top) when touching `window` / `document` / `localStorage`. Never set jsdom globally — per-file construction is ~5.5× slower. -## Test Environment +## Assertions & Structure -Default is `node` (set in `vite.config.ts`). Only files touching the real DOM (`window` / `document` / `localStorage`) opt in with a top-of-file `// @vitest-environment jsdom`. **Never set jsdom globally** — most tests are pure units and per-file jsdom construction is ~5.5x slower. - -### Toggling `browser` per describe - -**Never register `vi.mock('$app/environment')` twice in one file.** Every `vi.mock` is hoisted above the imports and runs once before any test, so a `{ browser: false }` mock written inside an SSR `describe`/`beforeEach` silently overwrites the `{ browser: true }` one at the top — the whole file ends up pinned to `browser = false`, the localStorage branches never execute, and tests that "verify" them become false-positives while still passing. - -Use **one dynamic mock** driven by a `vi.hoisted` flag, and toggle the flag in `beforeEach`. Default the flag to `false` so singletons constructed at import time stay SSR-safe: - -```typescript -// @vitest-environment jsdom -const browserState = vi.hoisted(() => ({ value: false })); - -vi.mock('$app/environment', () => ({ - get browser() { - return browserState.value; - }, -})); - -describe('MyStore', () => { - let store: MyStore; - - beforeEach(() => { - browserState.value = true; - localStorage.clear(); - store = new MyStore(); // constructed after the flag flips — reads localStorage - }); - // … -}); - -describe('MyStore in SSR', () => { - beforeEach(() => { - browserState.value = false; - }); - // … -}); -``` - -**Never assert a browser branch on the import-time singleton.** The flag is `false` while the module graph is evaluated, so the exported singleton is always built in SSR mode and never touches localStorage — asserting on it under `browser = true` is the same false-positive as the double-`vi.mock` above. Construct a fresh instance inside the browser `describe` (which is why the store class, not just the singleton, needs a named export). The singleton is still worth one assertion: that import-time construction is SSR-safe. - -In jsdom files, do **not** stub localStorage with `vi.stubGlobal` — use jsdom's real `Storage` and assert on state (`localStorage.getItem(key)`), not on spy calls. Cover both SSR guards with separate cases, since one assertion cannot carry both: - -- **read guard**: pre-seed localStorage, construct a fresh store, expect the _default_ (the pre-seeded value stays in storage, so do not expect `getItem(key)` to be null here) -- **write guard**: start from empty localStorage, call the setter, expect `getItem(key)` to still be null - -## Unit Testing Patterns - -### Assertions - -- Use `toBe(true)` / `toBe(false)` not `toBeTruthy()` / `toBeFalsy()` +- `toBe(true)` / `toBe(false)`, not `toBeTruthy()` / `toBeFalsy()` - DB query tests: assert `orderBy`, `include` with `expect.objectContaining`, not just `where` -- Enum membership: `Object.hasOwn(Enum, value)` not `in` (avoids prototype chain) -- `Promise`: `.resolves` requires a matcher — without one it does not assert and becomes a false-positive; use `await fn()` to assert no throw, or `.resolves.toBeUndefined()` for explicit form - -```typescript -// ✓ Preferred: simplest way to assert Promise does not throw -await ensureSessionOrRedirect(mockLocals); - -// ✓ Also correct: explicit resolves form -await expect(ensureSessionOrRedirect(mockLocals)).resolves.toBeUndefined(); - -// ✗ Wrong: .resolves without a matcher does not assert anything (false-positive) -await expect(ensureSessionOrRedirect(mockLocals)).resolves; -``` - -### Describe Organization +- Enum membership: `Object.hasOwn(Enum, value)`, not `in` (avoids prototype chain) +- `Promise`: use `await fn()` to assert no throw, or `.resolves.toBeUndefined()`. **Never** bare `.resolves` (false-positive) +- **Stubs**: parameter types must match production signature — use domain types (`TaskGrade`), not `string` +- **Test data**: realistic values (real task IDs, grade names). Extract shared fixtures to file/describe scope; inline for single-use -Group by scenario (successful vs error cases), not flat: +Group by scenario, not flat: ```typescript describe('validate', () => { @@ -108,25 +53,27 @@ describe('validate', () => { }); ``` -### Test Data +Parameterized: test enum boundaries + typical value, then separate test for distinct behavior: + +```typescript +test.each([TaskGrade.PENDING, TaskGrade.Q11, TaskGrade.Q10, TaskGrade.D6])( + 'returns grade %s', (grade) => { ... } +); +``` -- Use realistic values (real task IDs, grade names), not `'t1'` placeholders -- Extract shared fixture data to file scope; inline for single-use -- Verify fixture alignment: test name "tie-break" must exercise that code path -- Shared fixtures at `describe` scope avoid duplication (DRY + auto-sync) +## Mocking -### Mock Cleanup (Vitest v5) +### Cleanup (Vitest v5) - `clearMocks: true` is the v5 default — **never add `vi.clearAllMocks()`**; use `mockResolvedValue` (not `Once`) since auto-clear handles reset - `restoreMocks` is still `false` — `vi.restoreAllMocks()` remains needed for `vi.spyOn` -### Service Layer Mocking +### Service Layer (Prisma) -Mock Prisma with `vi.mock('$lib/server/database', ...)` — no real DB mutations. Use helpers: +Mock with `vi.mock('$lib/server/database', ...)`. Use helpers: ```typescript const mockFindUnique = (data) => db.task.findUnique.mockResolvedValue(data); -const mockFindMany = (data) => db.task.findMany.mockResolvedValue(data); ``` When the same mock is called with different arguments in one test, use `vi.when()` instead of `mockResolvedValueOnce` chains: @@ -141,27 +88,22 @@ vi.when(vi.mocked(prisma.workBook.findMany)) .thenResolve(curriculumRows); ``` -### Cache Module Tests - -Prevent timer leaks and test isolation: +### Cache Modules ```typescript afterAll(() => disposeDomainCaches()); beforeEach(() => invalidateDomainCaches()); -``` - -Mock cache modules in service tests so caching is bypassed: -```typescript +// Bypass caching in service tests: vi.mock('$lib/server/tasks/cache', () => ({ getCachedTasksMap: (fetchFn: () => Promise) => fetchFn(), invalidateTaskCaches: vi.fn(), })); ``` -### HTTP Mocking (Nock) +### HTTP (Nock) -Extract setup into helpers, declare once at describe scope: +Extract setup into helpers at describe scope: ```typescript const mockGetUser = (statusCode, user?) => { @@ -171,52 +113,45 @@ const mockGetUser = (statusCode, user?) => { }; ``` -### Parameterized Tests +### Environment Variables -Test enum boundaries + typical value, then separate test for distinct behavior: +Use `vi.stubEnv()` + `vi.unstubAllEnvs()` in `afterEach`. -```typescript -test.each([TaskGrade.PENDING, TaskGrade.Q11, TaskGrade.Q10, TaskGrade.D6])( - 'returns grade %s', (grade) => { ... } -); -test('returns null when no vote', () => { ... }); -``` +### globalThis / Mutable Exports -### Environment Variables +Save and restore `globalThis` state via `Object.defineProperty` in `beforeEach`/`afterEach`. -Use `vi.stubEnv()` + `vi.unstubAllEnvs()`: +For mutable module-level `const` objects (override maps), mutate directly in `beforeEach`/`afterEach` — no `vi.mock` needed. -```typescript -beforeEach(() => { - vi.stubEnv('MY_VAR', 'value'); -}); -afterEach(() => { - vi.unstubAllEnvs(); -}); -``` +## SvelteKit-Specific -### Mutable Module-Level Exports +### Browser Toggle per Describe -When a module exports a mutable `const` object (e.g. an override map), mutate it directly in `beforeEach`/`afterEach` to test override paths — no `vi.mock` needed: +**Never register `vi.mock('$app/environment')` twice in one file** — the second hoisted call silently overwrites the first, pinning the whole file to one value. Use one dynamic mock with a `vi.hoisted` flag: ```typescript -import { buildFn, OVERRIDE_MAP } from './module'; - -beforeEach(() => { - OVERRIDE_MAP['testKey'] = { '100': 'A', '102': 'C' }; -}); -afterEach(() => { - delete OVERRIDE_MAP['testKey']; -}); +// @vitest-environment jsdom +const browserState = vi.hoisted(() => ({ value: false })); +vi.mock('$app/environment', () => ({ + get browser() { + return browserState.value; + }, +})); -test('uses override map when entry exists', () => { - expect(buildFn('testKey', ['100', '102']).get('100')).toBe('A'); -}); +// In browser describe: browserState.value = true; construct fresh instance +// In SSR describe: browserState.value = false ``` -### Route load() Unit Tests +**Never assert a browser branch on the import-time singleton** — it is always constructed in SSR mode. Construct a fresh instance inside the browser `describe`. + +In jsdom files, use jsdom's real `Storage` and assert state (`localStorage.getItem(key)`), not spy calls. Cover SSR guards separately: + +- **read guard**: pre-seed localStorage, construct store, expect the default +- **write guard**: empty localStorage, call setter, expect `getItem(key)` still null -`load` in `+page.server.ts` is a plain async function — call it directly with a mock event. Pass `setHeaders` as a `vi.fn()` spy to assert whether and how headers are set. What unit tests **cannot** verify: whether the header actually reaches the wire, or that `Set-Cookie` is absent (auth mocks bypass that) — cover those in E2E. +### Route load() Tests + +Call `load` directly with a mock event. Pass `setHeaders` as `vi.fn()` to assert header behavior. Wire assertions cannot be verified in unit tests — cover in E2E. ```typescript const createMockEvent = ({ session = null } = {}) => @@ -226,67 +161,3 @@ const createMockEvent = ({ session = null } = {}) => setHeaders: vi.fn(), }) as unknown as Parameters[0] & { setHeaders: ReturnType }; ``` - -### Test Stubs - -Parameter types **must match** production signature — use domain types (`TaskGrade`), not `string`. Mismatch compiles silently but breaks type safety. - -## Component Testing - -- Extract logic to `utils/` or `_utils/` and test there, not in component -- Omit component Vitest if template-only **and** E2E covers rendering paths - -## Coverage - -Cover meaningful boundaries: happy path, error cases, and edge cases specific to the domain (e.g. empty arrays, null, enum extremes). Run `pnpm coverage` to spot untested branches — treat low coverage as a signal to review, not a target to hit mechanically. - -## Multiple Test Location Patterns - -During migration, support both centralized (`src/test/`) and co-located (`src/features/`, `src/lib/`) tests. -Configure `vite.config.ts` with explicit ordering: - -```typescript -include: [ - 'src/lib/**/*.test.ts', // shared utilities (adjacent) - 'src/test/**/*.test.ts', // legacy centralized - 'src/features/**/*.test.ts', // feature co-location -], -``` - -## Test Files Ship with Code - -Never defer tests. For non-trivial logic without explicit test requirement, add them anyway. - -## Mocking globalThis Properties - -Save and restore `globalThis` state to prevent test leaks: - -```typescript -const original = globalThis.location; - -beforeEach(() => { - Object.defineProperty(globalThis, 'location', { - value: { origin: 'http://test' }, - writable: true, - }); -}); - -afterEach(() => { - if (original !== undefined) { - Object.defineProperty(globalThis, 'location', { value: original, writable: true }); - } -}); -``` - -## Guard Clause Reachability - -Ensure guard clauses don't make later code unreachable. Example anti-pattern: - -```typescript -// Bad: 'http://localhost' is unreachable -if (location?.origin) return location.origin; -if (!browser) return ''; -return 'http://localhost'; // Never reached in browser -``` - -Simplify to remove dead code after the final guard. From 9aa50ab9717d062cff96ca0b47820ba517925772 Mon Sep 17 00:00:00 2001 From: "k.hiro1818" Date: Fri, 4 Sep 2026 22:27:26 +0000 Subject: [PATCH 6/6] docs(rules): clarify clearMocks behavior for mockResolvedValue and vi.when Co-Authored-By: Claude Opus 4.6 Claude-Session: https://claude.ai/code/session_01LpBu3jvGWLNEeyWVL2PQZT --- .claude/rules/testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.claude/rules/testing.md b/.claude/rules/testing.md index 01ee0a917..b804f1c4f 100644 --- a/.claude/rules/testing.md +++ b/.claude/rules/testing.md @@ -65,7 +65,7 @@ test.each([TaskGrade.PENDING, TaskGrade.Q11, TaskGrade.Q10, TaskGrade.D6])( ### Cleanup (Vitest v5) -- `clearMocks: true` is the v5 default — **never add `vi.clearAllMocks()`**; use `mockResolvedValue` (not `Once`) since auto-clear handles reset +- `clearMocks: true` is the v5 default — **never add `vi.clearAllMocks()`**; it clears call history only — `mockResolvedValue` / `vi.when()` implementations persist, so each test re-sets what it needs - `restoreMocks` is still `false` — `vi.restoreAllMocks()` remains needed for `vi.spyOn` ### Service Layer (Prisma)