Skip to content

perf(vitest): halve spec suite time with node specs and worker threads - #1377

Open
askov wants to merge 3 commits into
solana-foundation:masterfrom
hoodieshq:chore/vitest-environments
Open

askov wants to merge 3 commits into
solana-foundation:masterfrom
hoodieshq:chore/vitest-environments

Conversation

@askov

@askov askov commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Description

A .ts spec runs in node instead of jsdom, and each spec file runs in a worker thread. The spec suite takes about half the time.

Speed

Before After Change
Local vitest run 1m15s 37s −51%
CI Test specs, slowest shard 2m39s 1m32s −42%
CI Test specs, four shards added up 9m08s 4m58s −46%
Wait for all checks 7m01s 6m40s no change

Local numbers are the mean of two runs each, at 7e25002. CI before is the median of 52 green PR runs since 2026-09-29. CI after is the mean of this PR's two runs. The wait for all checks stays the same, because Build-Info starts after Build-And-Test and finishes last.

Changes

  • Splits the specs Vitest project into specs-dom for .tsx specs and specs-node for .ts specs.
  • Runs a .ts spec in jsdom only when its first line is // @vitest-environment jsdom.
  • Uses the threads pool for the spec projects.
  • Removes the server.deps.inline list from vite.config.mts.
  • Merges duplicate spec files into it.each tables.
  • Deletes spec files that copy a surviving spec.
  • Imports date-fns functions from subpaths.
  • Extracts formatAnchorIdl and formatCodamaIdl from their hooks.

Type of change

  • Other (please describe): test suite speed

Screenshots

No UI change.

Testing

  • Every spec runs in one project. Run pnpm exec vitest list --filesOnly --project specs-dom, then the same with --project specs-node. Expect each spec file outside packages/ in exactly one list.
  • Suite time. Run pnpm test:ci:specs on master and on this branch. Expect about half the wall time on this branch.

Related Issues

Closes HOO-1913

Checklist

  • My code follows the project's style guidelines
  • I have added tests that prove my fix/feature works
  • All checks pass locally (pnpm test, pnpm lint, pnpm typecheck)
  • I have updated documentation as needed
  • I have run build:info script to update build information

@vercel

vercel Bot commented Oct 6, 2026

Copy link
Copy Markdown

@askov is attempting to deploy a commit to the Solana Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium impact] The changes since the last review appear safe to merge.

Summary

The PR speeds up specs by splitting Node and jsdom projects, using worker threads, and sharing test helpers.

  • Spec files use the lightest environment they need and run in worker threads.
  • Tests share reusable helpers for common mocks and rendered components.
  • Anchor and Codama IDL formatting can run outside the React hooks.
  • Date helpers load only the date-fns functions they use.
  • Download size limits are checked before base64 decoding validation.

Reviews (5) · Last reviewed commit: "chore(vitest): rename hoo-611 fixture an..." · Reviewed by Greptile

Comment thread scripts/feature-gates/lib/__tests__/module-graph.spec.ts Outdated
@askov
askov force-pushed the chore/vitest-environments branch from 314d7f6 to ae00b36 Compare October 6, 2026 09:24
@vercel

vercel Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
explorer Ready Ready Preview Oct 6, 2026 9:27am UTC

Request Review

@askov askov changed the title chore(vitest): run .ts specs in node and merge duplicate specs perf(vitest): halve spec suite time with node specs and worker threads Oct 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: why do we need this HOO-611 file?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bad name for a helper, renamed

'6dNUCJLdccKGSSQvQDNvQMKfWiV5j3XSTTGqNsCJ8mSA',
'4QUZQ4c7bZuJ4o4L8tYAEGnePFV27SUFEVmC7BYfsXRp',
] as const;
const ACCOUNTS = [1, 2, 3, 4, 5, 6].map(seed => gen.address(seed));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
const ACCOUNTS = [1, 2, 3, 4, 5, 6].map(seed => gen.address(seed));
const ACCOUNTS = [...Array(6).keys()].map(i => gen.address(i + 1));

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I would keep it as is, more readable imo. But I don't have any strong objections against your variant. Ping me if you still think it's worth it

import { gen } from '@__fixtures__/gen';
import { Transaction, TransactionInstruction } from '@solana/web3.js';

export function makeTx(): Transaction {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: should we make it a global shared helper?

@askov askov Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think not yet, two usages in the same folder. This PR is mostly about the speed/duplication problem, not about the perfect structure/code quality..

});
});

async function runningTimers<T>(pending: Promise<T>): Promise<T> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: may be rename into runAfterAllTimers, or awaitAllTimers, not sure. But runningTimers name doesn't say anything to me.

@askov askov Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, inlined it, can't come up with a good readable name, so maybe we don't need it at all

askov added 2 commits October 12, 2026 20:05
Every spec file created a jsdom window, which costs about half a second
per file. A `.ts` spec runs in node unless it declares
`// @vitest-environment jsdom`, and specs run in worker threads.
The spec suite takes 37 s instead of 75 s.
Node 22.1 to 22.6 read `--eval` input as CommonJS, so top-level await
throws before the pipeline imports run.
@askov
askov force-pushed the chore/vitest-environments branch from ae00b36 to 9eec314 Compare October 12, 2026 13:05

This branch was successfully deployed

1 active (outdated) deployment
Preview — ae00b36e Deployed Oct 6, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants