-
Couldn't load subscription status.
- Fork 3.5k
gitignore the sources text files #22408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?