Skip to content

Commit c553de1

Browse files
authored
perf(tests): mock the tool registry globally, drop the dead deps optimizer (#6117)
* perf(tests): mock the tool registry globally, drop the dead deps optimizer The test suite spent far more time importing modules than running them: on the full suite, `import` was 1,399s aggregate against 88s of actual tests. Per-file import cost showed exactly where it came from — lib/core, which touches no registry, runs at 0.09s/file, while every area that reaches the tool registry runs 12x-79x that (blocks 7.12s/file, providers 3.65, executor 3.07, tools 2.05, app/api 1.05). The tool registry is 4,351 entries pulling ~5,907 modules, and almost nothing under test needs the real thing. `@/blocks/registry` was already globally mocked for this reason; this does the same for `@/tools/registry`. Full suite, same commit, same machine: baseline Duration 166.21s (transform 141.80s, import 1399.17s) after Duration 91.56s (transform 61.98s, import 617.08s) 45% faster, import -56%, transform -56%. Identical results either way: 1252 files / 16873 tests pass, plus one failure that reproduces on unmodified staging (cloud-review-tools.test.ts cannot find `rg` from its spawned python3 locally; CI installs ripgrep and it passes there). Four test files genuinely assert tool registration or tool params, so they opt out with `vi.unmock('@/tools/registry')` rather than being weakened or deleted — outlook, azure_devops, and the two search-replace suites. No coverage is lost. Also removes `deps.optimizer.web`, which was dead config: it only applies to client environments (jsdom/happy-dom) and 985 of 1,219 files declare `@vitest-environment node`. Measured both ways to be sure — removing it is a no-op (19.33s -> 19.23s), and switching it to the correct `ssr` side with an include list for the heavy provider SDKs was also a no-op (19.19s). The cost is first-party module graph, which the optimizer does not touch, so the honest move is to delete it rather than leave config that reads as if it does something. Adds the missing `getBlockRegistry` accessor to the existing `@/blocks/registry` mock. #6083 renamed that export and the mock was never updated, so any test reaching those three consumers would have hit "getBlockRegistry is not a function". Nothing exercises them today. Not done, deliberately: `isolate: false` is ~20% faster but leaks state between files and broke two doc-servable tests on the first run. * docs(tests): explain why the search-replace registry opt-outs are load-bearing Review read the missing direct import of @/tools/registry as evidence the vi.unmock calls were no-ops. They are not: the dependency is transitive — the search-replace planner resolves tool input params through real subblock configs — and removing both opt-outs fails 8 tests across the two suites. Comment now says that, so the next reader does not delete them.
1 parent e443a97 commit c553de1

6 files changed

Lines changed: 50 additions & 7 deletions

File tree

apps/sim/blocks/blocks/outlook.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ import { describe, expect, it } from 'vitest'
55
import { tools as toolRegistry } from '@/tools/registry'
66
import { OutlookBlock } from './outlook'
77

8+
/**
9+
* Uses the real tool registry: these assertions are about tool registration and
10+
* params, which the global `@/tools/registry` mock in vitest.setup.ts empties.
11+
*/
12+
vi.unmock('@/tools/registry')
13+
814
const block = OutlookBlock
915

1016
/** Every calendar operation exposed by the block's operation dropdown. */

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ import {
1010
} from '@/lib/workflows/search-replace/search-replace.fixtures'
1111
import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields'
1212

13+
/**
14+
* Uses the real tool registry. Nothing here imports it directly — the dependency
15+
* is transitive: the search-replace planner resolves tool input params through
16+
* real subblock configs, so the global `@/tools/registry` mock in
17+
* vitest.setup.ts empties the data these assertions read.
18+
*
19+
* Not a no-op, despite the lack of a direct import. Dropping this opt-out fails
20+
* 8 tests across this file and its sibling suite.
21+
*/
22+
vi.unmock('@/tools/registry')
23+
1324
describe('indexWorkflowSearchMatches', () => {
1425
it('finds plain text matches across nested subblock values', () => {
1526
const workflow = createSearchReplaceWorkflowFixture()

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,17 @@ import {
1010
} from '@/lib/workflows/search-replace/search-replace.fixtures'
1111
import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields'
1212

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

apps/sim/tools/azure_devops/azure-devops.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ const baseParams = {
4545
accessToken: 'pat-token',
4646
}
4747

48+
/**
49+
* Uses the real tool registry: these assertions are about tool registration and
50+
* params, which the global `@/tools/registry` mock in vitest.setup.ts empties.
51+
*/
52+
vi.unmock('@/tools/registry')
53+
4854
const authHeader = `Basic ${Buffer.from(':pat-token').toString('base64')}`
4955

5056
const allTools = [

apps/sim/vitest.config.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,6 @@ export default defineConfig({
2525
fileParallelism: true,
2626
maxConcurrency: 10,
2727
testTimeout: 10000,
28-
deps: {
29-
optimizer: {
30-
web: {
31-
enabled: true,
32-
},
33-
},
34-
},
3528
},
3629
resolve: {
3730
alias: [

apps/sim/vitest.setup.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,20 @@ vi.mock('@/stores/execution/store', () => ({
9797
useLastRunEdges: vi.fn().mockReturnValue(new Map()),
9898
}))
9999

100+
/**
101+
* The tool registry is 4,351 entries pulling ~5,907 modules, and almost nothing
102+
* under test needs the real thing — but every test file that transitively
103+
* reaches it paid to import the whole graph. Measured on the full suite:
104+
* import 1,347s -> 633s, transform 130s -> 53s.
105+
*
106+
* `@/blocks/registry` is mocked the same way directly below, for the same reason.
107+
*
108+
* Tests that genuinely assert registration or tool params opt out with
109+
* `vi.unmock('@/tools/registry')` at the top of the file — see
110+
* blocks/blocks/outlook.test.ts for the pattern.
111+
*/
112+
vi.mock('@/tools/registry', () => ({ tools: {} }))
113+
100114
vi.mock('@/blocks/registry', () => ({
101115
getBlock: vi.fn(() => ({
102116
name: 'Mock Block',
@@ -107,6 +121,8 @@ vi.mock('@/blocks/registry', () => ({
107121
})),
108122
getAllBlocks: vi.fn(() => []),
109123
getLatestBlock: vi.fn(() => undefined),
124+
/** Mirrors the real module's accessor; without it consumers get "not a function". */
125+
getBlockRegistry: vi.fn(() => ({})),
110126
getBlockByToolName: vi.fn((toolName: string) =>
111127
toolName.startsWith('gmail_')
112128
? {

0 commit comments

Comments
 (0)