Skip to content

test: run unit tests on Vitest 5 in Node by default - #367

Merged
chhoumann merged 7 commits into
masterfrom
build/vitest5-node-env
Oct 11, 2026
Merged

chhoumann merged 7 commits into
masterfrom
build/vitest5-node-env

Conversation

@chhoumann

@chhoumann chhoumann commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner

Summary

Dependabot's Vitest 5 bump (#339) fails type checking. On master, Vitest also builds a jsdom environment for every test file, and that setup is the largest cost of each run. This PR upgrades to Vitest 5.0.3, runs tests in Node by default, and opts only the 27 of 79 files that need a DOM or load Svelte code into jsdom.

Supersedes #339.

  • The setup file and tsconfig.test.json use the @testing-library/jest-dom/vitest entry, which fixes the 42 type errors in build(deps-dev): bump vitest from 4.1.11 to 5.0.3 #339.
  • 27 test files start with // @vitest-environment jsdom. The DOM setup moved to vitest.setup.dom.ts, unchanged apart from that import, and loads only under jsdom.
  • Two guards ported from QuickAdd (#2201) fail a Node test that loads Svelte code or reads window or document, and name the docblock to add. QuickAdd traps only document, but PodNotes guards its DOM paths with typeof window, so this trap covers both. Reads by Vitest itself, such as its object formatter probing window, do not count. Svelte's store runtime reads document once when it loads, so the Node setup loads svelte/store before it installs the trap.
  • Out of scope: a persistent transform cache (fsModuleCache) and further CI trimming.

Verification

  • npm run lint, format:check, typecheck, build, test, and docs:build pass. No Obsidian runtime check, because no plugin code changes.
  • The suite gives the same 1153 results as master, test for test. Vitest 5 drops the quotes around %s string params in 9 it.each titles.
  • vitest run wall time 11.2 s → 6.7 s and CPU time 109 s → 55 s, median of 3 interleaved runs per side against master on a shared 12-core host. The comment below has the timing table and each guard firing on a misconfigured file.

Release impact

None. Test tooling only.

Note

Run unit tests in Node by default with per-file jsdom opt-in

  • Upgrades Vitest from 4.1.11 to 5.0.3 in package.json and removes the suite-wide jsdom environment from vitest.config.ts; test files now run in Node unless they opt into jsdom via a file-level @vitest-environment jsdom directive
  • Splits setup code: DOM shims and Jest-DOM matchers move to vitest.setup.dom.ts, loaded only when window exists
  • Adds a svelteNeedsJsdom Vite plugin in vitest.config.ts that rejects Svelte file loads in non-client environments with a message telling the test to opt into jsdom; about 30 test files add the jsdom directive
  • Updates AGENTS.md and README.md to describe the new environment rules
  • Behavioral Change: Node-environment tests that read window or document now fail after the test (via the first-read stack trace in vitest.setup.ts), and tsconfig.test.json swaps the Jest-DOM type package for the Vitest-specific one

Macroscope summarized a873bfe.

Vitest 5's Assertion type no longer picks up the matchers that the root
@testing-library/jest-dom entry declares, so 42 matcher calls failed to
type-check. The setup file and tsconfig.test.json now use the
@testing-library/jest-dom/vitest entry, which augments Vitest's types.
Creating a jsdom environment for every test file was the suite's largest
cost. Only the 25 files that need a browser global now opt in with a
@vitest-environment docblock; the other 53 run in Node.

The DOM shims and jest-dom move to vitest.setup.dom.ts, which the setup
file loads only under jsdom.
Under the Node environment Vite compiles Svelte for the server, where
$state is a plain object, so a test can pass while checking server
semantics. A config plugin now fails such tests and names the jsdom
docblock. It flagged main.persistence.test.ts, which loads Svelte
components through main.ts and now runs under jsdom.
Code that guards a DOM path with `typeof window` or `typeof document`
skips that path under Node, so a test could pass without running it.
The setup file now records any read of window or document under Node
and fails the test with the jsdom hint. A read during late cleanup fails
the file. Reads by Vitest itself do not count: its object formatter
probes `window` for every snapshot and assertion diff.

Svelte's store runtime reads document once when it loads. The setup file
loads svelte/store before installing the trap, so that read never counts.
vitest.setup.dom.ts takes master's slimmer DOM setup with the
@testing-library/jest-dom/vitest import. package-lock.json is master's
lock plus the Vitest 5 tree, with the libc fields npm 10.9 drops
restored. The new src/ui/TimerNotice.test.ts opts into jsdom, because
TimerNotice calls window.setInterval.
A read in onTestFinished ran after the afterEach check, so the test that
made it passed and the next test failed. The check now runs as a finish
hook of each test, after that test's own afterEach and onTestFinished
callbacks.

Vitest's formatter reads `window` once per object it prints. Each read
built a full source-mapped stack, so 1000 object snapshots took about
1000 ms instead of 67 ms. The trap now finds the reader from a one-frame
stack and builds the full error only for a first-party read (about
200 ms).
@chhoumann

chhoumann commented Oct 11, 2026 •

Copy link
Copy Markdown
Owner Author

Evidence for the timing and guard claims in the description.

Timing

Merge commit f5231b4 against master 0d83671. The later commit ba91660 changes only the trap getter, which the suite never triggers. Each side ran 3 times, interleaved (before, after, before, after), on the same 12-core Linux host with Node 22.23.2. Other agents shared the host (load average 13 to 16), so the table also gives CPU time, which other load affects less. Every run passed 79 files and 1153 tests.

Command Before wall (median, range) After wall (median, range) Before CPU After CPU
vitest run --configLoader bundle 11.2 s (10.2 to 12.7) 6.7 s (6.1 to 7.4) 109 s 55 s
npm run test 18.4 s (17.2 to 23.9) 13.2 s (12.3 to 21.3) 122 s 69 s

Before the merge with master, a 5-round set of the same comparison gave vitest run 12.6 s to 7.7 s, faster in 5 of 5 paired rounds.

What bounds the after number: the run is CPU-bound, about 55 CPU seconds spread over the cores the host has free. Of Vitest's tracked time, jsdom setup for the 27 DOM files is the largest share (39%), then module transforms that rerun every time (27%), then test bodies (15%). In npm run test, the two svelte-check passes before Vitest take 7 to 11 s and do not change here.

CI, one sample for this PR: its Test run spent 35 s in "Run tests", 22.8 s of it in Vitest. Master's last six Test runs spent a median 57 s (34 to 60) in that step, and Vitest took a median 43.7 s (26.8 to 46.9).

Svelte guard

src/main.persistence.test.ts passed under Node before the guard existed, with server-compiled Svelte. With its docblock removed:

 FAIL  src/main.persistence.test.ts [ src/main.persistence.test.ts ]
Error: ./src/ui/ReorderQueue.svelte is Svelte code; add `// @vitest-environment jsdom` to the test file that loads it.
  Plugin: svelte-needs-jsdom
  File: ./src/ui/ReorderQueue.svelte
 Test Files  1 failed (1)
      Tests  no tests

window and document trap

With the docblock removed from src/utility/networkRequest.test.ts, 35 of 57 tests fail. The first error points at the read, a window.setTimeout inside a try/catch that would otherwise swallow it:

Error: This test reads `window`; add `// @vitest-environment jsdom` to its file.
 ❯ src/utility/networkRequest.ts:433:4
    431|    }
    432|    if (settled) return;
    433|    timeoutId = window.setTimeout(() => rejectOnce(new TimeoutError()),…
       |    ^
    434|   } catch {
    435|    rejectOnce(invalidOptions());
 ❯ runNativeRequest src/utility/networkRequest.ts:403:9
 ❯ requestProjected src/utility/networkRequest.ts:612:25
 ❯ Module.requestWithTimeout src/utility/networkRequest.ts:626:9
 ❯ src/utility/networkRequest.test.ts:117:9

Throwaway Node probes, deleted afterwards:

  • These fail with the hint: a typeof document read inside a store subscriber, getCachedEpisodes reaching its typeof window guard, a typeof document read in onTestFinished (the reading test fails, not the next one), and a test's own typeof window read after Vitest formats an object.
  • toMatchSnapshot on a plain object passes, and a failing object assertion reports only its own AssertionError.
  • 1000 object snapshots in one Node test take about 200 ms with the trap and 67 ms without it.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 11, 2026 •

Copy link
Copy Markdown

Deploying podnotes with  Cloudflare Pages  Cloudflare Pages

Latest commit: a873bfe
Status: ✅  Deploy successful!
Preview URL: https://488ffc99.podnotes.pages.dev
Branch Preview URL: https://build-vitest5-node-env.podnotes.pages.dev

View logs

@chhoumann
chhoumann merged commit 39fa7ca into master Oct 11, 2026
10 checks passed
@chhoumann
chhoumann deleted the build/vitest5-node-env branch October 11, 2026 00:24
chhoumann added a commit that referenced this pull request Oct 11, 2026
Unit tests run in Node since #367, where the platform FormData encodes, so the request test can check the bytes that would go on the wire instead of the FormData object.
chhoumann added a commit that referenced this pull request Oct 11, 2026
…366)

* refactor: transcribe through an in-house OpenAI client

Replace the openai SDK with createTranscription, one multipart POST to
/v1/audio/transcriptions through fetch with the caller's abort signal and
the SDK's 10-minute timeout. It sends exactly once, so PodNotes' three
attempts no longer stack on the SDK's own two retries (up to nine sends
per chunk before). Error messages match the SDK's for OpenAI error bodies,
plain-text and empty bodies, network failures and timeouts.

TranscriptionService and the OpenAI diarization provider now pass the API
key instead of a cached SDK client. The credential-cache hook in main.ts
is left empty here and removed with its callers in the next commit.

* refactor: drop the transcription credential cache

The cache only held the SDK client so a changed OpenAI key reference could
evict it. TranscriptionService now reads the key per run, so the settings
tab no longer needs to invalidate anything on import or key selection.

* build: add Vite's dev/prod resolve condition

Setting resolve.conditions replaces Vite's defaults, which include the
"development|production" condition. Without it esm-env fell back to
reading process.env.NODE_ENV at runtime, so Svelte's dev-only error
branches shipped in the production main.js and dev builds depended on the
shell that launched Obsidian.

Production main.js: 313,095 -> 303,042 bytes raw, 98,849 -> 95,630 bytes
gzip -9.

* refactor: check only the fetch error message for timeouts

Chromium and WebKit reject fetch with a bare TypeError and never attach a
cause, so the cause lookup could not trigger in Obsidian. Name what
errorDetail mirrors so its branches are not simplified away.

* test: read the current OpenAI key for each transcription

* fix: honor OpenAI retry-after delays when retrying transcription

The openai SDK waited for the server-directed delay before retrying, so a
transient 429 with Retry-After: 4 recovered. createTranscription sends once
and the retry loops waited a fixed 1 s then 2 s, so the same 429 exhausted
all three attempts and the transcription failed.

A non-2xx response now carries the delay from retry-after-ms or
retry-after (seconds or HTTP date), bounded to 0-60 s as openai-node does.
Both the Whisper and OpenAI diarization loops wait that delay through
their existing abortable waits and fall back to the fixed backoff
otherwise.

* test: parse the encoded multipart body back in the client test

Unit tests run in Node since #367, where the platform FormData encodes, so the request test can check the bytes that would go on the wire instead of the FormData object.
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.

1 participant