test(install): run the dummy-registry half of bun-update.test.ts concurrently and tighten its assertions

Closed
RO
1mo ago
+981-1390

Problem

  • test/cli/install/bun-update.test.ts takes 23s on debian 13 x64-asan, 11s on windows 11 aarch64, 6s on windows 2019 x64 and 2.2s to 2.9s elsewhere (build 101153): it starts its bun processes one after another.
  • The first 50 cases share the module-level handler, counter and package_dir of dummy.registry.ts, so they run serially. The 106 verdaccio cases are already concurrent.
  • runInstall only checked stderr for error:, and most cases checked one .dependencies.baz field per file.

Fix

  • Each of the 50 cases gets its own createTestContext() and runs as it.concurrent. The hooks are gone. Locally these cases take 25.8s serially and 8.4s concurrently.
  • perNameRegistry builds its tarballs with Bun.Archive instead of 20 bun pm pack children. The issue #5888 case runs 2 updates instead of 6.
  • runInstall requires stderr to be exactly Saved lockfile (empty with --frozen-lockfile), runUpdate that line or nothing. expectWorkspaces checks whole package.json objects and the matching bun.lock workspaces entries in one call. The install, update and outdated summaries are 18 inline snapshots.
  • Verified: bun bd test test/cli/install/bun-update.test.ts, 156 pass. Whole file locally (debug build): 42.5s and 45.3s before, 21.8s to 22.5s after (4 runs). In CI (build 101233) the 50 cases take about 3s on x64-asan instead of 15s. The totals of a PR build do not show it: see the notes.

Background

  • The context API of dummy.registry.ts routes each test's requests by a /test-N/ URL prefix, and cache = false keeps the install cache in the test dir. The cases share nothing.
  • bun test runs consecutive it.concurrent cases together (5 at a time under ASAN, 20 otherwise). A hook or a plain it() between them is a barrier.
  • bun.lock repeats each workspace's name, version and dependency groups under workspaces, keyed by path ("" is the root).
Notes

Reading the CI numbers. The runner puts the files a PR touches first in the job, so in a PR build this file runs while the job is still starting its services (the coordinator: ... ready lines land in the middle of its output). That inflates the verdaccio start in the middle of the file from under 0.5s (build 101153, where the file ran 8th to 44th) to 4s to 11s, in this PR's build and in any other PR's build that touches the file. The dots reporter flushes about once a second with a timestamp, so the three parts can still be read off the logs. Build 100212 (#38770, the same file plus one case, also first in the job) is the comparison:

lane50 dummy-registry casesverdaccio start106 verdaccio casestotal
debian 13 x64-asan, old (100212)about 15.5sabout 7s8.3s31.8s
debian 13 x64-asan, this PR (101233)about 3sabout 11s8.8s23.6s
windows 2019 x64, oldabout 3.4sabout 5.5s2.9s11.8s
windows 2019 x64, this PRabout 1.5sabout 6s2.1s10.1s
windows 11 aarch64, oldabout 6.8sabout 6s5.7s18.5s
windows 11 aarch64, this PRabout 2sabout 4.5s5.2s12.3s

At the file's normal position (build 101153: 22.7s on x64-asan, of which the first 50 cases were about 13s; 5.7s and 10.8s on the two Windows lanes, first 50 cases about 2.3s and 4.5s), the same 3x to 5x on that part gives roughly 12s, 5s and 8s. The verdaccio start and the verdaccio half are the rest and are unchanged. On a quiet machine with the release build, the whole file goes from 2.7s to 2.8s to 1.8s to 2.2s (3 runs each).

The file also passes with the release build (USE_SYSTEM_BUN=1). bun-install.test.ts and bun-remove.test.ts already run concurrently on the same context API.

The verdaccio half of the file (from describe("bun update <name> semantics") on) is byte for byte unchanged. It is now about 17s of the 22s locally, all of it the 3 to 4 processes each case needs (install, sometimes a second install to make the state stale, the update, and the --frozen-lockfile install in expectInSync). Removing those would remove coverage, so they stay.

Considered and not done: writing the starting bun.lock by hand instead of running bun install in the 40 workspace cases. It would save about 50 of the roughly 130 child processes left in this half, but the synthesized lockfile would replace the state a real install leaves behind (nested copies, what is installed, whether the first update re-saves), which several cases are about. It is about one second of the file in CI.

Assertion changes in detail:

  • run removes the two progress lines (Resolving dependencies, Resolved, downloaded and extracted [N]) from stderr and normalizes stdout, so callers assert stderr exactly and snapshot stdout. runInstall returns the stdout; the three single-package cases snapshot it.
  • Cases pin Saved lockfile where the lockfile must move, and empty stderr where nothing may be written: --dry-run, --no-save, the second run of the idempotence case, a named update run from a workspace that does not own the row, and the #5888 case (which now also compares against the lockfile the install wrote). setupWorkspaces, fanOutRepo and nestedBazRepo check the lockfile mirrors the files right after the install, so a later "unchanged" means something.
  • Error and dry-run cases also check bun.lock is byte-identical. The fan-out cases check every untouched file is byte-identical and every rewritten one differs in the version only.
  • The --no-save case used to install 0.0.5 and then check that 0.0.5 was installed. It now installs from a registry that only has 0.0.3 and bumps it, like the dist-tag cases, so node_modules really moves while package.json and bun.lock stay byte-identical.
  • The overrides cases also check the member's literal is left alone. --recursive tolerates a member with no dependency groups also checks the root moved. bun outdated -r is an exact snapshot of the empty output instead of not.toContain("baz"). The UTF-8 case checks the whole row ↑ baz 0.0.3 → 0.0.5 after stripping the colors, and that package.json was rewritten. --filter excluding a workspace checks bun prune printed nothing to stderr.
  • The for (const { input } of [...]) loop had one input, so tilde was always true. The loop and its dead branches are gone, and the stdout arrays are snapshots. Those two cases lost the (~) suffix of their names. All other names are unchanged.
  • The per-name cases keep their lockedSharedResolutions assertions, plus exact stderr and snapshots. #38770 changes their behavior, so nothing else about them is pinned.

Overlap found by the self-review, left for a follow-up. Two of the converted groups duplicate coverage that exists elsewhere. The fan-out group (FAN_OUT, 7 cases) overlaps the verdaccio bun update <name> -r / --filter describe in this file: -r/--filter * rewrites every workspace, --filter rewrites one, the two error texts, --dry-run, and the indentation case. Only -F and the stale registry move (root's exact pin and pkg-b's ^0.0.3 stay while pkg-a's ~ moves) are not in the describe. The per-name group (7 cases) overlaps bun-update-transitive.test.ts, whose serveRegistry is the same Bun.Archive construction as perNameRegistry, including the case where a named update run from the root leaves a member's entry alone. This PR keeps all of them: the task was to speed the file up without removing cases and without touching the verdaccio half, which the open PRs listed below add to. Consolidating the two groups (about 120 lines and about 20 child processes) and lifting the in-process registry into dummy.registry.ts are separate changes. In the same spirit, testContext() exists only because createTestContext() returns a plain object; giving that object a [Symbol.dispose] in dummy.registry.ts would remove this wrapper and the withContext copies in bun-install.test.ts and bun-install-tarball-integrity.test.ts. dummy.registry.ts is shared by about nine files, so that is also left out of this PR. perNameRegistry should not grow.

For the open PRs that add cases to this half (#35542, #34109, #29794, #38687, #38770): a case becomes it.concurrent, starts with using ctx = await testContext();, calls serve(ctx, versions, urls) instead of setHandler(dummyRegistry(urls, versions)), reads ctx.package_dir, ctx.registry_url and ctx.requested instead of the module-level package_dir, root_url and requested, and starts bun with runInstall(ctx), runUpdate(ctx, [...args]) or run(ctx, [...args]).


[stamp-90s] gate passed · iteration 0 · 1 files touched

passes on PR (with fix)
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/cli/install/bun-update.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/cli/install/bun-update.test.ts
bun test v1.4.0 (4c689909e)

test/cli/install/bun-update.test.ts:
(pass) --recursive updates dependencies and peerDependencies in workspace members [634.81ms]
(pass) --recursive --latest updates workspace members to the latest version [635.93ms]
(pass) should update to latest versions of dependencies [763.02ms]
(pass) should update to latest version of dependency [857.93ms]
(pass) lockfile should not be modified when there are no version changes, issue#5888 [793.07ms]
(pass) --filter updates only matching workspaces, leaving siblings and root untouched [473.72ms]
(pass) --filter with multiple patterns selects the union of matching workspaces [475.01ms]
(pass) named update -r --latest rewrites every workspace that declares the name, keeping each file's style [502.29ms]
(pass) --filter pkg-a removes the nested copy whose row it collapsed [677.86ms]
(pass) --filter excluding a workspace leaves that workspace's node_modules alone [888.88ms]
(pass) named update --filter rewrites only the selected workspace [515.77ms]
(pass) named update --filter of a workspace that does not depend on the name is an error [427.97ms]
(pass) named update accepts -F as the short form of --filter [547.62ms]
(pass) named update -r with a name missing from the lockfile is an error [594.73ms]
(pass) named update -r --dry-run writes nothing [553.43ms]
(pass) --recursive preserves a workspace member's catalog: reference [540.82ms]
(pass) named update -r moves the ranges and keeps bun.lock in sync [754.39ms]
(pass) --recursive --latest from a member updates root's catalog and direct deps together [512.48ms]
(pass) --filter excluding root leaves root (and its catalog) untouched [679.44ms]
(pass) --filter with a path targets only the matching workspace [451.47ms]
(pass) --filter with a negated pattern updates everything except the excluded workspace [534.46ms]
(pass) --filter
... (truncated)
Exit: 0
diff hotspot
test/cli/install/bun-update.test.ts | 2371 +++++++++++++++--------------------
 1 file changed, 981 insertions(+), 1390 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                 reads  edits  tests
test/cli/install/bun-update.test.ts     15      8      0

self-review · 6 surviving concerns

  • should-fix: Test-infra change (speedup/port of an existing ~17-helper dummy-half layer onto the ctx API): the port itself is a fair shape for ~30 of the 50 cases, but it re-tools two blocks (~375 of ~1240 lines: the per-name registry + its 7 cases, and the fanOut family + its 7 cases) whose registry helper a…
  • consider: Test-infra speed-up: this converts 1 of ~9 files still on dummy.registry's deprecated module-level API, and the two helpers it adds to do so (a Disposable wrapper and an in-process per-name registry) are the 3rd and 5th file-local copies of things that belong in dummy.registry.ts / the shared reg…
  • consider: run() hardcodes env, so this PR's own diff already carries a second copy of run()'s spawn body (the UTF-8 case) with the one remaining loose stderr assertion in the converted half, and the conversion recipe offered to the 5 pending PRs has no way to express the env override #34109 needs; the BUN_…
  • consider: Test-infra/perf change (one test file made concurrent; the speedup case is fine) -- but the Disposable-ctx half of its two local wrappers belongs in createTestContext itself: it is a ~3-line, provably non-breaking change to dummy.registry.ts, this PR instead adds a third in-tree lifecycle shape w…
  • consider: The new inline snapshot at bun-update.test.ts:851-855 certifies a summary gap as expected output: bare update -r moves pkg-a's baz: "latest" 0.0.3->0.0.5 but prints no row, while --dry-run/--lockfile-only on the identical repo, the ~0.0.3 control, and the named form all print `^ baz 0.0…
  • consider: Test-infra refactor: the 139-line dummy fanOut family this PR rebuilt (481-619) is the second copy of the verdaccio -r / --filter describe's matrix (both copies landed together in #38333 five days earlier); its residual is two shapes, -F and "declaring member whose literal cannot move under n…

46 concerns were raised and did not survive verification.