test: convert tempDirWithFiles callers to using tempDir - #36194
Merged
Merged
Conversation
Mechanically converts ~700 `const dir = tempDirWithFiles(...)` declarations that live directly inside test/it callbacks to `await using dir = tempDir(...)` (async callbacks) or `using dir = tempDir(...)` (sync ones), so the temp directory is removed when the test scope exits instead of leaking under os.tmpdir(). tempDir() returns a String object (DisposableString), so call sites that hit typeof-string checks are wrapped in String(): ===, expect().toBe / toEqual / toContain, process.chdir, child_process cwd, cwdScope, `new $.Shell().cwd()`, and glob.scan(cwd). Left alone: module/describe/beforeAll-scope directories, helper functions, `let`/reassignments, sync callbacks that return a promise or take a done callback, and one loop that awaits its work after the loop body. No-Verification-Needed: test-only refactor
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
A hand-applied replaceAll turned `$$.cwd` into `$.cwd` (the `$$` escape in replacement strings), and writeFile takes a different validation path when handed a String object as the path. No-Verification-Needed: test-only fixup
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughChangesTemporary fixture lifecycle migration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
test/js/bun/io/bun-write-leak.test.ts (1)
11-17: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
String(dir)before strict path/process calls.
tempDir()returns a boxedString, andpath.join/Bun.spawn({ cwd })reject boxed strings here.
test/js/bun/io/bun-write-leak.test.ts#L11-L17: createconst dirPath = String(dir)and use it for bothpath.joincalls.test/js/bun/namespace-prototype-pollution.test.ts#L5-L34: passcwd: String(dir).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/js/bun/io/bun-write-leak.test.ts` around lines 11 - 17, Convert the boxed tempDir result to a primitive with a dirPath variable in test/js/bun/io/bun-write-leak.test.ts lines 11-17, and use it for both path.join calls. In test/js/bun/namespace-prototype-pollution.test.ts lines 5-34, pass String(dir) as the cwd value for Bun.spawn.test/js/bun/resolve/jsonc.test.ts (1)
5-12: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCoerce
dirto a primitive beforepath.join()
tempDir()returns aDisposableString, andpath.join()rejects that wrapper object. UseString(dir)(ordir.toString()) in all four calls so these tests reach their assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/js/bun/resolve/jsonc.test.ts` around lines 5 - 12, Update all four path.join calls in the test setup to convert the DisposableString returned by tempDir to a primitive using String(dir) or dir.toString() before joining, while preserving the existing path segments and assertions.test/js/bun/resolve/resolver-permission-denied-ancestor.test.ts (1)
13-21: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConvert the temp dir to a primitive before
path.join
tempDir()returns aStringobject, andpath.join()only accepts primitive strings. UseString(dir)before thejoin()calls in both fixtures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/js/bun/resolve/resolver-permission-denied-ancestor.test.ts` around lines 13 - 21, Convert the tempDir() result to a primitive string before every path.join call in both resolver-permission fixtures and the ls test. Update test/js/bun/resolve/resolver-permission-denied-ancestor.test.ts at lines 13-21 and 39-42, and test/js/bun/shell/commands/ls.test.ts at lines 159-161; make no other changes.test/js/node/process/process-on.test.ts (1)
19-28: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConvert
tempDir()toString(dir)before strict path/process boundaries.
tempDir()returns aDisposableStringwrapper, and these calls pass it directly intopath.join(...)orcwd, which reject wrapper objects. Normalize once per fixture and reuse the primitive path.
test/js/node/process/process-on.test.ts:19-28,:52-63test/js/node/tls/test-node-extra-ca-certs.test.ts:19-22,:42-44,:73-76,:99test/js/third_party/body-parser/express-bun-build-compile.test.ts:11-13test/js/web/fetch/blob-write.test.ts:33-40🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/js/node/process/process-on.test.ts` around lines 19 - 28, Normalize each tempDir() result with String() before passing it to path.join or process cwd/options, then reuse the primitive path within the fixture setup. Apply this in test/js/node/process/process-on.test.ts at 19-28 and 52-63; test/js/node/tls/test-node-extra-ca-certs.test.ts at 19-22, 42-44, 73-76, and 99; test/js/third_party/body-parser/express-bun-build-compile.test.ts at 11-13; and test/js/web/fetch/blob-write.test.ts at 33-40, preserving the existing fixture behavior.test/cli/install/bun-run-bunfig.test.ts (2)
21-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer synchronous
usingfortempDir().These callbacks only perform synchronous fixture setup and process execution. Replace
await using cwdwithusing cwd;DisposableStringsupports synchronous disposal, avoiding unnecessary async teardown.Based on learnings, prefer plain
usingfortempDir()in Bun tests.Also applies to: 107-118, 145-156, 184-196, 221-232
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/cli/install/bun-run-bunfig.test.ts` around lines 21 - 32, Replace await using with synchronous using for each tempDir() fixture declaration in the affected test cases, including the declarations near the referenced ranges. Keep the existing fixture setup and test execution unchanged, relying on DisposableString’s synchronous disposal.Source: Learnings
184-205: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winCoerce
cwdbefore callingpathJoin.tempDir()returns aDisposableStringobject, andnode:path.join()rejectsStringobjects. BothpathJoin(cwd, "./subdir")andpathJoin(cwd, "./my-home")can throw before Bun spawns; useString(cwd)in both places.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/cli/install/bun-run-bunfig.test.ts` around lines 184 - 205, Update the `Bun.spawnSync` setup in the `where-node` test and the corresponding `./my-home` setup to pass `String(cwd)` into `pathJoin`, ensuring the `DisposableString` returned by `tempDir()` is coerced before path construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/cli/inspect/inspect.test.ts`:
- Line 466: Update the test setup around tempdir to convert the String object
returned by tempDir() into a primitive string before path operations. Pass
String(tempdir) to randomSocketPathFn(...) and join(...), preserving the
existing temporary-directory behavior.
In `@test/cli/run/tsconfig-override.test.ts`:
- Line 7: Convert each tempDir() result to a primitive string at string-only
boundaries by wrapping the directory value with String(...). Apply this to
path.join inputs, cwd values, and bunRunAsScript arguments across
test/cli/run/tsconfig-override.test.ts lines 7, 73, 114, 161, 210, and 266;
test/cli/run/run-quote.test.ts lines 12-14 and 18;
test/cli/run/sql-preconnect.test.ts lines 25 and 61; and
test/cli/run/workspaces.test.ts lines 5, 45, and 78. Use the existing dir values
and do not change tempDir lifecycle handling.
In `@test/js/bun/io/bun-write-leak.test.ts`:
- Line 11: Replace await using with using for the tempDir() declarations in
test/js/bun/io/bun-write-leak.test.ts lines 11-11,
test/js/bun/namespace-prototype-pollution.test.ts lines 5-5, and every migrated
tempDir() declaration in test/js/bun/patch/patch.test.ts lines 83-88, preserving
synchronous disposal via Symbol.dispose.
In `@test/js/bun/test/done-async.test.ts`:
- Around line 18-21: Use the local Shell instance consistently by changing
$.cwd(String(dir)) to $$.cwd(String(dir)) in test/js/bun/test/done-async.test.ts
lines 18-21 and test/js/bun/test/expect-assertions.test.ts lines 18-21; no other
changes are needed.
---
Outside diff comments:
In `@test/cli/install/bun-run-bunfig.test.ts`:
- Around line 21-32: Replace await using with synchronous using for each
tempDir() fixture declaration in the affected test cases, including the
declarations near the referenced ranges. Keep the existing fixture setup and
test execution unchanged, relying on DisposableString’s synchronous disposal.
- Around line 184-205: Update the `Bun.spawnSync` setup in the `where-node` test
and the corresponding `./my-home` setup to pass `String(cwd)` into `pathJoin`,
ensuring the `DisposableString` returned by `tempDir()` is coerced before path
construction.
In `@test/js/bun/io/bun-write-leak.test.ts`:
- Around line 11-17: Convert the boxed tempDir result to a primitive with a
dirPath variable in test/js/bun/io/bun-write-leak.test.ts lines 11-17, and use
it for both path.join calls. In
test/js/bun/namespace-prototype-pollution.test.ts lines 5-34, pass String(dir)
as the cwd value for Bun.spawn.
In `@test/js/bun/resolve/jsonc.test.ts`:
- Around line 5-12: Update all four path.join calls in the test setup to convert
the DisposableString returned by tempDir to a primitive using String(dir) or
dir.toString() before joining, while preserving the existing path segments and
assertions.
In `@test/js/bun/resolve/resolver-permission-denied-ancestor.test.ts`:
- Around line 13-21: Convert the tempDir() result to a primitive string before
every path.join call in both resolver-permission fixtures and the ls test.
Update test/js/bun/resolve/resolver-permission-denied-ancestor.test.ts at lines
13-21 and 39-42, and test/js/bun/shell/commands/ls.test.ts at lines 159-161;
make no other changes.
In `@test/js/node/process/process-on.test.ts`:
- Around line 19-28: Normalize each tempDir() result with String() before
passing it to path.join or process cwd/options, then reuse the primitive path
within the fixture setup. Apply this in test/js/node/process/process-on.test.ts
at 19-28 and 52-63; test/js/node/tls/test-node-extra-ca-certs.test.ts at 19-22,
42-44, 73-76, and 99;
test/js/third_party/body-parser/express-bun-build-compile.test.ts at 11-13; and
test/js/web/fetch/blob-write.test.ts at 33-40, preserving the existing fixture
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b6deae46-c5eb-4728-b406-1721809d383a
📒 Files selected for processing (141)
test/bake/dev/import-meta-inline-negative.test.tstest/bake/dev/response-to-bake-response.test.tstest/bake/framework-router.test.tstest/bundler/bun-build-compile-wasm.test.tstest/bundler/bundler_compile.test.tstest/bundler/bundler_defer.test.tstest/bundler/html-import-manifest.test.tstest/cli/bunfig-test-options.test.tstest/cli/console-depth.test.tstest/cli/hot/watch-many-dirs.test.tstest/cli/hot/watch.test.tstest/cli/init/init.test.tstest/cli/inspect/inspect.test.tstest/cli/install/bun-audit.test.tstest/cli/install/bun-install-patch.test.tstest/cli/install/bun-install-registry.test.tstest/cli/install/bun-install.test.tstest/cli/install/bun-pack.test.tstest/cli/install/bun-patch.test.tstest/cli/install/bun-pm-pkg.test.tstest/cli/install/bun-pm-scan.test.tstest/cli/install/bun-pm-version.test.tstest/cli/install/bun-pm-why.test.tstest/cli/install/bun-run-bunfig.test.tstest/cli/install/bun-security-scanner-workspaces.test.tstest/cli/install/bun-update-security-edge-cases.test.tstest/cli/install/bun-update-security-scan-all.test.tstest/cli/install/bun-update-security-simple.test.tstest/cli/install/migration/migrate.test.tstest/cli/install/migration/pnpm-comprehensive.test.tstest/cli/install/migration/pnpm-lock-migration.test.tstest/cli/install/migration/pnpm-migration-complete.test.tstest/cli/install/migration/yarn-lock-migration.test.tstest/cli/install/test-dev-peer-dependency-priority.test.tstest/cli/run/as-node.test.tstest/cli/run/env.test.tstest/cli/run/filter-workspace.test.tstest/cli/run/require-and-import-trailing.test.tstest/cli/run/require-cache.test.tstest/cli/run/run-process-env.test.tstest/cli/run/run-quote.test.tstest/cli/run/sql-preconnect.test.tstest/cli/run/tsconfig-override.test.tstest/cli/run/workspaces.test.tstest/cli/test/bun-test.test.tstest/cli/test/claudecode-flag.test.tstest/cli/test/coverage.test.tstest/cli/test/path-ignore-patterns.test.tstest/cli/test/test-randomize.test.tstest/cli/update_interactive_formatting.test.tstest/cli/update_interactive_snapshots.test.tstest/cli/user-agent.test.tstest/internal/int_from_float.test.tstest/js/bun/bundler/yaml-bundler.test.jstest/js/bun/fetch/node-use-system-ca.test.tstest/js/bun/glob/proto.test.tstest/js/bun/glob/scan.test.tstest/js/bun/http/bun-listen-connect-args.test.tstest/js/bun/http/bun-serve-html-405.test.tstest/js/bun/http/bun-serve-html-entry.test.tstest/js/bun/http/bun-serve-html-manifest.test.tstest/js/bun/http/bun-serve-html.test.tstest/js/bun/http/bun-serve-propagate-errors.test.tstest/js/bun/http/bun-server.test.tstest/js/bun/import-attributes/import-attributes.test.tstest/js/bun/ini/ini.test.tstest/js/bun/io/bun-write-leak.test.tstest/js/bun/namespace-prototype-pollution.test.tstest/js/bun/patch/patch.test.tstest/js/bun/resolve/bun-lock.test.tstest/js/bun/resolve/jsonc.test.tstest/js/bun/resolve/resolve.test.tstest/js/bun/resolve/resolver-permission-denied-ancestor.test.tstest/js/bun/s3/s3.leak.test.tstest/js/bun/s3/s3.test.tstest/js/bun/shell/bunshell.test.tstest/js/bun/shell/commands/ls.test.tstest/js/bun/shell/commands/rm.test.tstest/js/bun/shell/lazy.test.tstest/js/bun/shell/leak.test.tstest/js/bun/sqlite/sqlite.test.jstest/js/bun/test/describe.test.tstest/js/bun/test/done-async.test.tstest/js/bun/test/expect-assertions.test.tstest/js/bun/test/expect-extend-preload.test.tstest/js/bun/test/pretty-format-overflow.test.tstest/js/bun/test/snapshot-tests/snapshots/snapshot.test.tstest/js/bun/test/test-test.test.tstest/js/bun/typescript/type-export.test.tstest/js/bun/util/bun-file.test.tstest/js/bun/util/which.test.tstest/js/junit-reporter/junit.test.jstest/js/node/fs/cp.test.tstest/js/node/fs/fs-birthtime-linux.test.tstest/js/node/fs/fs-promises-writeFile-async-iterator.test.tstest/js/node/fs/fs-stat-seccomp-linux.test.tstest/js/node/fs/fs.test.tstest/js/node/fs/glob.test.tstest/js/node/fs/promises.test.jstest/js/node/module/require-extensions.test.tstest/js/node/process/process-on.test.tstest/js/node/tls/test-node-extra-ca-certs.test.tstest/js/node/watch/fs.watchFile.test.tstest/js/sql/sql.test.tstest/js/sql/sqlite-sql.test.tstest/js/third_party/body-parser/express-bun-build-compile.test.tstest/js/web/fetch/blob-write.test.tstest/napi/napi.test.tstest/regression/issue/026039.test.tstest/regression/issue/07740.test.tstest/regression/issue/09340.test.tstest/regression/issue/09555.test.tstest/regression/issue/09559.test.tstest/regression/issue/10887.test.tstest/regression/issue/11664.test.tstest/regression/issue/11806.test.tstest/regression/issue/14945-lifecycle-script-crash.test.tstest/regression/issue/14976/14976.test.tstest/regression/issue/17327.test.tstest/regression/issue/20321.test.tstest/regression/issue/21680.test.tstest/regression/issue/21907.test.tstest/regression/issue/22317.test.tstest/regression/issue/23649.test.tstest/regression/issue/24502/bun-pm-ls-all-invalid-package-id.test.tstest/regression/issue/25716.test.tstest/regression/issue/25794.test.tstest/regression/issue/26207.test.tstest/regression/issue/26647.test.tstest/regression/issue/29787.test.tstest/regression/issue/3657.test.tstest/regression/issue/5228.test.jstest/regression/issue/comma-operator-this-binding.test.tstest/regression/issue/ctrl-c.test.tstest/regression/issue/cyclic-imports-async-bundler.test.jstest/regression/issue/hashbang-still-works.test.tstest/regression/issue/malformed-integrity-base64.test.tstest/regression/issue/patch-bounds-check.test.tstest/regression/issue/test_env_loader_threading.test.tstest/regression/issue/update-interactive-formatting.test.tstest/regression/issue/utf16-encoding-crash.test.ts
| let session: InspectorSession; | ||
|
|
||
| const tempdir = tempDirWithFiles("junit-reporter", { | ||
| await using tempdir = tempDir("junit-reporter", { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 '\btempdir\b' test/cli/inspect/inspect.test.ts
rg -n -C3 'class DisposableString|function tempDir' test/harness.tsRepository: oven-sh/bun
Length of output: 2790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '460,530p' test/cli/inspect/inspect.test.ts
python3 - <<'PY'
from pathlib import Path
import re
text = Path("test/cli/inspect/inspect.test.ts").read_text()
for i, line in enumerate(text.splitlines(), 1):
if "tempdir" in line:
print(f"{i}:{line}")
PY
node - <<'JS'
const path = require('path');
const tempdirObj = new String('.tmp-demo');
console.log('typeof tempdirObj:', typeof tempdirObj);
console.log('String(tempdirObj):', String(tempdirObj));
console.log('path.join(tempdirObj, "a.test.js"):', path.join(tempdirObj, 'a.test.js'));
console.log('replaceAll on String object result:', tempdirObj.replaceAll('\\', '/'));
console.log('template concat:', `unix://${tempdirObj}`);
JSRepository: oven-sh/bun
Length of output: 3576
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline test/cli/inspect/inspect.test.ts --view expandedRepository: oven-sh/bun
Length of output: 623
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,40p' test/cli/inspect/inspect.test.tsRepository: oven-sh/bun
Length of output: 1363
Coerce tempdir to a primitive string before the path joins. tempDir() returns a String object; pass String(tempdir) into randomSocketPathFn(...) and join(...).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/cli/inspect/inspect.test.ts` at line 466, Update the test setup around
tempdir to convert the String object returned by tempDir() into a primitive
string before path operations. Pass String(tempdir) to randomSocketPathFn(...)
and join(...), preserving the existing temporary-directory behavior.
| describe("bun run --tsconfig-override", () => { | ||
| test("should use custom tsconfig for path resolution", async () => { | ||
| const dir = tempDirWithFiles("run-tsconfig-override", { | ||
| await using dir = tempDir("run-tsconfig-override", { |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate tempDir definition and the affected tests.
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'function tempDir|const tempDir|export .*tempDir|class DisposableString|Symbol\.dispose|Symbol\.asyncDispose' \
test harness packages src . 2>/dev/null || true
echo '--- FILE LIST ---'
git ls-files 'test/cli/run/*.test.ts' 'test/**/*harness*' 'test/**/harness*' 'test/**/*tempDir*' | sed -n '1,200p'Repository: oven-sh/bun
Length of output: 50367
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the harness implementation and the exact call sites.
for f in \
test/cli/run/tsconfig-override.test.ts \
test/cli/run/run-quote.test.ts \
test/cli/run/sql-preconnect.test.ts \
test/cli/run/workspaces.test.ts
do
echo "--- $f ---"
sed -n '1,340p' "$f"
done
echo "--- tempDir definition candidates ---"
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'tempDir\s*\(' test harness . 2>/dev/null | sed -n '1,200p'Repository: oven-sh/bun
Length of output: 35731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find bunRunAsScript and tempDir implementations.
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'bunRunAsScript|function tempDir|const tempDir|export .*tempDir|class .*Disposable' \
test . 2>/dev/null | sed -n '1,220p'Repository: oven-sh/bun
Length of output: 6793
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '420,540p' test/harness.tsRepository: oven-sh/bun
Length of output: 3574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the exact return type and helper behavior around tempDir/bunRunAsScript.
sed -n '420,540p' test/harness.tsRepository: oven-sh/bun
Length of output: 3574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '500,560p' test/harness.tsRepository: oven-sh/bun
Length of output: 1744
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- test/harness.ts around tempDir and bunRunAsScript ---'
sed -n '424,540p' test/harness.ts
echo '--- node path.join with String objects ---'
node - <<'JS'
const path = require('node:path');
for (const value of [new String('/tmp/x'), Object(new String('/tmp/y'))]) {
try {
console.log('join', String(value), '=>', path.join(value, 'file.txt'));
} catch (e) {
console.log('join-error', String(value), e.name, e.code, e.message);
}
}
JSRepository: oven-sh/bun
Length of output: 3895
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' \
'function join|join\s*=\s*function|validateString|invalid arg type|ERR_INVALID_ARG_TYPE|PathLike' \
src/js/node test/harness.ts | sed -n '1,220p'Repository: oven-sh/bun
Length of output: 20181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- tempDir and bunRunAsScript ---'
sed -n '424,540p' test/harness.tsRepository: oven-sh/bun
Length of output: 3581
Convert tempDir() values to plain strings at string-only boundaries.
tempDir() returns a disposable String object, and path.join(...) / cwd validation reject that shape at runtime. Wrap these call sites in String(...), including bunRunAsScript(dir, ...) and every cwd: dir use.
test/cli/run/tsconfig-override.test.ts#L7-L7, L73-L73, L114-L114, L161-L161, L210-L210, L266-L266test/cli/run/run-quote.test.ts#L12-L14, L18-L18test/cli/run/sql-preconnect.test.ts#L25-L25, L61-L61test/cli/run/workspaces.test.ts#L5-L5, L45-L45, L78-L78
📍 Affects 4 files
test/cli/run/tsconfig-override.test.ts#L7-L7(this comment)test/cli/run/run-quote.test.ts#L12-L14test/cli/run/run-quote.test.ts#L18-L18test/cli/run/sql-preconnect.test.ts#L25-L25test/cli/run/sql-preconnect.test.ts#L61-L61test/cli/run/tsconfig-override.test.ts#L73-L73test/cli/run/tsconfig-override.test.ts#L114-L114test/cli/run/tsconfig-override.test.ts#L161-L161test/cli/run/tsconfig-override.test.ts#L210-L210test/cli/run/tsconfig-override.test.ts#L266-L266test/cli/run/workspaces.test.ts#L5-L5test/cli/run/workspaces.test.ts#L45-L45test/cli/run/workspaces.test.ts#L78-L78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/cli/run/tsconfig-override.test.ts` at line 7, Convert each tempDir()
result to a primitive string at string-only boundaries by wrapping the directory
value with String(...). Apply this to path.join inputs, cwd values, and
bunRunAsScript arguments across test/cli/run/tsconfig-override.test.ts lines 7,
73, 114, 161, 210, and 266; test/cli/run/run-quote.test.ts lines 12-14 and 18;
test/cli/run/sql-preconnect.test.ts lines 25 and 61; and
test/cli/run/workspaces.test.ts lines 5, 45, and 78. Use the existing dir values
and do not change tempDir lifecycle handling.
| "Bun.write should not leak the output data", | ||
| async () => { | ||
| const dir = tempDirWithFiles("bun-write-leak-fixture", { | ||
| await using dir = tempDir("bun-write-leak-fixture", { |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Use synchronous disposal for tempDir().
tempDir() implements Symbol.dispose, so plain using avoids unnecessary asynchronous cleanup.
test/js/bun/io/bun-write-leak.test.ts#L11-L11: replaceawait usingwithusing.test/js/bun/namespace-prototype-pollution.test.ts#L5-L5: replaceawait usingwithusing.test/js/bun/patch/patch.test.ts#L83-L88: apply the same change to every migratedtempDir()declaration.
📍 Affects 3 files
test/js/bun/io/bun-write-leak.test.ts#L11-L11(this comment)test/js/bun/namespace-prototype-pollution.test.ts#L5-L5test/js/bun/patch/patch.test.ts#L83-L88
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/js/bun/io/bun-write-leak.test.ts` at line 11, Replace await using with
using for the tempDir() declarations in test/js/bun/io/bun-write-leak.test.ts
lines 11-11, test/js/bun/namespace-prototype-pollution.test.ts lines 5-5, and
every migrated tempDir() declaration in test/js/bun/patch/patch.test.ts lines
83-88, preserving synchronous disposal via Symbol.dispose.
Source: Learnings
This was referenced Jul 31, 2026
This was referenced Aug 11, 2026
robobun
added a commit
that referenced
this pull request
Aug 13, 2026
The readdir promise completion moved into AsyncReaddirRecursiveTask::then on main, so the WithFileTypesBuffer arm now lives there; DirentBuffer follows the narrowed visibility of Dirent; the new cp tests use the tempDir helper that cp.test.ts switched to in #36194.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Mechanically converts ~700
const dir = tempDirWithFiles(...)declarations that sit directly insidetest/itcallbacks toawait using dir = tempDir(...)(async callbacks) orusing dir = tempDir(...)(sync), so temp directories are removed when the test scope exits instead of leaking underos.tmpdir().tempDir()returns aStringobject (DisposableString), so call sites hittingtypeof x === "string"checks are wrapped inString(...):Stringobject===,expect().toBe/toEqual/toContainprocess.chdir,child_processcwd,cwdScopenew $.Shell().cwd()glob.scan(dir)Deliberately left alone: module/
describe/beforeAll-scope dirs, helper functions,let/reassignments, sync callbacks that return a promise or takedone, and a loop inbun-install-proxy.test.tsthat awaits after the loop body.How did you verify your code works?
USE_SYSTEM_BUN=1 bun test --parallel --bailover all 141 changed files against1.4.0-canary.1+d54984555(matches base commit). One residual: a--parallelworker crash (preload not found) onbun-listen-connect-args.test.tsthat doesn't reproduce alone, in ordered pairs, or in small parallel batches — watching CI for it.