From 30ed5db81f13e235e20e543b8d7c762b82202ca7 Mon Sep 17 00:00:00 2001 From: Andrew Louis Date: Mon, 5 Oct 2026 08:43:11 -0700 Subject: [PATCH] test: pass the PTY and jj suites on macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 ", like the failed-read one. --- .changeset/quiet-jj-paths.md | 5 +++++ packages/hunk-jj/src/source.ts | 7 +++++-- test/pty/extensions-integration.test.ts | 16 ++++++++++++---- test/pty/lifecycle.test.ts | 17 +++++++++++++++++ 4 files changed, 39 insertions(+), 6 deletions(-) create mode 100644 .changeset/quiet-jj-paths.md diff --git a/.changeset/quiet-jj-paths.md b/.changeset/quiet-jj-paths.md new file mode 100644 index 000000000..ea677fd79 --- /dev/null +++ b/.changeset/quiet-jj-paths.md @@ -0,0 +1,5 @@ +--- +"hunkdiff": patch +--- + +Name the repository in every Jujutsu source-read diagnostic. A missing `jj` binary and an unreadable `jj` stream now log the repo path beside the revision and file, as a failed `jj file show` already did. diff --git a/packages/hunk-jj/src/source.ts b/packages/hunk-jj/src/source.ts index ed02d56c5..64406a284 100644 --- a/packages/hunk-jj/src/source.ts +++ b/packages/hunk-jj/src/source.ts @@ -117,7 +117,7 @@ export async function readJjFileSource( }); } catch (error) { logSourceDiagnostic( - `failed to run Jujutsu while reading source ${spec.commitId}:${spec.path}`, + `failed to run Jujutsu while reading source ${spec.commitId}:${spec.path} in ${spec.repoRoot}`, error, ); return null; @@ -146,7 +146,10 @@ export async function readJjFileSource( return tooLarge(error.maxBytes); } - logSourceDiagnostic(`failed to collect Jujutsu source ${spec.commitId}:${spec.path}`, error); + logSourceDiagnostic( + `failed to collect Jujutsu source ${spec.commitId}:${spec.path} in ${spec.repoRoot}`, + error, + ); return null; } diff --git a/test/pty/extensions-integration.test.ts b/test/pty/extensions-integration.test.ts index a7f29e22f..cf7e37bb4 100644 --- a/test/pty/extensions-integration.test.ts +++ b/test/pty/extensions-integration.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, setDefaultTimeout, test } from "bun:test"; -import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import { existsSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; import { join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { createPtyHarness, dragMouse, lineIndexOf } from "./harness"; @@ -43,6 +43,14 @@ afterEach(() => { harness.cleanup(); }); +/** + * The key Hunk records a repo's trust under: its canonical path. A macOS temp dir reads as + * /var/folders/… and resolves to /private/var/folders/…. + */ +function trustKey(repoDir: string) { + return realpathSync.native(repoDir); +} + /** Read the persisted repo-trust decisions from one isolated config home. */ function readTrustState(configHome: string): Record { const statePath = join(configHome, "hunk", "state.json"); @@ -472,7 +480,7 @@ describe("PTY extensions", () => { expect(reloaded).not.toContain("beta.ts"); expect(reloaded).toContain("alpha.ts"); - expect(readTrustState(configHome)[fixture.dir]).toBe("trusted"); + expect(readTrustState(configHome)[trustKey(fixture.dir)]).toBe("trusted"); } finally { session.close(); } @@ -531,7 +539,7 @@ describe("PTY extensions", () => { expect(dismissed).toContain("beta.ts"); expect(dismissed).not.toContain("REPO EXTENSION ACTIVE"); - expect(readTrustState(configHome)[fixture.dir]).toBeUndefined(); + expect(readTrustState(configHome)[trustKey(fixture.dir)]).toBeUndefined(); } finally { session.close(); } @@ -563,7 +571,7 @@ describe("PTY extensions", () => { expect(denied).toContain("beta.ts"); expect(denied).not.toContain("REPO EXTENSION ACTIVE"); - expect(readTrustState(configHome)[fixture.dir]).toBe("denied"); + expect(readTrustState(configHome)[trustKey(fixture.dir)]).toBe("denied"); } finally { session.close(); } diff --git a/test/pty/lifecycle.test.ts b/test/pty/lifecycle.test.ts index dc00410be..96e1724c4 100644 --- a/test/pty/lifecycle.test.ts +++ b/test/pty/lifecycle.test.ts @@ -171,6 +171,22 @@ async function waitForPtyOutput(fd: number, pattern: RegExp, timeoutMs = 20_000) throw new Error(`Timed out waiting for ${pattern} on the PTY. Saw:\n${text}`); } +/** + * Keep reading the PTY until it closes. A PTY buffers little output, so once the test stops + * reading, Hunk's teardown writes fill it and block, and Hunk never exits. + */ +function drainPty(fd: number) { + const buffer = Buffer.alloc(64 * 1024); + const next = () => { + read(fd, buffer, 0, buffer.length, null, (error, bytesRead) => { + if (!error && bytesRead > 0) { + next(); + } + }); + }; + next(); +} + describe("PTY lifecycle", () => { test.skipIf(process.platform === "win32")( "restores a directly launched renderer when SIGTSTP is discarded", @@ -292,6 +308,7 @@ describe("PTY lifecycle", () => { try { await waitForPtyOutput(master, /this is a very long wrapped line/); + drainPty(master); process.kill(child.pid!, signal); await expect(waitForChildExit(child)).resolves.toEqual({ code: 0, signal: null });