perf(tests): mock the tool registry globally, drop the dead deps optimizer - #6117
Conversation
…mizer
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.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview Four test files that assert real registration or tool params opt out with Vitest config: removes the Blocks registry mock: adds Reviewed by Cursor Bugbot for commit 27af851. Configure here. |
Greptile SummaryThis PR reduces Vitest startup overhead while preserving tests that require real registry data.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| apps/sim/vitest.setup.ts | Adds the global lightweight tool-registry mock and brings the block-registry mock in sync with its current accessor API. |
| apps/sim/vitest.config.ts | Removes dependency-optimizer configuration that did not benefit the predominantly Node-based test suite. |
| apps/sim/lib/workflows/search-replace/indexer.test.ts | Restores the real tool registry because tool-input indexing resolves parameter configuration through the registry. |
| apps/sim/lib/workflows/search-replace/replacements.test.ts | Restores the real tool registry for replacement scenarios that first index registry-backed tool-input parameters. |
| apps/sim/blocks/blocks/outlook.test.ts | Restores the real registry for assertions covering Outlook tool registration and parameters. |
| apps/sim/tools/azure_devops/azure-devops.test.ts | Restores the real registry for Azure DevOps registration and parameter assertions. |
Reviews (2): Last reviewed commit: "docs(tests): explain why the search-repl..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…d-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.
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 27af851. Configure here.
1 participant
Summary
The test suite spent far more time importing modules than running them —
importwas 1,399s aggregate against 88s of actual tests. This cuts the full suite 45%, with zero coverage lost.Lint and Testis currently the CI critical path (p50 189s vs Build App's 150s), and ~143s of it is the test run.Where the cost was — measured per area
lib/coretouches no registry and sets the floor at 0.09s/file. Everything that reaches the tool registry graph:The tool registry is 4,351 entries pulling ~5,907 modules, and almost nothing under test needs the real thing.
@/blocks/registrywas already globally mocked for exactly this reason — this extends the same pattern to@/tools/registry.Result — full suite, same commit, same machine
The single failure reproduces on unmodified staging —
cloud-review-tools.test.tscan't findrgfrom its spawnedpython3on macOS. CI installs ripgrep explicitly and it passes there.No coverage was removed
Four files genuinely assert tool registration or tool params. They opt out with
vi.unmock('@/tools/registry')rather than being weakened or deleted —outlook,azure_devops, and the twosearch-replacesuites. Each carries a comment explaining why, so the pattern is discoverable for the next person who needs the real registry.These are exactly the tests that catch blocks and tools drifting apart, which is the kind of breakage that ships silently. Deleting them to make the number work would have been trading correctness for speed.
deps.optimizerwas dead config — measured, not assumedIt was set to
web, which only applies to client environments (jsdom/happy-dom), while 985 of 1,219 files declare@vitest-environment node. Vitest's docs are explicit that node/edge usessr.I measured both ways rather than just "fixing" it to
ssr:web)ssr+ include list for openai / Anthropic / Google / Bedrock SDKsAll identical within noise. The cost is first-party module graph, which the optimizer doesn't touch — it only bundles
node_modules. So the honest move is deleting it, not leaving config that reads as if it does something.Also fixes a latent mock gap
Adds the missing
getBlockRegistryaccessor to the@/blocks/registrymock. #6083 renamed that export and the mock was never updated, so any test reaching those three consumers would have hitgetBlockRegistry is not a function. Nothing exercises them today — this closes it before someone trips on it.Rejected, with evidence
isolate: falseis ~20% faster (import 136.6s → 110.4s on a 145-file set) but leaks state between files — it broke twodoc-servabletests on the first run that pass under isolation. Correctness over speed.Type of Change
Testing
Full suite run both ways on the same machine (table above).
tsc --noEmitexit 0.biome checkclean. Each of the four opt-out files verified individually: outlook 21/21, azure-devops 28/28, indexer 58/58, replacements 32/32.