From 36550a157d5fe8a1be1ad667df5b6fcdf36f8d9c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 19:49:38 -0700 Subject: [PATCH 1/2] perf(tests): mock the tool registry globally, drop the dead deps optimizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/blocks/blocks/outlook.test.ts | 6 ++++++ .../lib/workflows/search-replace/indexer.test.ts | 6 ++++++ .../search-replace/replacements.test.ts | 6 ++++++ apps/sim/tools/azure_devops/azure-devops.test.ts | 6 ++++++ apps/sim/vitest.config.ts | 7 ------- apps/sim/vitest.setup.ts | 16 ++++++++++++++++ 6 files changed, 40 insertions(+), 7 deletions(-) diff --git a/apps/sim/blocks/blocks/outlook.test.ts b/apps/sim/blocks/blocks/outlook.test.ts index cd60240b048..c8b36c5ad46 100644 --- a/apps/sim/blocks/blocks/outlook.test.ts +++ b/apps/sim/blocks/blocks/outlook.test.ts @@ -5,6 +5,12 @@ import { describe, expect, it } from 'vitest' import { tools as toolRegistry } from '@/tools/registry' import { OutlookBlock } from './outlook' +/** + * Uses the real tool registry: these assertions are about tool registration and + * params, which the global `@/tools/registry` mock in vitest.setup.ts empties. + */ +vi.unmock('@/tools/registry') + const block = OutlookBlock /** Every calendar operation exposed by the block's operation dropdown. */ diff --git a/apps/sim/lib/workflows/search-replace/indexer.test.ts b/apps/sim/lib/workflows/search-replace/indexer.test.ts index 2fe792b2073..cf6e68a3cb2 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.test.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.test.ts @@ -10,6 +10,12 @@ import { } from '@/lib/workflows/search-replace/search-replace.fixtures' import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields' +/** + * Uses the real tool registry: these assertions are about tool registration and + * params, which the global `@/tools/registry` mock in vitest.setup.ts empties. + */ +vi.unmock('@/tools/registry') + describe('indexWorkflowSearchMatches', () => { it('finds plain text matches across nested subblock values', () => { const workflow = createSearchReplaceWorkflowFixture() diff --git a/apps/sim/lib/workflows/search-replace/replacements.test.ts b/apps/sim/lib/workflows/search-replace/replacements.test.ts index 354dff37f8c..9fcb60b8635 100644 --- a/apps/sim/lib/workflows/search-replace/replacements.test.ts +++ b/apps/sim/lib/workflows/search-replace/replacements.test.ts @@ -10,6 +10,12 @@ import { } from '@/lib/workflows/search-replace/search-replace.fixtures' import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields' +/** + * Uses the real tool registry: these assertions are about tool registration and + * params, which the global `@/tools/registry` mock in vitest.setup.ts empties. + */ +vi.unmock('@/tools/registry') + describe('buildWorkflowSearchReplacePlan', () => { it('replaces selected text ranges across blocks without touching unselected matches', () => { const workflow = createSearchReplaceWorkflowFixture() diff --git a/apps/sim/tools/azure_devops/azure-devops.test.ts b/apps/sim/tools/azure_devops/azure-devops.test.ts index 104232b18a5..520b143675e 100644 --- a/apps/sim/tools/azure_devops/azure-devops.test.ts +++ b/apps/sim/tools/azure_devops/azure-devops.test.ts @@ -45,6 +45,12 @@ const baseParams = { accessToken: 'pat-token', } +/** + * Uses the real tool registry: these assertions are about tool registration and + * params, which the global `@/tools/registry` mock in vitest.setup.ts empties. + */ +vi.unmock('@/tools/registry') + const authHeader = `Basic ${Buffer.from(':pat-token').toString('base64')}` const allTools = [ diff --git a/apps/sim/vitest.config.ts b/apps/sim/vitest.config.ts index cd5eea87377..453c0009b74 100644 --- a/apps/sim/vitest.config.ts +++ b/apps/sim/vitest.config.ts @@ -25,13 +25,6 @@ export default defineConfig({ fileParallelism: true, maxConcurrency: 10, testTimeout: 10000, - deps: { - optimizer: { - web: { - enabled: true, - }, - }, - }, }, resolve: { alias: [ diff --git a/apps/sim/vitest.setup.ts b/apps/sim/vitest.setup.ts index 82760d33715..25c34639607 100644 --- a/apps/sim/vitest.setup.ts +++ b/apps/sim/vitest.setup.ts @@ -97,6 +97,20 @@ vi.mock('@/stores/execution/store', () => ({ useLastRunEdges: vi.fn().mockReturnValue(new Map()), })) +/** + * The tool registry is 4,351 entries pulling ~5,907 modules, and almost nothing + * under test needs the real thing — but every test file that transitively + * reaches it paid to import the whole graph. Measured on the full suite: + * import 1,347s -> 633s, transform 130s -> 53s. + * + * `@/blocks/registry` is mocked the same way directly below, for the same reason. + * + * Tests that genuinely assert registration or tool params opt out with + * `vi.unmock('@/tools/registry')` at the top of the file — see + * blocks/blocks/outlook.test.ts for the pattern. + */ +vi.mock('@/tools/registry', () => ({ tools: {} })) + vi.mock('@/blocks/registry', () => ({ getBlock: vi.fn(() => ({ name: 'Mock Block', @@ -107,6 +121,8 @@ vi.mock('@/blocks/registry', () => ({ })), getAllBlocks: vi.fn(() => []), getLatestBlock: vi.fn(() => undefined), + /** Mirrors the real module's accessor; without it consumers get "not a function". */ + getBlockRegistry: vi.fn(() => ({})), getBlockByToolName: vi.fn((toolName: string) => toolName.startsWith('gmail_') ? { From 27af851c2bd051b9c2996cc55efc789a4c710e3e Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Thu, 30 Jul 2026 19:55:24 -0700 Subject: [PATCH 2/2] docs(tests): explain why the search-replace registry opt-outs are load-bearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- apps/sim/lib/workflows/search-replace/indexer.test.ts | 9 +++++++-- .../lib/workflows/search-replace/replacements.test.ts | 9 +++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/apps/sim/lib/workflows/search-replace/indexer.test.ts b/apps/sim/lib/workflows/search-replace/indexer.test.ts index cf6e68a3cb2..e970b80c787 100644 --- a/apps/sim/lib/workflows/search-replace/indexer.test.ts +++ b/apps/sim/lib/workflows/search-replace/indexer.test.ts @@ -11,8 +11,13 @@ import { import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields' /** - * Uses the real tool registry: these assertions are about tool registration and - * params, which the global `@/tools/registry` mock in vitest.setup.ts empties. + * Uses the real tool registry. Nothing here imports it directly — the dependency + * is transitive: the search-replace planner resolves tool input params through + * real subblock configs, so the global `@/tools/registry` mock in + * vitest.setup.ts empties the data these assertions read. + * + * Not a no-op, despite the lack of a direct import. Dropping this opt-out fails + * 8 tests across this file and its sibling suite. */ vi.unmock('@/tools/registry') diff --git a/apps/sim/lib/workflows/search-replace/replacements.test.ts b/apps/sim/lib/workflows/search-replace/replacements.test.ts index 9fcb60b8635..340fa434350 100644 --- a/apps/sim/lib/workflows/search-replace/replacements.test.ts +++ b/apps/sim/lib/workflows/search-replace/replacements.test.ts @@ -11,8 +11,13 @@ import { import { WORKFLOW_SEARCH_SUBFLOW_FIELD_IDS } from '@/lib/workflows/search-replace/subflow-fields' /** - * Uses the real tool registry: these assertions are about tool registration and - * params, which the global `@/tools/registry` mock in vitest.setup.ts empties. + * Uses the real tool registry. Nothing here imports it directly — the dependency + * is transitive: the search-replace planner resolves tool input params through + * real subblock configs, so the global `@/tools/registry` mock in + * vitest.setup.ts empties the data these assertions read. + * + * Not a no-op, despite the lack of a direct import. Dropping this opt-out fails + * 8 tests across this file and its sibling suite. */ vi.unmock('@/tools/registry')