Skip to content

test: use tempDir in log-test.test.ts instead of hardcoded /tmp path - #34294

Merged
dylan-conway merged 1 commit into
mainfrom
farm/d9176aaf/log-test-tempdir
Jul 16, 2026
Merged

dylan-conway merged 1 commit into
mainfrom
farm/d9176aaf/log-test-tempdir

Conversation

@robobun

@robobun robobun commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Failure

test/cli/run/log-test.test.ts went red on macOS aarch64 in build 73489:

EACCES: permission denied, open '/tmp/log-test-silent/.env'
    path: "/tmp/log-test-silent/.env",
 syscall: "open",
   errno: -13,
      at writeDirectoryTree (.../test/cli/run/log-test.test.ts:52:8)

Cause

Since it was added in 2023 this test has written its fixture to the hardcoded path /tmp/log-test-silent. The macOS CI hosts are persistent bare-metal machines that now run buildkite agents under more than one local user. When a run under one user leaves /tmp/log-test-silent behind with the default drwxr-xr-x mode, a subsequent run under a different user cannot unlink or rewrite its contents and the fs.writeFileSync call fails with EACCES. No source change triggered this; it is a latent non-hermetic test that surfaced when a second agent user started leaving the directory behind.

Fix

Use tempDir from harness so each run gets a unique, user-owned temporary directory that is removed via using on exit. The assertions and the files written (.env, bunfig.toml, index.ts) are unchanged, so the same .env-loading / logLevel code path is still exercised. The bespoke writeDirectoryTree helper is no longer needed and is removed.

Verification

bun bd test test/cli/run/log-test.test.ts
 2 pass
 0 fail

Confirmed on the affected host that /tmp contains entries owned by both ciadmin and administrator, which is what makes the fixed path unsafe.

The test was writing to /tmp/log-test-silent, a fixed path shared across
runs. On the persistent macOS CI hosts two buildkite agent users write to
/tmp; when one creates the directory with mode 755 the other hits
EACCES: permission denied, open '/tmp/log-test-silent/.env'.

Switch to tempDir from harness so each run gets a unique, user-owned,
auto-cleaned directory. The assertions and the files under test are
unchanged.
@robobun

robobun commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 8:31 PM PT - Jul 15th, 2026

@robobun, your commit 2806e14 is building: #73599

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: afa8bed2-2512-42a1-a75b-47d6e50e13b6

📥 Commits

Reviewing files that changed from the base of the PR and between d29b050 and 2806e14.

📒 Files selected for processing (1)
  • test/cli/run/log-test.test.ts

Walkthrough

Changes

The CLI logging tests now use tempDir for .env, bunfig.toml, and index.ts fixtures, replacing fixed /tmp paths and manual directory writing. Both tests pass the temporary directory as spawnSync’s working directory.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: switching the test to tempDir instead of a hardcoded /tmp path.
Description check ✅ Passed The description covers the failure, cause, fix, and verification, though it does not use the repository's exact template headings.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — mechanical test hermeticity fix.

What was reviewed:

  • Confirmed tempDir from harness returns a disposable path wrapper (mkdtemp-based, unique per run) and String(dir) yields the cwd — matches the usage here.
  • Assertions and fixture contents (.env, bunfig.toml, index.ts) are byte-identical to before; only the directory location changed.
  • Removed writeDirectoryTree had no other callers in this file; fs/path imports are correctly dropped with it.
Extended reasoning...

Overview

This PR touches a single test file, test/cli/run/log-test.test.ts, replacing a hardcoded /tmp/log-test-silent fixture directory with the harness tempDir helper and a using declaration. The bespoke writeDirectoryTree helper (which swallowed errors and never cleaned up) is deleted along with its now-unused fs and path imports. The two test bodies otherwise keep identical fixture contents and identical assertions.

Security risks

None. Test-only change; no runtime, network, auth, or crypto surfaces are involved.

Level of scrutiny

Low. This is the canonical hermeticity pattern the repo's own guidelines prescribe (tempDir + using over ad-hoc /tmp paths), applied to fix a real CI flake caused by cross-user directory ownership on persistent macOS runners. I verified the tempDir implementation in test/harness.ts: it calls mkdtempSync under os.tmpdir() with a unique suffix, writes the file tree, and returns a DisposableString whose [Symbol.dispose] recursively removes the directory — so using dir = tempDir(...) and cwd: String(dir) behave exactly as needed here.

Other factors

The PR description shows the tests still pass (2 pass, 0 fail) under the debug build. The removed helper had no other call sites in the file, and the change follows the exact idiom used across dozens of other tests in the suite. No prior reviews or outstanding comments on the PR.

@dylan-conway
dylan-conway merged commit 10ff028 into main Jul 16, 2026
23 of 40 checks passed
@dylan-conway
dylan-conway deleted the farm/d9176aaf/log-test-tempdir branch July 16, 2026 03:38
hughescr added a commit to hughescr/bun that referenced this pull request Jul 16, 2026
* upstream/main: (57 commits)
  node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed (oven-sh#32488)
  expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266)
  lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253)
  Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289)
  test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297)
  worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278)
  buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273)
  fs.promises.watch: yield events with a null prototype (oven-sh#34279)
  child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268)
  Fix asString assertion when passing String objects as signals (oven-sh#34265)
  Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274)
  test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294)
  tty: track raw mode per handle instead of per process (oven-sh#33527)
  test: expect the bumped mimalloc SHA in process.versions
  Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181)
  Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131)
  test: update block-scoped enum lowering expectations to let (oven-sh#34287)
  Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259)
  js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246)
  js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245)
  ...
hughescr added a commit to hughescr/bun that referenced this pull request Jul 16, 2026
* upstream/main: (70 commits)
  node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed (oven-sh#32488)
  expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266)
  lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253)
  Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289)
  test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297)
  worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278)
  buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273)
  fs.promises.watch: yield events with a null prototype (oven-sh#34279)
  child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268)
  Fix asString assertion when passing String objects as signals (oven-sh#34265)
  Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274)
  test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294)
  tty: track raw mode per handle instead of per process (oven-sh#33527)
  test: expect the bumped mimalloc SHA in process.versions
  Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181)
  Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131)
  test: update block-scoped enum lowering expectations to let (oven-sh#34287)
  Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259)
  js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246)
  js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245)
  ...
hughescr added a commit to hughescr/bun that referenced this pull request Jul 16, 2026
* upstream/main: (52 commits)
  node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed (oven-sh#32488)
  expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266)
  lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253)
  Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289)
  test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297)
  worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278)
  buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273)
  fs.promises.watch: yield events with a null prototype (oven-sh#34279)
  child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268)
  Fix asString assertion when passing String objects as signals (oven-sh#34265)
  Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274)
  test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294)
  tty: track raw mode per handle instead of per process (oven-sh#33527)
  test: expect the bumped mimalloc SHA in process.versions
  Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181)
  Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131)
  test: update block-scoped enum lowering expectations to let (oven-sh#34287)
  Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259)
  js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246)
  js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245)
  ...
hughescr added a commit to hughescr/bun that referenced this pull request Jul 16, 2026
* upstream/main: (52 commits)
  node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed (oven-sh#32488)
  expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266)
  lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253)
  Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289)
  test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297)
  worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278)
  buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273)
  fs.promises.watch: yield events with a null prototype (oven-sh#34279)
  child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268)
  Fix asString assertion when passing String objects as signals (oven-sh#34265)
  Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274)
  test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294)
  tty: track raw mode per handle instead of per process (oven-sh#33527)
  test: expect the bumped mimalloc SHA in process.versions
  Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181)
  Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131)
  test: update block-scoped enum lowering expectations to let (oven-sh#34287)
  Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259)
  js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246)
  js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245)
  ...

# Conflicts:
#	test/js/bun/websocket/websocket-server.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants