Conversation
|
Warning Review limit reachedYour included review limit has been reached. You’re in a promotional period — use the checkbox below to run this review for free:
On-demand reviews are free for the next 29 days. After that, they cost $0.25 per reviewed file. How can I continue?Run this review now using the option above, or comment You can also wait for the limit to reset (next review available in 2 minutes), then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 12:17 AM PT - Aug 22nd, 2026
❌ @robobun, your commit a15d445 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 40052That installs a local version of the PR into your bun-40052 --bun |
|
Status: closed, superseded by #40062. #40062 merged at 08:10Z with the same four snapshot updates ( Culprit merge: #38952 (new count wording). Stale snapshots came from #39962 and #39157. |
There was a problem hiding this comment.
LGTM — mechanical snapshot updates to match the wording #38952 already ships.
What was reviewed:
- Verified
src/runtime/cli/package_manager_command.rs:639emitsnode_modules ({} installed)andsrc/install/prune.rs:336,359emit(checked {} installed package{})— the four updated snapshots match the source exactly. - Confirmed every other
bun-prune.test.tssnapshot already usesinstalled packages; these four were the only stale ones. - Checked the remaining
(checked N)hits inbun-audit.test.tscome fromaudit_fix.rs, which #38952 did not touch — correctly left alone.
Extended reasoning...
Overview
This PR updates four toMatchInlineSnapshot literals across two test files (test/cli/install/bun-pm.test.ts lines 603/625/673 and test/cli/install/bun-prune.test.ts line 661) to reflect the count-wording change that landed in #38952. The change is purely additive text inside snapshot strings: (5) → (5 installed) and (checked 5) → (checked 5 installed packages). No source code, test logic, or assertion structure changes.
Security risks
None. Test-only snapshot text edits with no runtime, build, or dependency impact.
Level of scrutiny
Low. This is a merge-race snapshot fixup: #38952 changed the summary wording and updated the snapshots it knew about, but #39962 and #39157 landed concurrently with the old wording. I verified against the current source that package_manager_command.rs:639 prints node_modules ({} installed) and prune.rs:336/359 print (checked {} installed package{}), so the new snapshots are byte-accurate to what the binary emits. All neighboring snapshots in bun-prune.test.ts already use the new wording, and the PR's grep of remaining old-form matches (only bun-audit.test.ts, which belongs to audit_fix.rs — unchanged by #38952) checks out.
Other factors
The tests still assert full output line-by-line, so no coverage is weakened. The PR description documents bun bd test runs on both files plus the other files #38952 touched. The head commit is already on main per the recent-commits list, and no prior reviewer has left outstanding comments.
Problem
test/cli/install/bun-pm.test.tsandtest/cli/install/bun-prune.test.tsfail on every lane on main since 3b3687e (pm: polish dedupe, prune, pm ls and pm licenses output #38952). FourtoMatchInlineSnapshotcalls mismatch by one line:- "<dir> node_modules (5)/+ "<dir> node_modules (5 installed)and- 2 packages removed (checked 5)/+ 2 packages removed (checked 5 installed packages).src/install/prune.rs,src/runtime/cli/package_manager_command.rs) and updated the snapshots it knew about. Two PRs merged before it kept the old text: pm ls: list a workspace the root also depends on once #39962 added the threebun pm lssnapshots (bun-pm.test.ts:603,:625,:673) and install: name build_store's timing flag with an enum instead of a bool #39157 added the prune snapshot (bun-prune.test.ts:661). Neither branch saw the other.Fix
src/change.docs/pm/cli/{prune,pm}.mdx. The neighboring prune test atbun-prune.test.ts:626already asserts(checked 9 installed packages).bun pm lsandbun prune --production --verboseoutput, compared line by line.bun bd test test/cli/install/bun-pm.test.ts(22 pass, 4 fail before) andbun bd test test/cli/install/bun-prune.test.ts(110 pass, 1 fail before). Alsobun-pm-licenses.test.ts,bun-dedupe.test.tsandisolated-relink.test.ts, the other files pm: polish dedupe, prune, pm ls and pm licenses output #38952 touched, all green.Background
bun pm lsprints a header line<dir> node_modules (N installed)followed by a tree of the installed packages.Ncounts node_modules entries.bun pruneends withN packages removed (checked M installed packages).Mcounts the installed entries it compared against the lockfile.dedupe(lockfile entries),prune(installed entries) andpm ls(tree positions) no longer look contradictory.Notes
test/anddocs/for every wording pm: polish dedupe, prune, pm ls and pm licenses output #38952 changed (node_modules (N),(checked N),can be removed (checked,No duplicates,nothing to prune,across N licenses). The only stale matches are the four lines in this PR. The(checked N)lines inbun-audit.test.tsbelong tobun audit --fix, which pm: polish dedupe, prune, pm ls and pm licenses output #38952 did not change.--jsonoutput) carries the same four snapshot updates inside a much larger diff. Main needs the fix on its own.