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 });