Skip to content

fix(test): pass the lifecycle, trust prompt and jj source tests on macOS - #1146

Merged
benvinegar merged 1 commit into
modem-dev:mainfrom
andrewloux:andrewloux/macos-test-fixes
Oct 10, 2026
Merged

benvinegar merged 1 commit into
modem-dev:mainfrom
andrewloux:andrewloux/macos-test-fixes

Conversation

@andrewloux

Copy link
Copy Markdown
Contributor

six tests fail on macOS on main, so bun run test and bun run test:integration don't go green on a mac. CI runs both suites on ubuntu-latest only, which is why main stays green. #1120 fixed the install VM ones and listed the jj test as pre-existing. with this branch both suites pass on my machine. each failure has its own cause:

lifecycle, exits cleanly on SIGHUP / SIGQUIT / SIGPIPE: the test stops reading the PTY once the first screen shows up. when the signal lands, hunk's teardown output fills the PTY buffer and the write blocks, so hunk never exits inside the 2s wait. drainPty keeps reading until the PTY closes. a hunk that hangs on the signal still fails the test.

extension trust, trust prompt runs repo extensions… and never records a denial…: hunk records trust under the repo's canonical path. the fixture lives under tmpdir(), which is /var/folders/… on macOS and resolves to /private/var/folders/…, so the lookup by fixture.dir returns undefined. the three lookups now go through trustKey(), which resolves the same way. the toBeUndefined() check for a dismissed prompt was passing because of that same mismatch, and now checks the real key.

jj source, logs unexpected source failures with revision and path context: without jj installed, Bun.spawn throws, and that log line names the revision and file but leaves out the repo. only the non-zero exit branch included in <repoRoot>. the spawn and stream-collect diagnostics now end with it too. that's the one user-visible change, so it carries a patch changeset.

one command per fix, on main (a3321c82) and on this branch:

bun test ./test/pty/lifecycle.test.ts -t 'exits cleanly on'
#   main: 0 pass, 3 fail        branch: 3 pass, 0 fail
bun test ./test/pty/extensions-integration.test.ts -t 'trust prompt runs|never records a denial'
#   main: 0 pass, 2 fail        branch: 2 pass, 0 fail
bun test ./packages/hunk-jj/src/source.test.ts
#   main: 2 pass, 5 skip, 1 fail    branch: 3 pass, 5 skip, 0 fail

full runs on this branch: bun run test 4556 pass, 52 skip, 0 fail. bun run test:integration 191 pass, 0 fail. typecheck, oxlint and oxfmt are clean.

tested on macOS (Darwin 25.6.0) with Bun 1.4.2. i haven't run it on Linux. CI is green there, so i'd expect the PTY to hold the teardown output and the tmpdir to already be a real path, and drainPty and trustKey() then change nothing.

Six tests failed on macOS on upstream main; each now passes.

- lifecycle "exits cleanly on SIGHUP/SIGQUIT/SIGPIPE": the test stopped
  reading the PTY master after its first match. Hunk's teardown writes
  then filled the PTY buffer and blocked, so Hunk never exited and the
  2s wait timed out. drainPty keeps reading until the PTY closes; a Hunk
  that hangs on a signal still fails the test.
- extensions "trust prompt runs repo extensions…" and "never records a
  denial…": Hunk records trust under the repo's canonical path, and a
  macOS temp dir reads as /var/folders/… but resolves to
  /private/var/folders/…. The lookups now use trustKey(), which
  resolves the same way; the toBeUndefined() check had passed for the
  wrong reason.
- hunk-jj "logs unexpected source failures with revision and path
  context": without jj installed, the spawn-failure log named the
  revision and file and left out the repo path. Both the spawn and the
  stream-collect diagnostics now end with "in <repoRoot>", like the
  failed-read one.
@vercel

vercel Bot commented Oct 5, 2026

Copy link
Copy Markdown

@andrewloux is attempting to deploy a commit to the Modem Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

@benvinegar

Copy link
Copy Markdown
Member

Thank you. Looks like we need a macOS test runner too.

@benvinegar
benvinegar enabled auto-merge (squash) October 10, 2026 12:06
@benvinegar
benvinegar merged commit e1ec8c1 into modem-dev:main Oct 10, 2026
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants