perf(tests): trim sweep, copilot, and timer-bound suites; run three CI shards - #7375
Merged
Conversation
Nothing in apps/docs imports mdx/types or a bare *.mdx module; the generated .source imports carry a ?collection= suffix that the *.mdx wildcard does not match, and @types/mdx still resolves transitively via @mdx-js/mdx for fumadocs. The docs type-check passes without the direct edge.
Audited the 229 test files declaring @vitest-environment jsdom. 217 need a real DOM (React rendering, TipTap/ProseMirror, window/document/localStorage, DOMParser, ProgressEvent) and stay on jsdom; 12 exercise pure logic (yjs collab-doc conversion, http tool URL building, panel editor store, log utils, attachment upload, headless TipTap table commands, terminal-session tooltip helpers) and now run on node, skipping the jsdom boot. Dropped one redundant test in apply-streamed-markdown.test.ts: the hand-rolled one-shot peer case is a strict subset of the NON-OVERLAPPING two-peer test in apply-streamed-markdown.concurrent.test.ts, which also asserts CRDT convergence and exactly-once survival of the peer edit.
Real sleeps and repeated setup were the run-phase cost in seven files: - tools/index.test.ts: `retryDelayMs: 0` falls through to the 500ms default, so every retry test slept for real. Fake the clock for the retry describe. - embeddings/client.test.ts: two retry tests waited the 1s initial backoff. - hosted-key-rate-limiter.test.ts: the queue-head test slept two 200ms polls; the abort test slept 20ms wall clock and asserted on Date.now. - event-buffer.test.ts: sleep(60)/sleep(5) waited on the 15ms flush timer. - events.attribution.test.ts: walked and read the whole app twice, serially. Walk once in a beforeAll with batched concurrent reads. - sso-trust.test.ts: imported the Better Auth graph in each of three tests. Import once in a beforeAll that carries the 30s budget. - markdown-parse.test.ts: the one-shot helper built and destroyed a tiptap Editor per document; reuse one across the 400-seed property corpus. No production timing constants, corpus sizes, or assertions changed.
…m 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.
…ock, and trigger registries The router test stubs every handler implementation module so the handler map binds without loading the executor and block registry; the 90s timeout goes. Handler, VFS, payload, and request tests mock the execution-side leaves and catalog registries they never exercise. Source edges that dragged in barrels for a single symbol now import the defining module: the Mothership block icon from @sim/emcn/icons, contract schemas from their contract file, and autolayout constants from new @sim/workflow-renderer/dimensions and /note-content subpaths. lib/copilot: 141 files, per-file collect+run 74.2s -> 37.4s, wall 19.1s -> 15.0s router.test.ts: 15.1s -> 0.45s
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryThis PR reduces test-suite runtime by increasing the apps/sim CI shard count and replacing expensive test setup, real-time waits, and broad registry imports with narrower equivalents.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| .github/workflows/test-build.yml | Expands apps/sim test execution to three consistently configured shards with shard-specific Turbo caches. |
| packages/testing/src/mocks/block-registry.mock.ts | Adds a focused registry-map helper used to avoid loading the complete production block catalog in narrow tests. |
| apps/sim/lib/copilot/tool-executor/router.test.ts | Narrows router-test dependencies while retaining wiring assertions, including the real cancellation handler binding. |
| apps/sim/tools/request-transport.test.ts | Hosts the real-registry operation-partition sweep in a suite that already incurs registry initialization. |
| packages/workflow-renderer/package.json | Exposes direct dimensions and note-related module entry points for narrower imports. |
| apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/markdown-parse.test.ts | Reuses one TipTap editor across property tests and destroys it after the suite. |
Reviews (2): Last reviewed commit: "fix(tests): bind the real cancellation h..." | Re-trigger Greptile
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 56 files
Heads up: you’re close to your included review allowance. Set a flex budget so reviews don’t pause.
Fix all with cubic | Re-trigger cubic
…ck the narrowed contract path
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Third pass on the apps/sim suite after #7367 and #7369, done as five independent audits with regression-injection checks (each fix was verified by breaking the guarded behavior and watching the test fail).
registry.server.test.tsno longer unmocks the executable tool registry (registration is checked through the generatedtool-ids; its operation-partition sweep moved torequest-transport.test.ts, which already pays for the registry). Seven tests that unmocked the full block map to read one or two blocks use a partial map built from those block modules (partialBlockRegistryin@sim/testing).tools/metadata.test.tscollects offenders into one assertion instead of ~30k per-entry expects. A deadvi.unmock('@/tools/registry')was removed from the search-replace tests. Real sweeps stay real.@sim/emcn/icons, contract schemas from their contract file, and autolayout constants from new@sim/workflow-renderer/dimensionsand/note-contentsubpaths. lib/copilot per-file collect+run 74s -> 37s.tools/index,embeddings/client, and the hosted-key rate limiter waited on real backoff (retryDelayMs: 0fell through to the 500ms default); the event buffer waited on its flush timer;events.attributionwalked the repo twice.markdown-parse.test.tswent 22.9s -> 2.1s by reusing one TipTap editor across the 400-seed property tests, with every assertion and input size unchanged.@types/mdxin apps/docs) and is removed. No duplicated test tooling was found.Local full run: 2,858 files, 195s -> 178s wall; cumulative import 1,108s -> 992s. Rejected after measurement, not shipped: happy-dom (11 files fail on DOM differences) and a non-isolated project for the sweep tests (mocks leak, no gain).
Type of Change
Testing
Root
bun run test(all 19 workspaces green, apps/sim 39,4xx tests),bunx turbo run type-check,bun run lint:check,bun run check:audits. Regression-injection checks per change are listed in the commit messages.Checklist