Conversation
There was a problem hiding this comment.
Pull request overview
Updates the repository’s test harness and dependency graph to resolve failing tests in the Cloudflare Workers + Vitest setup, while also refreshing the generated Wrangler worker type definitions.
Changes:
- Reworked
test/setup.tsto centralize Better Auth + Drizzle mocks (and a globalfetchstub) viasetupFiles. - Refactored
test/mocks/auth.tsandtest/mocks/drizzle.tsto read mocks from a sharedglobalThis.__testMocksinitialized by the setup file. - Added
vitetodevDependenciesand updatedpnpm-lock.yamlaccordingly; regeneratedworker-configuration.d.ts.
Reviewed changes
Copilot reviewed 4 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| worker-configuration.d.ts | Regenerated Wrangler/workerd type definitions; updates Env binding types. |
| test/setup.ts | New centralized initialization for auth/drizzle mocks and global fetch stub used by Vitest setupFiles. |
| test/mocks/drizzle.ts | Switched from defining mocks locally to exporting mocks initialized in test/setup.ts. |
| test/mocks/auth.ts | Same pattern change as drizzle mock: exports globals set up in test/setup.ts. |
| package.json | Adds explicit vite devDependency (likely to satisfy Vitest/Vite peer/runtime needs). |
| pnpm-lock.yaml | Lockfile updates reflecting new/updated Vite/Vitest-related dependency resolution. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| vi.mock('../lib/auth', () => ({ | ||
| createAuth: vi.fn(() => ({ | ||
| api: { | ||
| getSession: mocks.betterAuth.getSession, | ||
| signUpEmail: mocks.betterAuth.signUpEmail, |
There was a problem hiding this comment.
test/setup.ts mocks ../lib/auth, but the codebase imports the same module using multiple specifiers (e.g. ../lib/auth in src/index.ts and ../../lib/auth in src/routes/* / src/utils/authMiddleware.ts). Depending on the test runner’s module ID normalization, mocking only one specifier can leave some imports unmocked and accidentally exercise the real Better Auth implementation. Consider mocking both specifiers (or standardizing imports to a single path) so all createAuth imports resolve to the same mocked module.
| if (!testGlobals.__testMocks) { | ||
| throw new Error('Expected auth mocks to be initialized in test/setup.ts'); | ||
| } |
There was a problem hiding this comment.
These test helpers throw at module import time when globalThis.__testMocks is missing. That makes failures harder to diagnose and can break any tooling that imports the module outside the Vitest runtime (or if setupFiles is misconfigured). Prefer making this a lazy assertion (e.g., throw when mockBetterAuth/mockAuth() is accessed) or export a getTestMocks() accessor with a clear error message.
| if (!testGlobals.__testMocks) { | ||
| throw new Error('Expected drizzle mocks to be initialized in test/setup.ts'); | ||
| } |
There was a problem hiding this comment.
These test helpers throw at module import time when globalThis.__testMocks is missing. Consider deferring this assertion until the exported mocks are accessed (or providing an accessor) to avoid hard-to-debug import-time crashes if setupFiles changes or if the module is imported by tooling outside Vitest.
No description provided.