test(abort): Add AbortSignal.any basic tests #25406
Conversation
WalkthroughAdds a new test suite for AbortSignal.any() at Changes
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)test/**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Files:
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}📄 CodeRabbit inference engine (test/CLAUDE.md)
Files:
test/**/*.test.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (20)📓 Common learnings📚 Learning: 2025-11-24T18:36:59.706ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-11-24T18:35:08.612ZApplied to files:
📚 Learning: 2025-11-24T18:35:08.612ZApplied to files:
📚 Learning: 2025-11-24T18:35:08.612ZApplied to files:
📚 Learning: 2025-11-24T18:35:08.612ZApplied to files:
📚 Learning: 2025-11-24T18:35:08.612ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-09-03T01:30:58.001ZApplied to files:
📚 Learning: 2025-09-20T00:57:56.685ZApplied to files:
📚 Learning: 2025-09-20T00:58:38.042ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-10-08T13:48:02.430ZApplied to files:
📚 Learning: 2025-10-18T05:23:24.403ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
🧬 Code graph analysis (1)test/js/web/abort/abort-signal-any.test.ts (1)
🔇 Additional comments (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (1)
test/js/web/abort/abort-signal-any.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
test/**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
test/**/*.{js,ts,jsx,tsx}: Write tests as JavaScript and TypeScript files using Jest-style APIs (test,describe,expect) and import frombun:test
Usetest.eachand data-driven tests to reduce boilerplate when testing multiple similar cases
Files:
test/js/web/abort/abort-signal-any.test.ts
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}
📄 CodeRabbit inference engine (test/CLAUDE.md)
test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Usebun:testwith files that end in*.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always useport: 0to get a random port
Prefer concurrent tests over sequential tests usingtest.concurrentordescribe.concurrentwhen multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, usebunExeandbunEnvfromharnessto ensure the same build of Bun is used and debug logging is silenced
Use-eflag for single-file tests when spawning Bun processes
UsetempDir()from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, usePromise.withResolversto create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
UseBuffer.alloc(count, fill).toString()instead of'A'.repeat(count)to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Usedescribeblocks for grouping related tests
Always useawait usingorusingto ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Usedescribe.each()for parameterized tests
UsetoMatchSnapshot()for snapshot testing
UsebeforeAll(),afterEach(),beforeEach()for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup inafterEach()
Files:
test/js/web/abort/abort-signal-any.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: For single-file tests in Bun test suite, prefer using-eflag overtempDir
For multi-file tests in Bun test suite, prefer usingtempDirandBun.spawn
Always useport: 0when spawning servers in tests - do not hardcode ports or use custom random port functions
UsenormalizeBunSnapshotto normalize snapshot output in tests instead of manual output comparison
Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
UsetempDirfromharnessto create temporary directories in tests - do not usetmpdirSyncorfs.mkdtempSync
In tests, callexpect(stdout).toBe(...)beforeexpect(exitCode).toBe(0)when spawning processes for more useful error messages on failure
Do not write flaky tests - do not usesetTimeoutin tests; insteadawaitthe condition to be met since you're testing the CONDITION, not TIME PASSING
Verify your test fails withUSE_SYSTEM_BUN=1 bun test <file>and passes withbun bd test <file>- tests are not valid if they pass withUSE_SYSTEM_BUN=1
Avoid shell commands in tests - do not usefindorgrep; use Bun's Glob and built-in tools instead
Test files must end in.test.tsor.test.tsxand be created in the appropriate test folder structure
Files:
test/js/web/abort/abort-signal-any.test.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-11-24T18:35:39.205Z
Learning: Add tests for new Bun runtime functionality
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer async/await over callbacks in tests
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Never write tests that check for no 'panic', 'uncaught exception', or similar strings in test output - that is not a valid test
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-11-24T18:36:59.706Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: See `test/harness.ts` for common test utilities and helpers
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-11-24T18:35:08.612Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:08.612Z
Learning: Applies to test/bake/**/*.test.ts : Explicitly annotate hard-reloads with `expectReload()` to tell the test framework that a page reload is expected; all other hard reloads automatically fail the test
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-09-03T01:30:58.001Z
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-11-24T18:35:50.422Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-11-24T18:35:50.422Z
Learning: Applies to test/**/*.{js,ts,jsx,tsx} : Write tests as JavaScript and TypeScript files using Jest-style APIs (`test`, `describe`, `expect`) and import from `bun:test`
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-12-02T05:59:51.485Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-02T05:59:51.485Z
Learning: Applies to test/**/*.test.{ts,tsx} : Verify your test fails with `USE_SYSTEM_BUN=1 bun test <file>` and passes with `bun bd test <file>` - tests are not valid if they pass with `USE_SYSTEM_BUN=1`
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-10-08T13:48:02.430Z
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 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/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-11-24T18:37:30.259Z
Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Unit tests for specific features are organized by module (e.g., `/test/js/bun/`, `/test/js/node/`)
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-10-18T05:23:24.403Z
Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
🧬 Code graph analysis (1)
test/js/web/abort/abort-signal-any.test.ts (1)
src/js/node/test.ts (1)
signal(63-68)
🔇 Additional comments (4)
test/js/web/abort/abort-signal-any.test.ts (4)
1-9: LGTM! Clean imports and documentation.The imports follow coding guidelines by using
bun:test, and the documentation clearly explains the purpose of AbortSignal.any().
10-45: LGTM! Solid basic functionality coverage.The tests correctly verify empty array handling, single signal composition, and multi-signal abort propagation. The
@ts-ignorecomments are appropriate since AbortSignal.any() may not yet be in TypeScript's type definitions.
47-85: LGTM! Comprehensive already-aborted signal handling.These tests correctly verify immediate abort behavior when inputs are pre-aborted, and properly validate reason propagation. The inline comment on line 82 helpfully documents the expected "first wins" behavior.
89-112: Excellent use of table-driven tests!The use of
test.each()with parameterized reason types follows coding guidelines perfectly and reduces boilerplate. The coverage of Error, string, object, and default DOMException reasons is comprehensive.
| describe("AbortSignal.any()", () => { | ||
| describe("basic functionality", () => { | ||
| test("should return a non-aborted signal for empty array", () => { | ||
| // @ts-ignore - TypeScript may not have this typed | ||
| const signal = AbortSignal.any([]); | ||
| expect(signal).toBeInstanceOf(AbortSignal); | ||
| expect(signal.aborted).toBe(false); | ||
| }); | ||
|
|
||
| test("should follow a single signal", () => { | ||
| const controller = new AbortController(); | ||
| // @ts-ignore | ||
| const composite = AbortSignal.any([controller.signal]); | ||
|
|
||
| expect(composite.aborted).toBe(false); | ||
| controller.abort(); | ||
| expect(composite.aborted).toBe(true); | ||
| }); | ||
|
|
||
| test("should abort when any of multiple signals abort", () => { | ||
| const controller1 = new AbortController(); | ||
| const controller2 = new AbortController(); | ||
| const controller3 = new AbortController(); | ||
|
|
||
| // @ts-ignore | ||
| const composite = AbortSignal.any([ | ||
| controller1.signal, | ||
| controller2.signal, | ||
| controller3.signal | ||
| ]); | ||
|
|
||
| expect(composite.aborted).toBe(false); | ||
| controller2.abort("middle signal aborted"); | ||
| expect(composite.aborted).toBe(true); | ||
| }); | ||
| }); | ||
|
|
||
| describe("already-aborted signals", () => { | ||
| test("should immediately abort if any input signal is already aborted", () => { | ||
| const abortedController = new AbortController(); | ||
| abortedController.abort("pre-aborted"); | ||
|
|
||
| const freshController = new AbortController(); | ||
|
|
||
| // @ts-ignore | ||
| const composite = AbortSignal.any([ | ||
| freshController.signal, | ||
| abortedController.signal | ||
| ]); | ||
|
|
||
| expect(composite.aborted).toBe(true); | ||
| }); | ||
|
|
||
| test("should use AbortSignal.abort() result correctly", () => { | ||
| const alreadyAborted = AbortSignal.abort("already aborted reason"); | ||
| const controller = new AbortController(); | ||
|
|
||
| // @ts-ignore | ||
| const composite = AbortSignal.any([controller.signal, alreadyAborted]); | ||
|
|
||
| expect(composite.aborted).toBe(true); | ||
| expect(composite.reason).toBe("already aborted reason"); | ||
| }); | ||
|
|
||
| test("should work with all signals already aborted", () => { | ||
| const aborted1 = AbortSignal.abort("first"); | ||
| const aborted2 = AbortSignal.abort("second"); | ||
|
|
||
| // @ts-ignore | ||
| const composite = AbortSignal.any([aborted1, aborted2]); | ||
|
|
||
| expect(composite.aborted).toBe(true); | ||
| // First aborted signal's reason should be used | ||
| expect(composite.reason).toBe("first"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("reason propagation", () => { | ||
| // Table-driven tests for different reason types per coderabbitai suggestion | ||
| const reasonCases = [ | ||
| { name: "Error", reason: new Error("custom abort reason") }, | ||
| { name: "string", reason: "string reason" }, | ||
| { name: "object", reason: { code: 42, message: "custom object" } }, | ||
| ]; | ||
|
|
||
| test.each(reasonCases)("should propagate $name reasons", ({ reason }) => { | ||
| const controller = new AbortController(); | ||
| // @ts-ignore | ||
| const composite = AbortSignal.any([controller.signal]); | ||
| controller.abort(reason); | ||
| expect(composite.reason).toBe(reason); | ||
| }); | ||
|
|
||
| test("should use DOMException for default abort reason", () => { | ||
| const controller = new AbortController(); | ||
| // @ts-ignore | ||
| const composite = AbortSignal.any([controller.signal]); | ||
|
|
||
| controller.abort(); | ||
|
|
||
| expect(composite.reason).toBeInstanceOf(DOMException); | ||
| expect(composite.reason.name).toBe("AbortError"); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding abort event listener tests.
While the current tests comprehensively cover basic functionality, consider adding tests that verify abort event listeners fire correctly on the composite signal when any input aborts. Since this PR focuses on "basic functionality," this is optional and may already be covered in the parent PR #25341.
Example test:
test("should fire abort event when any signal aborts", async () => {
const controller = new AbortController();
const composite = AbortSignal.any([controller.signal]);
const { promise, resolve } = Promise.withResolvers<void>();
composite.addEventListener("abort", () => resolve());
controller.abort();
await promise; // Wait for event to fire
expect(composite.aborted).toBe(true);
});🤖 Prompt for AI Agents
In test/js/web/abort/abort-signal-any.test.ts around lines 10 to 113, add a test
that verifies the composite signal from AbortSignal.any fires the "abort" event
when any input signal aborts: create an AbortController, build the composite
with AbortSignal.any([controller.signal]), attach an "abort" event listener that
resolves a promise (or uses Promise.withResolvers if available), call
controller.abort(), await the promise to ensure the event fired, and then assert
composite.aborted is true (and optionally check composite.reason) to ensure
event propagation works.
Address CodeRabbit review comment by removing 'per coderabbitai suggestion' from the comment. Comments should focus on code intent rather than review provenance.
1 participant
Splitting #25341: Basic functionality.