node:test: run(), expectFailure, and Node v26.3.0 skip/todo semantics - #34444
Merged
Claude / Claude Code Review
completed
Jul 18, 2026 in 22m 8s
Code review found 1 important issue
Found 4 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/runtime/cli/test_command.rs:771-776 |
std::env::var_os is clippy-disallowed; use bun_core::env_var |
| 🟡 Nit | src/js/node/events.ts:769-772 |
events.addAbortListener return value lost proto: null after delegation |
Annotations
Check failure on line 776 in src/runtime/cli/test_command.rs
claude / Claude Code Review
std::env::var_os is clippy-disallowed; use bun_core::env_var
`should_drain_event_loop()` calls `std::env::var_os("BUN_TEST_DRAIN_EVENT_LOOP")`, which is in `clippy.toml`'s `disallowed-methods` list (reason: "use bun_core::env_var") and will fail `cargo clippy` in CI. robobun already flagged this and the reply was "dont ignore clippy issues", but the branch head still contains the disallowed call — declare `BUN_TEST_DRAIN_EVENT_LOOP` as a typed accessor in `bun_core::env_var` and call `.get()` here (which also makes the manual `OnceLock` redundant, since `
Check warning on line 772 in src/js/node/events.ts
claude / Claude Code Review
events.addAbortListener return value lost __proto__: null after delegation
Nit: the delegation drops `__proto__: null` from the returned disposable — the removed inline implementation returned `{ __proto__: null, [Symbol.dispose]() {...} }` (matching Node's `lib/internal/events/abort_listener.js`), but `internal/abort_listener.ts` returns `{ [Symbol.dispose]() {...} }`, so `Object.getPrototypeOf(events.addAbortListener(sig, fn))` changes from `null` to `Object.prototype`. One-line fix: add `__proto__: null` to the return object in `src/js/internal/abort_listener.ts` (w
Loading