Skip to content

Commit 5cf7e8d

Browse files
waleedlatif1claude
andauthored
improvement(codebase): migrate tests to dbChainMock, extract react-query hooks (#4235)
* improvement(codebase): migrate tests to dbChainMock, extract react-query hooks Migrate 97 test files to centralized dbChainMock/dbChainMockFns helpers from @sim/testing — removes hoisted chain-wiring boilerplate. Extend dbChainMock to cover insert/update/delete/transaction/execute patterns. Extract useGitHubStars and useVoiceSettings react-query hooks from inline fetches. Centralize additional mocks (authMockFns, hybridAuthMockFns) and update docs. * fix(github-stars): centralize fallback via initialData, remove stale constants Move the placeholder star count into useGitHubStars as initialData with initialDataUpdatedAt: 0 so `data` is always a narrowed string while still refetching on mount. Fixes two Bugbot issues: stale '25.8k' in chat.tsx (vs '27.8k' in navbar) and empty-string return in fetchGitHubStars that bypassed `??` fallbacks in consumers. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(testing): wire dbChainMock.db to shared transaction and execute fns dbChainMock.db.transaction was an inline vi.fn() separate from the exported dbChainMockFns.transaction, so dbChainMockFns.transaction.mockResolvedValueOnce and assertions silently targeted the wrong instance. dbChainMock.db also omitted execute, so tests for any module that calls db.execute (logging-session, table service, billing balance) would throw TypeError. Both mocks now reference the module-level constants so overrides and resetDbChainMock affect the same fn. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(chat,testing): memoize welcome message and add selectDistinct to dbChainMock.db Why: - Welcome ChatMessage was rebuilt inline each render, producing a fresh timestamp and new array identity — cascading to ChatMessageContainer and VoiceInterface props on every tick. - dbChainMockFns exports selectDistinct/selectDistinctOn but the dbChainMock.db object omitted them, so tests that stub those builders hit undefined on the mocked module. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(chat): re-attach scroll listener once container mounts The scroll effect's empty dep array meant it ran only on the first render, when `chatConfig` is still loading and the component returns `<ChatLoadingState />` — so `messagesContainerRef.current` was null and the listener was never attached. Depend on the gating conditions that control which tree renders, so the effect re-runs once the real container is in the DOM (and re-attaches when toggling in/out of voice mode). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(chat): reset chat state on identifier change via key prop Keying `<ChatClient>` on `identifier` guarantees a full remount on route transitions between chats, so `conversationId`, `messages`, and every other piece of local state start fresh — no reset effect required. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 1ced54a commit 5cf7e8d

File tree

100 files changed

+972
-1451
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

100 files changed

+972
-1451
lines changed

.claude/rules/sim-testing.md

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@ Use Vitest. Test files: `feature.ts` → `feature.test.ts`
1313
These modules are mocked globally — do NOT re-mock them in test files unless you need to override behavior:
1414

1515
- `@sim/db``databaseMock`
16+
- `@sim/db/schema``schemaMock`
1617
- `drizzle-orm``drizzleOrmMock`
1718
- `@sim/logger``loggerMock`
19+
- `@/lib/auth``authMock`
20+
- `@/lib/auth/hybrid``hybridAuthMock` (with default session-delegating behavior)
21+
- `@/lib/core/utils/request``requestUtilsMock`
1822
- `@/stores/console/store`, `@/stores/terminal`, `@/stores/execution/store`
1923
- `@/blocks/registry`
2024
- `@trigger.dev/sdk`
@@ -192,24 +196,38 @@ hybridAuthMockFns.mockCheckSessionOrInternalAuth.mockResolvedValue({
192196

193197
### Database chain mocking
194198

199+
Use the centralized `dbChainMock` + `dbChainMockFns` helpers — no `vi.hoisted()` or chain-wiring boilerplate needed.
200+
195201
```typescript
196-
const { mockSelect, mockFrom, mockWhere } = vi.hoisted(() => ({
197-
mockSelect: vi.fn(),
198-
mockFrom: vi.fn(),
199-
mockWhere: vi.fn(),
200-
}))
202+
import { dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing'
201203

202-
vi.mock('@sim/db', () => ({
203-
db: { select: mockSelect },
204-
}))
204+
vi.mock('@sim/db', () => dbChainMock)
205+
// Spread for custom exports: vi.mock('@sim/db', () => ({ ...dbChainMock, myTable: {...} }))
205206

206207
beforeEach(() => {
207-
mockSelect.mockReturnValue({ from: mockFrom })
208-
mockFrom.mockReturnValue({ where: mockWhere })
209-
mockWhere.mockResolvedValue([{ id: '1', name: 'test' }])
208+
vi.clearAllMocks()
209+
resetDbChainMock() // only needed if tests use permanent (non-`Once`) overrides
210+
})
211+
212+
it('reads a row', async () => {
213+
dbChainMockFns.limit.mockResolvedValueOnce([{ id: '1', name: 'test' }])
214+
// exercise code that hits db.select().from().where().limit()
215+
expect(dbChainMockFns.where).toHaveBeenCalled()
210216
})
211217
```
212218

219+
**Default chains supported:**
220+
- `select()/selectDistinct()/selectDistinctOn() → from() → where()/innerJoin()/leftJoin() → where() → limit()/orderBy()/returning()/groupBy()`
221+
- `insert() → values() → returning()/onConflictDoUpdate()/onConflictDoNothing()`
222+
- `update() → set() → where() → limit()/orderBy()/returning()`
223+
- `delete() → where() → limit()/orderBy()/returning()`
224+
- `db.execute()` resolves `[]`
225+
- `db.transaction(cb)` calls cb with `dbChainMock.db`
226+
227+
All terminals default to `Promise.resolve([])`. Override per-test with `dbChainMockFns.<terminal>.mockResolvedValueOnce(...)`.
228+
229+
Use `resetDbChainMock()` in `beforeEach` only when tests replace wiring with `.mockReturnValue` / `.mockResolvedValue` (permanent). Tests using only `...Once` variants don't need it.
230+
213231
## @sim/testing Package
214232

215233
Always prefer over local test data.

.cursor/rules/sim-testing.mdc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,12 @@ Use Vitest. Test files: `feature.ts` → `feature.test.ts`
1313
These modules are mocked globally — do NOT re-mock them in test files unless you need to override behavior:
1414

1515
- `@sim/db` → `databaseMock`
16+
- `@sim/db/schema` → `schemaMock`
1617
- `drizzle-orm` → `drizzleOrmMock`
1718
- `@sim/logger` → `loggerMock`
19+
- `@/lib/auth` → `authMock`
20+
- `@/lib/auth/hybrid` → `hybridAuthMock` (with default session-delegating behavior)
21+
- `@/lib/core/utils/request` → `requestUtilsMock`
1822
- `@/stores/console/store`, `@/stores/terminal`, `@/stores/execution/store`
1923
- `@/blocks/registry`
2024
- `@trigger.dev/sdk`

apps/sim/app/(landing)/actions/github.ts

Lines changed: 0 additions & 26 deletions
This file was deleted.

apps/sim/app/(landing)/components/navbar/components/github-stars.tsx

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,7 @@
11
'use client'
22

3-
import { useEffect, useState } from 'react'
4-
import { createLogger } from '@sim/logger'
53
import { GithubOutlineIcon } from '@/components/icons'
6-
import { getFormattedGitHubStars } from '@/app/(landing)/actions/github'
7-
8-
const logger = createLogger('github-stars')
9-
10-
const INITIAL_STARS = '27.7k'
4+
import { useGitHubStars } from '@/hooks/queries/github-stars'
115

126
/**
137
* Client component that displays GitHub stars count.
@@ -16,15 +10,7 @@ const INITIAL_STARS = '27.7k'
1610
* a Server Component for optimal SEO/GEO crawlability.
1711
*/
1812
export function GitHubStars() {
19-
const [stars, setStars] = useState(INITIAL_STARS)
20-
21-
useEffect(() => {
22-
getFormattedGitHubStars()
23-
.then(setStars)
24-
.catch((error) => {
25-
logger.warn('Failed to fetch GitHub stars', error)
26-
})
27-
}, [])
13+
const { data: stars } = useGitHubStars()
2814

2915
return (
3016
<a

apps/sim/app/api/auth/oauth/connections/route.test.ts

Lines changed: 21 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3,38 +3,23 @@
33
*
44
* @vitest-environment node
55
*/
6-
import { authMock, authMockFns, createMockRequest } from '@sim/testing'
6+
import {
7+
authMockFns,
8+
createMockRequest,
9+
dbChainMock,
10+
dbChainMockFns,
11+
resetDbChainMock,
12+
} from '@sim/testing'
713
import { beforeEach, describe, expect, it, vi } from 'vitest'
814

9-
const { mockDb, mockLogger, mockParseProvider, mockJwtDecode, mockEq } = vi.hoisted(() => {
10-
const db = {
11-
select: vi.fn().mockReturnThis(),
12-
from: vi.fn().mockReturnThis(),
13-
where: vi.fn().mockReturnThis(),
14-
limit: vi.fn(),
15-
}
16-
const logger = {
17-
info: vi.fn(),
18-
warn: vi.fn(),
19-
error: vi.fn(),
20-
debug: vi.fn(),
21-
trace: vi.fn(),
22-
fatal: vi.fn(),
23-
child: vi.fn(),
24-
}
25-
return {
26-
mockDb: db,
27-
mockLogger: logger,
28-
mockParseProvider: vi.fn(),
29-
mockJwtDecode: vi.fn(),
30-
mockEq: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'eq' })),
31-
}
32-
})
33-
34-
vi.mock('@/lib/auth', () => authMock)
15+
const { mockParseProvider, mockJwtDecode, mockEq } = vi.hoisted(() => ({
16+
mockParseProvider: vi.fn(),
17+
mockJwtDecode: vi.fn(),
18+
mockEq: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'eq' })),
19+
}))
3520

3621
vi.mock('@sim/db', () => ({
37-
db: mockDb,
22+
...dbChainMock,
3823
account: { userId: 'userId', providerId: 'providerId' },
3924
user: { email: 'email', id: 'id' },
4025
eq: mockEq,
@@ -48,10 +33,6 @@ vi.mock('jwt-decode', () => ({
4833
jwtDecode: mockJwtDecode,
4934
}))
5035

51-
vi.mock('@sim/logger', () => ({
52-
createLogger: vi.fn().mockReturnValue(mockLogger),
53-
}))
54-
5536
vi.mock('@/lib/oauth/utils', () => ({
5637
parseProvider: mockParseProvider,
5738
}))
@@ -61,10 +42,7 @@ import { GET } from '@/app/api/auth/oauth/connections/route'
6142
describe('OAuth Connections API Route', () => {
6243
beforeEach(() => {
6344
vi.clearAllMocks()
64-
65-
mockDb.select.mockReturnThis()
66-
mockDb.from.mockReturnThis()
67-
mockDb.where.mockReturnThis()
45+
resetDbChainMock()
6846

6947
mockParseProvider.mockImplementation((providerId: string) => ({
7048
baseProvider: providerId.split('-')[0] || providerId,
@@ -98,14 +76,8 @@ describe('OAuth Connections API Route', () => {
9876

9977
const mockUserRecord = [{ email: 'user@example.com' }]
10078

101-
mockDb.select.mockReturnValueOnce(mockDb)
102-
mockDb.from.mockReturnValueOnce(mockDb)
103-
mockDb.where.mockResolvedValueOnce(mockAccounts)
104-
105-
mockDb.select.mockReturnValueOnce(mockDb)
106-
mockDb.from.mockReturnValueOnce(mockDb)
107-
mockDb.where.mockReturnValueOnce(mockDb)
108-
mockDb.limit.mockResolvedValueOnce(mockUserRecord)
79+
dbChainMockFns.where.mockResolvedValueOnce(mockAccounts)
80+
dbChainMockFns.limit.mockResolvedValueOnce(mockUserRecord)
10981

11082
const req = createMockRequest('GET')
11183

@@ -138,22 +110,15 @@ describe('OAuth Connections API Route', () => {
138110

139111
expect(response.status).toBe(401)
140112
expect(data.error).toBe('User not authenticated')
141-
expect(mockLogger.warn).toHaveBeenCalled()
142113
})
143114

144115
it('should handle user with no connections', async () => {
145116
authMockFns.mockGetSession.mockResolvedValueOnce({
146117
user: { id: 'user-123' },
147118
})
148119

149-
mockDb.select.mockReturnValueOnce(mockDb)
150-
mockDb.from.mockReturnValueOnce(mockDb)
151-
mockDb.where.mockResolvedValueOnce([])
152-
153-
mockDb.select.mockReturnValueOnce(mockDb)
154-
mockDb.from.mockReturnValueOnce(mockDb)
155-
mockDb.where.mockReturnValueOnce(mockDb)
156-
mockDb.limit.mockResolvedValueOnce([])
120+
dbChainMockFns.where.mockResolvedValueOnce([])
121+
dbChainMockFns.limit.mockResolvedValueOnce([])
157122

158123
const req = createMockRequest('GET')
159124

@@ -169,9 +134,7 @@ describe('OAuth Connections API Route', () => {
169134
user: { id: 'user-123' },
170135
})
171136

172-
mockDb.select.mockReturnValueOnce(mockDb)
173-
mockDb.from.mockReturnValueOnce(mockDb)
174-
mockDb.where.mockRejectedValueOnce(new Error('Database error'))
137+
dbChainMockFns.where.mockRejectedValueOnce(new Error('Database error'))
175138

176139
const req = createMockRequest('GET')
177140

@@ -180,7 +143,6 @@ describe('OAuth Connections API Route', () => {
180143

181144
expect(response.status).toBe(500)
182145
expect(data.error).toBe('Internal server error')
183-
expect(mockLogger.error).toHaveBeenCalled()
184146
})
185147

186148
it('should decode ID token for display name', async () => {
@@ -204,14 +166,8 @@ describe('OAuth Connections API Route', () => {
204166
name: 'Decoded User',
205167
})
206168

207-
mockDb.select.mockReturnValueOnce(mockDb)
208-
mockDb.from.mockReturnValueOnce(mockDb)
209-
mockDb.where.mockResolvedValueOnce(mockAccounts)
210-
211-
mockDb.select.mockReturnValueOnce(mockDb)
212-
mockDb.from.mockReturnValueOnce(mockDb)
213-
mockDb.where.mockReturnValueOnce(mockDb)
214-
mockDb.limit.mockResolvedValueOnce([])
169+
dbChainMockFns.where.mockResolvedValueOnce(mockAccounts)
170+
dbChainMockFns.limit.mockResolvedValueOnce([])
215171

216172
const req = createMockRequest('GET')
217173

apps/sim/app/api/auth/oauth/credentials/route.test.ts

Lines changed: 1 addition & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -4,36 +4,10 @@
44
* @vitest-environment node
55
*/
66

7-
import {
8-
hybridAuthMock,
9-
hybridAuthMockFns,
10-
permissionsMock,
11-
requestUtilsMock,
12-
schemaMock,
13-
workflowsUtilsMock,
14-
} from '@sim/testing'
7+
import { hybridAuthMockFns, permissionsMock, workflowsUtilsMock } from '@sim/testing'
158
import { NextRequest } from 'next/server'
169
import { beforeEach, describe, expect, it, vi } from 'vitest'
1710

18-
const { mockLogger } = vi.hoisted(() => {
19-
const logger = {
20-
info: vi.fn(),
21-
warn: vi.fn(),
22-
error: vi.fn(),
23-
debug: vi.fn(),
24-
trace: vi.fn(),
25-
fatal: vi.fn(),
26-
child: vi.fn(),
27-
}
28-
return {
29-
mockLogger: logger,
30-
}
31-
})
32-
33-
vi.mock('@/lib/auth/hybrid', () => hybridAuthMock)
34-
35-
vi.mock('@/lib/core/utils/request', () => requestUtilsMock)
36-
3711
vi.mock('@/lib/credentials/oauth', () => ({
3812
syncWorkspaceOAuthCredentialsForUser: vi.fn(),
3913
}))
@@ -42,12 +16,6 @@ vi.mock('@/lib/workflows/utils', () => workflowsUtilsMock)
4216

4317
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
4418

45-
vi.mock('@sim/db/schema', () => schemaMock)
46-
47-
vi.mock('@sim/logger', () => ({
48-
createLogger: vi.fn().mockReturnValue(mockLogger),
49-
}))
50-
5119
import { GET } from '@/app/api/auth/oauth/credentials/route'
5220

5321
describe('OAuth Credentials API Route', () => {
@@ -73,7 +41,6 @@ describe('OAuth Credentials API Route', () => {
7341

7442
expect(response.status).toBe(401)
7543
expect(data.error).toBe('User not authenticated')
76-
expect(mockLogger.warn).toHaveBeenCalled()
7744
})
7845

7946
it('should handle missing provider parameter', async () => {
@@ -90,7 +57,6 @@ describe('OAuth Credentials API Route', () => {
9057

9158
expect(response.status).toBe(400)
9259
expect(data.error).toBe('Provider or credentialId is required')
93-
expect(mockLogger.warn).toHaveBeenCalled()
9460
})
9561

9662
it('should handle no credentials found', async () => {

0 commit comments

Comments
 (0)