Skip to content

Fix s3 leak tests run wrong times. Related to #20487 - #25526

Open
gogogoghost wants to merge 1 commit into
oven-sh:mainfrom
gogogoghost:main
Open

Fix s3 leak tests run wrong times. Related to #20487#25526
gogogoghost wants to merge 1 commit into
oven-sh:mainfrom
gogogoghost:main

Conversation

@gogogoghost

@gogogoghost gogogoghost commented Dec 15, 2025 •
edited
Loading

Copy link
Copy Markdown

What does this PR do?

Fix s3 leak tests run wrong times.

How did you verify your code works?

bun test/js/bun/s3/bun-write-leak-fixture.js
bun test/js/bun/s3/s3-stream-leak-fixture.js
bun test/js/bun/s3/s3-text-leak-fixture.js
bun test/js/bun/s3/s3-write-leak-fixture.js
bun test/js/bun/s3/s3-writer-leak-fixture.js

For example this test unit using below code to run writeLargeFile with 10 times. But writeLargeFile just run once:

await Promise.all(new Array(10).fill(writeLargeFile()));

So I change it to:

await Promise.all(Array.from({ length: 10 }, () => writeLargeFile()));

Related to #20487

@coderabbitai

coderabbitai Bot commented Dec 15, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The PR modifies five S3 test fixture files in Bun, replacing Array.fill() with Array.from() when creating Promise arrays. This ensures functions like writeLargeFile() and readLargeFile() are invoked independently for each array element rather than reusing a single promise.

Changes

Cohort / File(s) Summary
Promise array iteration pattern fixes
test/js/bun/s3/bun-write-leak-fixture.js, test/js/bun/s3/s3-stream-leak-fixture.js, test/js/bun/s3/s3-text-leak-fixture.js, test/js/bun/s3/s3-write-leak-fixture.js, test/js/bun/s3/s3-writer-leak-fixture.js		 Replaced new Array(n).fill(asyncFunction()) with Array.from({ length: n }, () => asyncFunction()) to ensure distinct promises are created for each iteration rather than reusing a single promise, changing concurrency behavior in test fixtures.

Suggested reviewers

  • Jarred-Sumner
  • pfgithub

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main fix: correcting S3 leak tests that were running the wrong number of times.
Description check ✅ Passed The description follows the template, explains what the PR does, provides concrete verification commands, and includes a clear code example demonstrating the issue and fix.
📜 Recent 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 7c98b0f and e34cfef.

📒 Files selected for processing (5)
  • test/js/bun/s3/bun-write-leak-fixture.js (1 hunks)
  • test/js/bun/s3/s3-stream-leak-fixture.js (1 hunks)
  • test/js/bun/s3/s3-text-leak-fixture.js (1 hunks)
  • test/js/bun/s3/s3-write-leak-fixture.js (1 hunks)
  • test/js/bun/s3/s3-writer-leak-fixture.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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 from bun:test
Use test.each and data-driven tests to reduce boilerplate when testing multiple similar cases

Files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
🧠 Learnings (21)
📓 Common learnings 
Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22946
File: test/js/sql/sql.test.ts:195-202
Timestamp: 2025-09-25T22:07:13.851Z
Learning: PR oven-sh/bun#22946: JSON/JSONB result parsing updates (e.g., returning parsed arrays instead of legacy strings) are out of scope for this PR; tests keep current expectations with a TODO. Handle parsing fixes in a separate PR.

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 set a timeout on tests. Bun already has timeouts

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

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} : Use `Buffer.alloc(count, fill).toString()` instead of `'A'.repeat(count)` to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds

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`

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`

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} : Use `test.each` and data-driven tests to reduce boilerplate when testing multiple similar cases

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/js/node/test/{parallel,sequential}/*.js : For test/js/node/test/{parallel,sequential}/*.js files without a .test extension, use `bun bd <file>` instead of `bun bd test <file>` since these expect exit code 0 and don't use bun's test runner

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

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 : Use `dev.write()`, `dev.patch()`, and `dev.delete()` to mutate the filesystem instead of `node:fs` APIs, as dev server functions are hooked to wait for hot-reload and notify clients

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/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves
📚 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} : Use `Buffer.alloc(count, fill).toString()` instead of `'A'.repeat(count)` to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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 : Use `dev.write()`, `dev.patch()`, and `dev.delete()` to mutate the filesystem instead of `node:fs` APIs, as dev server functions are hooked to wait for hot-reload and notify clients

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 Learning: 2025-11-06T00:58:23.965Z 
Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
📚 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/js/node/test/{parallel,sequential}/*.js : For test/js/node/test/{parallel,sequential}/*.js files without a .test extension, use `bun bd <file>` instead of `bun bd test <file>` since these expect exit code 0 and don't use bun's test runner

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
📚 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} : When spawning Bun processes in tests, use `bunExe` and `bunEnv` from `harness` to ensure the same build of Bun is used and debug logging is silenced

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 Learning: 2025-08-31T09:08:12.104Z 
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22279
File: test/js/web/structured-clone-fastpath.test.ts:12-12
Timestamp: 2025-08-31T09:08:12.104Z
Learning: In Bun, process.memoryUsage.rss() is called as a direct function, not as process.memoryUsage().rss like in Node.js. This is the correct API shape for Bun.

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 Learning: 2025-11-08T04:06:33.198Z 
Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24491
File: test/js/bun/transpiler/declare-global.test.ts:17-17
Timestamp: 2025-11-08T04:06:33.198Z
Learning: In Bun test files, `await using` with Bun.spawn() is the preferred pattern for spawned processes regardless of whether they are short-lived or long-running. Do not suggest replacing `await using proc = Bun.spawn(...)` with `const proc = Bun.spawn(...); await proc.exited;`.

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 Learning: 2025-11-24T18:35:39.205Z 
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

Applied to files:

  • test/js/bun/s3/bun-write-leak-fixture.js
  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
📚 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 concurrent tests over sequential tests using `test.concurrent` or `describe.concurrent` when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent

Applied to files:

  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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} : Track resources (servers, clients) in arrays for cleanup in `afterEach()`

Applied to files:

  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-writer-leak-fixture.js
📚 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} : Use `test.each` and data-driven tests to reduce boilerplate when testing multiple similar cases

Applied to files:

  • test/js/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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/bun/s3/s3-write-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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 set a timeout on tests. Bun already has timeouts

Applied to files:

  • test/js/bun/s3/s3-stream-leak-fixture.js
  • test/js/bun/s3/s3-text-leak-fixture.js
📚 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} : Use `describe.each()` for parameterized tests

Applied to files:

  • test/js/bun/s3/s3-text-leak-fixture.js
🧬 Code graph analysis (3)
test/js/bun/s3/s3-write-leak-fixture.js (4)
test/js/bun/s3/bun-write-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-stream-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-text-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-writer-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-stream-leak-fixture.js (4)
test/js/bun/s3/bun-write-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-text-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-write-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-writer-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-writer-leak-fixture.js (4)
test/js/bun/s3/bun-write-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-stream-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-text-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
test/js/bun/s3/s3-write-leak-fixture.js (2)
  • MAX_ALLOWED_MEMORY_USAGE (3-3)
  • MAX_ALLOWED_MEMORY_USAGE_INCREMENT (4-4)
🔇 Additional comments (5)
test/js/bun/s3/s3-text-leak-fixture.js (1)

19-19: Critical fix: Creates distinct promises for each iteration.

The original code new Array(n).fill(readLargeFile()) evaluated readLargeFile() once and reused the same promise n times, so the operation only executed once. This fix correctly invokes readLargeFile() for each array element, creating 10 and 100 concurrent operations respectively, which properly stress-tests memory usage as intended.

Also applies to: 25-25

test/js/bun/s3/s3-stream-leak-fixture.js (1)

24-24: Critical fix: Creates distinct promises for each iteration.

The original code new Array(n).fill(readLargeFile()) evaluated readLargeFile() once and reused the same promise n times, resulting in only a single operation. This fix correctly invokes readLargeFile() for each array element, creating 10 and 100 concurrent stream operations respectively, which properly stress-tests memory usage as intended.

Also applies to: 30-30

test/js/bun/s3/s3-writer-leak-fixture.js (1)

24-24: Critical fix: Creates distinct promises for each iteration.

The original code new Array(n).fill(writeLargeFile()) evaluated writeLargeFile() once and reused the same promise n times, so only one write operation executed. This fix correctly invokes writeLargeFile() for each array element, creating 10 and 100 concurrent write operations respectively, which properly stress-tests memory usage as intended.

Also applies to: 31-31

test/js/bun/s3/s3-write-leak-fixture.js (1)

16-16: Critical fix: Creates distinct promises for each iteration.

The original code new Array(n).fill(writeLargeFile()) evaluated writeLargeFile() once and reused the same promise n times, resulting in only a single write operation. This fix correctly invokes writeLargeFile() for each array element, creating 10 and 100 concurrent write operations respectively, which properly stress-tests memory usage as intended.

Also applies to: 23-23

test/js/bun/s3/bun-write-leak-fixture.js (1)

16-16: Critical fix: Creates distinct promises for each iteration.

The original code new Array(n).fill(writeLargeFile()) evaluated writeLargeFile() once and reused the same promise n times, so only one write operation executed. This fix correctly invokes writeLargeFile() for each array element, creating 10 and 100 concurrent write operations respectively, which properly stress-tests memory usage as intended.

Also applies to: 23-23

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@robobun robobun mentioned this pull request Jul 27, 2026
Closed
@robobun robobun mentioned this pull request Oct 24, 2026
Open
@robobun

robobun commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Thank you for finding this. #46686 has this loop fix, and you are a co-author of its commit.

#46686 also moves the file from Cloudflare R2 to the local S3 server of test/packages/s3-server. With the loop fix alone, one CI attempt sends about 550 bodies of 1 MiB to R2, and a slow period of R2 then fails the file at its 30 s limit.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gogogoghost @robobun