bun:test: defer the AsyncContextFrame wrap past .length/.bind() for tests and hooks - #35334
Closed
robobun wants to merge 3 commits into
Closed
bun:test: defer the AsyncContextFrame wrap past .length/.bind() for tests and hooks#35334robobun wants to merge 3 commits into
robobun wants to merge 3 commits into
Claude / Claude Code Review
completed
Jul 24, 2026 in 38m 18s
Code review found 2 potential issues
Found 4 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 1 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | test/js/bun/test/als-hook-arity.test.ts:32 |
Vacuous assertion: 'timed out' checked on stdout instead of stderr |
| 🟣 Pre-existing | src/runtime/test_runner/ScopeFunctions.rs:668-676 |
test.each()/describe.each() inside ALS still broken: bind() throws on non-callable AsyncContextFrame |
Annotations
Check warning on line 32 in test/js/bun/test/als-hook-arity.test.ts
claude / Claude Code Review
Vacuous assertion: 'timed out' checked on stdout instead of stderr
This assertion is vacuously true: `bun test` writes timeout messages to **stderr**, not stdout, and the fixture writes nothing to stdout — so `expect(stdout).not.toContain("timed out")` passes on both the fixed and unfixed builds. Either change it to `expect(stderr).not.toContain("timed out")` or just drop the line, since the stderr snapshot above already proves no timeout occurred.
Check notice on line 676 in src/runtime/test_runner/ScopeFunctions.rs
claude / Claude Code Review
test.each()/describe.each() inside ALS still broken: bind() throws on non-callable AsyncContextFrame
Pre-existing, same bug class: `test.each()` / `describe.each()` registered inside `als.run()` still fails at collection time — `args.callback` is the non-callable `AsyncContextFrame` wrapper, and the `.each` path passes it to `JSValueTestExt::bind()`, which throws `TypeError: bind() called on non-callable`. Not introduced by this PR, but it's the sibling site of the arity fix (and the new `callback_length.saturating_sub(...)` on line 245 is unreachable because `bind()` throws first); consider al
Loading