Repository navigation
Conversation
|
Updated 7:49 AM PT - Aug 14th, 2026
✅ Your commit 4d5e632 has passed in 🧪 To try this PR locally: bunx bun-pr 30284That installs a local version of the PR into your bun-30284 --bun |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a TestOptions flag tracking whether ChangesTest discovery: pathIgnorePatterns behavior
Codegen define serialization change
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/test/path-ignore-patterns.test.ts`:
- Around line 467-492: Update the test "explicit path still honors
user-configured pathIgnorePatterns" to use a user pattern that does not overlap
built-in defaults: change the bunfig.toml pathIgnorePatterns value to a unique
directory pattern (e.g., ["**/custom_build/**"]) and create the test file under
that directory (e.g., "custom_build/explicit.test.ts"), then call Bun.spawnSync
with that explicit file path ("./custom_build/explicit.test.ts") so the
assertion truly verifies the user-configured pattern is honored; modify the
tempDirWithFiles call, the bunfig.toml content, and the Bun.spawnSync invocation
accordingly while keeping the same assertions checking stderr and exitCode.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c3d3c23b-44bb-4b19-a345-5928fdc9c8c3
📒 Files selected for processing (8)
docs/runtime/bunfig.mdxdocs/test/configuration.mdxsrc/cli/Arguments.zigsrc/cli/bunfig.zigsrc/cli/test/Scanner.zigsrc/cli/test_command.zigsrc/options_types/Context.zigtest/cli/test/path-ignore-patterns.test.ts
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/test/path-ignore-patterns.test.ts`:
- Around line 343-375: The current parameterized block uses
test.each([...])("skips %s by default", ...) but must follow the repo guideline
to use describe.each for parameterized suites; change test.each to
describe.each(...) and move the existing assertions into a nested test("skips %s
by default", (_label, duplicatePath) => { ... }) callback so the parameters
drive a describe block while the assertions live in a test; update the
surrounding anonymous function names accordingly and keep the same parameter
tuple and inner logic (references: the existing test.each call and the inner
assertions that check stderr for only.test.ts, duplicate.test.ts, "1 pass", and
exitCode).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c723df2f-e508-493d-baad-149b47230789
📒 Files selected for processing (1)
test/cli/test/path-ignore-patterns.test.ts
…and #30284 (6-7/8 shards pass, 1-2 fail with exit 2; retries are manual-only per getRetry())
CI red: not this PROnly failing jobs on build #51785 are the three Windows All four retries time out on each Windows target. This is the same Windows-only flake that #30217 tried to patch by swapping the client from My changes here are in Not pushing a workaround here per the guidance on #30217. Available to rebase once the UV_DISCONNECT fix lands. |
c8c6ce4 to
7b67230
Compare
CI red: infra only, diff is greenBuild #54499 — only failure is 1/20 shards of
Already spent my one CI re-roll on the earlier darwin-agents-expired flake. Stopping here so the re-roll isn't abused — needs a maintainer to either merge or kick the agent. Happy to keep iterating if anything concrete surfaces. |
943198f to
473e4f6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/test/configuration.mdx`:
- Around line 123-130: Update the paragraph that explains the default
pathIgnorePatterns to explicitly state that those defaults only affect test
discovery and do not prevent explicitly supplied CLI file or directory arguments
from running; clarify that providing any explicit pathIgnorePatterns value
(including an empty array) replaces the defaults, but passing files or
directories on the command line (e.g., a file under dist/ or build/) will still
execute the specified tests even when the defaults are in effect. Reference the
symbol pathIgnorePatterns and the CLI/file-or-directory arguments in the note so
readers understand discovery vs explicit invocation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 13f77f93-6c46-4b59-8da4-87d621492a3a
📒 Files selected for processing (4)
docs/runtime/bunfig.mdxdocs/test/configuration.mdxsrc/bunfig/bunfig.rssrc/codegen/bake-codegen.ts
💤 Files with no reviewable changes (2)
- src/codegen/bake-codegen.ts
- src/bunfig/bunfig.rs
dffb0d7 to
151fc48
Compare
CI red: infra only, diff is greenBuild #60750 at
Already spent my one CI re-roll earlier in this PR. Leaving this for a maintainer to re-run the baseline job or merge. Ready to rebase again if needed. |
151fc48 to
4af350a
Compare
CI red: unrelated flaky test, diff is greenBuild #61170 at That test runs All other shards green, including the ones running |
4af350a to
1e56923
Compare
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
CI red: infra only, diff is greenBuild #64958 at That is the agent failing to download the prebuilt bun binary from the artifact store (S3) within 120s, before any test executes. Pure infra. The only annotation is a Every other shard is green, including the ones running |
bun test previously ran any *.test.* file it found — including compiled copies in dist/ or build/ produced by tsc or a bundler. Every test in src/ would then execute twice, once from the original and once from the mirrored output. Scanner now falls back to `**/dist/**` and `**/build/**` when the user hasn't set pathIgnorePatterns via bunfig or --path-ignore-patterns. Any explicit configuration (including an empty array) opts out. Explicit file/directory arguments on the CLI bypass the defaults — user-supplied patterns still apply — so `bun test ./build/foo.test.ts` continues to work. Closes #30282
Use a unique directory name (user-ignored/) instead of build/ so the assertion can't pass just because the default **/build/** pattern happened to match the same path. Addresses coderabbit review.
The _are_defaults flag guarantees the active slice is the const, so iterating the const yields &'static [u8] without the unsafe lifetime-extend; the compiler now enforces what the SAFETY comment asserted. Also add a positive anchor assertion to the negative-only user-pattern test so a pre-scanner crash can't satisfy it.
The previous narrowing implementation stored a raw pointer to the scanner in a Drop guard and rebuilt the pattern slice with a lifetime-erasing from_raw_parts. Later &mut self uses invalidated that pointer under Stacked Borrows, making the guard's restore write UB under Miri. Track which default patterns an explicit scan root disables in a u8 mask recomputed at the start of every scan instead: no unsafe, no restore. Also drop the bake-codegen OVERLAY_CSS quoting change; main now auto-quotes raw CSS define values in the build API.
|
Closing this PR because it has been inactive for more than 90 days. |
What
Closes #30282.
bun testwalked every non-node_modulesdirectory under the project root. Iftscor a bundler mirroredsrc/intobuild/ordist/the copied*.test.*files were picked up alongside the originals, so every test ran twice.Fix
Scannernow falls back to["**/dist/**", "**/build/**"]when the user hasn't setpathIgnorePatterns(neither viabunfig.tomlnor via--path-ignore-patterns). Any explicit configuration -- including an empty array -- opts out of the defaults, matching Vitest's replace semantics.Explicit file/directory arguments on the CLI bypass the built-in defaults so
bun test ./build/foo.test.tsandbun test ./buildkeep working. Only the defaults that match the explicit path itself are dropped (tracked in a per-scan bitmask), sobun test ./packages/foostill prunes a nestedpackages/foo/dist/. User-configured patterns always apply to explicit arguments.Tests
test/cli/test/path-ignore-patterns.test.ts:skips build/dist by default(and nested variants) -- the core fixuser-configured pathIgnorePatterns replaces the defaultsempty array opts out of built-in defaultsexplicit file/directory path bypasses the defaultsexplicit path still honors user-configured pathIgnorePatternsAll 20 tests pass locally on the debug (ASAN) build; the default-pattern tests fail when the fix is removed.
[review] gate passed · iteration 21 · 8 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 3 passed · 0 rejected · iteration 21
evidence per changed file