fix(cli): the contact sheet png test no longer waits on a Windows font cache build

[miguel-heygen]

What changes

The contact-sheet PNG test in packages/cli kept timing out on the Windows CI runner. Cause, measured on Windows CI:

• On a fresh runner, sharp's first text render makes Fontconfig build its cache for every installed OS font. In a fresh child process that first render took 9.4 s and 4.9 s in two runs; with a one-font Fontconfig file present when the process started it took 42 ms and 15 ms. A first SVG with no text took 3 ms, so it is the font step, not the image work.
• Earlier logs (the timing labels from test(cli): time the native Sharp calls in a flaky Windows contact-sheet test #4070, 7 runs) put the whole createContactSheet call at 3.7 s to 18.4 s while the two PNG writes took 2 to 15 ms and the metadata probe 3 to 4 ms.
• A setting made inside the test process does not reach the native library on Windows (a first attempt that did this still took 10.4 s). It has to be in the environment when the process starts.

The fix:

• On Windows, packages/cli/vitest.config.ts writes a Fontconfig file that points at one copied font (arial.ttf from the Windows fonts folder) and sets FONTCONFIG_FILE in the main process, so test workers start with it.
• The contact-sheet test no longer carries a per-test override, and its console.time lines are gone.
• The test now checks pixels: in the label band of the output sheet, at least one pixel is not the label background. Before, it only checked that the file was a PNG, which would pass even if no label drew.

The test's 60 second budget is unchanged. On the final head the Windows job ran the test in 1.0 s (it was 3.7 s to 18.4 s before).

Not a product change

contactSheet.ts is unchanged. A Windows user pays the cold Fontconfig cache once per machine and user, because the cache persists on disk; CI pays it on every fresh runner. I have not verified that persistence on a real Windows machine (it is inferred: a later render in the same job took 10 ms after the first process had built the cache), so I am not proposing a bundled font for the product here.

Verification

• contactSheet.test.ts: 3 pass on Linux; lint and format clean; comment check clean.
• Deliberate mutation: making the label fill invisible turns the new pixel check red.
• Windows job on the final head: pass, contact-sheet test 1.0 s.
• Not exercised: macOS.

Review

An independent review found one blocking risk in an earlier version (an empty Fontconfig may render no glyphs, so the pixel check fails). The one-font config resolves that, and the Windows job shows it passes there.

[jrusso1020]

Approving. Test-infra only, and the new pixel check is a real control rather than a decorative one — that was the part worth verifying, so here is what I measured.

The band the test scans is exactly the label band, and nothing but a glyph can satisfy it. With cellWidth: 16, cols: 2 and the defaults the comment names (padding = 4, labelH = 26), createContactSheet composes a 44×43 sheet: the label SVGs land at top = 4 with height 26, and the image overlays land at top = y + labelH = 30 (contactSheet.ts:70,77,97). So rows [4, 30) — precisely the loop bounds — contain no image pixels at all. Both the sheet canvas (background: {r:26,g:26,b:26}, :105) and the label rect (fill="#1a1a1a", :92) are the same (26,26,26), so every pixel in that band is byte-exactly the background unless text drew. nonBackgroundPixels > 0 therefore has one and only one cause. That is the strong form of this check, and it replaces an assertion (format: "png") that could not have failed for the reason we care about.

The env var reaches the right process and only that process. The win32 block runs in the module body of packages/cli/vitest.config.ts, i.e. in the vitest main process before any worker forks. The Windows lane that runs this test is studio-engine-cli (windows-render.yml:405-407, bun run --filter @hyperframes/engine --filter @hyperframes/cli test) — one vitest process per package, so FONTCONFIG_FILE is carried by the cli process alone and the Studio and studio-server steps in the same job are untouched.

Blast radius of the one-font config inside packages/cli: this test, and nothing else. contactSheet.ts is the only site that rasterizes SVG <text> through sharp in a test. The other <text> producer, motionShotLayout.ts (font-family: "ui-monospace,monospace", :178), would be the one to worry about under a single-Arial config — but motionShotLayout.test.ts never imports sharp; it asserts on the SVG string. captureFontValidation.test.ts is filename/extension validation with no rendering, and the Chrome-driven tests go through DirectWrite on Windows, not fontconfig. So collapsing the font set cannot change another test's result.

Deleting the old comment is correct, not collateral. It attributed the wall clock to "a native-binary fork per operation" plus runner I/O jitter; this PR disproves that. Leaving it would have left a falsified cause documented next to the fix. The 60s budget it was arguing for is untouched (contactSheet.test.ts:69).

Two non-blocking notes, neither of which changes the verdict:

1. The band bounds are a hand-copied mirror of two source constants, and the coupling fails quietly. padding is an option the test could pass explicitly; labelH = 26 is hardcoded in contactSheet.ts:60 and not exposed. If either default moves, the loop scans rows that now hold image pixels — red and green, never (26,26,26) — so the check goes green for the wrong reason instead of red. The comment naming both constants is the right mitigation for a test this small; I mention it only because the failure direction is "still passes."
2. copyFileSync on C:\Windows\Fonts\arial.ttf has nothing around it. If a future windows-latest image drops Arial, the config module throws at load and the entire cli suite fails to start on Windows, pointing at the config rather than at fonts. Wrapping the block so a failure just leaves FONTCONFIG_FILE unset would degrade to today's slow-but-passing behaviour instead.

contactSheet.ts is genuinely unchanged — the diff is two files. The studio-engine-cli lane is the one that executes this test, and it is the lane that ran at this head.

— Rames

[jrusso1020]

Re-approved at 5adae0c3. The delta is exactly what you described and nothing else: compare/2ab5a656...5adae0c3 is 1 commit ahead, 0 behind (no rebase), one file, packages/cli/vitest.config.ts at +3/−3. So contactSheet.test.ts is byte-identical to the head I read, and the pixel-control verification from that review carries over without re-deriving it — rows [4,30) are still exactly the label band, images still composite at top=30, and canvas and label rect are still both (26,26,26), so nonBackgroundPixels > 0 still has one possible cause.

On the fix itself:

mkdtempSync is the right remediation, and it closes a hazard wider than the alert. The old fixed hf-vitest-fontconfig was shared by anything that loaded this config on the same machine — two vitest processes on one runner would have raced on fonts.conf and on the cache writes into that same dir. A per-process directory removes the race along with the predictable name.

The cleanup is registered before the call that can throw, which is the ordering that matters. process.once("exit", …) on :10 precedes copyFileSync on :11, so a runner image without arial.ttf still crashes loudly but does not also leak the directory. Had those two lines been in the other order, the failure path would have done both. rmSync is synchronous, which is the only kind of work an exit handler can actually complete.

Losing the cross-run cache costs nothing here. The fixed directory used to let the one-font Fontconfig cache persist between runs; a unique directory rebuilds it every run. That is inside your own measurement — the 42 ms and 15 ms you recorded with a one-font config were on a fresh runner, i.e. already paying a cold one-font cache build. The 9 s you were removing was the cache for every installed OS font, which this still avoids.

FONTCONFIG_FILE is still assigned at :20 in the same module-body position, so it is still set before any worker forks — the property the whole fix rests on is unchanged.

Both notes from my previous review are unchanged and both are still non-blocking. The 4..30 bounds still mirror padding/labelH by hand and still fail upward rather than red. copyFileSync is still unguarded; with the handler now registered first, the only cost of a missing Arial is the loud failure, not a leak — a try/catch leaving FONTCONFIG_FILE unset would still degrade to slow-but-passing instead.

One accuracy note on the description, not a request: process.once("exit") does not fire on SIGINT/SIGTERM, so "removes it on exit" is best-effort — a Ctrl-C during a local Windows run leaves one mkdtemp directory behind. That is strictly better than the old behaviour, which left a fixed directory behind every time, so there is nothing to change.

— Rames