diff --git a/.gitignore b/.gitignore index cdc6e12..7c8db67 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,7 @@ mobile-*.png # Test scratch directories tests/__tmp_*/ +tests/tmp/ # Local task tracking tasks/ diff --git a/tests/cas-locking.test.ts b/tests/cas-locking.test.ts index 4630d9e..c46c7f1 100644 --- a/tests/cas-locking.test.ts +++ b/tests/cas-locking.test.ts @@ -1,10 +1,17 @@ import { describe, it, expect, afterAll } from "bun:test"; -import { mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from "fs"; +import { rmdirSync, mkdirSync, writeFileSync, readFileSync, rmSync, existsSync } from "fs"; import { join, dirname } from "path"; import { computeHash } from "../src/core/read"; import { acquireLock, acquireSortedLocks, LOCK_TIMEOUT_MS, lockPathFor, LockAcquireError, pruneStaleLocks } from "../src/core/locking"; import { routeEdit } from "../src/core/router"; +// #110: each describe removes its own fixture, but the shared parent dirs were +// left behind and dirtied `git status` after every run. Sweep them at file end. +afterAll(() => { + try { rmSync("tests/tmp/cas-locking", { recursive: true, force: true }); } catch { /* ignore */ } + try { rmdirSync("tests/tmp"); } catch { /* non-empty or already gone */ } +}); + function makeTestDir(name: string): { dir: string; cleanup: () => void } { const dir = join("tests/tmp/cas-locking", name); mkdirSync(dir, { recursive: true }); diff --git a/tests/intent.test.ts b/tests/intent.test.ts index be43a76..14be0d0 100644 --- a/tests/intent.test.ts +++ b/tests/intent.test.ts @@ -1,4 +1,4 @@ -import { describe, test, expect, beforeEach, afterEach } from "bun:test"; +import { describe, test, expect, beforeEach, afterEach, afterAll } from "bun:test"; import { parseIntent, findSymbolDefinition, findReferences, generatePlan, UnsupportedIntentError, resolveReferences } from "../src/core/intent"; import { executePlan, executeIntent } from "../src/core/plan-executor"; import type { VerifyResult } from "../src/core/verify"; @@ -8,6 +8,7 @@ import { simulateCrashAfterTempWrite } from "../src/core/paths"; import { exitCodeFor } from "../src/core/exit-codes"; const TMP_DIR = join(import.meta.dir, "__tmp_intent_tests__"); +const B15 = join(import.meta.dir, "__tmp_b15__"); const FILE_A = join(TMP_DIR, "a.ts"); const FILE_B = join(TMP_DIR, "b.ts"); @@ -71,6 +72,15 @@ function cleanup() { try { rmSync(TMP_DIR, { recursive: true, force: true }); } catch {} } +// #110: several describe blocks call setup() without a matching afterEach, so +// the fixture tree outlived the run and dirtied the working copy. A file-level +// afterAll guarantees the scratch dirs are gone no matter which block created +// them or whether a test threw partway through. +afterAll(() => { + cleanup(); + try { rmSync(B15, { recursive: true, force: true }); } catch {} +}); + // ── parseIntent ────────────────────────────────────────────────────── describe("parseIntent", () => { @@ -1069,8 +1079,6 @@ describe("executePlan verification / rollback correctness", () => { // references, decoys excluded, and unsupported languages reported as // `unresolved` rather than guessed. -const B15 = join(import.meta.dir, "__tmp_b15__"); - function b15write(rel: string, content: string) { const abs = join(B15, rel); mkdirSync(join(abs, ".."), { recursive: true }); diff --git a/tests/locking-multiprocess.test.ts b/tests/locking-multiprocess.test.ts index 802d59d..d1337c5 100644 --- a/tests/locking-multiprocess.test.ts +++ b/tests/locking-multiprocess.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeAll, afterAll } from "bun:test"; -import { mkdirSync, writeFileSync, rmSync, readFileSync, existsSync } from "fs"; +import { rmdirSync, mkdirSync, writeFileSync, rmSync, readFileSync, existsSync } from "fs"; import { join, resolve, dirname } from "path"; import { acquireLock, lockPathFor, pruneStaleLocks, LockAcquireError } from "../src/core/locking"; @@ -9,6 +9,13 @@ import { acquireLock, lockPathFor, pruneStaleLocks, LockAcquireError } from "../ const LOCKING_MODULE = resolve("src/core/locking.ts"); +// #110: each describe removes its own fixture, but the shared parent dirs were +// left behind and dirtied `git status` after every run. Sweep them at file end. +afterAll(() => { + try { rmSync("tests/tmp/locking-mp", { recursive: true, force: true }); } catch { /* ignore */ } + try { rmdirSync("tests/tmp"); } catch { /* non-empty or already gone */ } +}); + function makeTestDir(name: string): { dir: string; nested: string; cleanup: () => void } { const dir = resolve("tests/tmp/locking-mp", name); const nested = join(dir, "pkg", "deep"); diff --git a/tests/scratch-hygiene.test.ts b/tests/scratch-hygiene.test.ts new file mode 100644 index 0000000..36ed721 --- /dev/null +++ b/tests/scratch-hygiene.test.ts @@ -0,0 +1,41 @@ +import { describe, it, expect } from "bun:test"; +import { existsSync, readFileSync } from "fs"; +import { join, resolve } from "path"; + +// #110: `bun test` used to leave `tests/__tmp_intent_tests__/` and `tests/tmp/` +// behind, so every run dirtied the working copy and the next run started from +// stale fixtures. The guard is functional: run the owning test files in a child +// process and assert the scratch trees are gone when it exits. .gitignore +// covers the same roots as a backstop for a run that dies before afterAll fires. + +const REPO_ROOT = resolve(import.meta.dir, ".."); + +async function runTestFile(file: string): Promise { + const proc = Bun.spawn(["bun", "test", file], { + cwd: REPO_ROOT, + stdout: "pipe", + stderr: "pipe", + env: { ...process.env, HASHPILOT_TELEMETRY: "0" }, + }); + return await proc.exited; +} + +describe("test scratch hygiene (#110)", () => { + it("intent tests leave no scratch tree under tests/", async () => { + await runTestFile("tests/intent.test.ts"); + expect(existsSync(join(REPO_ROOT, "tests/__tmp_intent_tests__"))).toBe(false); + expect(existsSync(join(REPO_ROOT, "tests/__tmp_b15__"))).toBe(false); + }, 120_000); + + it("locking tests leave no tests/tmp tree behind", async () => { + await runTestFile("tests/cas-locking.test.ts"); + expect(existsSync(join(REPO_ROOT, "tests/tmp/cas-locking"))).toBe(false); + expect(existsSync(join(REPO_ROOT, "tests/tmp"))).toBe(false); + }, 120_000); + + it("gitignore covers both scratch roots", () => { + const ignore = readFileSync(join(REPO_ROOT, ".gitignore"), "utf8"); + expect(ignore).toContain("tests/__tmp_*/"); + expect(ignore).toContain("tests/tmp/"); + }); +});