test(install): run bun-install-registry.test.ts concurrently and tighten its assertions

Closed
RO
1mo ago
+839-584

Problem

  • test/cli/install/bun-install-registry.test.ts is one of the slowest serial-phase files: 58-60s on windows 11 aarch64, ~53s on debian 13 x64-asan, 23s on windows 2019 x64, against 9-11s on the linux release lanes (expected-durations.json: default 10.1s, asan 47.9s, windows 22.1s).
  • Nearly all of that time is ~280 bun install/bun update spawns run one at a time: a module-level beforeEach assigned a shared packageDir/packageJson and mutated the shared env object, so no test could overlap with another. The per-test floor is one spawn (~150-200ms on a debug build), and ~120 of the cases (semver, prereleases, yarn, peer index, ...) are exactly one spawn each.
  • The Windows lanes additionally run the 44 Windows-only bin shim cases (describe("windows bin linking shim should work")), each of which spawns a chain of 2-3 processes; those were serial too, which is most of the gap between the Windows and Linux lanes.
  • A few assertions were weaker than they looked (details under Fix): one test was a no-op, one bun.lockb variant was silently testing bun.lock, one bun outdated assertion checked a misspelled package name, and a number of installs only checked the exit code.

Fix

  • Every test now starts with const { packageDir, packageJson, env } = await setupTest(), which wraps the same registry.createTestDir() call the beforeEach made and returns an env whose cache/tmp dirs live inside that directory. The beforeEach and the module-level packageDir/packageJson/env/users are gone, so tests share nothing but the (read-only) registry. This is the same shape bun-install-lifecycle-scripts.test.ts and bun-workspaces.test.ts already use.
  • With that, the describes become describe.concurrent and the top-level tests test.concurrent. Describe-scoped helpers that read the module variables (runBin, checkHoistedFiles/checkUnhoistedFiles, setupWorkspace, installAndCaptureAuth) take the directory/env as parameters instead. No test is deleted, skipped, or renamed; two tests that looped over independent cases are split into one test per case under a describe with the old name (update > exact versions stay exact > a-dep|aliased, update > update different dependency groups > ...), which is where the +8 in the test count comes from.
  • Kept serial, and marked serial so the choice is visible: the whoami tests (they register registry users, and createTestDir() deletes the registry's htpasswd file, so an overlapping test would log the other one out; "rejects npm aliases ..." also registered a user, but its request is refused before anything is sent, so it uses a literal token now, like the test after it, and runs concurrently); the 16 tests that use toMatchSnapshot() (bun rejects external snapshots inside concurrent tests; the outdated tables contain ANSI escapes and are not worth inlining); and hoisting > peers, whose outcome depends on manifest arrival order (#33972, #37713), so it keeps running against an idle registry rather than one serving 20 installs. The Windows shim tests only execute bins from one install made at collection time, so they share it and run concurrently.
  • Two things the shared registry/temp state still needed once tests overlap (both found by running this on Windows):
    • createTestDir() deletes the registry's htpasswd file, and when a concurrent group starts every test in it calls setupTest() at once. On Windows, concurrent rm(file, { force: true }) of the same file rejects with EFAULT (bun bug, reproduced standalone and reported separately; node resolves all of them), which is what failed in the first CI run on windows 11 aarch64 in the test right after the whoami block. setupTest() therefore chains the createTestDir() calls through a promise; each one takes a few milliseconds, and the Linux timings below are unchanged. bun-workspaces.test.ts and bun-install-lifecycle-scripts.test.ts have the same latent race.
    • bun --bun creates its fake node.exe under %TEMP%, and debug builds wipe and recreate that directory on every run (create_fake_temporary_node_executable in src/install/lib.rs), so concurrent --bun shim tests occasionally got a debug-only SHARING_VIOLATION warning on stderr, which these tests assert is empty (about 1 run in 6 with a Windows debug build; release builds, i.e. the CI lanes, never wipe the directory). Each --bun shim test now gets its own TMP/TEMP directory, so nothing is shared; 12 of 12 debug runs of the block were clean afterwards, and the fake node.exe shows up inside the per-test directory, so the --bun path is still exercised.
  • Why this is safe: each test only touches its own directory, its own cache (install.cache in its bunfig plus BUN_INSTALL_CACHE_DIR/TMPDIR in its env), and its own port: 0 mock servers; the registry is only read. Bun's default --max-concurrency (20, or 5 in ASAN builds, src/options_types/context.rs) bounds how many are in flight, the same as in the other concurrent install files. setDefaultTimeout is left at 5 minutes on purpose: a concurrent test's wall time now includes waiting on its 19 neighbours, so it should not be tightened in the same change.
  • Slowest cases before (debug+ASAN, serial) and what happened to each:
    • "it should install with missing bun.lockb, node_modules, and/or cache" (2.6s, 14 sequential installs; the 10-iteration loop is deliberate per its comment): unchanged internally, now overlaps with the rest of its group.
    • "update > exact versions stay exact" (1.6s, 2 x 4 runs) and "update > update different dependency groups" (1.5s, 8 runs): split into 2 and 8 independent cases that run concurrently, each now asserting the bun update output (and, for the groups, the installed version).
    • "binaries > bin types" (1.3s): the six bun <bin> checks were sequential; they run under Promise.all now (0.5s) and the bun add summary is asserted.
    • "transitive file dependencies > from (non-)hoisted workspace dependencies" (1.1s each): only blocked on the helpers reading the module variable; now concurrent, and the final reinstall in each also checks the files.
    • the long tail of ~120 one-spawn cases (semver 19, prereleases 43, yarn 15, peerDependency index 7, certificate authority 8, ...) and the 44 Windows shim cases: concurrent.
    • the beforeEach itself: it also ran two Bun.$ rms of paths that do not exist (the real htpasswd is removed by createTestDir()); dropping it took the runner's own CPU time on the debug build from 23s to 18s.
  • Assertion changes:
    • "disabled dev/peer/optional dependencies are still included in the lockfile" did await runBunInstall; (the function object) and asserted nothing. It now installs with --omit=dev --omit=peer --omit=optional, checks that nothing is installed but all three packages are in bun.lock, and that a plain install then installs all three from that lockfile without re-saving it.
    • bundledDependencies: the 20 installs had stdout/stderr: "ignore" and checked the exit code. They now go through two small helpers that assert stderr, Saved lockfile, that the lockfile flavor named in the test title was actually written, and that the second install reports (no changes). The flavor check exposed that "(bun.lockb) git dependencies" wrote a bun.lock: its private bunfig (needed because the git packages depend on zod, which the local registry does not have) predates bun.lock becoming the default and lacked saveTextLockfile = false; fixed.
    • outdated > dependency pattern args asserted not.toContain("prerelease-1"); the package is prereleases-1, so the assertion could never fail. Fixed, and it holds.
    • hoisting > peers: the second and third installs asserted not.toContain("package installed"), which a multi-package reinstall never prints; they now assert the lockfile-less re-resolve reports (no changes) and that the reinstall lists every root dependency. A leftover console.log of stdout is gone.
    • semver (19 cases): also checks the installed node_modules/dep-with-tags/package.json, like the prerelease cases already did.
    • package.json indentation (one spawn was stdio: "inherit"), --config cli flag works, "it will skip ... directories.bin does not exist", transitive file dependencies > from root dependencies: install output and resulting node_modules are asserted instead of just the exit code. The outdated edge cases read stdout/stderr before the exit code.
    • auto-install's 4-line snapshot is inlined; the file-local copy of generateRegistryUser delegates to registry.generateUser (same duplicate-name guard, reset by createTestDir()); the runBin helper reports which bin failed.
  • Verification (the runs alternate baseline and this branch on the same machine; the debug build stands in for the slow lanes):
    • linux x64, debug+ASAN, bun bd test test/cli/install/bun-install-registry.test.ts: before 89.5s / 89.7s / 90.2s (238 tests, 3184 expect calls); after 35.8s / 36.9s / 35.8s (246 tests, 3357 expect calls). Eight further runs of the new file (the last three with the chained setupTest()): 36.0-40.2s, all green (shared machine, hence the spread).
    • windows x64 (Server 2019), debug build of the same source: before 122.9s (282 tests); after 32.1-32.7s across seven full runs (290 tests), plus the 12 runs of the shim block mentioned above. CI durations are only comparable between PR builds, because the runner puts a touched file at the front of its shard (position 3, while the docker prestart is still running), which inflates it; against build 95342 (#38303, which touches this file but leaves it serial, same position) this PR's build 96080 measured: x64-asan 55.2s -> 24.1s, windows 11 aarch64 63.3s -> 36.5s (46.8s on the attempt that hit the peer * flake), alpine x64 27.2s -> 17.5s, windows 2019 x64 30.0s -> 23.5s, debian x64 15.8s -> 14.1s. The release lanes gain less than the local runs because they have 4 vCPUs and, with the tests themselves cheap there, the single-threaded verdaccio process serving every install is most of what is left; the asan lane also only runs 5 tests at a time. One test fails on Windows with a debug build before and after this change ("npm manifest cache entries are only reused for the package name they were saved for", from #37669; it passes on the release Windows lanes); #38580 fixes that test on main, and the lines it touches are identical here, so the two do not conflict.
    • tsc over the file reports the same pre-existing diagnostics as before and nothing new; prettier is clean.
  • Not changed, for the record: the two git dependencies cases still fetch from github.com and resolve zod from registry.npmjs.org, and "manifest cache will invalidate when registry changes" still uses the public registry for its second install; making those hermetic is a separate change. VerdaccioRegistry.stop() leaving the registry process behind is #36352. Once #37713 lands, hoisting > peers can drop its describe.serial. The natural follow-up, kept out of this PR because #33115 and #36352 are both reshaping that class, is to have createTestDir() itself return the per-directory env and stop deleting htpasswd (start()/stop() already do), which would replace setupTest() here and the identical blocks in bun-install-lifecycle-scripts, bun-workspaces and bun-install-native-binlink, remove the chaining, and make the remaining serial verdaccio files (isolated-install, bun-publish, bun-lock, catalogs, bun-lockb, public-hoist-pattern, pnpm-migration) a mechanical conversion.

Background

  • Concurrent groups: bun runs consecutive test.concurrent tests (including across describe boundaries) as one group with up to --max-concurrency (20; 5 in ASAN builds) in flight; a serial test waits for the group to drain and runs alone. So each serial holdout costs its own duration plus one drain; the remaining serial stretches are the whoami block, the outdated snapshot block, hoisting > peers, and the individual snapshot tests.
  • describe.concurrent marks every test declared inside it (including nested describes) concurrent; test.serial / describe.serial inside it opt individual tests back out. This keeps the per-test headers unchanged for tests inside describes, which is what keeps this diff mostly to one added line per test.
  • registry.createTestDir() (test/harness.ts) makes a fresh temp directory with a bunfig.toml pointing at the file's single verdaccio instance (started once at module load, as before) and resets the registry's htpasswd file; setupTest() is a thin wrapper that adds the per-directory env.
  • toMatchSnapshot() stores snapshots by test name in a shared .snap file and is rejected inside concurrent tests; toMatchInlineSnapshot() is allowed, which is why the one small snapshot was inlined and the others stayed serial.
  • The serial-phase note: the CI runner executes regular test files one at a time, so a file's own wall time adds directly to its shard, and concurrency inside the file uses cores that are otherwise idle.