test(backfill-images): fix the real signature regression #2812 misdiagnosed as a flake (#2813)

[jung-thomas]

TL;DR

The CI red that #2808/#2812 chased as a forks-pool timing flake was a deterministic signature regression. It fails 1/1 locally in isolation. #2812's hermetic-fixture rewrite shipped to DEV and the test still failed both attempts on the merge commit fdceca151.

Root cause

PR #2805 (48da7d835) widened:

- export function collectImageUrls(publicDir: string, slug?: string)
+ export function collectImageUrls(publicDir: string, slugs?: string[])

…updated the production call site and the new .js sibling test (test/unit/backfill-images-slug-filter.test.js, which calls with arrays), but left this older .ts test passing a bare string 'beta'. The impl does for (const s of slugs?.length ? slugs : readdir(...)) — a string has truthy .length and iterates by character ('b','e','t','a') → no matching dirs → empty map → expected [] to deeply equal ['beta']. retry:1 re-ran it, it lost again, and the plausible "Node22/Vitest-4 forks pool" story made a dead signature read as stochastic noise.

#2812's guard — expect(typeof collectImageUrls).toBe('function') — was a no-op for this bug: the binding was a function; the call shape was wrong.

Fix (test-only — no production code touched)

• Call sites pass arrays (['beta'], ['does-not-exist']), matching the Image-refresh step hit 361s on a full rebuild (publish spike #2802 follow-up) #2805 signature.
• The beforeEach guard now asserts the call contract (collectImageUrls(publicDir, ['beta']).size === 1), so a future signature/shape drift fails loudly at the guard instead of as a cryptic empty-map assertion three lines down.

Verified: 5/5 pass locally; reverting any call to a bare string fails all 5 tests at the guard line (expected +0 to be 1), confirming the guard is not a no-op.

retry:1 policy (the issue's scope step)

Ran the "surface every retry-dependent pass" step against the real CI environment — scanned the unit-tier log of DEV merge fdceca151 (the commit that contains #2812 and still went red):

• Exactly one (retry xN) marker in the whole 1490-file / 11,541-test run — this test, and it lost both attempts. Summary: 1 failed | 1487 passed | 2 skipped.
• No swarm of timing flakes hiding behind retry:1. The single retried test was the deterministic regression above.

Recommendation: keep retry:1 (it genuinely absorbs the documented cold-boot starvation blip; the smoke tier has the same lever for the same reason) but treat any retried-then-failed test as a deterministic bug first — reproduce in isolation before blaming the forks pool. No config change warranted on this evidence; the fix is diagnostic discipline, codified in docs/developers/reference/test-hygiene.md.

Structural audit — don't mechanically guard the shape

The #2812 flake shape (per-file beforeAll shared fixture + reaching a module through the workspace-first async-import shim) is pervasive: 119 modules contain the shim, 615 test/** files use beforeAll/afterAll, 432 boot a shared CAP fixture via module-level cds.test(). Hundreds have both ingredients and are green. Bolting a contract guard onto all of them would be churn against a hypothesis this audit disproved — the shim/beforeAll shape was not the cause. Add a contract guard where a test's call shape can drift from its SUT's signature, not everywhere the shim appears.

Scope

• scripts/__tests__/backfill-images-slug-filter.test.ts — the actual fix.
• docs/developers/reference/test-hygiene.md — audit writeup + retry:1 policy (same doc as the Tests hardcoding counts/values that drift from source-of-truth constants — audit & convert #1089 source-of-truth-literal class; this is that class one layer up, at the function-signature level).

Closes #2813.

🤖 Generated with Claude Code