Skip to content

test: pack the rename-retry fixture with Bun.Archive, not the system tar - #45588

Merged
dylan-conway merged 1 commit into
mainfrom
claude/rename-retry-test-bun-archive
Oct 16, 2026
Merged

dylan-conway merged 1 commit into
mainfrom
claude/rename-retry-test-bun-archive

Conversation

@dylan-conway

Copy link
Copy Markdown
Member

What does this PR do?

Fixes test/cli/install/bun-install-windows-rename-retry.test.ts, which is red on the Windows 11 aarch64 lane on main and on most branches: the two "reinstalled project-local store entry" tests find 299 files in the package instead of 300, on every retry.

The test is at fault, not bun install. Its beforeAll packed the fixture with the system's tar -czf ... .quiet(). On the failing machines tar.exe (bsdtar 3.8.8) prints package/files/N.txt: Can't add archive to itself, leaves that file out and still exits with 0, so the tarball has 299 files.

That check in libarchive is wrong on Windows. It compares a 16-bit hash of the archive's file id (getino in archive_windows.c) with each file's whole NTFS record number (bhfi_ino in archive_read_disk_windows.c), so an unrelated file can be taken for the archive. The current Windows aarch64 image has two ranges of free records that are 2 * 65536 apart, which makes the two numbers meet. The measurements are in #45579.

The fixture is now packed in memory with Bun.Archive, as other tests in test/cli/install do. The temp directory and the afterAll that only served tar are gone. Bun's own packing never sets that check.

How did you verify your code works?

The tests only run on Windows, so CI's Windows lanes are the check. Locally (Linux) the file loads with its 8 tests skipped, and the same Bun.Archive call gives a gzip tarball in which both Bun and GNU tar find all 303 entries, 300 of them under package/files/.

@robobun

robobun commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator
Updated 4:05 PM PT - Oct 16th, 2026

@dylan-conway, your commit 3f2ad82 is building: #126887

@coderabbitai

coderabbitai Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 832dd2f3-5b18-4d08-bbbb-b97f51424845
📥 Commits

Reviewing files that changed from the base of the PR and between f8bc171 and 3f2ad82.

📒 Files selected for processing: 1
  • test/cli/install/bun-install-windows-rename-retry.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.

Walkthrough

The Windows rename-retry test now creates its package archive in memory with Bun.Archive. It types the archive bytes as Uint8Array and removes the temporary package directory, tar creation, file readback, and cleanup hook.

Changes

Windows rename-retry test

Layer / File(s) Summary
In-memory archive setup
test/cli/install/bun-install-windows-rename-retry.test.ts
The test creates and hashes package archive bytes in memory. It removes the temporary package directory and its cleanup code.

Suggested reviewers: jarred-sumner

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 3f2ad

The in-memory fixture preserves the package archive used by the install tests, and no concrete regression is established. The change is mergeable.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: using Bun.Archive instead of system tar for the rename-retry fixture.
Description check Passed The description includes both required sections. It explains the Windows tar failure, the implementation change, and verification results.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, a small test-only swap from the system tar to in-memory Bun.Archive, matching the pattern already used by a dozen sibling files in test/cli/install.

What was reviewed:

  • Bun.Archive input/output types: Record<string, BlobPart> accepts the Buffer from randomBytes, and .bytes() returns Uint8Array<ArrayBuffer>, which createHash().update, new Response, .length and .subarray all consume view-correctly, so the served body and shasum stay consistent.
  • Removed afterAll, pkgDir and the tar shell-out have no remaining references; tempDir, readFileSync and join are still used elsewhere in the file.
  • Header and inline comments never described the tar step, so nothing is left stale.
Extended reasoning...

The PR changes one Windows-only test file, test/cli/install/bun-install-windows-rename-retry.test.ts, replacing a beforeAll that wrote fixture files to a tempDir and shelled out to tar -czf with a single in-memory new Bun.Archive(files, { compress: "gzip" }).bytes() call, dropping the now-unneeded pkgDir variable, afterAll cleanup and afterAll import. It touches no security-sensitive surface; it is test fixture construction served from a local Bun.serve registry. The change is 3 added / 11 removed lines, follows the exact pattern used in bun-prune, hoist, bun-add, catalogs and other sibling tests, the file is not covered by CODEOWNERS, and the bug hunt ran dry with no findings, which decided approve.

@dylan-conway
dylan-conway merged commit f0e4f6b into main Oct 16, 2026
5 of 7 checks passed
@dylan-conway
dylan-conway deleted the claude/rename-retry-test-bun-archive branch October 16, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants