Repository navigation
Fix crash in toContainAnyKeys and toContainKeys with non-object values - #22977
Conversation
|
Updated 1:15 AM PT - Sep 29th, 2025
❌ @dylan-conway, your commit cc1e3f2 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 22977That installs a local version of the PR into your bun-22977 --bun |
WalkthroughZig expect helpers now require Changes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.39.5)test/js/bun/test/expect.test.jsComment |
3a2e685 to
0e2aedb
Compare
robobun
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/bun.js/test/expect/toContainAllKeys.zig (1)
27-28: Unnecessary initialization of keys variableThe
keysvariable is initialized to.js_undefinedbut is only used within theif (value.isObject())branch where it's immediately reassigned. Consider declaring it within the branch scope or uninitialized.const not = this.flags.not; var pass = false; -var keys: JSValue = .js_undefined; const count = try expected.getLength(globalObject); if (value.isObject()) { - keys = try value.keys(globalObject); + const keys = try value.keys(globalObject);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📥 Commits
Reviewing files that changed from the base of the PR and between 3a2e685ae412058e87a154acfcaf9fa9aaf15174 and 0e2aedbda2e5e903514b1223f283384da2b7020f.
📒 Files selected for processing (4)
src/bun.js/test/expect/toContainAllKeys.zig(2 hunks)src/bun.js/test/expect/toContainAnyKeys.zig(1 hunks)src/bun.js/test/expect/toContainKeys.zig(1 hunks)test/js/bun/test/expect.test.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/bun.js/test/expect/toContainAnyKeys.zigsrc/bun.js/test/expect/toContainAllKeys.zigsrc/bun.js/test/expect/toContainKeys.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/bun.js/test/expect/toContainAnyKeys.zigsrc/bun.js/test/expect/toContainAllKeys.zigsrc/bun.js/test/expect/toContainKeys.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/test/expect/toContainAnyKeys.zigsrc/bun.js/test/expect/toContainAllKeys.zigsrc/bun.js/test/expect/toContainKeys.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/test/expect.test.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/test/expect.test.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/test/expect.test.js
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/test/expect.test.js
test/js/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests for specific features under
test/js/by module
Files:
test/js/bun/test/expect.test.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/js/bun/test/expect.test.js
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Follow existing patterns in similar V8 classes, add comprehensive Node.js parity tests, update all symbol files, and document any special behavior
Applied to files:
test/js/bun/test/expect.test.js
📚 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} : Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Applied to files:
test/js/bun/test/expect.test.js
📚 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:
test/js/bun/test/expect.test.js
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Add tests for new functionality
Applied to files:
test/js/bun/test/expect.test.js
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Applied to files:
src/bun.js/test/expect/toContainKeys.zig
⏰ 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 (6)
src/bun.js/test/expect/toContainKeys.zig (1)
29-29: Good fix for preventing crash on non-objectsThe change from truthiness check to explicit object type check correctly prevents the crash when
hasOwnPropertyValueis called on non-object values. This aligns with the PR objective of fixing the segmentation fault.src/bun.js/test/expect/toContainAnyKeys.zig (1)
30-41: Correct implementation of object guardThe object type check properly guards the key iteration logic, preventing crashes on non-object values while maintaining the correct pass/fail semantics. The implementation correctly returns false for non-objects, matching jest-extended's behavior.
src/bun.js/test/expect/toContainAllKeys.zig (2)
31-48: Well-structured object guard with proper edge case handlingThe implementation correctly handles both object and non-object cases, with the
pass = count == 0logic properly handling empty key arrays for non-objects. This prevents crashes while maintaining semantically correct behavior.
56-59: Robust error message formattingGood defensive programming by checking if
keys.isUndefined()before formatting, ensuring proper error messages even when the value is not an object.test/js/bun/test/expect.test.js (2)
2567-2615: Comprehensive test coverage for crash preventionExcellent test coverage! The tests thoroughly validate that all three matchers (
toContainAllKeys,toContainAnyKeys,toContainKeys) no longer crash when called on:
- Primitive values (null, undefined, numbers, strings, booleans, BigInt, Symbol)
- Arrays (validating they work with numeric string keys)
- Functions (which are objects)
- Objects created with
Object.create(null)This comprehensive coverage ensures the fix is robust across all edge cases mentioned in the PR objectives.
Based on learnings: Prefer data-driven tests (e.g., test.each) to reduce boilerplate
2625-2643: Good test coverage for toContainKeys edge casesThe tests properly validate that
toContainKeyshandles non-object values correctly, including the expected behavior of returning false for non-objects when keys are expected, and true when the expected keys array is empty.
The toContainAnyKeys matcher was crashing when called with non-object values (null, undefined, numbers, etc.) because it was calling hasOwnPropertyValue without first checking if the value is an object. As documented in JSValue.zig, hasOwnPropertyValue will crash if called on a non-object. This fix adds proper validation to check if the value is both truthy and an object before attempting to check for properties. This matches jest-extended's behavior where non-objects simply return false rather than throwing an error. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
0e2aedb to
21f82f9
Compare
|
Converting to draft. I haven't manually verified the recent changes are good |
dylan-conway
changed the title
toContainAnyKeys and toContainKeys with non-object values
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/bun.js/test/expect/toContainKeys.zig(1 hunks)test/js/bun/test/expect.test.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/test/expect.test.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/test/expect.test.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/test/expect.test.js
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/test/expect.test.js
test/js/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests for specific features under
test/js/by module
Files:
test/js/bun/test/expect.test.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/js/bun/test/expect.test.js
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/bun.js/test/expect/toContainKeys.zig
**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.zig: Declare the extern C symbol in Zig and export a Zig-friendly alias for use
Wrap the Bun____toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/bun.js/test/expect/toContainKeys.zig
src/bun.js/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/zig-javascriptcore-classes.mdc)
src/bun.js/**/*.zig: In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS and re-export toJS/fromJS/fromJSDirect
Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Implement getters as get(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Provide deinit() for resource cleanup and finalize() that calls deinit(); use bun.destroy(this) or appropriate destroy pattern
Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
For properties marked cache: true, use the generated Zig accessors (NameSetCached/GetCached) to work with GC-owned values
In finalize() for objects holding JS references, release them using .deref() before destroy
Files:
src/bun.js/test/expect/toContainKeys.zig
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{js,ts} : Use `$isObject` and throw appropriate TypeErrors for constructor/initializer inputs that must be objects
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Implement getters as get<PropertyName>(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
📚 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} : Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Applied to files:
test/js/bun/test/expect.test.js
📚 Learning: 2025-09-07T05:41:52.563Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{js,ts} : Use `$isObject` and throw appropriate TypeErrors for constructor/initializer inputs that must be objects
Applied to files:
src/bun.js/test/expect/toContainKeys.zig
🔇 Additional comments (1)
src/bun.js/test/expect/toContainKeys.zig (1)
29-32: Object guard prevents property lookups from crashingUsing
value.isObject()here is the correct fix: we now skiphasOwnPropertyValueon primitives while still honoring the empty-expected success path (count == 0). Looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
test/js/bun/test/expect.test.js (2)
2578-2604: Consolidate repetitive non-object checks with data-driven inputsThe non-object assertions are spot on. To reduce boilerplate and ease future additions, consider iterating a shared list instead of individual lines.
Example inside this test:
- expect(null).not.toContainAnyKeys(["a", "b"]); - expect(undefined).not.toContainAnyKeys(["a", "b"]); - expect(42).not.toContainAnyKeys(["a", "b"]); - expect("string").not.toContainAnyKeys(["a", "b"]); - expect(true).not.toContainAnyKeys(["a", "b"]); - expect(false).not.toContainAnyKeys(["a", "b"]); - expect(Symbol("test")).not.toContainAnyKeys(["a", "b"]); - expect(BigInt(123)).not.toContainAnyKeys(["a", "b"]); + for (const v of [null, undefined, 42, "string", true, false, Symbol("x"), 123n]) { + expect(v).not.toContainAnyKeys(["a", "b"]); + }As per coding guidelines (prefer data-driven tests).
2613-2631: Add explicit empty-expected cases for null/undefined in toContainKeysPR notes say non-objects with empty expected should pass. You already cover primitives; add null/undefined to lock this in.
Apply this diff near the “Non-object values” block:
// Non-object values - expect(undefined).not.toContainKeys(["id"]); + // Empty expected array should pass for non-objects (jest-extended parity) + expect(undefined).toContainKeys([]); + expect(null).toContainKeys([]); + // Non-empty should not + expect(undefined).not.toContainKeys(["id"]); - expect(null).not.toContainKeys(["id"]); + expect(null).not.toContainKeys(["id"]);Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
test/js/bun/test/expect.test.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/test/expect.test.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/test/expect.test.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/test/expect.test.js
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/test/expect.test.js
test/js/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests for specific features under
test/js/by module
Files:
test/js/bun/test/expect.test.js
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/js/bun/test/expect.test.js
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Implement getters as get<PropertyName>(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Learnt from: CR
PR: oven-sh/bun#0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-09-07T05:41:52.563Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{js,ts} : Use `$isObject` and throw appropriate TypeErrors for constructor/initializer inputs that must be objects
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} : Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Follow existing patterns in similar V8 classes, add comprehensive Node.js parity tests, update all symbol files, and document any special behavior
📚 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} : Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Applied to files:
test/js/bun/test/expect.test.js
📚 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} : Explicitly assert client console logs with c.expectMessage; unasserted logs should not appear
Applied to files:
test/js/bun/test/expect.test.js
📚 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/ecosystem.test.ts : ecosystem.test.ts should focus on concrete library integration bugs rather than whole-package coverage
Applied to files:
test/js/bun/test/expect.test.js
📚 Learning: 2025-09-20T00:57:56.685Z
Learnt from: markovejnovic
PR: oven-sh/bun#22568
File: test/js/valkey/valkey.test.ts:268-276
Timestamp: 2025-09-20T00:57:56.685Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods like ctx.redis.set() - the maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/js/bun/test/expect.test.js
📚 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:
test/js/bun/test/expect.test.js
📚 Learning: 2025-09-20T00:58:38.042Z
Learnt from: markovejnovic
PR: oven-sh/bun#22568
File: test/js/valkey/valkey.test.ts:561-564
Timestamp: 2025-09-20T00:58:38.042Z
Learning: For test/js/valkey/valkey.test.ts, do not comment on synchronous throw assertions for async Redis methods (like ctx.redis.set(), ctx.redis.unsubscribe(), etc.) - Bun's Redis client implementation differs from Node.js and can throw synchronously even for async methods. The maintainer has explicitly requested to stop looking at this error pattern.
Applied to files:
test/js/bun/test/expect.test.js
📚 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/js/bun/test/expect.test.js
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Applied to files:
test/js/bun/test/expect.test.js
📚 Learning: 2025-09-03T17:10:13.486Z
Learnt from: CR
PR: oven-sh/bun#0
File: test/CLAUDE.md:0-0
Timestamp: 2025-09-03T17:10:13.486Z
Learning: Applies to test/**/*.test.ts : Use `describe` blocks for grouping, `describe.each` for parameterized tests, snapshots with `toMatchSnapshot`, and lifecycle hooks (`beforeAll`, `beforeEach`, `afterEach`); track resources for cleanup in `afterEach`
Applied to files:
test/js/bun/test/expect.test.js
📚 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/esm.test.ts : esm.test.ts should cover ESM feature behavior in development mode
Applied to files:
test/js/bun/test/expect.test.js
🔇 Additional comments (1)
test/js/bun/test/expect.test.js (1)
2567-2568: Array index coverage for toContainAllKeys looks goodPositive and negative cases for numeric-string indices on arrays are covered and match intended behavior. Nice.
3 participants
Summary
toContainAnyKeys,toContainKeys, andtoContainAllKeyson non-object values (null, undefined, numbers, strings, etc.)hasOwnPropertyValueorkeys()Problem
The matchers were crashing with a segmentation fault when called with non-object values because:
toContainAnyKeysandtoContainKeyswere callinghasOwnPropertyValuewithout checking if the value is an object firsttoContainAllKeyswas callingkeys()without checking if the value is an object firsthasOwnPropertyValuefunction documentation explicitly states: "If the object is not an object, it will crash. You must check if the object is an object before calling this function."Solution
value.isObject()check intoContainAnyKeysbefore attempting to check for propertiestoContainKeysby replacing thetoBoolean()check withisObject()checktoContainAllKeysby adding proper object validation before callingkeys()Test plan
test/js/bun/test/expect.test.js🤖 Generated with Claude Code