Repository navigation
gitignore the sources text files - #22408
Conversation
|
Updated 3:29 PM PT - Sep 4th, 2025
❌ @Jarred-Sumner, your commit 2066db1 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 22408That installs a local version of the PR into your bun-22408 --bun |
WalkthroughRemoves many entries from multiple cmake/sources/*.txt lists (several now empty), adds a gitignore rule for those generated lists, moves glob-sources invocation into a Bun-only top-level import inside scripts/build.mjs, removes its separate call from package.json build:debug, and adds an import of glob-sources.mjs in a test. Other changes include JS/C/TS source list pruning and small test/title/initialization adjustments. Changes
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
scripts/build.mjs (1)
15-17: Gate and await globbing with grouped logging + error surfacingThis makes failures clearer in CI output and keeps behavior identical (Bun-only, pre-configure).
Apply:
-if (globalThis.Bun) { - await import("./glob-sources.mjs"); -} +if (globalThis.Bun) { + await startGroup("Glob sources", async () => { + try { + await import("./glob-sources.mjs"); + } catch (err) { + console.error("Failed to glob sources", err); + throw err; + } + }); +}Confirm CI’s Node version parses TLA in ESM (it should, but worth verifying across all agents).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
.gitignore(1 hunks)cmake/sources/BakeRuntimeSources.txt(0 hunks)cmake/sources/BindgenSources.txt(0 hunks)cmake/sources/BunErrorSources.txt(0 hunks)cmake/sources/CSources.txt(0 hunks)cmake/sources/JavaScriptCodegenSources.txt(0 hunks)cmake/sources/JavaScriptSources.txt(0 hunks)cmake/sources/NodeFallbacksSources.txt(0 hunks)cmake/sources/ZigGeneratedClassesSources.txt(0 hunks)package.json(1 hunks)scripts/build.mjs(1 hunks)test/internal/ban-words.test.ts(1 hunks)
💤 Files with no reviewable changes (8)
- cmake/sources/JavaScriptCodegenSources.txt
- cmake/sources/JavaScriptSources.txt
- cmake/sources/BunErrorSources.txt
- cmake/sources/BakeRuntimeSources.txt
- cmake/sources/BindgenSources.txt
- cmake/sources/CSources.txt
- cmake/sources/ZigGeneratedClassesSources.txt
- cmake/sources/NodeFallbacksSources.txt
🧰 Additional context used
📓 Path-based instructions (5)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/internal/ban-words.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/internal/ban-words.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Tests must be named with the suffix .test.ts or .test.tsx and live under the test/ directory
In tests, always use port: 0 and never hardcode ports or use custom random-port utilities
Prefer snapshot tests using normalizeBunSnapshot(...) over direct string equality on stdout/stderr
Do not write tests that assert absence of 'panic', 'uncaught exception', or similar strings in output
Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
Files:
test/internal/ban-words.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Run Prettier to format JS/TS files (bun run prettier)
Files:
test/internal/ban-words.test.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/internal/ban-words.test.ts
🧠 Learnings (11)
📚 Learning: 2025-09-03T17:09:28.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.100Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
Applied to files:
test/internal/ban-words.test.tsscripts/build.mjs
📚 Learning: 2025-09-03T17:10:13.460Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.460Z
Learning: Applies to test/**/*.test.ts : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)
Applied to files:
test/internal/ban-words.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Use shared utilities from test/harness.ts where applicable
Applied to files:
test/internal/ban-words.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/{dev/*.test.ts,dev-and-prod.ts} : Import testing utilities (devTest, prodTest, devAndProductionTest, Dev, Client) from test/bake/bake-harness.ts
Applied to files:
test/internal/ban-words.test.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/internal/ban-words.test.tsscripts/build.mjs
📚 Learning: 2025-09-03T17:09:28.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.100Z
Learning: Applies to test/js/bun/** : Place Bun-specific API tests (http, crypto, ffi, shell, etc.) under test/js/bun/
Applied to files:
scripts/build.mjs
📚 Learning: 2025-09-03T17:09:28.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.101Z
Learning: Run cross-platform Zig checks (bun run zig:check-all) when making platform-specific changes
Applied to files:
package.json
📚 Learning: 2025-08-30T00:07:54.573Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-08-30T00:07:54.573Z
Learning: Applies to src/**/*.zig : Implement debug logs in Zig using `const log = bun.Output.scoped(.${SCOPE}, false);` and invoking `log("...", .{})`
Applied to files:
package.json
📚 Learning: 2025-08-30T00:07:54.573Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/building-bun.mdc:0-0
Timestamp: 2025-08-30T00:07:54.573Z
Learning: Build debug versions using `bun bd` or `bun run build:debug`; use `bun bd <command>` to run any command against the debug build
Applied to files:
package.json
📚 Learning: 2025-09-03T17:09:28.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.101Z
Learning: Applies to **/*.zig : Run zig-format to format Zig files (bun run zig-format)
Applied to files:
package.json
📚 Learning: 2025-09-03T17:10:13.460Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.460Z
Learning: Use `bun bd <...cmd>` to run commands with your debug build of Bun
Applied to files:
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (3)
test/internal/ban-words.test.ts (1)
4-4: LGTM: ensure sources are generated during this test runStatic side-effect import is fine here since the file already requires Bun at top level.
.gitignore (1)
191-192: Ignore generated CMake source lists
Verified that all references tocmake/sources/*.txtare in build-time scripts and CMake configurations (glob, shell, JS, CMakeLists), so no static consumers expect committed files.package.json (1)
35-35: Simplified build:debug path looks goodRelying on build.mjs to drive globbing reduces duplication.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
test/js/node/test/parallel/test-common-must-not-call.js (2)
29-29: Make expectation resilient: accept optional column instead of hard-coding ':29'.Stack-trace columns vary across engines/build flags and can be brittle. Prefer matching the line and allowing an optional “:col”.
Apply this minimal change in the edited line:
-const validate1 = createValidate('9:29'); +const validate1 = createValidate('9');And update the assertion once (outside this hunk) to tolerate an optional column:
// inside createValidate(...) const argsInfo = args.length > 0 ? `\ncalled with arguments: ${args.map(util.inspect).join(', ')}` : ''; // tolerate optional column in stack traces const [location, ...restLines] = rest.split('\n'); const actualArgsInfo = restLines.length ? '\n' + restLines.join('\n') : ''; assert.ok( location === String(line) || new RegExp(`^${line}:\\d+$`).test(location), `unexpected location "${location}"` ); assert.strictEqual(actualArgsInfo, argsInfo);
36-36: Apply the same resilience to the second expectation.Keep the check stable across platforms by not pinning the column.
-const validate2 = createValidate('11:29', ['hello', 42]); +const validate2 = createValidate('11', ['hello', 42]);test/js/bun/crypto/wpt-webcrypto.generateKey.test.ts (1)
439-447: Preserve context, avoid collisions, and only truncate when necessary.
- Slicing the last 50 chars can drop the “Success: generateKey” context and create duplicate names.
- It also shortens labels even when they’re already under 50 chars (negative start index).
- Suggest: truncate only if over a max length, keep the prefix, and add a short hash for uniqueness.
Apply:
- // This generates about 1.3 MB of test logs. - let testLabel = testTag + ": generateKey" + parameterString(algorithm, extractable, usages); - - if (isCI) { - testLabel = testLabel.slice(testLabel.length - 50); - } - - test(testLabel, async function () { + // CI logs can get large; keep names readable and unique. + const fullLabel = `${testTag}: generateKey${parameterString(algorithm, extractable, usages)}`; + const MAX_LABEL = 120; + let testLabel = fullLabel; + if (isCI && fullLabel.length > MAX_LABEL) { + const hash = + typeof Bun !== "undefined" && typeof Bun.hash === "function" ? (Bun.hash(fullLabel) >>> 0).toString(36).slice(0,6) : ""; + const suffix = fullLabel.slice(-(MAX_LABEL - 10)); + testLabel = `${testTag}: … ${suffix}${hash ? " #" + hash : ""}`; + } + test(testLabel, async function () {src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts (2)
218-226: Remove the unused stub_types block (dead code).stub_types is created/modified but never used. This adds noise and risks future drift with the test-plugin types handling.
Apply this diff:
- // For stub generation, we need semicolons but no initialization - const stub_types = { ...types }; - stub_types.decls = stub_types.decls.map(d => d + ";"); - if (stub_types.args.length === 1 && stub_types.args[0] === "void") { - stub_types.decls = []; - stub_types.args = []; - }
387-392: Ensure the plugin directory exists before writing.Write will fail if test/napi/uv-stub-stuff doesn’t exist on a fresh checkout or in CI. Create it first.
Apply this diff:
-const plugin_path_ = join(import.meta.dir, "../", "../", "../", "../", "test", "napi", "uv-stub-stuff", "plugin.c"); +const plugin_path_ = join(import.meta.dir, "../", "../", "../", "../", "test", "napi", "uv-stub-stuff", "plugin.c"); +await Bun.$`mkdir -p ${join(import.meta.dir, "../", "../", "../", "../", "test", "napi", "uv-stub-stuff")}`.quiet(); await Bun.write(plugin_path_, test_plugin_contents);Also, per repo guidelines, please run Prettier for this TS file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts(2 hunks)test/js/bun/crypto/wpt-webcrypto.generateKey.test.ts(2 hunks)test/js/node/test/parallel/test-common-must-not-call.js(1 hunks)test/napi/uv_stub.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/napi/uv_stub.test.ts
🧰 Additional context used
📓 Path-based instructions (12)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.tstest/js/node/test/parallel/test-common-must-not-call.js
test/js/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place JavaScript and TypeScript tests under test/js/
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.tstest/js/node/test/parallel/test-common-must-not-call.js
test/js/bun/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.ts
test/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.tstest/js/node/test/parallel/test-common-must-not-call.js
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Tests must be named with the suffix .test.ts or .test.tsx and live under the test/ directory
In tests, always use port: 0 and never hardcode ports or use custom random-port utilities
Prefer snapshot tests using normalizeBunSnapshot(...) over direct string equality on stdout/stderr
Do not write tests that assert absence of 'panic', 'uncaught exception', or similar strings in output
Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.ts
test/js/bun/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place Bun-specific API tests (http, crypto, ffi, shell, etc.) under test/js/bun/
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.ts
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
Run Prettier to format JS/TS files (bun run prettier)
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.tstest/js/node/test/parallel/test-common-must-not-call.jssrc/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.ts: Name test files*.test.tsand usebun:test
Do not write flaky tests: never wait for arbitrary time; wait for conditions instead
Never hardcode port numbers in tests; useport: 0to get a random port
When spawning Bun in tests, usebunExe()andbunEnvfromharness
Preferasync/awaitin tests; for a single callback, usePromise.withResolvers()
Do not set explicit test timeouts; rely on Bun’s built-in timeouts
UsetempDir/tempDirWithFilesfromharnessfor temporary files and directories in tests
For large/repetitive strings in tests, preferBuffer.alloc(count, fill).toString()over"A".repeat(count)
Import common test utilities fromharness(e.g.,bunExe,bunEnv,tempDirWithFiles,tmpdirSync, platform checks, GC helpers)
In error tests, assert non-zero exit codes for failing processes and usetoThrowfor synchronous errors
Usedescribeblocks for grouping,describe.eachfor parameterized tests, snapshots withtoMatchSnapshot, and lifecycle hooks (beforeAll,beforeEach,afterEach); track resources for cleanup inafterEach
Useusing/await usingwith Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.ts
test/js/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests for specific features under
test/js/by module
Files:
test/js/bun/crypto/wpt-webcrypto.generateKey.test.tstest/js/node/test/parallel/test-common-must-not-call.js
test/js/node/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place Node.js module compatibility tests under test/js/node/, separated by module (e.g., test/js/node/assert/)
Files:
test/js/node/test/parallel/test-common-must-not-call.js
test/js/node/test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Vendored Node.js tests under test/js/node/test/ are exceptions and do not follow Bun’s test style
Files:
test/js/node/test/parallel/test-common-must-not-call.js
test/js/node/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place Node.js compatibility tests under test/js/node/
Files:
test/js/node/test/parallel/test-common-must-not-call.js
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.100Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
📚 Learning: 2025-09-03T17:09:28.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.101Z
Learning: Applies to test/napi/** : Place N-API compatibility tests under test/napi/
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/dev/plugins.test.ts : plugins.test.ts should contain plugin-related development-mode tests
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-09-03T17:09:28.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.100Z
Learning: Applies to test/js/bun/** : Place Bun-specific API tests (http, crypto, ffi, shell, etc.) under test/js/bun/
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/napi/**/* : Place N-API tests under test/napi/
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/node/test/** : Vendored Node.js tests under test/js/node/test/ are exceptions and do not follow Bun’s test style
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-09-03T17:10:13.460Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.460Z
Learning: Applies to test/**/*.test.ts : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-08-30T00:12:56.803Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
📚 Learning: 2025-09-03T17:09:28.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.100Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
Applied to files:
src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts
| } else { | ||
| types.decls = types.decls.map(d => { | ||
| if (d.includes("argv") || d.includes("argc")) { | ||
| return d.trim() + ";"; | ||
| } | ||
|
|
||
| // Initialize function pointers and multi-pointers to NULL, everything else to {0} | ||
| if (d.includes("**") || d.includes("(*") || d.includes("_cb ")) { | ||
| return d + " = NULL;"; | ||
| } | ||
|
|
||
| return d + " = {0};"; | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialize pointers with NULL (including single pointers); avoid {0} for pointer types.
Current check only NULL-inits multi-pointers/function ptrs, leaving single pointers initialized with {0}, which can trigger braced-scalar-init warnings and is brittle. Treat any pointer declarator as needing NULL.
Apply this diff:
- types.decls = types.decls.map(d => {
- if (d.includes("argv") || d.includes("argc")) {
- return d.trim() + ";";
- }
-
- // Initialize function pointers and multi-pointers to NULL, everything else to {0}
- if (d.includes("**") || d.includes("(*") || d.includes("_cb ")) {
- return d + " = NULL;";
- }
-
- return d + " = {0};";
- });
+ types.decls = types.decls.map(d => {
+ if (/\b(argv|argc)\b/.test(d)) return d.trim() + ";";
+ // Any pointer (incl. function pointers) -> NULL; everything else -> zero-init
+ if (d.includes("*")) return d + " = NULL;";
+ return d + " = {0};";
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else { | |
| types.decls = types.decls.map(d => { | |
| if (d.includes("argv") || d.includes("argc")) { | |
| return d.trim() + ";"; | |
| } | |
| // Initialize function pointers and multi-pointers to NULL, everything else to {0} | |
| if (d.includes("**") || d.includes("(*") || d.includes("_cb ")) { | |
| return d + " = NULL;"; | |
| } | |
| return d + " = {0};"; | |
| }); | |
| } | |
| } else { | |
| types.decls = types.decls.map(d => { | |
| if (/\b(argv|argc)\b/.test(d)) return d.trim() + ";"; | |
| // Any pointer (incl. function pointers) -> NULL; everything else -> zero-init | |
| if (d.includes("*")) return d + " = NULL;"; | |
| return d + " = {0};"; | |
| }); | |
| } |
🤖 Prompt for AI Agents
In src/bun.js/bindings/libuv/generate_uv_posix_stubs.ts around lines 231 to 244,
the code currently only initializes multi-pointers and function pointers to NULL
but leaves single pointer declarations initialized with "{0}", which can trigger
braced-scalar-init warnings; update the logic to treat any pointer declarator
(e.g., any declara tor text containing "*" ) as needing " = NULL;" so that
single pointers, multi-pointers, and function pointers are all initialized to
NULL, while non-pointer types keep " = {0};" and keep the existing argc/argv
special-case unchanged.
| // or wpt test runner is fully adopted. | ||
| // FYI: https://github.com/oven-sh/bun/issues/19673 | ||
|
|
||
| import { isCI } from "harness"; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM on using isCI from harness; please verify it’s exported everywhere tests run.
Prevents ad-hoc env checks and aligns with shared utilities.
Run to confirm isCI is exported from the harness module:
🏁 Script executed:
#!/bin/bash
fd -t f -a 'harness\.(ts|tsx|js)$' test | while read -r f; do
echo "== $f =="; rg -n -C2 -e 'export\s+(const|let|var)\s+isCI\b' "$f"
doneLength of output: 1667
Ensure isCI is exported across all test harness modules
isCI is currently only defined in test/harness.ts. Add an export (or re-export from the root harness) in the Deno (test/js/deno/harness.ts), Node (test/js/node/harness.ts), Bake (test/bake/bake-harness.ts), and N-API (test/napi/node-napi-tests/harness.ts) harness files so that all environments can import it.
🤖 Prompt for AI Agents
In test/js/bun/crypto/wpt-webcrypto.generateKey.test.ts around line 10, the test
imports isCI from "harness" but isCI is only exported from test/harness.ts;
update the Deno (test/js/deno/harness.ts), Node (test/js/node/harness.ts), Bake
(test/bake/bake-harness.ts), and N-API (test/napi/node-napi-tests/harness.ts)
harness files to export or re-export isCI so the top-level harness import works
in all environments; specifically add an export statement (or a re-export from
the root harness) that exposes isCI from whichever module defines it, and ensure
the module paths/exports match the project’s harness entry so tests can import
isCI uniformly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/format.yml (1)
108-108: Use git's--ignore-unmatchinstead of masking errors with|| true.More robust and quiet; avoids shell glob pitfalls and returns 0 if no tracked files match.
- git rm -f cmake/sources/*.txt || true + git rm -qf --ignore-unmatch -- cmake/sources/*.txt
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/format.yml(1 hunks)cmake/sources/ZigSources.txt(0 hunks)
💤 Files with no reviewable changes (1)
- cmake/sources/ZigSources.txt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-03T17:09:28.100Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shelling out to tools like find or grep in tests; use Bun.Glob and built-in utilities
📚 Learning: 2025-09-03T17:10:13.460Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.460Z
Learning: Applies to test/**/*.test.ts : Name test files `*.test.ts` and use `bun:test`
Applied to files:
.github/workflows/format.yml
What does this PR do?
How did you verify your code works?