Skip to content

Commit b2cd2a9

Browse files
waleedlatif1waleedlatif1
authored
chore(tests): tighten check:test-patterns and shrink its baseline (#8886)
* chore(tests): tighten check:test-patterns and shrink its baseline - Exempt *.live.test.ts from global-remock and local-factory: live suites bind real boundaries (real Postgres via a re-mocked @sim/db). - Fail when a central mock vi.mocks an @/ module id that no longer resolves; repoint copilot-http to @/lib/mothership/request/http (renamed in #8208) and drop executor.mock's mocks of deleted modules. The check also scans vitest.setup.ts, whose mock and alias of the deleted @/stores/console/store are removed. - Gate local-helper on the workspace depending on @sim/testing. - redundant-hook inspects every afterEach statement and the leading run of a beforeEach (a later reset can deliberately discard setup's own calls). - New module-scope-stub rule: module-scope vi.stubGlobal/stubEnv/spyOn is undone before the first test; delete the six dead fetch stubs. - Rewrite @sim/testing @example barrel imports to direct paths. - Replace real sleeps with fake timers in file-doc-store and update-cost. - Convert table-constants and sim-search-connectors local factories to the central mocks; fold v1-route's subscription/rate-limiter duplicates into billingSubscriptionMock/rateLimiterMock. Baseline: 140 -> 92 entries. * fix(tests): catch exported and type-asserted module-scope stubs, and cover the stub and hook rules with fixtures
1 parent fb08094 commit b2cd2a9

60 files changed

Lines changed: 477 additions & 333 deletions

File tree

Some content is hidden

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

‎.claude/rules/sim-testing.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ describe('GET /api/my-route', () => {
8888

8989
`apps/sim/vitest.setup.ts` mocks the modules nearly every test touches. `@sim/testing` holds one
9090
central mock for every other module that more than a couple of tests mock. Never hand-roll a
91-
`vi.mock` factory for either — `bun run check:test-patterns` fails on a new one.
91+
`vi.mock` factory for either — `bun run check:test-patterns` fails on a new one (integration and
92+
`*.live.test.ts` files bind real boundaries and are exempt).
9293

9394
- **Global module**: don't `vi.mock` it; drive it through its knobs (table below).
9495
- **Any other module**: find its central mock by copying an existing use —

‎.cursor/rules/sim-testing.mdc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ describe('GET /api/my-route', () => {
8686

8787
`apps/sim/vitest.setup.ts` mocks the modules nearly every test touches. `@sim/testing` holds one
8888
central mock for every other module that more than a couple of tests mock. Never hand-roll a
89-
`vi.mock` factory for either — `bun run check:test-patterns` fails on a new one.
89+
`vi.mock` factory for either — `bun run check:test-patterns` fails on a new one (integration and
90+
`*.live.test.ts` files bind real boundaries and are exempt).
9091

9192
- **Global module**: don't `vi.mock` it; drive it through its knobs (table below).
9293
- **Any other module**: find its central mock by copying an existing use —

‎apps/desktop/src/main/terminal/shell-startup.test.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ afterEach(() => {
7676
pty.emit = null
7777
pty.exit = null
7878
pty.writes.length = 0
79-
vi.unstubAllEnvs()
8079
})
8180

8281
describe('a shell that is still starting', () => {

‎apps/realtime/src/handlers/file-doc-store.test.ts‎

Lines changed: 23 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -423,23 +423,29 @@ describe('FileDocStore', () => {
423423
* The loop must back off instead, and re-open a client that was closed rather than reading a dead one.
424424
*/
425425
it('backs off and re-opens the reader when its connection is closed, instead of spinning', async () => {
426-
const store = await newStore()
427-
const doc = new Y.Doc()
428-
await store.attachRoom(NAME, doc)
429-
state.backing!.readerClosed = true
430-
431-
state.backing!.connects = 0 // ignore the two `init` connects; count only recovery attempts
432-
const before = state.backing!.reads
433-
await sleep(3000)
434-
const attempts = state.backing!.reads - before
435-
436-
// A fixed 500ms retry manages 6–7 attempts in this window; backing off (500 → 1s → 2s → …) manages
437-
// about 3. Exact counts are timing-dependent, so assert the property — it slowed down — not a number.
438-
expect(attempts).toBeGreaterThan(0)
439-
expect(attempts).toBeLessThanOrEqual(4)
440-
// …and it tried to bring the connection back rather than leaving the tailer dead forever.
441-
expect(state.backing!.connects).toBeGreaterThan(0)
442-
doc.destroy()
426+
// Faked before `init` so the reader loop's first sleep already runs on the fake clock.
427+
vi.useFakeTimers()
428+
try {
429+
const store = await newStore()
430+
const doc = new Y.Doc()
431+
await store.attachRoom(NAME, doc)
432+
state.backing!.readerClosed = true
433+
434+
state.backing!.connects = 0 // ignore the two `init` connects; count only recovery attempts
435+
const before = state.backing!.reads
436+
await vi.advanceTimersByTimeAsync(3000)
437+
const attempts = state.backing!.reads - before
438+
439+
// A fixed 500ms retry manages 6 attempts in this window; backing off (500 → 1s → 2s → …)
440+
// manages about 3. The ±20% jitter moves the exact count, so assert that it slowed down.
441+
expect(attempts).toBeGreaterThan(0)
442+
expect(attempts).toBeLessThanOrEqual(4)
443+
// …and it tried to bring the connection back rather than leaving the tailer dead forever.
444+
expect(state.backing!.connects).toBeGreaterThan(0)
445+
doc.destroy()
446+
} finally {
447+
vi.useRealTimers()
448+
}
443449
})
444450

445451
it('elects exactly one seeder across tasks (no split-brain seed)', async () => {

‎apps/sim/app/api/billing/update-cost/route.test.ts‎

Lines changed: 58 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
import { copilotHttpMock, copilotHttpMockFns } from '@sim/testing/mocks/copilot-http.mock'
1818
import { mothershipOtelMock } from '@sim/testing/mocks/mothership-otel.mock'
1919
import { sleep } from '@sim/utils/helpers'
20-
import { afterAll, beforeEach, describe, expect, it, vi } from 'vitest'
20+
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
2121

2222
const {
2323
mockCheckAndBillOverageThreshold,
@@ -1255,58 +1255,73 @@ describe('POST /api/billing/update-cost — mid-run usage gate', () => {
12551255
expect(body.usageExceeded).toBe(false)
12561256
})
12571257

1258-
it('answers not exceeded when the standing read outlasts the callback budget', async () => {
1259-
mockCheckAttributedUsageLimits.mockImplementation(async () => {
1260-
await sleep(1500)
1261-
return { isExceeded: true, scope: 'payer' }
1258+
describe('on a fake clock', () => {
1259+
beforeEach(() => {
1260+
// Only the clock the deadline and period checks read, so mocked I/O still settles.
1261+
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] })
12621262
})
1263-
const startedAt = Date.now()
12641263

1265-
const res = await POST(attributedCallback())
1264+
afterEach(() => {
1265+
vi.useRealTimers()
1266+
})
12661267

1267-
expect(res.status).toBe(200)
1268-
await expect(res.json()).resolves.toMatchObject({ success: true, usageExceeded: false })
1269-
expect(Date.now() - startedAt).toBeLessThan(1400)
1270-
})
1268+
it('answers not exceeded when the standing read outlasts the callback budget', async () => {
1269+
mockCheckAttributedUsageLimits.mockImplementation(async () => {
1270+
await sleep(1500)
1271+
return { isExceeded: true, scope: 'payer' }
1272+
})
12711273

1272-
it('does not pause a run on a verdict read across the end of its period', async () => {
1273-
const straddling = {
1274-
...CURRENT_ATTRIBUTION,
1275-
billingPeriod: {
1276-
start: '2026-07-01T00:00:00.000Z',
1277-
end: new Date(Date.now() + 40).toISOString(),
1278-
source: 'stripe' as const,
1279-
},
1280-
}
1281-
mockRefreshAttributionPeriod.mockResolvedValue(straddling)
1282-
mockCheckAttributedUsageLimits.mockImplementation(async () => {
1283-
await sleep(80)
1284-
return { isExceeded: true, scope: 'payer' }
1274+
const pending = POST(attributedCallback())
1275+
await vi.advanceTimersByTimeAsync(1000)
1276+
const res = await pending
1277+
1278+
expect(res.status).toBe(200)
1279+
await expect(res.json()).resolves.toMatchObject({ success: true, usageExceeded: false })
1280+
// Settle the abandoned read so no later test coalesces onto it.
1281+
await vi.advanceTimersByTimeAsync(500)
12851282
})
12861283

1287-
const body = await (await POST(attributedCallback())).json()
1284+
it('does not pause a run on a verdict read across the end of its period', async () => {
1285+
const straddling = {
1286+
...CURRENT_ATTRIBUTION,
1287+
billingPeriod: {
1288+
start: '2026-07-01T00:00:00.000Z',
1289+
end: new Date(Date.now() + 40).toISOString(),
1290+
source: 'stripe' as const,
1291+
},
1292+
}
1293+
mockRefreshAttributionPeriod.mockResolvedValue(straddling)
1294+
mockCheckAttributedUsageLimits.mockImplementation(async () => {
1295+
await sleep(80)
1296+
return { isExceeded: true, scope: 'payer' }
1297+
})
12881298

1289-
expect(body.usageExceeded).toBe(false)
1290-
})
1299+
const pending = POST(attributedCallback())
1300+
await vi.advanceTimersByTimeAsync(80)
1301+
const body = await (await pending).json()
12911302

1292-
it('reloads a cached current period once it has ended', async () => {
1293-
const ending = {
1294-
...CURRENT_ATTRIBUTION,
1295-
billingPeriod: {
1296-
start: '2026-07-01T00:00:00.000Z',
1297-
end: new Date(Date.now() + 50).toISOString(),
1298-
},
1299-
}
1300-
mockRefreshAttributionPeriod
1301-
.mockResolvedValueOnce(ending)
1302-
.mockResolvedValue(CURRENT_ATTRIBUTION)
1303-
refuseOnlyCurrentPeriod()
1304-
await POST(attributedCallback())
1305-
await sleep(100)
1303+
expect(body.usageExceeded).toBe(false)
1304+
})
13061305

1307-
const body = await (await POST(attributedCallback())).json()
1306+
it('reloads a cached current period once it has ended', async () => {
1307+
const ending = {
1308+
...CURRENT_ATTRIBUTION,
1309+
billingPeriod: {
1310+
start: '2026-07-01T00:00:00.000Z',
1311+
end: new Date(Date.now() + 50).toISOString(),
1312+
},
1313+
}
1314+
mockRefreshAttributionPeriod
1315+
.mockResolvedValueOnce(ending)
1316+
.mockResolvedValue(CURRENT_ATTRIBUTION)
1317+
refuseOnlyCurrentPeriod()
1318+
await POST(attributedCallback())
1319+
await vi.advanceTimersByTimeAsync(100)
13081320

1309-
expect(body.usageExceeded).toBe(true)
1321+
const body = await (await POST(attributedCallback())).json()
1322+
1323+
expect(body.usageExceeded).toBe(true)
1324+
})
13101325
})
13111326

13121327
it('keeps a recorded charge successful when the gate read fails', async () => {

‎apps/sim/app/api/knowledge/utils.test.ts‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,6 @@ function createEmbeddingFetchMock() {
146146
return vi.fn().mockResolvedValue(createEmbeddingResponse([0.1, 0.3], 'base64'))
147147
}
148148

149-
vi.stubGlobal('fetch', createEmbeddingFetchMock())
150-
151149
import { processDocumentAsync } from '@/lib/knowledge/documents/service'
152150

153151
describe('Knowledge Utils', () => {
@@ -156,8 +154,6 @@ describe('Knowledge Utils', () => {
156154
// The document claim gates on the row it writes back, so an unstubbed
157155
// `returning()` would abort processing before any completion write.
158156
dbChainMockFns.returning.mockResolvedValue([{ id: 'doc1' }])
159-
// `unstubGlobals: true` removes the module-scope fetch stub after the
160-
// first test in the worker; re-stub it per test.
161157
vi.stubGlobal('fetch', createEmbeddingFetchMock())
162158
// Under `isolate: false` the shared `@/lib/knowledge/embeddings` module may
163159
// be cached bound to the REAL env module, so reset the real `env` object

‎apps/sim/app/api/v1/capability-gate.test.ts‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,28 +15,30 @@
1515
* projections rather than gates — a route declaring `'none'` withholds fields
1616
* instead of refusing — and are pinned in `app/api/v1/logs/projection.test.ts`.
1717
*/
18-
import {
19-
permissionGroupScopeMock,
20-
permissionGroupScopeMockFns,
21-
resetPermissionGroupScopeMock,
22-
v1PersonalKeyCredential,
23-
v1RateLimitContextModuleMock,
24-
v1RateLimiterModuleMock,
25-
v1SubscriptionModuleMock,
26-
v1WorkspaceKeyCredential,
27-
} from '@sim/testing'
2818
import { auditMock } from '@sim/testing/mocks/audit.mock'
19+
import { billingSubscriptionMock } from '@sim/testing/mocks/billing-subscription.mock'
2920
import {
3021
knowledgeServiceMock,
3122
knowledgeServiceMockFns,
3223
} from '@sim/testing/mocks/knowledge-service.mock'
24+
import {
25+
permissionGroupScopeMock,
26+
permissionGroupScopeMockFns,
27+
resetPermissionGroupScopeMock,
28+
} from '@sim/testing/mocks/permission-group-scope.mock'
3329
import { permissionsMock, permissionsMockFns } from '@sim/testing/mocks/permissions.mock'
3430
import { posthogServerMock } from '@sim/testing/mocks/posthog-server.mock'
31+
import { rateLimiterMock } from '@sim/testing/mocks/rate-limiter.mock'
3532
import { createMockRequest } from '@sim/testing/mocks/request.mock'
3633
import { tableMock, tableMockFns } from '@sim/testing/mocks/table.mock'
3734
import { tableWireMock } from '@sim/testing/mocks/table-wire.mock'
3835
import { traceStoreMock } from '@sim/testing/mocks/trace-store.mock'
3936
import { v1LogsMetaMock, v1LogsMetaMockFns } from '@sim/testing/mocks/v1-logs-meta.mock'
37+
import {
38+
v1PersonalKeyCredential,
39+
v1RateLimitContextModuleMock,
40+
v1WorkspaceKeyCredential,
41+
} from '@sim/testing/mocks/v1-route.mock'
4042
import { workflowsOrchestrationMock } from '@sim/testing/mocks/workflows-orchestration.mock'
4143
import {
4244
workspaceUploadsMock,
@@ -59,8 +61,8 @@ vi.mock('@/lib/permission-groups/config-scope.server', () => permissionGroupScop
5961
vi.mock('@/app/api/v1/auth', () => ({ authenticateV1Request: mockAuthenticateV1Request }))
6062
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
6163
vi.mock('@/lib/workspaces/utils', () => workspacesUtilsMock)
62-
vi.mock('@/lib/billing/core/subscription', () => v1SubscriptionModuleMock)
63-
vi.mock('@/lib/core/rate-limiter', () => v1RateLimiterModuleMock)
64+
vi.mock('@/lib/billing/core/subscription', () => billingSubscriptionMock)
65+
vi.mock('@/lib/core/rate-limiter', () => rateLimiterMock)
6466
vi.mock('@/lib/api/server/rate-limit-context', () => v1RateLimitContextModuleMock)
6567

6668
vi.mock('@sim/audit', () => auditMock)

‎apps/sim/app/api/v1/logs/projection.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,20 @@
1111
* either surface stops projecting.
1212
*/
1313
import { createRouteContext } from '@sim/testing/helpers/http'
14+
import { billingSubscriptionMock } from '@sim/testing/mocks/billing-subscription.mock'
1415
import {
1516
permissionGroupScopeMock,
1617
permissionGroupScopeMockFns,
1718
resetPermissionGroupScopeMock,
1819
} from '@sim/testing/mocks/permission-group-scope.mock'
1920
import { permissionsMock, permissionsMockFns } from '@sim/testing/mocks/permissions.mock'
21+
import { rateLimiterMock } from '@sim/testing/mocks/rate-limiter.mock'
2022
import { createMockRequest } from '@sim/testing/mocks/request.mock'
2123
import { traceStoreMock, traceStoreMockFns } from '@sim/testing/mocks/trace-store.mock'
2224
import { v1LogsMetaMock, v1LogsMetaMockFns } from '@sim/testing/mocks/v1-logs-meta.mock'
2325
import {
2426
v1PersonalKeyCredential,
2527
v1RateLimitContextModuleMock,
26-
v1RateLimiterModuleMock,
27-
v1SubscriptionModuleMock,
2828
v1WorkspaceKeyCredential,
2929
} from '@sim/testing/mocks/v1-route.mock'
3030
import {
@@ -44,8 +44,8 @@ vi.mock('@/lib/permission-groups/config-scope.server', () => permissionGroupScop
4444
vi.mock('@/app/api/v1/auth', () => ({ authenticateV1Request: mockAuthenticateV1Request }))
4545
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
4646
vi.mock('@/lib/workspaces/utils', () => workspacesUtilsMock)
47-
vi.mock('@/lib/billing/core/subscription', () => v1SubscriptionModuleMock)
48-
vi.mock('@/lib/core/rate-limiter', () => v1RateLimiterModuleMock)
47+
vi.mock('@/lib/billing/core/subscription', () => billingSubscriptionMock)
48+
vi.mock('@/lib/core/rate-limiter', () => rateLimiterMock)
4949
vi.mock('@/lib/api/server/rate-limit-context', () => v1RateLimitContextModuleMock)
5050
vi.mock('@/lib/logs/public-queries', () => ({
5151
listPublicWorkflowLogs: mockListPublicWorkflowLogs,

‎apps/sim/app/api/v1/tables/capability-gate.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,20 @@
1313
* row and the governing group config are mocked.
1414
*/
1515
import { createRouteContext } from '@sim/testing/helpers/http'
16+
import { billingSubscriptionMock } from '@sim/testing/mocks/billing-subscription.mock'
1617
import {
1718
permissionGroupScopeMock,
1819
permissionGroupScopeMockFns,
1920
resetPermissionGroupScopeMock,
2021
} from '@sim/testing/mocks/permission-group-scope.mock'
2122
import { permissionsMock, permissionsMockFns } from '@sim/testing/mocks/permissions.mock'
23+
import { rateLimiterMock } from '@sim/testing/mocks/rate-limiter.mock'
2224
import { createMockRequest } from '@sim/testing/mocks/request.mock'
2325
import { tableMock, tableMockFns } from '@sim/testing/mocks/table.mock'
2426
import { tableWireMock } from '@sim/testing/mocks/table-wire.mock'
2527
import {
2628
v1PersonalKeyCredential,
2729
v1RateLimitContextModuleMock,
28-
v1RateLimiterModuleMock,
29-
v1SubscriptionModuleMock,
3030
v1WorkspaceKeyCredential,
3131
} from '@sim/testing/mocks/v1-route.mock'
3232
import {
@@ -55,8 +55,8 @@ vi.mock('@/lib/permission-groups/config-scope.server', () => permissionGroupScop
5555
vi.mock('@/app/api/v1/auth', () => ({ authenticateV1Request: mockAuthenticateV1Request }))
5656
vi.mock('@/lib/workspaces/permissions/utils', () => permissionsMock)
5757
vi.mock('@/lib/workspaces/utils', () => workspacesUtilsMock)
58-
vi.mock('@/lib/billing/core/subscription', () => v1SubscriptionModuleMock)
59-
vi.mock('@/lib/core/rate-limiter', () => v1RateLimiterModuleMock)
58+
vi.mock('@/lib/billing/core/subscription', () => billingSubscriptionMock)
59+
vi.mock('@/lib/core/rate-limiter', () => rateLimiterMock)
6060
vi.mock('@/lib/api/server/rate-limit-context', () => v1RateLimitContextModuleMock)
6161
vi.mock('@/lib/table', () => tableMock)
6262
vi.mock('@/lib/table/orchestration', () => ({ performDeleteTable: vi.fn() }))

‎apps/sim/app/o/[organizationId]/settings/integrations/providers/[connectorType]/page.test.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { authMockFns } from '@sim/testing/mocks/auth.mock'
22
import { nextNavigationMock, nextNavigationMockFns } from '@sim/testing/mocks/next-navigation.mock'
3+
import { simSearchConnectorsMock } from '@sim/testing/mocks/sim-search-connectors.mock'
34
import { beforeEach, expect, it, vi } from 'vitest'
45

56
const mocks = vi.hoisted(() => ({ authorize: vi.fn() }))
@@ -8,6 +9,7 @@ vi.mock('@/lib/settings/application/organization-section-access', () => ({
89
}))
910
vi.mock('next/navigation', () => nextNavigationMock)
1011
vi.mock('@/lib/sim-search/connectors', () => ({
12+
...simSearchConnectorsMock,
1113
SEARCH_SOURCE_TYPES: [
1214
['jira', { name: 'Jira' }],
1315
['confluence', { name: 'Confluence' }],

0 commit comments

Comments
 (0)