fix: expect().resolves and expect().rejects now return promises #23425
Conversation
Previously, matchers under `expect().resolves` and `expect().rejects` would return `undefined` instead of a promise. This caused issues when trying to chain or await matcher results. This fix adds a helper function `returnMatcherValue()` that checks if async flags (.resolves/.rejects) are set and returns a resolved promise instead of undefined. All matchers now use this helper. Fixes #23420 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Updated 9:05 PM PT - Oct 14th, 2025
❌ @pfgithub, your commit 23d88b5 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 23425That installs a local version of the PR into your bun-23425 --bun |
WalkthroughAdds a public inline helper Expect.returnMatcherValue that returns a resolved Promise when async flags (.resolves/.rejects) are used, updates many matchers to call it on successful paths instead of returning undefined/thisValue, adjusts snapshot success returns, and adds regression tests verifying .resolves/.rejects return Promises. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (6)test/**📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/js/**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/js/bun/**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/**/*.{js,ts}📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/js/**📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/js/bun/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-10-08T13:48:02.430ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
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.6)test/js/bun/test/expect.test.jsComment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 (72)
src/bun.js/test/expect.zig(4 hunks)src/bun.js/test/expect/toBe.zig(1 hunks)src/bun.js/test/expect/toBeArray.zig(1 hunks)src/bun.js/test/expect/toBeArrayOfSize.zig(1 hunks)src/bun.js/test/expect/toBeBoolean.zig(1 hunks)src/bun.js/test/expect/toBeCloseTo.zig(2 hunks)src/bun.js/test/expect/toBeDate.zig(1 hunks)src/bun.js/test/expect/toBeDefined.zig(1 hunks)src/bun.js/test/expect/toBeEmpty.zig(1 hunks)src/bun.js/test/expect/toBeEmptyObject.zig(1 hunks)src/bun.js/test/expect/toBeEven.zig(1 hunks)src/bun.js/test/expect/toBeFalse.zig(1 hunks)src/bun.js/test/expect/toBeFalsy.zig(1 hunks)src/bun.js/test/expect/toBeFinite.zig(1 hunks)src/bun.js/test/expect/toBeFunction.zig(1 hunks)src/bun.js/test/expect/toBeGreaterThan.zig(1 hunks)src/bun.js/test/expect/toBeGreaterThanOrEqual.zig(1 hunks)src/bun.js/test/expect/toBeInstanceOf.zig(1 hunks)src/bun.js/test/expect/toBeInteger.zig(1 hunks)src/bun.js/test/expect/toBeLessThan.zig(1 hunks)src/bun.js/test/expect/toBeLessThanOrEqual.zig(1 hunks)src/bun.js/test/expect/toBeNaN.zig(1 hunks)src/bun.js/test/expect/toBeNegative.zig(1 hunks)src/bun.js/test/expect/toBeNil.zig(1 hunks)src/bun.js/test/expect/toBeNull.zig(1 hunks)src/bun.js/test/expect/toBeNumber.zig(1 hunks)src/bun.js/test/expect/toBeObject.zig(1 hunks)src/bun.js/test/expect/toBeOdd.zig(1 hunks)src/bun.js/test/expect/toBeOneOf.zig(1 hunks)src/bun.js/test/expect/toBePositive.zig(1 hunks)src/bun.js/test/expect/toBeString.zig(1 hunks)src/bun.js/test/expect/toBeSymbol.zig(1 hunks)src/bun.js/test/expect/toBeTrue.zig(1 hunks)src/bun.js/test/expect/toBeTruthy.zig(1 hunks)src/bun.js/test/expect/toBeTypeOf.zig(1 hunks)src/bun.js/test/expect/toBeUndefined.zig(1 hunks)src/bun.js/test/expect/toBeValidDate.zig(1 hunks)src/bun.js/test/expect/toBeWithin.zig(1 hunks)src/bun.js/test/expect/toContain.zig(1 hunks)src/bun.js/test/expect/toContainAllKeys.zig(1 hunks)src/bun.js/test/expect/toContainAllValues.zig(1 hunks)src/bun.js/test/expect/toContainAnyKeys.zig(1 hunks)src/bun.js/test/expect/toContainAnyValues.zig(1 hunks)src/bun.js/test/expect/toContainEqual.zig(1 hunks)src/bun.js/test/expect/toContainKey.zig(1 hunks)src/bun.js/test/expect/toContainKeys.zig(1 hunks)src/bun.js/test/expect/toContainValue.zig(1 hunks)src/bun.js/test/expect/toContainValues.zig(1 hunks)src/bun.js/test/expect/toEndWith.zig(1 hunks)src/bun.js/test/expect/toEqual.zig(1 hunks)src/bun.js/test/expect/toEqualIgnoringWhitespace.zig(1 hunks)src/bun.js/test/expect/toHaveBeenCalled.zig(1 hunks)src/bun.js/test/expect/toHaveBeenCalledOnce.zig(1 hunks)src/bun.js/test/expect/toHaveBeenCalledTimes.zig(1 hunks)src/bun.js/test/expect/toHaveBeenCalledWith.zig(1 hunks)src/bun.js/test/expect/toHaveBeenLastCalledWith.zig(1 hunks)src/bun.js/test/expect/toHaveBeenNthCalledWith.zig(1 hunks)src/bun.js/test/expect/toHaveLastReturnedWith.zig(1 hunks)src/bun.js/test/expect/toHaveLength.zig(1 hunks)src/bun.js/test/expect/toHaveNthReturnedWith.zig(1 hunks)src/bun.js/test/expect/toHaveProperty.zig(1 hunks)src/bun.js/test/expect/toHaveReturned.zig(1 hunks)src/bun.js/test/expect/toHaveReturnedWith.zig(1 hunks)src/bun.js/test/expect/toInclude.zig(1 hunks)src/bun.js/test/expect/toIncludeRepeated.zig(1 hunks)src/bun.js/test/expect/toMatch.zig(1 hunks)src/bun.js/test/expect/toMatchObject.zig(1 hunks)src/bun.js/test/expect/toSatisfy.zig(1 hunks)src/bun.js/test/expect/toStartWith.zig(1 hunks)src/bun.js/test/expect/toStrictEqual.zig(1 hunks)src/bun.js/test/expect/toThrow.zig(10 hunks)test/regression/issue/23420.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.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
Files:
src/bun.js/test/expect/toBeNegative.zigsrc/bun.js/test/expect/toBeDefined.zigsrc/bun.js/test/expect/toBeNull.zigsrc/bun.js/test/expect/toHaveBeenNthCalledWith.zigsrc/bun.js/test/expect/toBeInteger.zigsrc/bun.js/test/expect/toBeNil.zigsrc/bun.js/test/expect/toHaveBeenCalled.zigsrc/bun.js/test/expect/toBeOneOf.zigsrc/bun.js/test/expect/toContainAllValues.zigsrc/bun.js/test/expect/toHaveLastReturnedWith.zigsrc/bun.js/test/expect/toBeBoolean.zigsrc/bun.js/test/expect/toEqualIgnoringWhitespace.zigsrc/bun.js/test/expect/toBeFalse.zigsrc/bun.js/test/expect/toHaveReturnedWith.zigsrc/bun.js/test/expect/toHaveProperty.zigsrc/bun.js/test/expect/toBeFinite.zigsrc/bun.js/test/expect/toHaveReturned.zigsrc/bun.js/test/expect/toContainKeys.zigsrc/bun.js/test/expect/toSatisfy.zigsrc/bun.js/test/expect/toBeCloseTo.zigsrc/bun.js/test/expect/toBeLessThanOrEqual.zigsrc/bun.js/test/expect/toStrictEqual.zigsrc/bun.js/test/expect/toBe.zigsrc/bun.js/test/expect/toStartWith.zigsrc/bun.js/test/expect/toHaveBeenCalledTimes.zigsrc/bun.js/test/expect/toBeFunction.zigsrc/bun.js/test/expect/toContainEqual.zigsrc/bun.js/test/expect/toBeSymbol.zigsrc/bun.js/test/expect/toBeArray.zigsrc/bun.js/test/expect/toBeGreaterThanOrEqual.zigsrc/bun.js/test/expect/toEqual.zigsrc/bun.js/test/expect/toContainAnyKeys.zigsrc/bun.js/test/expect/toBeOdd.zigsrc/bun.js/test/expect/toContainAllKeys.zigsrc/bun.js/test/expect/toBeLessThan.zigsrc/bun.js/test/expect/toBeDate.zigsrc/bun.js/test/expect.zigsrc/bun.js/test/expect/toBeTruthy.zigsrc/bun.js/test/expect/toThrow.zigsrc/bun.js/test/expect/toHaveBeenLastCalledWith.zigsrc/bun.js/test/expect/toContainValue.zigsrc/bun.js/test/expect/toBeInstanceOf.zigsrc/bun.js/test/expect/toBeTrue.zigsrc/bun.js/test/expect/toBeGreaterThan.zigsrc/bun.js/test/expect/toBeEmpty.zigsrc/bun.js/test/expect/toBeEven.zigsrc/bun.js/test/expect/toContainAnyValues.zigsrc/bun.js/test/expect/toEndWith.zigsrc/bun.js/test/expect/toBeEmptyObject.zigsrc/bun.js/test/expect/toBeNaN.zigsrc/bun.js/test/expect/toHaveBeenCalledWith.zigsrc/bun.js/test/expect/toBeWithin.zigsrc/bun.js/test/expect/toBeString.zigsrc/bun.js/test/expect/toBePositive.zigsrc/bun.js/test/expect/toBeFalsy.zigsrc/bun.js/test/expect/toHaveLength.zigsrc/bun.js/test/expect/toContainKey.zigsrc/bun.js/test/expect/toContain.zigsrc/bun.js/test/expect/toIncludeRepeated.zigsrc/bun.js/test/expect/toHaveBeenCalledOnce.zigsrc/bun.js/test/expect/toContainValues.zigsrc/bun.js/test/expect/toBeNumber.zigsrc/bun.js/test/expect/toBeTypeOf.zigsrc/bun.js/test/expect/toMatchObject.zigsrc/bun.js/test/expect/toBeUndefined.zigsrc/bun.js/test/expect/toBeValidDate.zigsrc/bun.js/test/expect/toHaveNthReturnedWith.zigsrc/bun.js/test/expect/toMatch.zigsrc/bun.js/test/expect/toInclude.zigsrc/bun.js/test/expect/toBeArrayOfSize.zigsrc/bun.js/test/expect/toBeObject.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/toBeNegative.zigsrc/bun.js/test/expect/toBeDefined.zigsrc/bun.js/test/expect/toBeNull.zigsrc/bun.js/test/expect/toHaveBeenNthCalledWith.zigsrc/bun.js/test/expect/toBeInteger.zigsrc/bun.js/test/expect/toBeNil.zigsrc/bun.js/test/expect/toHaveBeenCalled.zigsrc/bun.js/test/expect/toBeOneOf.zigsrc/bun.js/test/expect/toContainAllValues.zigsrc/bun.js/test/expect/toHaveLastReturnedWith.zigsrc/bun.js/test/expect/toBeBoolean.zigsrc/bun.js/test/expect/toEqualIgnoringWhitespace.zigsrc/bun.js/test/expect/toBeFalse.zigsrc/bun.js/test/expect/toHaveReturnedWith.zigsrc/bun.js/test/expect/toHaveProperty.zigsrc/bun.js/test/expect/toBeFinite.zigsrc/bun.js/test/expect/toHaveReturned.zigsrc/bun.js/test/expect/toContainKeys.zigsrc/bun.js/test/expect/toSatisfy.zigsrc/bun.js/test/expect/toBeCloseTo.zigsrc/bun.js/test/expect/toBeLessThanOrEqual.zigsrc/bun.js/test/expect/toStrictEqual.zigsrc/bun.js/test/expect/toBe.zigsrc/bun.js/test/expect/toStartWith.zigsrc/bun.js/test/expect/toHaveBeenCalledTimes.zigsrc/bun.js/test/expect/toBeFunction.zigsrc/bun.js/test/expect/toContainEqual.zigsrc/bun.js/test/expect/toBeSymbol.zigsrc/bun.js/test/expect/toBeArray.zigsrc/bun.js/test/expect/toBeGreaterThanOrEqual.zigsrc/bun.js/test/expect/toEqual.zigsrc/bun.js/test/expect/toContainAnyKeys.zigsrc/bun.js/test/expect/toBeOdd.zigsrc/bun.js/test/expect/toContainAllKeys.zigsrc/bun.js/test/expect/toBeLessThan.zigsrc/bun.js/test/expect/toBeDate.zigsrc/bun.js/test/expect.zigsrc/bun.js/test/expect/toBeTruthy.zigsrc/bun.js/test/expect/toThrow.zigsrc/bun.js/test/expect/toHaveBeenLastCalledWith.zigsrc/bun.js/test/expect/toContainValue.zigsrc/bun.js/test/expect/toBeInstanceOf.zigsrc/bun.js/test/expect/toBeTrue.zigsrc/bun.js/test/expect/toBeGreaterThan.zigsrc/bun.js/test/expect/toBeEmpty.zigsrc/bun.js/test/expect/toBeEven.zigsrc/bun.js/test/expect/toContainAnyValues.zigsrc/bun.js/test/expect/toEndWith.zigsrc/bun.js/test/expect/toBeEmptyObject.zigsrc/bun.js/test/expect/toBeNaN.zigsrc/bun.js/test/expect/toHaveBeenCalledWith.zigsrc/bun.js/test/expect/toBeWithin.zigsrc/bun.js/test/expect/toBeString.zigsrc/bun.js/test/expect/toBePositive.zigsrc/bun.js/test/expect/toBeFalsy.zigsrc/bun.js/test/expect/toHaveLength.zigsrc/bun.js/test/expect/toContainKey.zigsrc/bun.js/test/expect/toContain.zigsrc/bun.js/test/expect/toIncludeRepeated.zigsrc/bun.js/test/expect/toHaveBeenCalledOnce.zigsrc/bun.js/test/expect/toContainValues.zigsrc/bun.js/test/expect/toBeNumber.zigsrc/bun.js/test/expect/toBeTypeOf.zigsrc/bun.js/test/expect/toMatchObject.zigsrc/bun.js/test/expect/toBeUndefined.zigsrc/bun.js/test/expect/toBeValidDate.zigsrc/bun.js/test/expect/toHaveNthReturnedWith.zigsrc/bun.js/test/expect/toMatch.zigsrc/bun.js/test/expect/toInclude.zigsrc/bun.js/test/expect/toBeArrayOfSize.zigsrc/bun.js/test/expect/toBeObject.zig
src/**/*.zig
📄 CodeRabbit inference engine (CLAUDE.md)
In Zig code, manage memory carefully and use defer for cleanup of allocations/resources
src/**/*.zig: Use the # prefix to declare private fields in Zig structs (e.g., struct { #foo: u32 })
Prefer decl literals when initializing values in Zig (e.g., const decl: Decl = .{ .binding = 0, .value = 0 })
Place @import directives at the bottom of Zig files
Use @import("bun") instead of @import("root").bunWhen adding debug logs in Zig, create a scoped logger and log via Bun APIs:
const log = bun.Output.scoped(.${SCOPE}, .hidden);thenlog("...", .{})
Files:
src/bun.js/test/expect/toBeNegative.zigsrc/bun.js/test/expect/toBeDefined.zigsrc/bun.js/test/expect/toBeNull.zigsrc/bun.js/test/expect/toHaveBeenNthCalledWith.zigsrc/bun.js/test/expect/toBeInteger.zigsrc/bun.js/test/expect/toBeNil.zigsrc/bun.js/test/expect/toHaveBeenCalled.zigsrc/bun.js/test/expect/toBeOneOf.zigsrc/bun.js/test/expect/toContainAllValues.zigsrc/bun.js/test/expect/toHaveLastReturnedWith.zigsrc/bun.js/test/expect/toBeBoolean.zigsrc/bun.js/test/expect/toEqualIgnoringWhitespace.zigsrc/bun.js/test/expect/toBeFalse.zigsrc/bun.js/test/expect/toHaveReturnedWith.zigsrc/bun.js/test/expect/toHaveProperty.zigsrc/bun.js/test/expect/toBeFinite.zigsrc/bun.js/test/expect/toHaveReturned.zigsrc/bun.js/test/expect/toContainKeys.zigsrc/bun.js/test/expect/toSatisfy.zigsrc/bun.js/test/expect/toBeCloseTo.zigsrc/bun.js/test/expect/toBeLessThanOrEqual.zigsrc/bun.js/test/expect/toStrictEqual.zigsrc/bun.js/test/expect/toBe.zigsrc/bun.js/test/expect/toStartWith.zigsrc/bun.js/test/expect/toHaveBeenCalledTimes.zigsrc/bun.js/test/expect/toBeFunction.zigsrc/bun.js/test/expect/toContainEqual.zigsrc/bun.js/test/expect/toBeSymbol.zigsrc/bun.js/test/expect/toBeArray.zigsrc/bun.js/test/expect/toBeGreaterThanOrEqual.zigsrc/bun.js/test/expect/toEqual.zigsrc/bun.js/test/expect/toContainAnyKeys.zigsrc/bun.js/test/expect/toBeOdd.zigsrc/bun.js/test/expect/toContainAllKeys.zigsrc/bun.js/test/expect/toBeLessThan.zigsrc/bun.js/test/expect/toBeDate.zigsrc/bun.js/test/expect.zigsrc/bun.js/test/expect/toBeTruthy.zigsrc/bun.js/test/expect/toThrow.zigsrc/bun.js/test/expect/toHaveBeenLastCalledWith.zigsrc/bun.js/test/expect/toContainValue.zigsrc/bun.js/test/expect/toBeInstanceOf.zigsrc/bun.js/test/expect/toBeTrue.zigsrc/bun.js/test/expect/toBeGreaterThan.zigsrc/bun.js/test/expect/toBeEmpty.zigsrc/bun.js/test/expect/toBeEven.zigsrc/bun.js/test/expect/toContainAnyValues.zigsrc/bun.js/test/expect/toEndWith.zigsrc/bun.js/test/expect/toBeEmptyObject.zigsrc/bun.js/test/expect/toBeNaN.zigsrc/bun.js/test/expect/toHaveBeenCalledWith.zigsrc/bun.js/test/expect/toBeWithin.zigsrc/bun.js/test/expect/toBeString.zigsrc/bun.js/test/expect/toBePositive.zigsrc/bun.js/test/expect/toBeFalsy.zigsrc/bun.js/test/expect/toHaveLength.zigsrc/bun.js/test/expect/toContainKey.zigsrc/bun.js/test/expect/toContain.zigsrc/bun.js/test/expect/toIncludeRepeated.zigsrc/bun.js/test/expect/toHaveBeenCalledOnce.zigsrc/bun.js/test/expect/toContainValues.zigsrc/bun.js/test/expect/toBeNumber.zigsrc/bun.js/test/expect/toBeTypeOf.zigsrc/bun.js/test/expect/toMatchObject.zigsrc/bun.js/test/expect/toBeUndefined.zigsrc/bun.js/test/expect/toBeValidDate.zigsrc/bun.js/test/expect/toHaveNthReturnedWith.zigsrc/bun.js/test/expect/toMatch.zigsrc/bun.js/test/expect/toInclude.zigsrc/bun.js/test/expect/toBeArrayOfSize.zigsrc/bun.js/test/expect/toBeObject.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/regression/issue/23420.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/regression/issue/23420.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/regression/issue/23420.test.ts
test/regression/issue/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Place regression tests under
test/regression/issue/and organize by issue numberPlace regression tests under test/regression/issue/ (one per bug fix)
Files:
test/regression/issue/23420.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must live under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or roll your own random port
Prefer normalizeBunSnapshot for snapshotting test output instead of asserting raw strings
Do not write tests that assert absence of crashes (e.g., 'no panic' or 'no uncaught exception')
Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Avoid shell commands like find or grep in tests; use Bun’s Glob and built-in tools instead
Prefer running tests via bun bd test and use provided harness utilities (bunEnv, bunExe, tempDir)
Use Bun.spawn with proper stdio handling and await proc.exited in process-spawning tests
Files:
test/regression/issue/23420.test.ts
🧠 Learnings (16)
📓 Common learnings
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 : Prefer `async/await` in tests; for a single callback, use `Promise.withResolvers()`
📚 Learning: 2025-09-05T19:49:26.188Z
Learnt from: markovejnovic
PR: oven-sh/bun#21728
File: src/valkey/js_valkey_functions.zig:852-867
Timestamp: 2025-09-05T19:49:26.188Z
Learning: In Bun's Zig codebase, `.js_undefined` is a valid way to represent JavaScript's undefined value when working with JSPromise.resolve() and similar JavaScript interop functions. This is the correct pattern to use rather than `jsc.JSValue.jsUndefined()`.
Applied to files:
src/bun.js/test/expect/toBeDefined.zigsrc/bun.js/test/expect.zigsrc/bun.js/test/expect/toBeUndefined.zig
📚 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 : Implement getters as get<PropertyName>(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Applied to files:
src/bun.js/test/expect/toHaveProperty.zig
📚 Learning: 2025-09-07T22:26:50.213Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#22478
File: test/regression/issue/22475.test.ts:0-0
Timestamp: 2025-09-07T22:26:50.213Z
Learning: The `toBeDate()` matcher is acceptable and supported in Bun's test suite - don't suggest replacing it with `toBeInstanceOf(Date)`.
Applied to files:
src/bun.js/test/expect/toBe.zigsrc/bun.js/test/expect/toBeDate.zigsrc/bun.js/test/expect/toBeValidDate.zig
📚 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/toBeFunction.zigsrc/bun.js/test/expect.zig
📚 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 : Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
Applied to files:
src/bun.js/test/expect/toBeFunction.zigsrc/bun.js/test/expect.zigsrc/bun.js/test/expect/toThrow.zig
📚 Learning: 2025-09-07T22:28:28.044Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#22478
File: test/regression/issue/22475.test.ts:0-0
Timestamp: 2025-09-07T22:28:28.044Z
Learning: toBeDate() is a valid custom matcher available in Bun's test environment and should be used instead of toBeInstanceOf(Date) for Date assertions in Bun tests.
Applied to files:
src/bun.js/test/expect/toBeDate.zig
📚 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 : Prefer `async/await` in tests; for a single callback, use `Promise.withResolvers()`
Applied to files:
test/regression/issue/23420.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
PR: oven-sh/bun#23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.
Applied to files:
test/regression/issue/23420.test.ts
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to test/**/*.test.{ts,tsx} : Use Bun’s Jest-compatible runner (import { test, expect } from "bun:test") for tests
Applied to files:
test/regression/issue/23420.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} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Applied to files:
test/regression/issue/23420.test.ts
📚 Learning: 2025-09-19T19:55:22.427Z
Learnt from: markovejnovic
PR: oven-sh/bun#22568
File: src/valkey/js_valkey_functions.zig:853-854
Timestamp: 2025-09-19T19:55:22.427Z
Learning: In Bun's JSPromise API, `resolvedPromiseValue(globalThis: *JSGlobalObject, value: JSValue) JSValue` returns a JSValue directly (not a JSPromise that needs `.toJS()` called on it). This is different from `resolvedPromise()` which returns a *JSPromise that would need `.toJS()`.
Applied to files:
src/bun.js/test/expect.zig
📚 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 : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Applied to files:
src/bun.js/test/expect.zig
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.zig : Wrap the Bun__<Type>__toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Applied to files:
src/bun.js/test/expect.zig
📚 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 : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect
Applied to files:
src/bun.js/test/expect.zig
📚 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: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Applied to files:
src/bun.js/test/expect.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 (82)
src/bun.js/test/expect/toBeNumber.zig (1)
1-53: Consistent and correct matcher return value updates.All eight matcher files follow the identical pattern of replacing the success-path return with
this.returnMatcherValue(globalThis). This change correctly implements the PR objective to return Promise instances for.resolvesand.rejectsmatchers.The modifications are:
- Mechanically consistent across all files
- Preserve all existing logic and error handling
- Only affect the success path return value
- Rely on the
returnMatcherValuehelper added to theExpectstructThe implementation is correct, and the regression test suite added in
test/regression/issue/23420.test.tsprovides coverage for the Promise-related behavior.src/bun.js/test/expect/toBeLessThanOrEqual.zig (2)
41-41: LGTM! Consistent with the matcher return value pattern.The change correctly updates the success path to return
this.returnMatcherValue(globalThis).
41-41: LGTM! Consistent with the matcher value pattern.The success path now returns
this.returnMatcherValue(globalThis), ensuring consistent behavior with other matchers in the PR.src/bun.js/test/expect/toBeFunction.zig (2)
12-12: LGTM! Correct implementation.The success path now returns the appropriate matcher value via
this.returnMatcherValue(globalThis).
12-12: LGTM! Success path updated correctly.The matcher now returns
this.returnMatcherValue(globalThis)on success, aligning with the Promise-return behavior for async matchers.src/bun.js/test/expect/toHaveLastReturnedWith.zig (2)
48-50: LGTM! Correct handling of negation and return value.The condition
pass != this.flags.notcorrectly handles both positive and negated matchers, and the return value now usesthis.returnMatcherValue(globalThis)for successful matches.
49-49: LGTM! Success condition now returns matcher value.When
pass != this.flags.not, the function correctly returnsthis.returnMatcherValue(globalThis)instead of.js_undefined, maintaining consistency with the broader PR changes.src/bun.js/test/expect/toBeOdd.zig (2)
35-35: LGTM! Correct implementation.The success path correctly returns
this.returnMatcherValue(globalThis)for passing odd number checks.
35-35: LGTM! Success path updated to return matcher value.The change from
.js_undefinedtothis.returnMatcherValue(globalThis)is consistent with the PR's objective to support Promise returns for async matchers.src/bun.js/test/expect/toStartWith.zig (2)
35-35: LGTM! Correct implementation.The success path correctly returns
this.returnMatcherValue(globalThis)for passing string prefix checks.
35-35: LGTM! Consistent success-path return.The matcher now returns
this.returnMatcherValue(globalThis)on success, aligning with the Promise-return pattern for async matchers.src/bun.js/test/expect/toContainValues.zig (2)
45-45: LGTM! Correct implementation with consistent parameter naming.The success path correctly returns
this.returnMatcherValue(globalObject). Note that this file usesglobalObjectas the parameter name (line 3), which aligns with the coding guidelines preference forglobalObjectoverctx.
45-45: LGTM! Success path now returns matcher value instead of thisValue.The change from
thisValuetothis.returnMatcherValue(globalObject)ensures consistent behavior with other matchers. Note that the parameter nameglobalObjectis used consistently with the function signature.src/bun.js/test/expect/toContainKey.zig (2)
31-31: LGTM! Correct implementation.The success path correctly returns
this.returnMatcherValue(globalThis)for passing key containment checks.
31-31: LGTM! Success path updated from thisValue to matcher value.The change from
thisValuetothis.returnMatcherValue(globalThis)aligns with the PR's objective to return Promise instances for async matchers, ensuring consistent behavior across the test suite.src/bun.js/test/expect/toEqual.zig (2)
21-21: Verified returnMatcherValue implementation correctly returns a resolved Promise withundefinedwhen.resolves/.rejectsflags are set and returns.js_undefinedfor synchronous matchers.
21-21: LGTM! Success path now returns a concrete matcher value.The change from
.js_undefinedtothis.returnMatcherValue(globalThis)aligns with the PR objective to return Promise instances for.resolvesand.rejectsmatchers. The helper function correctly receives theglobalThisparameter.src/bun.js/test/expect/toHaveBeenCalledWith.zig (1)
50-50: LGTM! Consistent with PR objectives.The change correctly returns a matcher value on success instead of undefined, enabling Promise chaining for
.resolvesand.rejectsmatchers.src/bun.js/test/expect/toBeNull.zig (1)
12-12: LGTM! Correct implementation.The matcher now returns the appropriate value (Promise for async cases, undefined otherwise) on success.
src/bun.js/test/expect/toContainAllValues.zig (1)
51-51: LGTM! Consistency improvement.This change unifies the return behavior with other matchers. Previously, this matcher returned
thisValueon success while most others returned.js_undefined. Now all matchers use the samereturnMatcherValuehelper for consistent behavior.src/bun.js/test/expect/toBeOneOf.zig (1)
64-64: LGTM! Correct implementation.The change properly returns a matcher value on success, maintaining compatibility with Promise-based expect chains.
src/bun.js/test/expect/toBeFinite.zig (1)
18-18: LGTM! Consistent with other matchers.The success path now correctly returns a matcher value, enabling proper Promise chaining for async expectations.
src/bun.js/test/expect/toBe.zig (1)
25-25: LGTM! Core matcher updated correctly.The
toBematcher (using Object.is() semantics) now returns the appropriate matcher value on success, consistent with the PR's objective to support Promise returns for.resolvesand.rejects.src/bun.js/test/expect/toBeBoolean.zig (1)
12-12: LGTM! Simple and correct.The boolean type matcher now returns the appropriate value on success, enabling Promise chaining for async expectations.
src/bun.js/test/expect/toBeSymbol.zig (1)
12-12: LGTM! Consistent implementation.The symbol type matcher now correctly returns a matcher value on success, completing the pattern applied across all matchers in this PR.
src/bun.js/test/expect/toContainEqual.zig (1)
85-85: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toHaveBeenLastCalledWith.zig (1)
46-46: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toIncludeRepeated.zig (1)
60-60: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toBeFalsy.zig (1)
17-17: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toBeTrue.zig (1)
12-12: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toBeDate.zig (1)
12-12: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toBePositive.zig (1)
18-18: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toBeTruthy.zig (1)
15-15: LGTM! Consistent success-path update.The change correctly replaces the success return with
this.returnMatcherValue(globalThis), enabling promise returns for.resolves/.rejectsmatchers. The logic and error handling remain intact.src/bun.js/test/expect/toMatchObject.zig (1)
40-40: LGTM! Success path correctly returns matcher value.The success return now uses
returnMatcherValue(globalThis)to properly return a Promise for async matchers (.resolves/.rejects) orundefinedotherwise, fixing the issue where these matchers returnedundefinedinstead of a Promise.src/bun.js/test/expect/toHaveReturnedWith.zig (1)
55-55: LGTM! Matcher value correctly returned on success.The change aligns with the PR's goal of returning Promise instances for
.resolves/.rejectsmatchers while maintaining backward compatibility with synchronous matchers.src/bun.js/test/expect/toContainValue.zig (1)
36-36: LGTM! Success return value updated correctly.Consistent with the systematic update across all matchers to return the appropriate value (Promise or undefined) via the new helper function.
src/bun.js/test/expect/toBeFalse.zig (1)
12-12: LGTM! Success path correctly updated.The matcher now returns the appropriate value for async contexts (Promise) or sync contexts (undefined) as intended by the fix.
src/bun.js/test/expect/toContain.zig (1)
76-76: LGTM! Return value correctly updated.The change maintains the existing matching logic while ensuring the correct return value for both synchronous and asynchronous (
.resolves/.rejects) contexts.src/bun.js/test/expect/toBeArrayOfSize.zig (1)
27-27: LGTM! Success return properly updated.Consistent with the pattern applied across all matchers to fix the Promise return value issue for
.resolvesand.rejectsassertions.src/bun.js/test/expect/toBeArray.zig (1)
12-12: LGTM! Matcher return value correctly updated.The change ensures proper Promise return for async matchers while preserving the existing validation logic.
src/bun.js/test/expect/toBeInstanceOf.zig (1)
27-27: LGTM! Success path correctly returns matcher value.The final matcher update follows the same pattern as the other 70+ matchers, ensuring consistent behavior across the test suite where
.resolvesand.rejectsproperly return Promise instances.src/bun.js/test/expect/toBeGreaterThanOrEqual.zig (1)
41-41: Align pass-path return with Promise-aware helperSwitching to
returnMatcherValueensures successful.resolves/.rejectschains surface a concrete value while preserving undefined for sync matchers. Looks great.src/bun.js/test/expect/toBeLessThan.zig (1)
41-41: Consistent success return handlingUsing
returnMatcherValuehere keeps matcher chaining intact for async expectations without altering sync behavior. All good.src/bun.js/test/expect/toSatisfy.zig (1)
35-35: Predicate success now honors async flagsReturning
this.returnMatcherValue(globalThis)correctly propagates promise results for.resolves/.rejectsflows. Implementation looks solid.src/bun.js/test/expect/toHaveBeenCalledTimes.zig (1)
29-29: Mock call-count pass path updated correctlyAdopting
returnMatcherValuehere keeps successful call-count assertions compatible with promise chaining while preserving previous sync semantics. 👍src/bun.js/test/expect/toHaveBeenCalled.zig (1)
26-26: Success path now respects matcher return contractThe switch to
returnMatcherValuealigns this matcher with the new promise-aware return behavior without impacting existing sync usage. Looks good.src/bun.js/test/expect/toBeEmptyObject.zig (1)
13-13: LGTM! Consistent with PR objective.The success path now returns
this.returnMatcherValue(globalThis)instead ofthisValue, which aligns with the PR's goal to return Promise instances for.resolvesand.rejectsmatchers. The change is minimal and focused.src/bun.js/test/expect/toBeTypeOf.zig (1)
68-68: LGTM! Consistent pattern application.The success path now uses
this.returnMatcherValue(globalThis)instead of.js_undefined. This change aligns with the broader PR objective to ensure matchers return Promise instances when used with.resolvesor.rejects.src/bun.js/test/expect/toContainKeys.zig (1)
47-47: LGTM! Correct application of the new pattern.The success path correctly returns
this.returnMatcherValue(globalThis)instead ofthisValue. This ensures proper Promise handling for async matchers.src/bun.js/test/expect/toHaveReturned.zig (1)
46-46: LGTM! Proper integration in shared helper.The success path in
toHaveReturnedTimesFnnow returnsthis.returnMatcherValue(globalThis), which correctly applies to bothtoHaveReturnedandtoHaveReturnedTimesmatchers. The change maintains consistency with the PR's async Promise handling objective.src/bun.js/test/expect/toBeEmpty.zig (1)
64-64: LGTM! Correct placement in complex matcher.The success path correctly uses
this.returnMatcherValue(globalThis). Despite the complexity oftoBeEmpty's logic (handling strings, objects, iterables), the change is properly placed and consistent with the PR pattern.src/bun.js/test/expect/toHaveProperty.zig (1)
42-42: LGTM! Consistent with PR objective.The success path now returns
this.returnMatcherValue(globalThis). This change correctly integrates with the property path traversal and deep equality logic while maintaining the PR's Promise-handling semantics.src/bun.js/test/expect/toBeDefined.zig (1)
12-12: LGTM! Proper evolution of undefined handling.The success path now returns
this.returnMatcherValue(globalThis)instead of the previous.js_undefined. While.js_undefinedis valid for JS interop (as noted in learnings), the new helper provides the necessary Promise-handling semantics for async matchers.src/bun.js/test/expect/toBeWithin.zig (1)
39-39: All matchers updated with returnMatcherValue
Verified 75to*.zigfiles in src/bun.js/test/expect usereturnMatcherValueon success and its implementation correctly handles async flags.src/bun.js/test/expect/toEndWith.zig (2)
35-35: LGTM! Consistent return value for successful matches.The change correctly returns
this.returnMatcherValue(globalThis)on successful matches, aligning with the PR objective to return Promises for.resolvesand.rejectsmatchers. The implementation is clean and the failure paths remain unchanged.
35-35: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path, aligning with the PR objective to return Promises for async matchers.src/bun.js/test/expect/toBeValidDate.zig (2)
13-13: LGTM! Correct return value for successful date validation.The change properly returns
this.returnMatcherValue(globalThis)instead ofthisValue, ensuring the matcher returns the appropriate value (Promise for async contexts, undefined otherwise) on successful validation.
13-13: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path.src/bun.js/test/expect/toHaveNthReturnedWith.zig (2)
58-58: LGTM! Proper return value for successful mock validation.The change correctly returns
this.returnMatcherValue(globalThis)when the matcher passes, maintaining consistency with the PR's objective to support Promise returns for async matchers. The complex mock function validation logic remains intact.
58-58: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path.src/bun.js/test/expect/toContainAllKeys.zig (2)
46-46: LGTM! Consistent return value for successful key validation.The change correctly uses
this.returnMatcherValue(globalThis)on successful matches, ensuring proper Promise returns for async contexts while maintaining undefined for synchronous cases.
46-46: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path.src/bun.js/test/expect/toBeObject.zig (2)
12-12: LGTM! Correct return value for type validation.The change properly returns
this.returnMatcherValue(globalThis)instead ofthisValue, aligning with the pattern used across all matchers in this PR.
12-12: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path.src/bun.js/test/expect/toBeInteger.zig (2)
12-12: LGTM! Consistent return value for integer validation.The change correctly uses
this.returnMatcherValue(globalThis)on successful matches, maintaining consistency with the PR's systematic approach to fixing matcher return values.
12-12: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path.src/bun.js/test/expect/toHaveBeenNthCalledWith.zig (2)
54-54: LGTM! Proper return value for successful call validation.The change correctly returns
this.returnMatcherValue(globalThis)when the mock function call validation passes, ensuring the matcher behaves correctly in both sync and async contexts.
54-54: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path.src/bun.js/test/expect/toBeGreaterThan.zig (2)
41-41: LGTM! Consistent return value for successful comparison.The change correctly returns
this.returnMatcherValue(globalThis)on successful matches, completing the systematic update across all matchers to support proper return values for.resolvesand.rejects.
41-41: LGTM: Consistent success-path return value.The change correctly uses the new
returnMatcherValuehelper for the success path.src/bun.js/test/expect/toBeUndefined.zig (1)
13-13: LGTM! Correctly returns Promise for async matchers.The change from
.js_undefinedtothis.returnMatcherValue(globalThis)ensures that async matchers (.resolves/.rejects) return a Promise, fixing the reported issue while maintaining backward compatibility for non-async cases.src/bun.js/test/expect/toBeString.zig (1)
12-12: LGTM! Consistent with the Promise-return pattern.The success path now correctly returns a Promise for async matchers via
returnMatcherValue.src/bun.js/test/expect/toStrictEqual.zig (1)
21-21: LGTM! Properly returns matcher value on success.The change aligns with the PR's goal of ensuring matchers return Promises for async cases.
src/bun.js/test/expect/toContainAnyValues.zig (1)
45-45: LGTM! Correctly usesglobalObjectparameter.The change properly returns
this.returnMatcherValue(globalObject)on success, using the parameter name that matches this function's signature.src/bun.js/test/expect/toBeCloseTo.zig (1)
46-46: LGTM! Both success paths correctly updated.Both the special case for infinities (Line 46) and the general pass case (Line 56) now correctly return the matcher value via
returnMatcherValue, ensuring Promise returns for async matchers.Also applies to: 56-56
src/bun.js/test/expect/toHaveBeenCalledOnce.zig (1)
22-22: LGTM! Mock function matcher correctly updated.The success path now properly returns the matcher value for Promise support in async contexts.
src/bun.js/test/expect/toContainAnyKeys.zig (1)
44-44: LGTM! Correctly returns matcher value on success.The change from returning the raw value to using
returnMatcherValue(globalThis)ensures proper Promise handling for async matchers.src/bun.js/test/expect/toBeEven.zig (1)
37-37: LGTM! Correct use of the new helper function.The change properly returns a Promise when async flags (.resolves/.rejects) are set, aligning with the PR objective to fix issue #23420.
src/bun.js/test/expect.zig (3)
1234-1242: LGTM! Well-implemented helper function.The implementation correctly:
- Returns a resolved Promise with undefined when async flags (.resolves/.rejects) are set
- Returns undefined for synchronous matchers
- Uses
resolvedPromiseValuewhich returns JSValue directly (no need for.toJS())- Is marked inline for performance
- Takes
*const Expectappropriately since it only reads flagsThis helper enables all matchers to properly return Promises for async scenarios, fixing issue #23420.
Based on learnings.
851-851: LGTM! Consistent application to snapshot matchers.Both snapshot success paths (existing match at line 851, new snapshot write at line 866) now correctly use
returnMatcherValueinstead of returning undefined directly. This ensures snapshot matchers also return Promises when used with .resolves/.rejects.Also applies to: 866-866
2259-2259: LGTM! Required import for Promise functionality.The JSPromise import is needed to support the new
returnMatcherValuehelper function.As per coding guidelines (imports at bottom of file).
src/bun.js/test/expect/toThrow.zig (1)
42-42: LGTM! Comprehensive and correct application to all success paths.All changes properly use
returnMatcherValueon matcher success paths across both negated and non-negated scenarios:Negated paths (expect().not.toThrow()):
- Line 42: Didn't throw → pass
- Line 79: String mismatch → pass
- Line 98: Regex mismatch → pass
- Line 115: Message mismatch → pass
- Line 120: Not instance of → pass
Normal paths (expect().toThrow()):
- Line 129: Threw with no expected value → pass
- Line 149: String match → pass
- Line 174: Regex match → pass
- Line 205: Asymmetric matcher match → pass
- Line 222: Message match → pass
- Line 240: Instance of match → pass
This ensures toThrow properly returns Promises when used with .resolves/.rejects, addressing the core issue in #23420.
Also applies to: 79-79, 98-98, 115-115, 120-120, 129-129, 149-149, 174-174, 205-205, 222-222, 240-240
| import { expect, test } from "bun:test"; | ||
|
|
||
| test("resolves matcher returns a Promise", () => { | ||
| const promise = Promise.resolve(42); | ||
| const matcherResult = expect(promise).resolves.toBe(42); | ||
| expect(matcherResult).toBeInstanceOf(Promise); | ||
| return matcherResult; | ||
| }); | ||
|
|
||
| test("rejects matcher returns a Promise", () => { | ||
| const promise = Promise.reject(new Error("test error")); | ||
| const matcherResult = expect(promise).rejects.toThrow("test error"); | ||
| expect(matcherResult).toBeInstanceOf(Promise); | ||
| return matcherResult; | ||
| }); | ||
|
|
||
| test("resolves.not matcher returns a Promise", () => { | ||
| const promise = Promise.resolve(42); | ||
| const matcherResult = expect(promise).resolves.not.toBe(100); | ||
| expect(matcherResult).toBeInstanceOf(Promise); | ||
| return matcherResult; | ||
| }); | ||
|
|
||
| test("rejects.not matcher returns a Promise", () => { | ||
| const promise = Promise.reject(42); | ||
| const matcherResult = expect(promise).rejects.not.toThrow("wrong error"); | ||
| expect(matcherResult).toBeInstanceOf(Promise); | ||
| return matcherResult; | ||
| }); | ||
|
|
||
| test("multiple resolves matchers can be chained with await", async () => { | ||
| const promise1 = Promise.resolve(1); | ||
| const promise2 = Promise.resolve(2); | ||
|
|
||
| await expect(promise1).resolves.toBe(1); | ||
| await expect(promise2).resolves.toBe(2); | ||
| }); | ||
|
|
||
| test("resolves.toEqual returns a Promise", async () => { | ||
| const promise = Promise.resolve({ foo: "bar" }); | ||
| const matcherResult = expect(promise).resolves.toEqual({ foo: "bar" }); | ||
| expect(matcherResult).toBeInstanceOf(Promise); | ||
| await matcherResult; | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Suggest expanding test coverage for comprehensive validation.
The current tests validate the core Promise-return behavior for .resolves and .rejects, which is good. However, consider adding:
- Non-async matcher tests: Verify that matchers without
.resolves/.rejectsstill return undefined (not Promises). - Failure case tests: Verify that failed matchers with
.resolves/.rejectsreturn rejected Promises. - Additional matcher coverage: Test other matchers that were updated (e.g.,
toBeString,toStrictEqual,toContainAnyKeys,toBeCloseTo).
Example additions:
test("non-async matcher returns undefined", () => {
const result = expect(42).toBe(42);
expect(result).toBeUndefined();
});
test("failed resolves matcher returns rejected Promise", async () => {
const promise = Promise.resolve(42);
const matcherResult = expect(promise).resolves.toBe(100);
expect(matcherResult).toBeInstanceOf(Promise);
await expect(async () => await matcherResult).toThrow();
});
test("resolves.toBeString returns a Promise", async () => {
const promise = Promise.resolve("hello");
const matcherResult = expect(promise).resolves.toBeString();
expect(matcherResult).toBeInstanceOf(Promise);
await matcherResult;
});🤖 Prompt for AI Agents
In test/regression/issue/23420.test.ts lines 1-44, expand the test file to add
coverage for non-async matcher behavior, failure cases of resolves/rejects, and
additional updated matchers: add a test asserting a non-async matcher (e.g.,
expect(42).toBe(42)) returns undefined, add tests that failing resolves/rejects
produce rejected Promises (e.g., expect(Promise.resolve(42)).resolves.toBe(100)
and assert the returned Promise rejects), and add short tests asserting other
updated matchers when used with resolves (e.g., resolves.toBeString,
resolves.toStrictEqual/toContainAnyKeys/toBeCloseTo) return a Promise and await
them; ensure each new test follows the existing style of checking
toBeInstanceOf(Promise) and awaiting or asserting rejection as appropriate.
|
The fastest PR in the wild west. I guess the generics in |
…ejects-return-promise
Updated the test expectations to verify that matchers under .resolves and .rejects now correctly return Promise instances instead of undefined. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
expect().resolvesandexpect().rejectsto return promises instead ofundefinedreturnMatcherValue()that checks async flags and returns appropriate valueTest plan
test/regression/issue/23420.test.ts.resolvesor.rejectsawaitand return valuesFixes #23420
🤖 Generated with Claude Code