test: add comprehensive AbortSignal.any() test coverage #25341
Conversation
This adds a comprehensive test suite for AbortSignal.any() covering: - Basic functionality (empty array, single signal, multiple signals) - Already-aborted signals in the input array - Reason propagation (custom errors, strings, objects, DOMExceptions) - Event handling (fires once, even with multiple source aborts) - Nested AbortSignal.any() calls - Integration with AbortSignal.timeout() These tests ensure standards-compliant behavior and help catch regressions in the composite signal implementation.
Abort event fires synchronously when controller.abort() is called, so no fallback timeout is needed.
WalkthroughAdds a new test file that implements a comprehensive test suite for Changes
Pre-merge checks✅ Passed checks (2 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 (23)📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied 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:37:30.259ZApplied to files:
📚 Learning: 2025-11-24T18:37:30.259ZApplied to files:
📚 Learning: 2025-09-03T01:30:58.001ZApplied to files:
📚 Learning: 2025-09-20T00:58:38.042ZApplied to files:
📚 Learning: 2025-11-06T00:58:23.965ZApplied to files:
📚 Learning: 2025-10-08T13:48:02.430ZApplied to files:
📚 Learning: 2025-09-20T00:57:56.685ZApplied to files:
📚 Learning: 2025-11-08T04:06:33.198ZApplied to files:
📚 Learning: 2025-11-24T18:35:50.422ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-12-02T05:59:51.485ZApplied to files:
📚 Learning: 2025-10-18T05:23:24.403ZApplied to files:
🔇 Additional comments (5)
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: 4
📜 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 (16)
📚 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-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-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: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} : 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
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/dev/html.test.ts : Organize HTML tests in html.test.ts for tests relating to HTML files themselves
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} : Do not write flaky tests - do not use `setTimeout` in tests; instead `await` the condition to be met since you're testing the CONDITION, not TIME PASSING
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: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/dev/esm.test.ts : Organize ESM tests in esm.test.ts for tests about various ESM features in development mode
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-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-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-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} : For multi-file tests in Bun test suite, prefer using `tempDir` and `Bun.spawn`
Applied to files:
test/js/web/abort/abort-signal-any.test.ts
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
Repo: oven-sh/bun PR: 22534
File: test/regression/issue/21830.fixture.ts:14-63
Timestamp: 2025-09-20T03:39:41.770Z
Learning: Bun's test runner supports async describe callbacks, unlike Jest/Vitest where describe callbacks must be synchronous. The syntax `describe("name", async () => { ... })` is valid in Bun.
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
🔇 Additional comments (5)
test/js/web/abort/abort-signal-any.test.ts (5)
11-45: Basic AbortSignal.any() behavior is well-coveredEmpty, single-signal, and multi-signal cases map cleanly to the standard semantics and look correct as written. No changes needed here.
47-85: Already-aborted input coverage looks correctThe tests around already-aborted signals (including
AbortSignal.abort()and “all aborted” ordering of reasons) match the expected behavior and give good regression coverage.
173-192: Nested AbortSignal.any() behavior is covered wellThe nested
AbortSignal.any()test nicely exercises propagation ofabortedandreasonthrough composed signals; this is a good regression target for implementation details.
211-222: Manual-abort vs timeout precedence test looks goodThe precedence test correctly ensures a manual abort wins over
AbortSignal.timeout()when it happens first, and the reason is preserved as the manual reason. This is a valuable edge case to lock in.
1-224: Validate new tests against system Bun vs bd buildPer the repo’s testing guidelines for
test/**/*.test.{ts,tsx}, please remember to manually verify that this new file:
- Fails with:
USE_SYSTEM_BUN=1 bun test test/js/web/abort/abort-signal-any.test.ts- Passes with:
bun bd test test/js/web/abort/abort-signal-any.test.tsThis confirms the coverage is exercising behavior that differs between the system Bun and the bd build.
Based on learnings, this check is required for new Bun tests.
| describe("reason propagation", () => { | ||
| test("should propagate the reason from the aborting signal", () => { | ||
| const controller1 = new AbortController(); | ||
| const controller2 = new AbortController(); | ||
|
|
||
| // @ts-ignore | ||
| const composite = AbortSignal.any([controller1.signal, controller2.signal]); | ||
|
|
||
| const customReason = new Error("custom abort reason"); | ||
| controller1.abort(customReason); | ||
|
|
||
| expect(composite.reason).toBe(customReason); | ||
| }); | ||
|
|
||
| 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"); | ||
| }); | ||
|
|
||
| test("should propagate string reasons", () => { | ||
| const controller = new AbortController(); | ||
| // @ts-ignore | ||
| const composite = AbortSignal.any([controller.signal]); | ||
|
|
||
| controller.abort("string reason"); | ||
|
|
||
| expect(composite.reason).toBe("string reason"); | ||
| }); | ||
|
|
||
| test("should propagate object reasons", () => { | ||
| const controller = new AbortController(); | ||
| // @ts-ignore | ||
| const composite = AbortSignal.any([controller.signal]); | ||
|
|
||
| const objectReason = { code: 42, message: "custom object" }; | ||
| controller.abort(objectReason); | ||
|
|
||
| expect(composite.reason).toBe(objectReason); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: consolidate reason propagation tests with test.each
The string, object, and custom-error reason tests all follow the same structure. You could reduce boilerplate and make it easier to add more cases later by using a table-driven test.each:
const cases = [
{ name: "Error", reason: new Error("custom abort reason") },
{ name: "string", reason: "string reason" },
{ name: "object", reason: { code: 42, message: "custom object" } },
];
test.each(cases)("should propagate %s reasons", ({ reason }) => {
const controller = new AbortController();
// @ts-ignore
const composite = AbortSignal.any([controller.signal]);
controller.abort(reason);
expect(composite.reason).toBe(reason);
});Purely stylistic; the current tests are fine functionally.
As per coding guidelines, using test.each for similar cases is encouraged.
🤖 Prompt for AI Agents
test/js/web/abort/abort-signal-any.test.ts lines 87-132: the three separate
tests that verify propagation of different abort reason types (Error, string,
object) are repetitive; replace them with a table-driven test using test.each
that enumerates the reason cases and runs the same setup/assertion for each case
(create AbortController, build composite via AbortSignal.any, call
controller.abort(reason), expect composite.reason toBe reason) so the test is
consolidated and easier to extend.
Per coderabbitai nitpick, consolidate the repetitive reason propagation tests (Error, string, object) into a single table-driven test using test.each for reduced boilerplate and easier extensibility.
1 participant
Summary
Adds comprehensive test coverage for
AbortSignal.any()- the web standard API that creates composite abort signals.What's Covered
AbortSignal.any()calls work correctlyAbortSignal.timeout()Why