Repository navigation
ci: give PR builds an x64 darwin test lane again - #39909
Conversation
x64 darwin tests were taken off PR builds in #39191 when the fleet was down to 4 Intel minis. With 9 back, PR builds get a darwin-x64-any lane shaped exactly like the aarch64 one: untiered (any Intel agent), 2 shards, and the same --skip-slower-than=10000 stopgap, so it fits the pool at PR volume. main keeps the full x64 suite as before. No-Verification-Needed: pipeline generator; checked with --dry-run (PR: aarch64-any + x64-any; main: 26/14/x64 unchanged)
|
Updated 1:57 AM PT - Aug 21st, 2026
✅ @alii, your commit 29d800528419305658f91a723885df7fe62a54cc passed in 🧪 To try this PR locally: bunx bun-pr 39909That installs a local version of the PR into your bun-39909 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: 1 review is currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. WalkthroughChangesDarwin testing
Suggested reviewers: Merge Risk: ⚪ Minimal · up to This adds the x64 Darwin test lane to pull-request builds without any supplied evidence of a correctness or production-impact issue; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
LGTM — mirrors the existing aarch64 PR lane pattern exactly; the one inline note is a stale comment.
Extended reasoning...
Overview
Adds a second entry ({ os: "darwin", arch: "x64", release: "any" }) to prDarwinTestPlatforms in .buildkite/ci.mjs, plus a comment update at testPlatforms. This restores an x64 darwin test lane on PR builds now that the Intel Mac fleet is back to 9 machines.
Security risks
None. Pure CI pipeline config — no runtime code, no auth/crypto/permissions.
Level of scrutiny
Low. This is a small, mechanical config addition that mirrors the existing aarch64 PR lane byte-for-byte in shape. I traced the new entry through each consumer: getTestAgent (darwin branch already handles x64 without a release-tier tag), getTestBunStep (!platform.tier → --skip-slower-than=10000, parallelism: 2, no EXPECTED_PLATFORM_RELEASE since that gate is aarch64+latest only), getPlatformKey → darwin-x64-any-test-bun, and depends_on → darwin-x64-build-bun which is already in buildPlatforms on PRs. The darwinTestsEnabled gating is unchanged, so main/manual/[macos tests] still get the full darwin-x64-14 lane instead.
Other factors
The PR description shows --dry-run verification of both PR and main pipelines. The only finding is a stale "arm64" in the comment directly above the array — a nit, not a blocker. The change has already landed on main (29d80052).
| @@ -1574,7 +1575,10 @@ async function getPipeline(options = {}) { | |||
| // on main along with its build. | |||
| // Untiered: any arm64 mac agent, whatever macOS it runs, can take it. | |||
There was a problem hiding this comment.
🟡 The comment immediately above prDarwinTestPlatforms still says "any arm64 mac agent" but the array now also has an x64 lane. The parallel comment at testPlatforms (lines 190-192) was updated to cover both pools; this one was missed — drop "arm64" or reword to "any mac agent of that arch".
Extended reasoning...
What the bug is
The comment on line 1576 reads:
// Untiered: any arm64 mac agent, whatever macOS it runs, can take it.but the prDarwinTestPlatforms array declared directly beneath it now contains both an aarch64 entry and an x64 entry. The comment describes only half of what the array holds.
Step-by-step
- Before this PR,
prDarwinTestPlatformswas a single-element array:[{ os: "darwin", arch: "aarch64", release: "any" }]. The comment "any arm64 mac agent, whatever macOS it runs, can take it" was accurate — it described exactly the one lane. - This PR adds
{ os: "darwin", arch: "x64", release: "any" }to the array (lines 1578-1581 in the new file). - The PR author did notice the parallel explanatory comment up in
testPlatformsand updated it (diff hunk at lines 188-192) from "so the whole arm64 pool serves PRs" to "so the whole arm64 pool serves one PR lane and the whole x64 pool the other". - The comment right above the declaration itself, however, was left unchanged and still says "arm64" only.
Why nothing else catches it
There is no code enforcing comment/array agreement — this is documentation drift. The fact that the sibling comment at testPlatforms was updated in the same diff shows the author intended both to reflect the new state and simply overlooked this one.
Impact
No runtime impact — comment only. But per REVIEW.md ("Name things truthfully"; "Only comment what the code cannot say"), stale comments are reviewed as code. A future reader skimming for "why is x64 not in the PR lane?" could be misled by a comment that says arm64 only, directly above an array that contains x64.
Fix
Reword to match the updated testPlatforms comment, e.g.:
// Untiered: any mac agent of that arch, whatever macOS it runs, can take it.or simply drop "arm64":
// Untiered: any mac agent, whatever macOS it runs, can take it.
x64 darwin tests came off PR builds in #39191 when the fleet was down to 4 Intel minis. MacStadium returned the other 5 today, so there are 9 again.
This adds
darwin-x64-any-test-bunto PR builds, shaped exactly like the existing aarch64 PR lane: untiered (queue=test-darwin, os=darwin, arch=x64, any Intel box), 2 shards, and the same--skip-slower-than=10000stopgap so it fits the pool at PR volume (~6-7 min shards; 9 slots ≈ 216 slot-hours/day vs ~180 needed).main, manual and[macos tests]builds still run the fulldarwin-x64-14lane.--dry-run: PR →darwin-aarch64-any-test-bun+darwin-x64-any-test-bun(both skip-slow, par 2); main →26/14/x64-14unchanged. The x64 lane depends ondarwin-x64-build-bun, which already runs on PRs.