Skip to content

Commit aa19e68

Browse files
committed
perf(tests): drop the executable tool registry and full block map from non-sweep tests
The vitest suite is bounded by module loading, and the biggest single cost is a test file opting out of the global `@/tools/registry` or `@/blocks/registry` mock. Trim that where the test never needed the whole thing: - `lib/internal/tool-operations/registry.server.test.ts` checked registration against `getToolIds()` but still imported the executable registry for one sweep. That sweep — every operation-backed tool has no `request` and a registered handler — is the in-process half of the transport partition, so it moves next to the external-URL sweep in `tools/request-transport.test.ts`, which already pays for the registry. Handler loads now run in parallel. Collect 12.5s -> 0.1s, run 6s -> 4.3s; request-transport unchanged (~8s). - New `partialBlockRegistry` in `@sim/testing/mocks/block-registry.mock` builds the `@/blocks/registry-maps` shape from the block modules a test actually reads, so the real registry code runs over those blocks alone. Applied to the six non-sweep tests that unmocked the block registry for one or two blocks (webhook, slack, whatsapp, condition/function, knowledge/start_trigger): collect 3-8s -> 0.05-2s each. Registry sweeps keep the real map. - `tools/index.test.ts` retry cases set `retryDelayMs: 0`, which the retry config replaces with the 500ms default; `1` is the smallest delay honored. Run 4.0s -> 0.45s. - `tools/metadata.test.ts` collects sweep offenders into one assertion instead of one `expect` per registry entry, and indexes ids once. Run 1.1s -> 0.03s. - The `vi.unmock('@/tools/registry')` in the search-replace tests was dead: nothing in their graph imports the registry any more, and all 106 tests pass without it, contrary to the comment that claimed eight would fail.
1 parent 7c93bbe commit aa19e68

13 files changed

Lines changed: 159 additions & 68 deletions

File tree

‎apps/sim/executor/utils/block-data.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,14 @@ import type { SerializedBlock } from '@/serializer/types'
88

99
/**
1010
* These assertions are about what the real block registry publishes, so the global stub — which
11-
* returns one mock block with no outputs — would make every case here pass vacuously.
11+
* returns one mock block with no outputs — would make every case here pass vacuously. Only the
12+
* generic webhook block is read, so only it is registered.
1213
*/
1314
vi.unmock('@/blocks/registry')
15+
vi.mock('@/blocks/registry-maps', async () => {
16+
const { partialBlockRegistry } = await import('@sim/testing/mocks/block-registry.mock')
17+
return partialBlockRegistry(await import('@/blocks/blocks/generic_webhook'))
18+
})
1419

1520
function triggerBlock(type: string, params: Record<string, unknown> = {}): SerializedBlock {
1621
return {

‎apps/sim/lib/copilot/tools/server/blocks/get-blocks-metadata-projection.test.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
99
* off `@/tools/registry`.
1010
*
1111
* The sibling suite exercises this tool's gating against a mocked registry; this
12-
* one runs it against the real block registry and the real generated tool
12+
* one runs it against the real Slack block config and the real generated tool
1313
* metadata, because the thing worth proving is exactly that the metadata
14-
* artifacts can answer everything the executable registry used to.
14+
* artifacts can answer everything the executable registry used to. Only the
15+
* Slack block is read, so only it is registered.
1516
*/
1617
vi.unmock('@/blocks/registry')
18+
vi.mock('@/blocks/registry-maps', async () => {
19+
const { partialBlockRegistry } = await import('@sim/testing/mocks/block-registry.mock')
20+
return partialBlockRegistry(await import('@/blocks/blocks/slack'))
21+
})
1722

1823
const mocks = vi.hoisted(() => ({
1924
getUserPermissionConfig: vi.fn(),

‎apps/sim/lib/internal/tool-operations/registry.server.test.ts‎

Lines changed: 12 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
11
/**
22
* @vitest-environment node
33
*/
4-
import { describe, expect, it, vi } from 'vitest'
4+
import { describe, expect, it } from 'vitest'
55
import {
66
getInternalToolOperationHandler,
77
getRegisteredInternalToolOperationIds,
88
isInternalToolOperationRegistered,
99
} from '@/lib/internal/tool-operations/registry.server'
10-
import { tools } from '@/tools/registry'
1110
import { getToolIds } from '@/tools/tool-ids'
12-
import { isInternalToolConfig } from '@/tools/types'
13-
14-
vi.unmock('@/tools/registry')
1511

12+
/**
13+
* Registration is checked against the generated tool ids rather than the
14+
* executable registry, whose import costs more than every handler load below
15+
* combined. The converse — that every operation-backed tool in the registry has
16+
* a handler here — is the in-process half of the transport partition sweep in
17+
* `tools/request-transport.test.ts`, which already pays for that registry.
18+
*/
1619
describe('internal tool operation registry', () => {
1720
it('registers only canonical internal tool definitions with loadable handlers', async () => {
1821
const registeredIds = getRegisteredInternalToolOperationIds()
@@ -22,29 +25,15 @@ describe('internal tool operation registry', () => {
2225

2326
for (const toolId of registeredIds) {
2427
expect(canonicalIds.has(toolId), `Missing canonical tool definition for ${toolId}`).toBe(true)
25-
expect(await getInternalToolOperationHandler(toolId)).toBeTypeOf('function')
28+
}
29+
const handlers = await Promise.all(registeredIds.map(getInternalToolOperationHandler))
30+
for (const [index, handler] of handlers.entries()) {
31+
expect(handler, `${registeredIds[index]} has no loadable handler`).toBeTypeOf('function')
2632
}
2733
// Cost scales with the number of registered internal tools, so this budget has to grow
2834
// with the registry rather than sit just above the current total.
2935
}, 90_000)
3036

31-
it('registers every operation-backed tool and keeps it free of HTTP request metadata', async () => {
32-
const operationTools = Object.entries(tools).filter(([, tool]) => isInternalToolConfig(tool))
33-
34-
expect(operationTools.length).toBeGreaterThan(0)
35-
for (const [toolId, tool] of operationTools) {
36-
expect(tool.request, `${toolId} must not declare an HTTP request`).toBeUndefined()
37-
expect(tool.operation.input, `${toolId} must materialize its operation input`).toBeTypeOf(
38-
'function'
39-
)
40-
if (toolId === 'function_execute' || toolId === 'workflow_executor') continue
41-
expect(
42-
isInternalToolOperationRegistered(toolId),
43-
`${toolId} is missing its in-process operation handler`
44-
).toBe(true)
45-
}
46-
})
47-
4837
it('loads dynamic MCP operations without an HTTP route', async () => {
4938
expect(isInternalToolOperationRegistered('mcp-server-id-tool-name')).toBe(true)
5039
expect(await getInternalToolOperationHandler('mcp-server-id-tool-name')).toBeTypeOf('function')

‎apps/sim/lib/workflows/canonical/reported-bug.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,14 @@ import { describe, expect, it, vi } from 'vitest'
1616

1717
/**
1818
* The canonical form reads declared defaults, so the globally-mocked registry
19-
* (every block reduced to `subBlocks: []`) would make this pass vacuously.
19+
* (every block reduced to `subBlocks: []`) would make this pass vacuously. Only
20+
* the webhook block is read, so only it is registered.
2021
*/
2122
vi.unmock('@/blocks/registry')
23+
vi.mock('@/blocks/registry-maps', async () => {
24+
const { partialBlockRegistry } = await import('@sim/testing/mocks/block-registry.mock')
25+
return partialBlockRegistry(await import('@/blocks/blocks/generic_webhook'))
26+
})
2227

2328
import { generateWorkflowDiffSummary } from '@/lib/workflows/comparison/compare'
2429
import type { WorkflowState } from '@/stores/workflows/workflow/types'

‎apps/sim/lib/workflows/migrations/whatsapp-interactive-type.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,15 @@
44
import { afterAll, describe, expect, it, vi } from 'vitest'
55
import type { BlockState } from '@/stores/workflows/workflow/types'
66

7+
/**
8+
* The backfill reads the WhatsApp block's declared sub-blocks, which the global
9+
* registry stub empties. Only that block is registered.
10+
*/
711
vi.unmock('@/blocks/registry')
12+
vi.mock('@/blocks/registry-maps', async () => {
13+
const { partialBlockRegistry } = await import('@sim/testing/mocks/block-registry.mock')
14+
return partialBlockRegistry(await import('@/blocks/blocks/whatsapp'))
15+
})
816

917
import * as blocksBarrel from '@/blocks'
1018
import { getBlock as getRealBlock } from '@/blocks/registry'

‎apps/sim/lib/workflows/operations/import-export.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
import { describe, expect, it, vi } from 'vitest'
22

3+
/**
4+
* Import parsing migrates sub-block ids against each block's declared config,
5+
* which the global registry stub empties. Only the blocks the fixtures name are
6+
* registered.
7+
*/
38
vi.unmock('@/blocks/registry')
9+
vi.mock('@/blocks/registry-maps', async () => {
10+
const { partialBlockRegistry } = await import('@sim/testing/mocks/block-registry.mock')
11+
return partialBlockRegistry(
12+
await import('@/blocks/blocks/knowledge'),
13+
await import('@/blocks/blocks/start_trigger')
14+
)
15+
})
416

517
vi.mock('@/lib/api/client/request', () => ({
618
requestJson: vi.fn().mockResolvedValue({}),

‎apps/sim/lib/workflows/sanitization/subblocks.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,18 @@
33
*/
44
import { describe, expect, it, vi } from 'vitest'
55

6+
/**
7+
* Sanitization reads each block's declared sub-block types, which the global
8+
* registry stub empties. Only the blocks the cases below name are registered.
9+
*/
610
vi.unmock('@/blocks/registry')
11+
vi.mock('@/blocks/registry-maps', async () => {
12+
const { partialBlockRegistry } = await import('@sim/testing/mocks/block-registry.mock')
13+
return partialBlockRegistry(
14+
await import('@/blocks/blocks/condition'),
15+
await import('@/blocks/blocks/function')
16+
)
17+
})
718

819
import { migrateSubblockIds } from '@/lib/workflows/migrations/subblock-migrations'
920
import { sanitizeMalformedSubBlocks } from '@/lib/workflows/sanitization/subblocks'

‎apps/sim/lib/workflows/search-replace/indexer.test.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,6 @@ import { NoteBlock } from '@/blocks/blocks/note'
2121
vi.unmock('@/tools/metadata')
2222
vi.unmock('@/tools/metadata-outputs')
2323

24-
/**
25-
* Uses the real tool registry. Nothing here imports it directly — the dependency
26-
* is transitive: the search-replace planner resolves tool input params through
27-
* real subblock configs, so the global `@/tools/registry` mock in
28-
* vitest.setup.ts empties the data these assertions read.
29-
*
30-
* Not a no-op, despite the lack of a direct import. Dropping this opt-out fails
31-
* 8 tests across this file and its sibling suite.
32-
*/
33-
vi.unmock('@/tools/registry')
34-
3524
describe('indexWorkflowSearchMatches', () => {
3625
it('marks generic tool-param fallbacks as non-authoritative', () => {
3726
expect(

‎apps/sim/lib/workflows/search-replace/replacements.test.ts‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,6 @@ import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replac
1717
vi.unmock('@/tools/metadata')
1818
vi.unmock('@/tools/metadata-outputs')
1919

20-
/**
21-
* Uses the real tool registry. Nothing here imports it directly — the dependency
22-
* is transitive: the search-replace planner resolves tool input params through
23-
* real subblock configs, so the global `@/tools/registry` mock in
24-
* vitest.setup.ts empties the data these assertions read.
25-
*
26-
* Not a no-op, despite the lack of a direct import. Dropping this opt-out fails
27-
* 8 tests across this file and its sibling suite.
28-
*/
29-
vi.unmock('@/tools/registry')
30-
3120
describe('buildWorkflowSearchReplacePlan', () => {
3221
it('replaces selected text ranges across blocks without touching unselected matches', () => {
3322
const workflow = createSearchReplaceWorkflowFixture()

‎apps/sim/tools/index.test.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5105,6 +5105,10 @@ describe('MCP Tool Execution', () => {
51055105
expect(fetchSpy).not.toHaveBeenCalled()
51065106
})
51075107

5108+
/**
5109+
* `retryDelayMs: 1` rather than `0`: the retry config falls back to the 500 ms
5110+
* default for a falsy delay, so 1 ms is the smallest delay the tool honors.
5111+
*/
51085112
describe('Tool request retries', () => {
51095113
beforeEach(() => {
51105114
vi.useFakeTimers()
@@ -5118,11 +5122,7 @@ describe('MCP Tool Execution', () => {
51185122
vi.useRealTimers()
51195123
})
51205124

5121-
/**
5122-
* Runs the request with every retry backoff elapsed on the fake clock. A
5123-
* `retryDelayMs` of 0 falls through to the 500ms default, so the sleeps
5124-
* between attempts are real unless the clock is faked.
5125-
*/
5125+
/** Runs the request with every retry backoff elapsed on the fake clock. */
51265126
async function executeWithRetries(params: Record<string, unknown>) {
51275127
const pending = executeTool('http_request', params)
51285128
await vi.runAllTimersAsync()
@@ -5152,7 +5152,7 @@ describe('MCP Tool Execution', () => {
51525152
url: 'https://api.example.com/test',
51535153
method: 'GET',
51545154
retries: 2,
5155-
retryDelayMs: 0,
5155+
retryDelayMs: 1,
51565156
retryMaxDelayMs: 0,
51575157
})
51585158

@@ -5182,7 +5182,7 @@ describe('MCP Tool Execution', () => {
51825182
url: 'https://api.example.com/test',
51835183
method: 'GET',
51845184
retries: 2,
5185-
retryDelayMs: 0,
5185+
retryDelayMs: 1,
51865186
retryMaxDelayMs: 0,
51875187
})
51885188

@@ -5197,7 +5197,7 @@ describe('MCP Tool Execution', () => {
51975197
url: 'https://api.example.com/test',
51985198
method: 'GET',
51995199
retries: 5,
5200-
retryDelayMs: 0,
5200+
retryDelayMs: 1,
52015201
retryMaxDelayMs: 0,
52025202
})
52035203

@@ -5214,7 +5214,7 @@ describe('MCP Tool Execution', () => {
52145214
url: 'https://api.example.com/test',
52155215
method: 'POST',
52165216
retries: 2,
5217-
retryDelayMs: 0,
5217+
retryDelayMs: 1,
52185218
retryMaxDelayMs: 0,
52195219
})
52205220

@@ -5232,7 +5232,7 @@ describe('MCP Tool Execution', () => {
52325232
method: 'POST',
52335233
retries: 1,
52345234
retryNonIdempotent: true,
5235-
retryDelayMs: 0,
5235+
retryDelayMs: 1,
52365236
retryMaxDelayMs: 0,
52375237
})
52385238

@@ -5288,7 +5288,7 @@ describe('MCP Tool Execution', () => {
52885288
url: 'https://api.example.com/test',
52895289
method: 'GET',
52905290
retries: 2,
5291-
retryDelayMs: 0,
5291+
retryDelayMs: 1,
52925292
retryMaxDelayMs: 5000,
52935293
})
52945294

@@ -5308,7 +5308,7 @@ describe('MCP Tool Execution', () => {
53085308
url: 'https://api.example.com/test',
53095309
method: 'GET',
53105310
retries: 1,
5311-
retryDelayMs: 0,
5311+
retryDelayMs: 1,
53125312
retryMaxDelayMs: 0,
53135313
})
53145314

0 commit comments

Comments
 (0)