diff --git a/test/extension/e2e/caret-handoff-mention-range.test.ts b/test/extension/e2e/caret-handoff-mention-range.test.ts index a311c5cb..839ac183 100644 --- a/test/extension/e2e/caret-handoff-mention-range.test.ts +++ b/test/extension/e2e/caret-handoff-mention-range.test.ts @@ -16,10 +16,16 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, tick, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDir, + tick, + VIEW_TYPE, +} from "./harness"; const PROTOCOL = 1; const INSERT_AT_MENTIONED = "claude-code.insertAtMentioned"; @@ -45,7 +51,7 @@ describe("caret-handoff does not clobber the ⌘⌥K mention range", function () }); it("keeps the reveal's line-range selection through the insertAtMentioned read", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("mention-range"); tempFile = path.join(dir, "mention-range.md"); await fs.writeFile(tempFile, "line0\nline1\nline2\nline3\n"); const uri = vscode.Uri.file(tempFile); @@ -136,7 +142,7 @@ describe("caret-handoff does not clobber the ⌘⌥K mention range", function () // (2,5) — so the assertion can only pass if the ordinary switch actually // applied the re-reported caret (latch consumed, not stranded; a stranded // latch would skip the apply and leave the fresh editor at its default). - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("mention-range-recovery"); tempFile = path.join(dir, "mention-range-recovery.md"); await fs.writeFile(tempFile, "line0\nline1\nline2\nline3\n"); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/caret-handoff.test.ts b/test/extension/e2e/caret-handoff.test.ts index eb0b7b2e..02a4ce7a 100644 --- a/test/extension/e2e/caret-handoff.test.ts +++ b/test/extension/e2e/caret-handoff.test.ts @@ -1,9 +1,15 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, tick, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDir, + tick, + VIEW_TYPE, +} from "./harness"; const PROTOCOL = 1; @@ -25,7 +31,7 @@ describe("caret-handoff", function () { }); it("caret-report inbound mutates no document and posts no Document event (reducer bypass)", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("caret-bypass"); tempFile = path.join(dir, "caret-bypass.md"); await fs.writeFile(tempFile, "line0\nline1\nline2\n"); const uri = vscode.Uri.file(tempFile); @@ -76,7 +82,7 @@ describe("caret-handoff", function () { }); it("applies the tracked caret to the live text editor on Quoll→text-editor switch", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("caret-apply"); tempFile = path.join(dir, "caret-apply.md"); await fs.writeFile(tempFile, "line0\nline1\nline2\nline3\n"); const uri = vscode.Uri.file(tempFile); @@ -109,7 +115,7 @@ describe("caret-handoff", function () { }); it("posts a caret-apply with the tracked caret on text-editor→Quoll switch (Codex #1)", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("caret-push"); tempFile = path.join(dir, "caret-push.md"); await fs.writeFile(tempFile, "line0\nline1\nline2\nline3\nline4\n"); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/crlf-roundtrip.test.ts b/test/extension/e2e/crlf-roundtrip.test.ts index 3a219cdd..e394209e 100644 --- a/test/extension/e2e/crlf-roundtrip.test.ts +++ b/test/extension/e2e/crlf-roundtrip.test.ts @@ -1,6 +1,5 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; @@ -9,6 +8,7 @@ import { getHarness, isDocumentAfter, isDocumentEvent, + makeTempDir, VIEW_TYPE, } from "./harness"; @@ -47,7 +47,7 @@ describe("crlf-roundtrip", function () { it("preserves \\r\\n bytes end-to-end through the host write path", async () => { // Per-test temp file (mirrors external-edit-propagates) so a mid-test // failure does not leave a shared fixture dirty for subsequent tests. - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-crlf-")); + const dir = await makeTempDir("crlf"); tempFile = path.join(dir, "crlf.md"); // Initial on-disk bytes: pure CRLF. The trailing CRLF after the last // line gives the file two distinct CRLF separators so a single- diff --git a/test/extension/e2e/decoration-external-edit-boundary.test.ts b/test/extension/e2e/decoration-external-edit-boundary.test.ts index e11c0e37..2e57bf05 100644 --- a/test/extension/e2e/decoration-external-edit-boundary.test.ts +++ b/test/extension/e2e/decoration-external-edit-boundary.test.ts @@ -9,10 +9,15 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDir, + VIEW_TYPE, +} from "./harness"; describe("C4a external-edit byte-identity across a token boundary", function () { this.timeout(20000); @@ -36,7 +41,7 @@ describe("C4a external-edit byte-identity across a token boundary", function () // Per-test temp file so a mid-test failure does not leave any shared // fixture dirty for subsequent tests. Mirrors the pattern used by // external-edit-propagates.test.ts. - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-c4a-")); + const dir = await makeTempDir("c4a"); tempFile = path.join(dir, "boundary.md"); const initial = "**bold** rest"; await fs.writeFile(tempFile, initial); diff --git a/test/extension/e2e/dirty-doc-disk-conflict.test.ts b/test/extension/e2e/dirty-doc-disk-conflict.test.ts index 8ef06e48..b38c89b4 100644 --- a/test/extension/e2e/dirty-doc-disk-conflict.test.ts +++ b/test/extension/e2e/dirty-doc-disk-conflict.test.ts @@ -12,7 +12,6 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { @@ -20,6 +19,7 @@ import { getHarness, isDocumentAfter, isDocumentEvent, + makeTempDir, VIEW_TYPE, } from "./harness"; @@ -51,7 +51,7 @@ describe("dirty-doc-disk-conflict", function () { // Open the temp file in Quoll, seed, then dirty it via an in-session edit that // does NOT match disk. Returns the seed docVersion. async function openAndDirty(bodyEdit: string): Promise { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-dirty-conflict-")); + tempDir = await makeTempDir("dirty-conflict"); tempFile = path.join(tempDir, "dirty-conflict.md"); await fs.writeFile(tempFile, "# Initial\n\nbody\n"); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/external-edit-propagates.test.ts b/test/extension/e2e/external-edit-propagates.test.ts index dc5b5e88..eeb15a60 100644 --- a/test/extension/e2e/external-edit-propagates.test.ts +++ b/test/extension/e2e/external-edit-propagates.test.ts @@ -1,6 +1,5 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; @@ -10,6 +9,7 @@ import { getHarness, isDocumentAfter, isDocumentEvent, + makeTempDir, tick, VIEW_TYPE, } from "./harness"; @@ -36,7 +36,7 @@ describe("external-edit-propagates", function () { it("propagates an externally-applied WorkspaceEdit as a higher-docVersion Document", async () => { // Per-test temp file so a mid-test failure does not leave the // shared fixture dirty for subsequent tests. - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("ext-edit"); tempFile = path.join(dir, "ext-edit.md"); await fs.writeFile(tempFile, "# Initial\n\nbody\n"); const uri = vscode.Uri.file(tempFile); @@ -75,7 +75,7 @@ describe("external-edit-propagates", function () { }); it("coalesces a burst of lock-free external edits into fewer Document posts (latest wins)", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("ext-edit-burst"); tempFile = path.join(dir, "ext-edit-burst.md"); await fs.writeFile(tempFile, "# Initial\n\nbody\n"); const uri = vscode.Uri.file(tempFile); @@ -143,7 +143,7 @@ describe("external-edit-propagates", function () { }); it("dispatches a lock-held racing external edit immediately (refused settlement posts the live version)", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("lock-race"); tempFile = path.join(dir, "lock-race.md"); await fs.writeFile(tempFile, "# Initial\n\nbody\n"); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/external-fs-write-propagates.test.ts b/test/extension/e2e/external-fs-write-propagates.test.ts index c9620947..8dc90733 100644 --- a/test/extension/e2e/external-fs-write-propagates.test.ts +++ b/test/extension/e2e/external-fs-write-propagates.test.ts @@ -33,7 +33,6 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { @@ -41,6 +40,7 @@ import { getHarness, isDocumentAfter, isDocumentEvent, + makeTempDir, VIEW_TYPE, } from "./harness"; import type { DocumentMessageShape, RecordedEventShape } from "./types"; @@ -70,7 +70,7 @@ describe("external-fs-write-propagates", function () { it("propagates an out-of-process fs.writeFile as a higher-docVersion Document", async () => { // Per-test temp dir so a mid-test failure does not leave the shared // fixture dirty for subsequent tests (mirrors external-edit-propagates). - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-fswrite-")); + tempDir = await makeTempDir("fswrite"); tempFile = path.join(tempDir, "ext-fs-write.md"); await fs.writeFile(tempFile, "# Initial\n\nbody\n"); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/format-document-active-edge.test.ts b/test/extension/e2e/format-document-active-edge.test.ts index cf8987ad..b8d763b9 100644 --- a/test/extension/e2e/format-document-active-edge.test.ts +++ b/test/extension/e2e/format-document-active-edge.test.ts @@ -1,10 +1,9 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; -import { cleanupBetweenTests, getHarness, tick, VIEW_TYPE } from "./harness"; +import { cleanupBetweenTests, getHarness, makeTempDir, tick, VIEW_TYPE } from "./harness"; import type { PanelControlsShape, TestHarnessShape } from "./types"; // Pins the host-side routing of `quoll.formatDocument`: the command forwards a @@ -43,7 +42,7 @@ async function openTempQuoll( slug: string, previous: PanelControlsShape | null ): Promise<{ uri: vscode.Uri; file: string; panel: PanelControlsShape }> { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), `quoll-fmtdoc-${slug}-`)); + const dir = await makeTempDir(`fmtdoc-${slug}`); const file = path.join(dir, `${slug}.md`); await fs.writeFile(file, content); const uri = vscode.Uri.file(file); diff --git a/test/extension/e2e/handoff-edit-applied-barrier.test.ts b/test/extension/e2e/handoff-edit-applied-barrier.test.ts index f9e9325d..6294d107 100644 --- a/test/extension/e2e/handoff-edit-applied-barrier.test.ts +++ b/test/extension/e2e/handoff-edit-applied-barrier.test.ts @@ -12,7 +12,6 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; @@ -21,6 +20,7 @@ import { deferred, getHarness, isDocumentEvent, + makeTempDir, tick, VIEW_TYPE, } from "./harness"; @@ -41,7 +41,7 @@ describe("handoff edit-applied barrier", function () { // its uri. The seed content is short (3 lines) so the applied 40-line clamp // is non-vacuous. const openTempDoc = async (): Promise => { - tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-barrier-")); + tempDir = await makeTempDir("barrier"); const tempFile = path.join(tempDir, "barrier.md"); await fs.writeFile(tempFile, "# seed\n\nbody\n"); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/harness.ts b/test/extension/e2e/harness.ts index 8516cc1d..4301c72e 100644 --- a/test/extension/e2e/harness.ts +++ b/test/extension/e2e/harness.ts @@ -1,7 +1,7 @@ import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; +import { makeTempDir } from "../temp-root"; import type { DocumentMessageShape, EditorConfigMessageShape, @@ -15,6 +15,14 @@ import type { export const EXTENSION_ID = "mtskf.quoll"; export const VIEW_TYPE = "quoll.editMarkdown"; +// Every temp dir an E2E suite creates lives under the run root that +// launch.ts made and disposes on exit — that single owner is why no suite +// needs dir teardown of its own, and why nothing here ever globs +// `quoll-e2e-*` (a parallel run owns its own root). Suites must NOT call +// fs.mkdtemp(os.tmpdir(), …); test/extension/temp-dir-choke-point.test.ts +// enforces that. Re-exported so suites keep importing from "./harness". +export { makeTempDir, makeTempDirSync } from "../temp-root"; + // __dirname at runtime is `out/test-e2e/e2e/`. Resolve up to the // repo root then back into the source-controlled fixtures directory. // Avoids needing to copy *.md into out/ as a build step. @@ -178,7 +186,7 @@ export async function openTempQuoll( slug: string, previous: PanelControlsShape | null ): Promise<{ uri: vscode.Uri; file: string; panel: PanelControlsShape }> { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), `quoll-e2e-${slug}-`)); + const dir = await makeTempDir(slug); const file = path.join(dir, `${slug}.md`); await fs.writeFile(file, content); const uri = vscode.Uri.file(file); diff --git a/test/extension/e2e/hidden-webview-resync.test.ts b/test/extension/e2e/hidden-webview-resync.test.ts index 232f8028..39f12488 100644 --- a/test/extension/e2e/hidden-webview-resync.test.ts +++ b/test/extension/e2e/hidden-webview-resync.test.ts @@ -1,9 +1,15 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, tick, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDir, + tick, + VIEW_TYPE, +} from "./harness"; describe("hidden-webview-resync", function () { this.timeout(25000); @@ -24,7 +30,7 @@ describe("hidden-webview-resync", function () { }); it("posts a fresh Document when a hidden panel becomes visible after an external edit", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-")); + const dir = await makeTempDir("hidden"); tempFile = path.join(dir, "hidden.md"); await fs.writeFile(tempFile, "# Original\n"); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/host-rejects-edit-preserves-webview.test.ts b/test/extension/e2e/host-rejects-edit-preserves-webview.test.ts index 7bbd85bf..ac91a3a2 100644 --- a/test/extension/e2e/host-rejects-edit-preserves-webview.test.ts +++ b/test/extension/e2e/host-rejects-edit-preserves-webview.test.ts @@ -1,6 +1,5 @@ import * as assert from "node:assert"; import * as fs from "node:fs"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; @@ -10,6 +9,7 @@ import { getHarness, hideQuollByOpeningOtherDoc, isDocumentEvent, + makeTempDirSync, openFixtureWithQuoll, tick, VIEW_TYPE, @@ -22,7 +22,7 @@ const isEditRejectedEvent = (e: { message: { type: string } }) => // resumption case, and the revert-check of the deferred-race guard) writes to a // throwaway file instead of mutating a committed fixture. function tempMd(name: string): vscode.Uri { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "quoll-reject-")); + const dir = makeTempDirSync("reject"); const p = path.join(dir, name); fs.writeFileSync(p, "# Title\n\nbody\n", "utf8"); return vscode.Uri.file(p); diff --git a/test/extension/e2e/lint-diagnostics-propagate.test.ts b/test/extension/e2e/lint-diagnostics-propagate.test.ts index babf967c..4acdaf51 100644 --- a/test/extension/e2e/lint-diagnostics-propagate.test.ts +++ b/test/extension/e2e/lint-diagnostics-propagate.test.ts @@ -1,9 +1,8 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, VIEW_TYPE } from "./harness"; +import { cleanupBetweenTests, getHarness, makeTempDir, VIEW_TYPE } from "./harness"; // Poll vscode.languages.getDiagnostics(uri) until `predicate` holds or the // deadline passes. Lint is debounced (250ms) in the webview, then posted across @@ -52,7 +51,7 @@ describe("lint-diagnostics-propagate", function () { }); it("mirrors lint into Problems with correct range, updates on fix, reopens, clears on close", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-lint-e2e-")); + const dir = await makeTempDir("lint"); tempFile = path.join(dir, "heading-skip.md"); // h1 -> h3 skips h2: heading-increment (MD001-equivalent) warning on "### Skip". await fs.writeFile(tempFile, "# Title\n\n### Skip\n"); @@ -113,7 +112,7 @@ describe("lint-diagnostics-propagate", function () { }); it("maps ranges correctly for a CRLF document (line/character is EOL-invariant)", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-lint-crlf-")); + const dir = await makeTempDir("lint-crlf"); tempFile = path.join(dir, "crlf.md"); // Same violation, CRLF line endings. An offset-based wire would mis-place // the range (CM is LF-internal, the TextDocument is CRLF); line/character @@ -147,7 +146,7 @@ describe("lint-diagnostics-propagate", function () { // (Task 3) and toLintDiagnostics is host-document-independent (Task 2), so the // host reproduces exactly the ranges the webview computed for its content. it("surfaces a violation introduced by an external edit, at the correct line", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-lint-dyn-")); + const dir = await makeTempDir("lint-dyn"); tempFile = path.join(dir, "baseline.md"); // Line 0 carries a single trailing space → a STABLE `no-trailing-spaces` // finding (a single trailing space is flagged; only exactly two on a diff --git a/test/extension/e2e/lint-problems-config-toggle.test.ts b/test/extension/e2e/lint-problems-config-toggle.test.ts index 5fea45fc..8e0f469c 100644 --- a/test/extension/e2e/lint-problems-config-toggle.test.ts +++ b/test/extension/e2e/lint-problems-config-toggle.test.ts @@ -1,9 +1,8 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, VIEW_TYPE } from "./harness"; +import { cleanupBetweenTests, getHarness, makeTempDir, VIEW_TYPE } from "./harness"; const KEY = "quoll.lint.problems.enabled"; @@ -73,7 +72,7 @@ describe("lint-problems-config-toggle", function () { // depend on inherited settings state). await vscode.workspace.getConfiguration().update(KEY, true, vscode.ConfigurationTarget.Global); - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-lint-toggle-")); + const dir = await makeTempDir("lint-toggle"); tempFile = path.join(dir, "heading-skip.md"); // h1 -> h3 skips h2: a heading-increment warning on "### Skip" (line 2). await fs.writeFile(tempFile, "# Title\n\n### Skip\n"); diff --git a/test/extension/e2e/minimal-range-apply.test.ts b/test/extension/e2e/minimal-range-apply.test.ts index 7d1d0c96..2b6a40f9 100644 --- a/test/extension/e2e/minimal-range-apply.test.ts +++ b/test/extension/e2e/minimal-range-apply.test.ts @@ -1,10 +1,15 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDir, + VIEW_TYPE, +} from "./harness"; describe("minimal-range-apply", function () { this.timeout(20000); @@ -23,7 +28,7 @@ describe("minimal-range-apply", function () { }); it("emits a minimal WorkspaceEdit range, not a whole-document replace", async () => { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-min-")); + const dir = await makeTempDir("min"); tempFile = path.join(dir, "min.md"); const base = "# Title\n\nhello world\n"; await fs.writeFile(tempFile, base); diff --git a/test/extension/e2e/mixed-eol-roundtrip.test.ts b/test/extension/e2e/mixed-eol-roundtrip.test.ts index 9567ae51..d2550bfb 100644 --- a/test/extension/e2e/mixed-eol-roundtrip.test.ts +++ b/test/extension/e2e/mixed-eol-roundtrip.test.ts @@ -1,9 +1,14 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDir, + VIEW_TYPE, +} from "./harness"; /** * Mixed / CR-only EOL round-trip (TODO: webview-mixed-eol-roundtrip). @@ -44,7 +49,7 @@ describe("mixed-eol-roundtrip", function () { }); async function assertContract(originalBytes: Buffer): Promise { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-mixedeol-")); + const dir = await makeTempDir("mixedeol"); tempFile = path.join(dir, "doc.md"); // `as Uint8Array` bridges a Buffer vs Uint8Array // invariance in @types/node@20.16.0 + TS 5.9 (fs/Buffer.equals signatures); diff --git a/test/extension/e2e/racing-applyedit-semantics.test.ts b/test/extension/e2e/racing-applyedit-semantics.test.ts index 55a3bb08..804afc49 100644 --- a/test/extension/e2e/racing-applyedit-semantics.test.ts +++ b/test/extension/e2e/racing-applyedit-semantics.test.ts @@ -1,10 +1,16 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, tick, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDir, + tick, + VIEW_TYPE, +} from "./harness"; // Plan S5 — experiment: racing positional applyEdit semantics (#7, gates S6). // @@ -70,7 +76,7 @@ describe("racing-applyedit-semantics (Plan S5 experiment)", function () { }); async function openTemp(base: string): Promise { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), "quoll-e2e-s5-")); + const dir = await makeTempDir("s5"); tempFile = path.join(dir, "race.md"); await fs.writeFile(tempFile, base); const uri = vscode.Uri.file(tempFile); diff --git a/test/extension/e2e/remember-last-editor-surface.test.ts b/test/extension/e2e/remember-last-editor-surface.test.ts index 4855cc88..e31a3680 100644 --- a/test/extension/e2e/remember-last-editor-surface.test.ts +++ b/test/extension/e2e/remember-last-editor-surface.test.ts @@ -10,14 +10,20 @@ import * as assert from "node:assert"; import * as fs from "node:fs"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, tick, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDirSync, + tick, + VIEW_TYPE, +} from "./harness"; function tempMd(name: string): vscode.Uri { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "quoll-surface-")); + const dir = makeTempDirSync("surface"); const p = path.join(dir, name); fs.writeFileSync(p, "# Title\n\nbody\n", "utf8"); return vscode.Uri.file(p); diff --git a/test/extension/e2e/status-bar-active-edge.test.ts b/test/extension/e2e/status-bar-active-edge.test.ts index 1b345bd5..67b47580 100644 --- a/test/extension/e2e/status-bar-active-edge.test.ts +++ b/test/extension/e2e/status-bar-active-edge.test.ts @@ -1,9 +1,8 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; -import { cleanupBetweenTests, getHarness, tick, VIEW_TYPE } from "./harness"; +import { cleanupBetweenTests, getHarness, makeTempDir, tick, VIEW_TYPE } from "./harness"; import type { PanelControlsShape, StatusBarItemProbeShape, TestHarnessShape } from "./types"; // Pins the panel-side status-bar wiring PR #158 left untested: the item shows on @@ -49,7 +48,7 @@ async function openTempQuoll( previous: PanelControlsShape | null, openOptions?: vscode.TextDocumentShowOptions ): Promise<{ uri: vscode.Uri; file: string; panel: PanelControlsShape }> { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), `quoll-sbar-${slug}-`)); + const dir = await makeTempDir(`sbar-${slug}`); const file = path.join(dir, `${slug}.md`); await fs.writeFile(file, content); const uri = vscode.Uri.file(file); diff --git a/test/extension/e2e/toggle-editor-in-place-swap.test.ts b/test/extension/e2e/toggle-editor-in-place-swap.test.ts index f36f46ac..6798a455 100644 --- a/test/extension/e2e/toggle-editor-in-place-swap.test.ts +++ b/test/extension/e2e/toggle-editor-in-place-swap.test.ts @@ -8,14 +8,20 @@ import * as assert from "node:assert"; import * as fs from "node:fs"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; -import { cleanupBetweenTests, getHarness, isDocumentEvent, tick, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isDocumentEvent, + makeTempDirSync, + tick, + VIEW_TYPE, +} from "./harness"; function tempMd(name: string): vscode.Uri { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "quoll-swap-")); + const dir = makeTempDirSync("swap"); const p = path.join(dir, name); fs.writeFileSync(p, "# Title\n\nbody\n", "utf8"); return vscode.Uri.file(p); diff --git a/test/extension/e2e/two-panel-config-caret.test.ts b/test/extension/e2e/two-panel-config-caret.test.ts index 95085c2f..cf63da46 100644 --- a/test/extension/e2e/two-panel-config-caret.test.ts +++ b/test/extension/e2e/two-panel-config-caret.test.ts @@ -1,10 +1,16 @@ import * as assert from "node:assert"; import * as fs from "node:fs/promises"; -import * as os from "node:os"; import * as path from "node:path"; import * as vscode from "vscode"; import { PROTOCOL_VERSION } from "./constants"; -import { cleanupBetweenTests, getHarness, isEditorConfigEvent, tick, VIEW_TYPE } from "./harness"; +import { + cleanupBetweenTests, + getHarness, + isEditorConfigEvent, + makeTempDir, + tick, + VIEW_TYPE, +} from "./harness"; import type { PanelControlsShape, RecordedEventShape, TestHarnessShape } from "./types"; const GUTTER_KEY = "quoll.lint.gutter.enabled"; @@ -42,7 +48,7 @@ async function openTempQuoll( slug: string, previous: PanelControlsShape | null ): Promise<{ uri: vscode.Uri; file: string; panel: PanelControlsShape }> { - const dir = await fs.mkdtemp(path.join(os.tmpdir(), `quoll-2panel-${slug}-`)); + const dir = await makeTempDir(`2panel-${slug}`); const file = path.join(dir, `${slug}.md`); await fs.writeFile(file, content); const uri = vscode.Uri.file(file); diff --git a/test/extension/launch-wiring.test.ts b/test/extension/launch-wiring.test.ts new file mode 100644 index 00000000..c6fc9e80 --- /dev/null +++ b/test/extension/launch-wiring.test.ts @@ -0,0 +1,151 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { runE2E } from "./launch"; +import type { RunTempRoot } from "./temp-root"; + +// The reclaim this PR exists for lives in launch.ts's wiring, not in the seam: +// which env key carries the root, which dir VS Code is given, and that +// dispose() runs on BOTH the pass and the fail path. Restoring `process.exit(1)` +// in the catch (the conventional CLI idiom) would keep every other suite green +// while silently skipping the finally — so pin it here. +const fakeRoot = (): RunTempRoot & { dispose: ReturnType } => ({ + root: "/t/r", + userDataDir: "/t/r/ud", + workDir: "/t/r/w", + dispose: vi.fn(), +}); + +const noopPreflight = (): void => undefined; + +afterEach(() => { + process.exitCode = undefined; + vi.restoreAllMocks(); +}); + +describe("runE2E wiring", () => { + it("hands the run root to the host and the user-data dir to VS Code", async () => { + const run = fakeRoot(); + const runTests = vi.fn().mockResolvedValue(0); + await runE2E({ runTests, createRoot: () => run, preflight: noopPreflight }); + + const opts = runTests.mock.calls[0][0]; + expect(opts.extensionTestsEnv).toEqual({ QUOLL_E2E_TEMP_ROOT: "/t/r" }); + expect(opts.launchArgs).toContain("--user-data-dir=/t/r/ud"); + expect(run.dispose).toHaveBeenCalledTimes(1); + expect(process.exitCode).toBeUndefined(); + }); + + it("still reclaims the root when the suite fails", async () => { + const run = fakeRoot(); + const runTests = vi.fn().mockRejectedValue(new Error("suite failed")); + vi.spyOn(console, "error").mockImplementation(() => undefined); + + await runE2E({ runTests, createRoot: () => run, preflight: noopPreflight }); + + expect(run.dispose).toHaveBeenCalledTimes(1); + expect(process.exitCode).toBe(1); + }); + + it("fails the run loudly when the root cannot be reclaimed", async () => { + const run = fakeRoot(); + run.dispose.mockImplementation(() => { + throw new Error("EBUSY"); + }); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + + await runE2E({ + runTests: vi.fn().mockResolvedValue(0), + createRoot: () => run, + preflight: noopPreflight, + }); + + expect(process.exitCode).toBe(1); + expect(errorSpy.mock.calls[0][0]).toContain("/t/r"); + }); + + it("never creates a root it would have no finally to reclaim", async () => { + // The preflight must run BEFORE createRoot: a throw from after it escapes + // with no finally, stranding the root. Ordering, not just the throw. + const createRoot = vi.fn(); + const preflight = vi.fn(() => { + throw new Error("[e2e] FIXTURES_DIR misresolved"); + }); + await expect(runE2E({ runTests: vi.fn(), createRoot, preflight })).rejects.toThrow( + /FIXTURES_DIR/ + ); + expect(createRoot).not.toHaveBeenCalled(); + }); +}); + +describe("runE2E signal wiring", () => { + it("stays passive while the library owns SIGINT, then reclaims and exits 130", async () => { + // Post-spawn, @vscode/test-electron's ctrlc1 is also listening (simulated + // here). We must NOT reclaim mid-run — the library is still gracefully + // closing the child — but the run must not report a pass either. + const run = fakeRoot(); + const before = process.listenerCount("SIGINT"); + const libraryHandler = (): void => undefined; + let duringRun = 0; + const runTests = vi.fn().mockImplementation(async () => { + // ctrlc1 is registered inside innerRunTests, i.e. only once the child has + // spawned — so it appears AFTER ours, which is what the baseline check + // detects. + process.on("SIGINT", libraryHandler); + duringRun = process.listenerCount("SIGINT"); + process.emit("SIGINT"); + expect(run.dispose).not.toHaveBeenCalled(); // passive: the finally reclaims + process.removeListener("SIGINT", libraryHandler); + return 0; + }); + + await runE2E({ runTests, createRoot: () => run, preflight: noopPreflight }); + + expect(duringRun).toBe(before + 2); + expect(run.dispose).toHaveBeenCalledTimes(1); + expect(process.exitCode).toBe(130); + expect(process.listenerCount("SIGINT")).toBe(before); + }); + + it("reclaims and exits itself when nothing else is listening (pre-spawn Ctrl+C)", async () => { + // Before the child spawns there is no ctrlc1, and merely holding a listener + // suppresses Node's default terminate — so Ctrl+C would otherwise do + // nothing at all for the whole download window. + const run = fakeRoot(); + const exit = vi.spyOn(process, "exit").mockImplementation((() => undefined) as never); + const runTests = vi.fn().mockImplementation(async () => { + process.emit("SIGINT"); + return 0; + }); + + await runE2E({ runTests, createRoot: () => run, preflight: noopPreflight }); + + expect(exit).toHaveBeenCalledWith(130); + // Before the exit, not merely "at some point" — the finally would satisfy that. + expect(run.dispose.mock.invocationCallOrder[0]).toBeLessThan(exit.mock.invocationCallOrder[0]); + }); + + it("does not downgrade a real failure to 130", async () => { + const run = fakeRoot(); + const libraryHandler = (): void => undefined; + vi.spyOn(console, "error").mockImplementation(() => undefined); + const runTests = vi.fn().mockImplementation(async () => { + process.on("SIGINT", libraryHandler); + process.emit("SIGINT"); + process.removeListener("SIGINT", libraryHandler); + throw new Error("suite failed"); + }); + + await runE2E({ runTests, createRoot: () => run, preflight: noopPreflight }); + + expect(process.exitCode).toBe(1); // the `??=`, not `=` + }); + + it("leaves no signal listeners behind on the happy path", async () => { + const before = process.listenerCount("SIGINT"); + await runE2E({ + runTests: vi.fn().mockResolvedValue(0), + createRoot: fakeRoot, + preflight: noopPreflight, + }); + expect(process.listenerCount("SIGINT")).toBe(before); + }); +}); diff --git a/test/extension/launch.ts b/test/extension/launch.ts index 17874028..7d480ae7 100644 --- a/test/extension/launch.ts +++ b/test/extension/launch.ts @@ -4,9 +4,9 @@ // engines.vscode bumps in package.json, this constant bumps with it. import * as fs from "node:fs"; -import * as os from "node:os"; import * as path from "node:path"; import { runTests } from "@vscode/test-electron"; +import { createRunTempRoot, RUN_TEMP_ROOT_ENV } from "./temp-root"; const VS_CODE_VERSION = "1.94.0"; @@ -18,41 +18,123 @@ const VS_CODE_VERSION = "1.94.0"; // starts; the previous module-load-time `existsSync` inside harness.ts // ran on every test file's first require and crashed the Electron // runner with no mocha context, which triaged as an activation bug. +// Throws rather than process.exit()ing, and runs BEFORE the run root exists. +// Both halves matter: exit() would skip the reclaim in the finally, and a throw +// from after createRoot() escapes with no finally to reclaim either. The +// ordering is load-bearing, so it rides the LaunchDeps seam and is pinned by +// launch-wiring.test.ts rather than left to a comment. function preflightFixturesDir(): void { // __dirname at runtime is `out/test-e2e/`. Resolve up to the repo // root then back into the source-controlled fixtures directory. const fixturesDir = path.resolve(__dirname, "../..", "test/extension/e2e/fixtures"); if (!fs.existsSync(fixturesDir)) { - console.error( + throw new Error( `[e2e] FIXTURES_DIR misresolved: ${fixturesDir} — tsconfig outDir may have changed` ); - process.exit(1); } } -async function main(): Promise { +/** Injectable seam so the wiring below — which env key carries the root, which + * dir VS Code gets, and that dispose() runs on BOTH the pass and fail paths — + * is unit-testable without downloading and spawning Electron. */ +export interface LaunchDeps { + runTests: typeof runTests; + createRoot: typeof createRunTempRoot; + preflight: typeof preflightFixturesDir; +} + +export async function runE2E( + deps: LaunchDeps = { runTests, createRoot: createRunTempRoot, preflight: preflightFixturesDir } +): Promise { + deps.preflight(); + + // VS Code creates a unix-domain IPC socket under user-data-dir; on + // macOS the socket path must fit in 103 chars. The repo path under + // ~/Dev/... + worktree name routinely exceeds that, so we put the + // user-data-dir in a short tmp path. mkdtemp guarantees a unique + // root per run so parallel CI shards don't collide. + // + // The root also parents every per-test workspace dir — the suites + // allocate through temp-root's makeTempDir, which reads the root from + // RUN_TEMP_ROOT_ENV below — so this one dispose() reclaims the run's + // whole tmp footprint. Before it, every run stranded its user-data dir + // plus one dir per temp-file test, forever (+53 dirs per run measured + // 2026-08-14, 2 544 accumulated). + const run = deps.createRoot(); + // Own SIGINT ourselves. @vscode/test-electron ends innerRunTests with + // `if (exitRequested && process.listenerCount('SIGINT') === 0) process.exit(1)` + // — so without a listener here, the FIRST Ctrl+C (graceful close, child exits + // 0) hard-exits this process from inside the await and the finally below + // never runs, stranding the whole root. Holding a listener keeps the exit on + // our side; a second Ctrl+C still force-closes through the library's own + // handler. + // + // Baseline rather than a bare count: "has the library taken over?" means "did + // a listener appear after ours", which reads correctly in a bare `node` run + // (0 → 1) and under any host that already listens. + const sigintListenersBefore = process.listenerCount("SIGINT"); + let interrupted = false; + const onSigint = (): void => { + interrupted = true; + // Post-spawn, the library's ctrlc1 has appeared and does the graceful child + // close, so we stay passive and reclaim in the finally. Pre-spawn — during + // downloadAndUnzipVSCode, minutes on a cold cache — ctrlc1 does not exist + // yet, and merely holding this listener suppresses Node's default + // termination. Without this branch Ctrl+C would be a dead key for that + // whole window. + if (process.listenerCount("SIGINT") <= sigintListenersBefore + 1) { + try { + run.dispose(); + } catch (err) { + console.error(`[e2e] failed to reclaim temp root ${run.root}:`, err); + } + process.exit(130); // 128 + SIGINT + } + }; + process.on("SIGINT", onSigint); + // SIGTERM is deliberately NOT handled. The library exposes no child pid and + // no graceful stop, so a handler could only reclaim the root while VS Code + // may still be running — and a plain `kill` of this process alone leaves the + // child alive to recreate user-data-dir under the path just removed, trading + // one stranded root for a stranded root plus an orphaned editor. An aborted + // run keeping its own single root is the accepted outcome. try { - preflightFixturesDir(); const extensionDevelopmentPath = path.resolve(__dirname, "../.."); const extensionTestsPath = path.resolve(__dirname, "./e2e/index"); - // VS Code creates a unix-domain IPC socket under user-data-dir; on - // macOS the socket path must fit in 103 chars. The repo path under - // ~/Dev/... + worktree name routinely exceeds that, so we put the - // user-data-dir in a short tmp path. mkdtemp guarantees a unique - // dir per run so parallel CI shards don't collide. - const userDataDir = fs.mkdtempSync(path.join(os.tmpdir(), "quoll-e2e-")); - - await runTests({ + await deps.runTests({ version: VS_CODE_VERSION, extensionDevelopmentPath, extensionTestsPath, - launchArgs: ["--disable-extensions", `--user-data-dir=${userDataDir}`], + extensionTestsEnv: { [RUN_TEMP_ROOT_ENV]: run.root }, + launchArgs: ["--disable-extensions", `--user-data-dir=${run.userDataDir}`], }); } catch (err) { console.error("Failed to run E2E tests:", err); - process.exit(1); + // exitCode, not process.exit: exit() skips the finally below and would + // strand the whole root on every failing run. + process.exitCode = 1; + } finally { + process.removeListener("SIGINT", onSigint); + if (interrupted) { + // An interrupted run is not a pass — without this it would exit 0. + process.exitCode ??= 130; // 128 + SIGINT + } + try { + run.dispose(); + } catch (err) { + // Never silent — an unreclaimable root is the bug this owns. + console.error(`[e2e] failed to reclaim temp root ${run.root}:`, err); + process.exitCode = 1; + } } } -void main(); +// `require.main === module` holds only under `node out/test-e2e/launch.js`, so +// the wiring test can import runE2E without spawning anything. +if (require.main === module) { + void runE2E().catch((err) => { + console.error(err instanceof Error ? err.message : err); + process.exitCode = 1; + }); +} diff --git a/test/extension/temp-dir-choke-point.test.ts b/test/extension/temp-dir-choke-point.test.ts new file mode 100644 index 00000000..13a3f09a --- /dev/null +++ b/test/extension/temp-dir-choke-point.test.ts @@ -0,0 +1,156 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; +import ts from "typescript"; +import { describe, expect, it } from "vitest"; + +// Default-deny: a suite that allocates straight from os.tmpdir() strands +// that dir forever — 2 544 accumulated, +53 per run, measured 2026-08-14. +// The only sanctioned allocation is temp-root.ts, whose dirs live under the +// per-run root that launch.ts disposes on exit. +// +// AST, not regex: a source-text scan trips on the word `mkdtemp` inside a +// comment or a string (this very file would trip it) and cannot tell a call +// from a mention. +const SCAN_ROOT = path.resolve(__dirname); +const BANNED = new Set(["mkdtemp", "mkdtempSync", "tmpdir"]); + +// The seam itself allocates — that is its job. +const ALLOCATION_SITE = path.join(SCAN_ROOT, "temp-root.ts"); +// The seam's test may NAME os.tmpdir() to pin where the root lands, but it +// still may not allocate: `mkdtemp*` stays banned there. +const TMPDIR_REFERENCE_ALLOWED = new Set([path.join(SCAN_ROOT, "temp-root.test.ts")]); + +function collectSources(dir: string, acc: string[] = []): string[] { + for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) { + if (entry.name === "node_modules" || entry.name === "fixtures") { + continue; + } + collectSources(full, acc); + } else if (entry.name.endsWith(".ts")) { + acc.push(full); + } + } + return acc; +} + +function calleeName(node: ts.CallExpression): string { + const target = node.expression; + if (ts.isPropertyAccessExpression(target)) { + return target.name.text; + } + // fs["mkdtempSync"](…) — an element access is still a call to the same thing. + if (ts.isElementAccessExpression(target) && ts.isStringLiteralLike(target.argumentExpression)) { + return target.argumentExpression.text; + } + if (ts.isIdentifier(target)) { + return target.text; + } + return ""; +} + +/** BANNED plus whatever local names those symbols were imported AS — without + * this, `import { mkdtemp as mkTmp }` walks straight past the guard, and that + * rename is something an import-organiser can introduce mechanically. */ +function bannedNamesIn(source: ts.SourceFile): Set { + const names = new Set(BANNED); + source.forEachChild((node) => { + if (!ts.isImportDeclaration(node)) { + return; + } + const named = node.importClause?.namedBindings; + if (named && ts.isNamedImports(named)) { + for (const spec of named.elements) { + if (BANNED.has((spec.propertyName ?? spec.name).text)) { + names.add(spec.name.text); + } + } + } + }); + return names; +} + +function findOffenders(source: ts.SourceFile, label: string): string[] { + const banned = bannedNamesIn(source); + const offenders: string[] = []; + const visit = (node: ts.Node): void => { + if (ts.isCallExpression(node) && banned.has(calleeName(node))) { + const { line } = source.getLineAndCharacterOfPosition(node.getStart(source)); + offenders.push(`${label}:${line + 1} ${calleeName(node)}()`); + } + ts.forEachChild(node, visit); + }; + visit(source); + return offenders; +} + +describe("e2e temp-dir choke point", () => { + const sources = collectSources(SCAN_ROOT).filter((file) => file !== ALLOCATION_SITE); + + it("scans the whole test/extension tree", () => { + // Without this, a moved directory would make the scan below vacuously + // pass over an empty file list. + expect(sources.length).toBeGreaterThan(25); + expect(sources.some((f) => f.endsWith(path.join("e2e", "harness.ts")))).toBe(true); + expect(sources.some((f) => f.endsWith("launch.ts"))).toBe(true); + }); + + it("finds no temp-dir allocation outside temp-root.ts", () => { + const offenders: string[] = []; + for (const file of sources) { + const source = ts.createSourceFile( + file, + fs.readFileSync(file, "utf8"), + ts.ScriptTarget.ES2022, + true + ); + const found = findOffenders(source, path.relative(SCAN_ROOT, file)); + // The seam's own test may NAME os.tmpdir() to pin where the root lands; + // it still may not allocate, so only `tmpdir` is exempted there. + offenders.push( + ...(TMPDIR_REFERENCE_ALLOWED.has(file) + ? found.filter((entry) => !entry.endsWith("tmpdir()")) + : found) + ); + } + expect( + offenders, + "use makeTempDir/makeTempDirSync from ./harness — temp-root.ts is the only allocation site" + ).toEqual([]); + }); + + it("catches the bypasses a rename or an indexed call would open", () => { + // Exercises the branches above, which no real source file currently hits — + // without this they would be dead code that silently stops working. + const probe = ts.createSourceFile( + "probe.ts", + [ + 'import { mkdtemp as mkTmp } from "node:fs/promises";', + 'import * as os from "node:os";', + 'await mkTmp(os.tmpdir() + "/x-");', + 'fs["mkdtempSync"]("/tmp/y-");', + ].join("\n"), + ts.ScriptTarget.ES2022, + true + ); + expect(findOffenders(probe, "probe.ts")).toEqual([ + "probe.ts:3 mkTmp()", + "probe.ts:3 tmpdir()", + "probe.ts:4 mkdtempSync()", + ]); + + // Banning `tmpdir` is what stops a hand-rolled + // `fs.mkdirSync(path.join(os.tmpdir(), …))` — `mkdirSync` is deliberately + // not in BANNED — so the exemption must stay scoped to the seam's own + // test. Widening it tree-wide is otherwise a green edit. + const tmpdirProbe = ts.createSourceFile( + "probe.ts", + 'import * as os from "node:os";\nfs.mkdirSync(os.tmpdir() + "/x");', + ts.ScriptTarget.ES2022, + true + ); + expect(findOffenders(tmpdirProbe, "probe.ts")).toEqual(["probe.ts:2 tmpdir()"]); + expect([...TMPDIR_REFERENCE_ALLOWED]).toEqual([path.join(SCAN_ROOT, "temp-root.test.ts")]); + }); +}); diff --git a/test/extension/temp-root-partial-init.test.ts b/test/extension/temp-root-partial-init.test.ts new file mode 100644 index 00000000..9c4f5d94 --- /dev/null +++ b/test/extension/temp-root-partial-init.test.ts @@ -0,0 +1,44 @@ +import * as path from "node:path"; +import { describe, expect, it, vi } from "vitest"; + +// The rollback that reclaims a half-built root is the one branch whose whole +// job is "do not leak", in a module whose whole purpose is not leaking — so it +// gets a test even though the obvious route fails. +// +// `vi.spyOn(fs, "mkdirSync")` throws "Cannot redefine property": the node:fs +// namespace exposes non-configurable properties. `vi.mock` redefines nothing — +// it swaps the module-registry entry the module under test imports — so the +// path IS reachable. Own file, because the mock is file-scoped and would +// otherwise apply to every case in temp-root.test.ts. +const attempted: string[] = []; + +vi.mock("node:fs", async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + default: real, + mkdirSync: (...args: Parameters) => { + const target = String(args[0]); + attempted.push(target); + if (target.endsWith(`${path.sep}ud`)) { + throw new Error("ENOSPC: synthetic"); + } + return real.mkdirSync(...args); + }, + }; +}); + +const realFs = await vi.importActual("node:fs"); +const { createRunTempRoot } = await import("./temp-root"); + +describe("createRunTempRoot partial init", () => { + it("reclaims the half-built root and rethrows the original cause", () => { + // The rollback's own failure must never mask the diagnosable one. + expect(() => createRunTempRoot()).toThrow(/ENOSPC: synthetic/); + // Named exactly rather than diffed from a directory listing, so a parallel + // run cannot flake this. + const root = path.dirname(attempted[0]); + expect(path.basename(root).startsWith("quoll-e2e-")).toBe(true); + expect(realFs.existsSync(root)).toBe(false); + }); +}); diff --git a/test/extension/temp-root.test.ts b/test/extension/temp-root.test.ts new file mode 100644 index 00000000..798683b6 --- /dev/null +++ b/test/extension/temp-root.test.ts @@ -0,0 +1,136 @@ +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + createRunTempRoot, + makeTempDir, + makeTempDirSync, + RUN_TEMP_ROOT_ENV, + type RunTempRoot, + resolveRunTempWorkDir, +} from "./temp-root"; + +const created: RunTempRoot[] = []; + +const newRoot = (): RunTempRoot => { + const run = createRunTempRoot(); + created.push(run); + return run; +}; + +afterEach(() => { + for (const run of created.splice(0)) { + fs.rmSync(run.root, { recursive: true, force: true }); + } + delete process.env[RUN_TEMP_ROOT_ENV]; +}); + +describe("createRunTempRoot", () => { + it("gives each run a unique root holding the user-data and work dirs", () => { + const a = newRoot(); + const b = newRoot(); + expect(a.root).not.toBe(b.root); + // os.tmpdir() unresolved — the module must NOT realpath it. On macOS the + // resolved form (`/private/var/…`) reaches VS Code as a document URI and + // breaks the disk-conflict watcher's exact-string event filter. + expect(path.dirname(a.root)).toBe(os.tmpdir()); + expect(path.basename(a.root).startsWith("quoll-e2e-")).toBe(true); + expect(path.dirname(a.userDataDir)).toBe(a.root); + expect(path.dirname(a.workDir)).toBe(a.root); + expect(fs.existsSync(a.userDataDir)).toBe(true); + expect(fs.existsSync(a.workDir)).toBe(true); + }); + + it("keeps the code's own contribution inside the macOS socket budget", () => { + // VS Code opens its IPC socket under --user-data-dir and macOS caps + // those paths at 103 chars; an overlong path fails the launch, not a + // test. Measure only the part this module controls — asserting the + // absolute length would pass on CI (where $TMPDIR is `/tmp`, 56 chars + // of slack) and so would never catch a rename of the segments. + const run = newRoot(); + const owned = run.userDataDir.slice(os.tmpdir().length); + expect(owned).toBe(`${path.sep}${path.basename(run.root)}${path.sep}ud`); + expect(owned.length).toBeLessThanOrEqual(24); + }); + + it.skipIf(process.getuid?.() === 0)( + "throws when the filesystem refuses, rather than leaking silently", + () => { + // The other half of the dispose contract. `not.toThrow()` on an + // already-removed root is satisfied by `force: true` alone, so a + // dispose() that swallowed everything would pass it — this case is what + // distinguishes "loud on unreclaimable" from "silently gives up". + const run = newRoot(); + const locked = path.join(run.workDir, "locked"); + fs.mkdirSync(locked); + fs.writeFileSync(path.join(locked, "f.md"), "x"); + fs.chmodSync(locked, 0o500); + try { + expect(() => run.dispose()).toThrow(); + } finally { + fs.chmodSync(locked, 0o700); // afterEach must still be able to reclaim + } + }, + 15000 // dispose retries with linear backoff before giving up + ); + + it("reclaims a non-empty root", () => { + const run = newRoot(); + fs.writeFileSync(path.join(run.workDir, "leftover.md"), "# x\n"); + run.dispose(); + expect(fs.existsSync(run.root)).toBe(false); + }); + + it("dispose is idempotent on an already-removed root", () => { + const run = newRoot(); + run.dispose(); + expect(() => run.dispose()).not.toThrow(); + }); +}); + +describe("resolveRunTempWorkDir", () => { + it("returns the work dir under the exported root", () => { + const run = newRoot(); + expect(resolveRunTempWorkDir({ [RUN_TEMP_ROOT_ENV]: run.root })).toBe(run.workDir); + }); + + it("throws a diagnosable error when the runner did not export the root", () => { + expect(() => resolveRunTempWorkDir({})).toThrow(RUN_TEMP_ROOT_ENV); + expect(() => resolveRunTempWorkDir({})).toThrow(/launch\.ts/); + }); +}); + +describe("makeTempDir", () => { + it("allocates unique slug-named dirs under the run's work dir", async () => { + const run = newRoot(); + process.env[RUN_TEMP_ROOT_ENV] = run.root; + + const first = await makeTempDir("crlf"); + const second = await makeTempDir("crlf"); + const sync = makeTempDirSync("swap"); + + expect(first).not.toBe(second); + for (const dir of [first, second, sync]) { + expect(path.dirname(dir)).toBe(run.workDir); + expect(fs.existsSync(dir)).toBe(true); + } + expect(path.basename(first).startsWith("crlf-")).toBe(true); + expect(path.basename(sync).startsWith("swap-")).toBe(true); + + // The whole footprint goes with the ONE root removal — this is why no + // suite needs per-dir teardown of its own. + run.dispose(); + expect(fs.existsSync(first)).toBe(false); + }); + + it("refuses a slug that would escape the run's work dir", async () => { + // `path.join` normalises `../`, so a separator in a slug lands the dir + // outside the root — where nothing disposes it. The choke-point test + // governs who allocates; this governs where the result lands. + const run = newRoot(); + process.env[RUN_TEMP_ROOT_ENV] = run.root; + await expect(makeTempDir("../escape")).rejects.toThrow(/bare name/); + expect(() => makeTempDirSync("a/b")).toThrow(/bare name/); + }); +}); diff --git a/test/extension/temp-root.ts b/test/extension/temp-root.ts new file mode 100644 index 00000000..2ad20c41 --- /dev/null +++ b/test/extension/temp-root.ts @@ -0,0 +1,108 @@ +// One temp root per E2E run. The process that CREATES the disposable state +// owns removing it: launch.ts mkdtemps this root, hands the path to the +// Electron host through extensionTestsEnv, and removes exactly this path in +// a finally. Nothing here ever globs `quoll-e2e-*` — a parallel run (a CI +// shard, a second agent's worktree) owns its own root and must survive ours. +import * as fs from "node:fs"; +import * as fsp from "node:fs/promises"; +import * as os from "node:os"; +import * as path from "node:path"; + +/** Env var carrying the run root into the Electron host. */ +export const RUN_TEMP_ROOT_ENV = "QUOLL_E2E_TEMP_ROOT"; + +// Deliberately two-letter: VS Code opens a unix-domain IPC socket under +// --user-data-dir and macOS caps those paths at 103 chars, of which $TMPDIR +// alone is 48. Nobody reads these paths, so a longer name buys no +// readability — only a launch-blocking risk. +const USER_DATA_SEGMENT = "ud"; +const WORK_SEGMENT = "w"; + +export interface RunTempRoot { + /** The only directory under os.tmpdir() this run may write to. */ + readonly root: string; + /** VS Code --user-data-dir. */ + readonly userDataDir: string; + /** Parent of every per-test workspace dir the suites allocate. */ + readonly workDir: string; + /** Remove the whole root. Safe to call twice; THROWS if the filesystem + * refuses (EBUSY / EACCES) — a leak we cannot reclaim must be loud. */ + dispose(): void; +} + +export function createRunTempRoot(): RunTempRoot { + // os.tmpdir() as-is, NOT realpath'd. On macOS it returns the `/var/folders/…` + // symlink whose target is `/private/var/folders/…`, and the paths built here + // become document URIs inside VS Code. The disk-conflict watcher filters + // events by exact URI string, so handing it the resolved form makes those + // comparisons miss and the dirty-doc-disk-conflict suite time out. + const root = fs.mkdtempSync(path.join(os.tmpdir(), "quoll-e2e-")); + const userDataDir = path.join(root, USER_DATA_SEGMENT); + const workDir = path.join(root, WORK_SEGMENT); + try { + fs.mkdirSync(userDataDir); + fs.mkdirSync(workDir); + } catch (err) { + // Partial init still owns the root: reclaim it before the caller ever + // holds a handle, otherwise the throw leaks the dir we just created. + // The rollback must never become the reported failure — whatever broke + // mkdir is the diagnosable cause (a read-only or full $TMPDIR breaks + // both), so log a failed rollback and rethrow the original. + try { + fs.rmSync(root, { recursive: true, force: true }); + } catch (cleanupErr) { + console.error(`[e2e] failed to reclaim partially-created temp root ${root}:`, cleanupErr); + } + throw err; + } + return { + root, + userDataDir, + workDir, + dispose(): void { + // ~3s of retries (linear backoff: 200, 400, … 1000). dispose() fires the + // instant the Electron main process closes, while VS Code's helpers may + // still be writing under user-data-dir. An ENOTEMPTY there is a race, + // not a leak, and must not flip a green run red; a throw past this + // budget means genuinely unreclaimable, which is worth failing for. + fs.rmSync(root, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }); + }, + }; +} + +/** The work dir of the run this process belongs to. Throws (rather than + * silently falling back to os.tmpdir()) because a fallback would restore + * the very leak this module exists to close. */ +export function resolveRunTempWorkDir(env: NodeJS.ProcessEnv = process.env): string { + const root = env[RUN_TEMP_ROOT_ENV]; + if (!root) { + throw new Error( + `${RUN_TEMP_ROOT_ENV} is unset — the E2E suite must be launched through test/extension/launch.ts` + ); + } + return path.join(root, WORK_SEGMENT); +} + +/** Reject anything that could climb out of the work dir. `path.join` + * normalises `../`, so a slug carrying a separator would land a dir OUTSIDE + * the run root — where nothing disposes it. The choke-point test governs who + * allocates; this governs where the result lands. */ +function assertSlug(slug: string): string { + if (!/^[a-z0-9][a-z0-9-]*$/i.test(slug)) { + throw new Error(`temp-dir slug must be a bare name, got ${JSON.stringify(slug)}`); + } + return slug; +} + +/** Allocate a per-test workspace dir under the run root. This is the ONLY + * sanctioned temp-dir allocation in the E2E tree — enforced by + * test/extension/temp-dir-choke-point.test.ts. + * `async` is load-bearing: `resolveRunTempWorkDir` / `assertSlug` throw + * synchronously, and a declared `Promise` must deliver that as a rejection. */ +export async function makeTempDir(slug: string): Promise { + return fsp.mkdtemp(path.join(resolveRunTempWorkDir(), `${assertSlug(slug)}-`)); +} + +export function makeTempDirSync(slug: string): string { + return fs.mkdtempSync(path.join(resolveRunTempWorkDir(), `${assertSlug(slug)}-`)); +} diff --git a/test/extension/tsconfig.json b/test/extension/tsconfig.json index da8ec1ab..e2ea513f 100644 --- a/test/extension/tsconfig.json +++ b/test/extension/tsconfig.json @@ -14,5 +14,5 @@ "strict": true, "isolatedModules": false }, - "include": ["launch.ts", "e2e/**/*.ts"] + "include": ["launch.ts", "temp-root.ts", "e2e/**/*.ts"] }