Concurrent limit --max-concurrency, defaults to 20
#22944
Conversation
|
Updated 4:41 PM PT - Sep 26th, 2025
❌ @pfgithub, your commit 5d215a0 has 5 failures in
🧪 To try this PR locally: bunx bun-pr 22944That installs a local version of the PR into your bun-22944 --bun |
WalkthroughAdd a max-concurrency CLI option and wiring, expose TestRunner.max_concurrency, refactor test execution to be group-centric by adding ConcurrentGroup.next_sequence_index and changing sequence-starting flow to respect reporter-configured concurrency limits, and add tests/fixture exercising max-concurrency behavior. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 3
🧹 Nitpick comments (9)
src/bun.js/test/Execution.zig (4)
47-49: Clarify/alignnext_sequence_indexsemantics.Doc says “next sequence that has not been started yet”, but it’s used as a scan cursor that may pass over executing sequences. Consider clarifying the comment (or rename to
next_scan_index) and ensure it’s reset on group start/creation (currently set ininit, which is fine).Is there any path where a
ConcurrentGroupis reused across files/runs without reinit? If so, resettingnext_sequence_indexinonGroupStarted()would be safer.Also applies to: 61-61
254-266: Guard against over-subscribing when backfilling after a completion.This loop unconditionally starts one more sequence. It should respect
max_concurrencyto avoid accidental overfill if invariants change. Recommend checking current executing count before starting a new sequence.Apply this diff:
- // this sequence is complete; execute the next sequence - while (group.next_sequence_index < group.sequences(this).len) : (group.next_sequence_index += 1) { + // this sequence is complete; execute the next sequence (respecting concurrency cap) + const concurrent_limit: usize = if (buntest.reporter) |reporter| reporter.jest.max_concurrency else 20; + var executing_count: usize = 0; + for (group.sequences(this)) |*seq| { + if (seq.executing) executing_count += 1; + } + while (group.next_sequence_index < group.sequences(this).len) : (group.next_sequence_index += 1) { + if (concurrent_limit != 0 and executing_count >= concurrent_limit) break; const target_sequence = &group.sequences(this)[group.next_sequence_index]; if (target_sequence.executing) continue; const sequence_status = try stepSequence(buntest_strong, globalThis, group, group.next_sequence_index, now); switch (sequence_status) { .done => continue, .execute => |exec| { + executing_count += 1; return .{ .waiting = .{ .timeout = exec.timeout } }; }, } }
323-335: Minor: avoid repeated slice calls in hot path.Cache
group.sequences(this)once before the loop to reduce repeated slicing.Apply this diff:
- for (0..group.sequences(this).len) |sequence_index| { - const sequence_status = try stepSequence(buntest_strong, globalThis, group, sequence_index, now); + const sequences = group.sequences(this); + for (0..sequences.len) |sequence_index| { + const sequence_status = try stepSequence(buntest_strong, globalThis, group, sequence_index, now);
346-350: Group-centric sequencing refactor LGTM.
stepSequence/stepSequenceOnenow operate via thegroupto resolve sequences. The use of&group.sequences(this)[sequence_index]is sound.You can also cache
const sequences = group.sequences(this);locally instepSequenceOne()before indexing.Also applies to: 352-359
src/bun.js/test/jest.zig (1)
61-62: Set a safe default for max_concurrency at the struct level.Currently
max_concurrencyhas no default initializer. If any code constructsTestRunner{}without setting this field, it will be uninitialized. Since CLI defaults to 20, mirror that default here to avoid UB in other construction paths.Apply this diff:
- max_concurrency: u32, + max_concurrency: u32 = 20,src/cli/Arguments.zig (1)
204-205: Clarify help text: document 0 as “unlimited”.Tests rely on
--maxConcurrency 0meaning “no cap”. Reflect that in the flag description to avoid confusion.Apply this diff:
- clap.parseParam("--maxConcurrency <NUMBER> Maximum number of concurrent tests to execute at once. Default is 20.") catch unreachable, + clap.parseParam("--maxConcurrency <NUMBER> Maximum number of concurrent tests to execute at once (0 = unlimited). Default is 20.") catch unreachable,test/js/bun/test/concurrent.test.ts (3)
61-82: Dispose spawned process with await using.Use
await usingfor Bun resources to ensure cleanup per test guidelines. Also,awaitonBun.spawn()is unnecessary.Apply this diff:
-test("maxConcurrency limits concurrent tests", async () => { - // Test with maxConcurrency=3 - const result = await Bun.spawn({ +test("maxConcurrency limits concurrent tests", async () => { + // Test with maxConcurrency=3 + await using result = Bun.spawn({ cmd: [bunExe(), "test", "--maxConcurrency", "3", import.meta.dir + "/concurrent-max.fixture.ts"], stdout: "pipe", stderr: "pipe", env: bunEnv, }); const exitCode = await result.exited; const stdout = await result.stdout.text();
84-104: Dispose spawned process with await using (repeat).Same suggestion as above to ensure cleanup.
Apply this diff:
-test("maxConcurrency default is 20", async () => { - const result = await Bun.spawn({ +test("maxConcurrency default is 20", async () => { + await using result = Bun.spawn({ cmd: [bunExe(), "test", import.meta.dir + "/concurrent-max.fixture.ts"], stdout: "pipe", stderr: "pipe", env: bunEnv, });
106-126: Dispose spawned process with await using and fix comment.Use
await usingand correct the comment to reflect unlimited behavior.Apply this diff:
-test("zero removes maxConcurrency", async () => { - const result = await Bun.spawn({ +test("zero removes maxConcurrency", async () => { + await using result = Bun.spawn({ cmd: [bunExe(), "test", "--maxConcurrency", "0", import.meta.dir + "/concurrent-max.fixture.ts"], stdout: "pipe", stderr: "pipe", env: bunEnv, }); @@ - // Should be 1,2,3,...,18,19,20,20,20,20,20,20,... + // Should be 1,2,3,4,5,... (no cap) const expected = Array.from({ length: 100 }, (_, i) => i + 1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 (8)
src/bun.js/test/Execution.zig(5 hunks)src/bun.js/test/bun_test.zig(1 hunks)src/bun.js/test/jest.zig(1 hunks)src/cli.zig(1 hunks)src/cli/Arguments.zig(2 hunks)src/cli/test_command.zig(1 hunks)test/js/bun/test/concurrent-max.fixture.ts(1 hunks)test/js/bun/test/concurrent.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/bun.js/test/jest.zigsrc/bun.js/test/Execution.zigsrc/bun.js/test/bun_test.zigsrc/cli/Arguments.zigsrc/cli.zigsrc/cli/test_command.zig
**/*.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
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/bun.js/test/jest.zigsrc/bun.js/test/Execution.zigsrc/bun.js/test/bun_test.zigsrc/cli/Arguments.zigsrc/cli.zigsrc/cli/test_command.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/jest.zigsrc/bun.js/test/Execution.zigsrc/bun.js/test/bun_test.zig
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
test/js/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place JavaScript and TypeScript tests under test/js/
Files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
test/js/bun/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.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/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.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/js/bun/test/concurrent.test.ts
test/js/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests for specific features under
test/js/by module
Files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions
Files:
test/js/bun/test/concurrent.test.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
🧠 Learnings (17)
📚 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 : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.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/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
📚 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 : Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.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/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.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/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 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 : Name test files `*.test.ts` and use `bun:test`
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
📚 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 : Use `using`/`await using` with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
📚 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 : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)
Applied to files:
test/js/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
PR: oven-sh/bun#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/bun/test/concurrent.test.tstest/js/bun/test/concurrent-max.fixture.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 Learning: 2025-09-24T05:48:59.851Z
Learnt from: nektro
PR: oven-sh/bun#22806
File: scripts/runner.node.mjs:687-689
Timestamp: 2025-09-24T05:48:59.851Z
Learning: In the Bun codebase, the `startGroup` utility function in scripts/runner.node.mjs automatically closes any previously open group when called, so `startGroup("End")` correctly closes the final test group and keeps subsequent output ungrouped.
Applied to files:
src/bun.js/test/Execution.zig
📚 Learning: 2025-09-02T18:29:58.304Z
Learnt from: taylordotfish
PR: oven-sh/bun#22227
File: src/memory.zig:37-43
Timestamp: 2025-09-02T18:29:58.304Z
Learning: In bun's `src/memory.zig`, the `initDefault` function intentionally calls `T.init()` without parameter validation to cause compile-time errors when `init` requires arguments or returns an error union. This design forces developers to add explicit `initDefault` methods for complex types rather than silently falling back to `{}`. The `{}` fallback is only intended for types that have no `init` function at all.
Applied to files:
src/bun.js/test/bun_test.zig
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/client-fixture.mjs : Client subprocess logic (page load, IPC, error/reload coordination) should live in client-fixture.mjs and be driven by Client
Applied to files:
test/js/bun/test/concurrent-max.fixture.ts
📚 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/**/*-fixture.ts : Name test fixture files that are spawned by tests with the suffix `-fixture.ts`
Applied to files:
test/js/bun/test/concurrent-max.fixture.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} : Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Applied to files:
test/js/bun/test/concurrent-max.fixture.ts
🧬 Code graph analysis (1)
test/js/bun/test/concurrent-max.fixture.ts (1)
src/js/node/test.ts (2)
beforeEach(151-160)afterEach(162-171)
⏰ 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 (5)
src/bun.js/test/Execution.zig (1)
249-253: Signature change usage looks correct.
stepSequencenow takes(group, sequence_index, now); call sites updated accordingly.src/bun.js/test/bun_test.zig (1)
117-125: Confirm all call sites now pass default_concurrent explicitly.Field
default_concurrentno longer has a default;enterFile()now requires it. I seetest_command.zigpassesshould_run_concurrent, which is good. Please verify there are no otherBunTestRoot.enterFileinvocations orBunTestinitializations that omit this argument.If helpful, I can generate a repo scan to confirm there are no missed call sites.
Also applies to: 160-193
src/cli.zig (1)
347-348: LGTM on default surface.Adding
max_concurrency: u32 = 20to TestOptions is clear and consistent with PR intent.src/cli/Arguments.zig (1)
416-423: Validation path looks good.u32 parse with explicit error is fine; negative values will error out as expected. No changes requested.
Consider rejecting extremely large values with a friendlier message if needed (optional).
src/cli/test_command.zig (1)
1308-1309: Wiring looks correct.
TestRunner.max_concurrency = ctx.test_options.max_concurrencyis correctly plumbed.
| /// null if the runner has moved on to the next file but a strong reference to BunTest is stll keeping it alive | ||
| reporter: ?*test_command.CommandLineReporter, |
There was a problem hiding this comment.
Fix typo in doc comment.
"stll" -> "still".
Apply this diff:
- /// null if the runner has moved on to the next file but a strong reference to BunTest is stll keeping it alive
+ /// null if the runner has moved on to the next file but a strong reference to BunTest is still keeping it alive📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /// null if the runner has moved on to the next file but a strong reference to BunTest is stll keeping it alive | |
| reporter: ?*test_command.CommandLineReporter, | |
| /// null if the runner has moved on to the next file but a strong reference to BunTest is still keeping it alive | |
| reporter: ?*test_command.CommandLineReporter, |
🤖 Prompt for AI Agents
In src/bun.js/test/bun_test.zig around lines 155 to 156, the doc comment
contains a typo "stll" — replace it with "still" so the comment reads "still
keeping it alive"; update the comment text only, preserving punctuation and
surrounding whitespace.
| const concurrent_limit = if (buntest.reporter) |reporter| reporter.jest.max_concurrency else blk: { | ||
| bun.assert(false); // probably can't get here because reporter is only set null when the file is exited | ||
| break :blk 20; | ||
| }; |
There was a problem hiding this comment.
Don’t assert on null reporter; use a safe default.
bun.assert(false) will crash in debug builds. Fall back to the default (20) without asserting.
Apply this diff:
- const concurrent_limit = if (buntest.reporter) |reporter| reporter.jest.max_concurrency else blk: {
- bun.assert(false); // probably can't get here because reporter is only set null when the file is exited
- break :blk 20;
- };
+ const concurrent_limit: usize = if (buntest.reporter) |reporter| reporter.jest.max_concurrency else 20;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const concurrent_limit = if (buntest.reporter) |reporter| reporter.jest.max_concurrency else blk: { | |
| bun.assert(false); // probably can't get here because reporter is only set null when the file is exited | |
| break :blk 20; | |
| }; | |
| const concurrent_limit: usize = if (buntest.reporter) |reporter| reporter.jest.max_concurrency else 20; |
🤖 Prompt for AI Agents
In src/bun.js/test/Execution.zig around lines 318 to 321, the current code
asserts on a null reporter with bun.assert(false) which will crash in debug
builds; replace the assert branch with a safe fallback so when reporter is null
the expression returns the default value 20 instead. Modify the if/else block so
the else path simply yields 20 (no assert or break needed) and ensure the
overall expression still assigns to concurrent_limit.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/bun.js/test/jest.zig (1)
62-62: Document max_concurrency semantics and verify all initializations set it.Add a brief comment (e.g., “0 = unlimited”) next to the field, and confirm every TestRunner literal assigns it (besides TestCommand).
- max_concurrency: u32, + /// Maximum concurrent test sequences per group (0 = unlimited) + max_concurrency: u32,src/cli/Arguments.zig (1)
208-209: Clarify help text: 0 means unlimited.Make the CLI description explicit to match runtime behavior.
- clap.parseParam("--maxConcurrency <NUMBER> Maximum number of concurrent tests to execute at once. Default is 20.") catch unreachable, + clap.parseParam("--maxConcurrency <NUMBER> Maximum number of concurrent tests to execute at once (0 = unlimited). Default is 20.") catch unreachable,src/bun.js/test/Execution.zig (1)
352-359: Minor readability nit (optional).Cache sequences slice once to avoid repeated calls in tight paths.
- const sequence = &group.sequences(this)[sequence_index]; + const seqs = group.sequences(this); + const sequence = &seqs[sequence_index];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 (6)
src/bun.js/test/Execution.zig(5 hunks)src/bun.js/test/bun_test.zig(1 hunks)src/bun.js/test/jest.zig(1 hunks)src/cli.zig(1 hunks)src/cli/Arguments.zig(2 hunks)src/cli/test_command.zig(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/bun.js/test/jest.zigsrc/cli/test_command.zigsrc/bun.js/test/Execution.zigsrc/cli.zigsrc/bun.js/test/bun_test.zigsrc/cli/Arguments.zig
**/*.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
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/bun.js/test/jest.zigsrc/cli/test_command.zigsrc/bun.js/test/Execution.zigsrc/cli.zigsrc/bun.js/test/bun_test.zigsrc/cli/Arguments.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/jest.zigsrc/bun.js/test/Execution.zigsrc/bun.js/test/bun_test.zig
🧠 Learnings (1)
📚 Learning: 2025-09-24T05:48:59.851Z
Learnt from: nektro
PR: oven-sh/bun#22806
File: scripts/runner.node.mjs:687-689
Timestamp: 2025-09-24T05:48:59.851Z
Learning: In the Bun codebase, the `startGroup` utility function in scripts/runner.node.mjs automatically closes any previously open group when called, so `startGroup("End")` correctly closes the final test group and keeps subsequent output ungrouped.
Applied to files:
src/bun.js/test/Execution.zig
🔇 Additional comments (10)
src/bun.js/test/bun_test.zig (2)
155-156: Fix typo in doc comment."stll" -> "still".
- /// null if the runner has moved on to the next file but a strong reference to BunTest is stll keeping it alive + /// null if the runner has moved on to the next file but a strong reference to BunTest is still keeping it alive
160-160: LGTM: explicit initializationRemoving the default initializer on default_concurrent makes construction explicit and avoids hidden defaults.
src/cli.zig (1)
348-349: LGTM: default set to 20Public TestOptions gains max_concurrency with sensible default.
src/cli/Arguments.zig (1)
420-427: LGTM: parsing and validationParses unsigned int, errors on invalid input, and leaves default when omitted.
src/cli/test_command.zig (1)
1309-1310: Wiring looks correctTestRunner.max_concurrency is initialized from parsed test options.
src/bun.js/test/Execution.zig (5)
47-49: Track next_sequence_index: verify extension path.Initialization is fine. If tryExtend() can be called after some sequences have started, consider whether next_sequence_index needs adjusting; otherwise, this is safe when only used pre-execution.
Also applies to: 61-62
249-265: Start one new sequence after completion (keeps the cap steady).This loop correctly finds the next non-executing sequence and starts at most one, preserving the concurrency limit established earlier.
323-333: LGTM: cap enforcement logicactive_count + early break enforces the configured limit; zero means unbounded due to the guard.
346-349: Signature/loop refactor looks goodstepSequence delegates via stepSequenceOne with the new parameters and continues until a stable state.
318-323: Do not assert on null reporter; fix integer type mismatch.
- Asserting here can crash debug builds; default to 20 safely.
- concurrent_limit (u32) compared with active_count (usize) will cause a type mismatch. Cast to usize.
- const concurrent_limit = if (buntest.reporter) |reporter| reporter.jest.max_concurrency else blk: { - bun.assert(false); // probably can't get here because reporter is only set null when the file is exited - break :blk 20; - }; + const concurrent_limit: usize = @intCast(if (buntest.reporter) |reporter| reporter.jest.max_concurrency else 20);
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/cli/Arguments.zig (1)
421-429: Validate non-zero to avoid deadlock.If a user passes 0, scheduling may stall. Enforce >= 1 at parse time.
Apply:
- if (args.option("--maxConcurrency")) |max_concurrency| { - if (max_concurrency.len > 0) { - ctx.test_options.max_concurrency = std.fmt.parseInt(u32, max_concurrency, 10) catch { - Output.prettyErrorln("<r><red>error<r>: Invalid maxConcurrency: \"{s}\"", .{max_concurrency}); - Global.exit(1); - }; - } - } + if (args.option("--maxConcurrency")) |max_concurrency| { + if (max_concurrency.len > 0) { + ctx.test_options.max_concurrency = std.fmt.parseInt(u32, max_concurrency, 10) catch { + Output.prettyErrorln("<r><red>error<r>: Invalid maxConcurrency: \"{s}\"", .{max_concurrency}); + Global.exit(1); + }; + if (ctx.test_options.max_concurrency == 0) { + Output.prettyErrorln("<r><red>error<r>: --maxConcurrency must be >= 1", .{}); + Global.exit(1); + } + } + }Also confirm whether downstream execution clamps or relies on this being >= 1.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 (5)
src/bun.js/test/bun_test.zig(1 hunks)src/bun.js/test/jest.zig(1 hunks)src/cli.zig(1 hunks)src/cli/Arguments.zig(2 hunks)src/cli/test_command.zig(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/bun.js/test/jest.zig
- src/cli.zig
🧰 Additional context used
📓 Path-based instructions (3)
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/cli/Arguments.zigsrc/bun.js/test/bun_test.zigsrc/cli/test_command.zig
**/*.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
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/cli/Arguments.zigsrc/bun.js/test/bun_test.zigsrc/cli/test_command.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/bun_test.zig
🧠 Learnings (1)
📚 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/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Applied to files:
src/bun.js/test/bun_test.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 (4)
src/bun.js/test/bun_test.zig (2)
155-156: Fix typo in doc comment."stll" -> "still".
Apply this diff:
- /// null if the runner has moved on to the next file but a strong reference to BunTest is stll keeping it alive + /// null if the runner has moved on to the next file but a strong reference to BunTest is still keeping it alive
160-160: LGTM: field default removal is safe.
default_concurrentis now set explicitly in init; removal of the default initializer is fine.src/cli/Arguments.zig (1)
209-209: Add option: looks good.The flag name and help text are clear and consistent.
src/cli/test_command.zig (1)
1315-1315: max_concurrency wiring is correct Verified thatTestRunnerdeclaresmax_concurrency: u32andTestOptionsdefaults it to 20.
pfgithub
changed the title
--maxConcurrency, defaults to 20--max-concurrency, defaults to 20
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
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 (4)
docs/cli/test.md(1 hunks)src/cli.zig(1 hunks)src/cli/Arguments.zig(2 hunks)test/js/bun/test/concurrent.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cli/Arguments.zig
🧰 Additional context used
📓 Path-based instructions (10)
test/**
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place all tests under the test/ directory
Files:
test/js/bun/test/concurrent.test.ts
test/js/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place JavaScript and TypeScript tests under test/js/
Files:
test/js/bun/test/concurrent.test.ts
test/js/bun/**/*.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)
Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Files:
test/js/bun/test/concurrent.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/js/bun/test/concurrent.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/js/bun/test/concurrent.test.ts
test/js/**
📄 CodeRabbit inference engine (test/CLAUDE.md)
Organize unit tests for specific features under
test/js/by module
Files:
test/js/bun/test/concurrent.test.ts
test/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
test/**/*.test.{ts,tsx}: Test files must be placed under test/ and end with .test.ts or .test.tsx
In tests, always use port: 0; do not hardcode ports or use custom random port functions
In tests, use normalizeBunSnapshot when asserting snapshots
Never write tests that merely assert absence of "panic" or "uncaught exception" in output
Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Prefer snapshot tests over exact stdout equality assertions
Files:
test/js/bun/test/concurrent.test.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Format JavaScript/TypeScript files with Prettier (bun run prettier)
Files:
test/js/bun/test/concurrent.test.ts
src/**/*.zig
📄 CodeRabbit inference engine (.cursor/rules/building-bun.mdc)
Implement debug logs in Zig using
const log = bun.Output.scoped(.${SCOPE}, false);and invokinglog("...", .{})
Files:
src/cli.zig
**/*.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
**/*.zig: Format Zig files with zig-format (bun run zig-format)
In Zig, manage memory carefully with allocators and use defer for cleanup
Files:
src/cli.zig
🧠 Learnings (14)
📚 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 : When spawning Bun in tests, use `bunExe()` and `bunEnv` from `harness`
Applied to files:
test/js/bun/test/concurrent.test.tsdocs/cli/test.md
📚 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/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes
Applied to files:
test/js/bun/test/concurrent.test.tsdocs/cli/test.md
📚 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 : Do not set explicit test timeouts; rely on Bun’s built-in timeouts
Applied to files:
test/js/bun/test/concurrent.test.tsdocs/cli/test.md
📚 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/js/bun/test/concurrent.test.tsdocs/cli/test.md
📚 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/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 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 : Name test files `*.test.ts` and use `bun:test`
Applied to files:
test/js/bun/test/concurrent.test.tsdocs/cli/test.md
📚 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 : Use `using`/`await using` with Bun resources (e.g., Bun.listen/connect/spawn/serve) to ensure cleanup in tests
Applied to files:
test/js/bun/test/concurrent.test.tsdocs/cli/test.md
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 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 : Import common test utilities from `harness` (e.g., `bunExe`, `bunEnv`, `tempDirWithFiles`, `tmpdirSync`, platform checks, GC helpers)
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 Learning: 2025-08-30T00:09:39.100Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/bake-harness.ts : Provide and maintain shared test utilities: devTest, prodTest, devAndProductionTest, Dev, Client, and helpers in the harness
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 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 : In error tests, assert non-zero exit codes for failing processes and use `toThrow` for synchronous errors
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 Learning: 2025-09-08T04:44:59.101Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T04:44:59.101Z
Learning: Applies to test/**/*.test.{ts,tsx} : Avoid shell commands (e.g., find, grep) in tests; use Bun.Glob and built-ins instead
Applied to files:
test/js/bun/test/concurrent.test.ts
📚 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: Run tests with `bun bd test <...test file>`; do not use `bun test`
Applied to files:
docs/cli/test.md
📚 Learning: 2025-09-20T03:39:41.770Z
Learnt from: pfgithub
PR: oven-sh/bun#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:
docs/cli/test.md
🪛 markdownlint-cli2 (0.18.1)
docs/cli/test.md
121-121: Dollar signs used before commands without showing output
(MD014, commands-show-output)
| This helps prevent resource exhaustion when running many concurrent tests. The default value is 20. | ||
|
|
There was a problem hiding this comment.
Document that 0 disables the concurrency cap
The implementation treats 0 as “no limit”, but the docs don’t mention it. Calling this out avoids confusion for users who want to remove the default cap.
-This helps prevent resource exhaustion when running many concurrent tests. The default value is 20.
+This helps prevent resource exhaustion when running many concurrent tests. The default value is 20; set it to 0 to remove the cap entirely and let Bun schedule as many concurrent tests as possible.🤖 Prompt for AI Agents
In docs/cli/test.md around lines 138 to 139, the docs state the default
concurrency is 20 but do not mention that setting the concurrency to 0 disables
the cap; update the sentence to explicitly document that using 0 means “no
limit” (no concurrency cap) and keep the default value note, e.g. add a clause
like “(use 0 to disable the cap)” so users know 0 is treated as unlimited
concurrency.
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | ||
| expect(maxMatch).toBeTruthy(); | ||
| const executionPattern = JSON.parse(maxMatch![1]); | ||
|
|
||
| // Should be 1,2,3,3,3,3,3,... | ||
| const expected = Array.from({ length: 100 }, (_, i) => Math.min(i + 1, 3)); | ||
| expect(executionPattern).toEqual(expected); | ||
| }); | ||
|
|
||
| test("max-concurrency default is 20", async () => { | ||
| const result = await Bun.spawn({ | ||
| cmd: [bunExe(), "test", import.meta.dir + "/concurrent-max.fixture.ts"], | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| env: bunEnv, | ||
| }); | ||
| const exitCode = await result.exited; | ||
| const stdout = await result.stdout.text(); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
|
|
||
| // Extract max concurrent value from output | ||
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | ||
| expect(maxMatch).toBeTruthy(); | ||
| const executionPattern = JSON.parse(maxMatch![1]); | ||
|
|
||
| // Should be 1,2,3,...,18,19,20,20,20,20,20,20,... | ||
| const expected = Array.from({ length: 100 }, (_, i) => Math.min(i + 1, 20)); | ||
| expect(executionPattern).toEqual(expected); | ||
| }); | ||
|
|
||
| test("zero removes max-concurrency", async () => { | ||
| const result = await Bun.spawn({ | ||
| cmd: [bunExe(), "test", "--max-concurrency", "0", import.meta.dir + "/concurrent-max.fixture.ts"], | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| env: bunEnv, | ||
| }); | ||
| const exitCode = await result.exited; | ||
| const stdout = await result.stdout.text(); | ||
|
|
||
| expect(exitCode).toBe(0); | ||
|
|
||
| // Extract max concurrent value from output | ||
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | ||
| expect(maxMatch).toBeTruthy(); | ||
| const executionPattern = JSON.parse(maxMatch![1]); | ||
|
|
||
| // Should be 1,2,3,...,18,19,20,20,20,20,20,20,... |
There was a problem hiding this comment.
Trim execution pattern before JSON.parse
On Windows the log line ends with \r\n, so the capture group includes \r. JSON.parse("[...]\\r") throws, making these tests fail cross-platform. Trim the capture before parsing (and apply the same fix to each test).
- const executionPattern = JSON.parse(maxMatch![1]);
+ const executionPattern = JSON.parse(maxMatch![1].trim());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | |
| expect(maxMatch).toBeTruthy(); | |
| const executionPattern = JSON.parse(maxMatch![1]); | |
| // Should be 1,2,3,3,3,3,3,... | |
| const expected = Array.from({ length: 100 }, (_, i) => Math.min(i + 1, 3)); | |
| expect(executionPattern).toEqual(expected); | |
| }); | |
| test("max-concurrency default is 20", async () => { | |
| const result = await Bun.spawn({ | |
| cmd: [bunExe(), "test", import.meta.dir + "/concurrent-max.fixture.ts"], | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| env: bunEnv, | |
| }); | |
| const exitCode = await result.exited; | |
| const stdout = await result.stdout.text(); | |
| expect(exitCode).toBe(0); | |
| // Extract max concurrent value from output | |
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | |
| expect(maxMatch).toBeTruthy(); | |
| const executionPattern = JSON.parse(maxMatch![1]); | |
| // Should be 1,2,3,...,18,19,20,20,20,20,20,20,... | |
| const expected = Array.from({ length: 100 }, (_, i) => Math.min(i + 1, 20)); | |
| expect(executionPattern).toEqual(expected); | |
| }); | |
| test("zero removes max-concurrency", async () => { | |
| const result = await Bun.spawn({ | |
| cmd: [bunExe(), "test", "--max-concurrency", "0", import.meta.dir + "/concurrent-max.fixture.ts"], | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| env: bunEnv, | |
| }); | |
| const exitCode = await result.exited; | |
| const stdout = await result.stdout.text(); | |
| expect(exitCode).toBe(0); | |
| // Extract max concurrent value from output | |
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | |
| expect(maxMatch).toBeTruthy(); | |
| const executionPattern = JSON.parse(maxMatch![1]); | |
| // Should be 1,2,3,...,18,19,20,20,20,20,20,20,... | |
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | |
| expect(maxMatch).toBeTruthy(); | |
| const executionPattern = JSON.parse(maxMatch![1].trim()); | |
| // Should be 1,2,3,3,3,3,3,... | |
| const expected = Array.from({ length: 100 }, (_, i) => Math.min(i + 1, 3)); | |
| expect(executionPattern).toEqual(expected); | |
| }); | |
| test("max-concurrency default is 20", async () => { | |
| const result = await Bun.spawn({ | |
| cmd: [bunExe(), "test", import.meta.dir + "/concurrent-max.fixture.ts"], | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| env: bunEnv, | |
| }); | |
| const exitCode = await result.exited; | |
| const stdout = await result.stdout.text(); | |
| expect(exitCode).toBe(0); | |
| // Extract max concurrent value from output | |
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | |
| expect(maxMatch).toBeTruthy(); | |
| const executionPattern = JSON.parse(maxMatch![1].trim()); | |
| // Should be 1,2,3,...,18,19,20,20,20,20,20,20,... | |
| const expected = Array.from({ length: 100 }, (_, i) => Math.min(i + 1, 20)); | |
| expect(executionPattern).toEqual(expected); | |
| }); | |
| test("zero removes max-concurrency", async () => { | |
| const result = await Bun.spawn({ | |
| cmd: [bunExe(), "test", "--max-concurrency", "0", import.meta.dir + "/concurrent-max.fixture.ts"], | |
| stdout: "pipe", | |
| stderr: "pipe", | |
| env: bunEnv, | |
| }); | |
| const exitCode = await result.exited; | |
| const stdout = await result.stdout.text(); | |
| expect(exitCode).toBe(0); | |
| // Extract max concurrent value from output | |
| const maxMatch = stdout.match(/Execution pattern: ([^\n]+)/); | |
| expect(maxMatch).toBeTruthy(); | |
| const executionPattern = JSON.parse(maxMatch![1].trim()); | |
| // Should be 1,2,3,...,18,19,20,20,20,20,20,20,... | |
| const expected = Array.from({ length: 100 }, (_, i) => Math.min(i + 1, 20)); | |
| expect(executionPattern).toEqual(expected); | |
| }); |
🤖 Prompt for AI Agents
test/js/bun/test/concurrent.test.ts around lines 75 to 123: the captured
execution pattern string can include trailing CR on Windows so
JSON.parse(maxMatch![1]) can throw; fix by trimming the captured group before
parsing (e.g. const executionPattern = JSON.parse(maxMatch![1].trim())); apply
the same .trim() change to every test in this block where
JSON.parse(maxMatch![1]) is used.
There was a problem hiding this comment.
JSON.parse seems to trim whitespace: this works without error JSON.parse(" {}\r\n\r"). It passed in CI already and that includes windows.
What does this PR do?
Adds a maxConcurrency flag to limit the amount of concurrent tests that run at once. Defaults to 20. Jest and Vitest both default to 5.
How did you verify your code works?
Tests