From 1bc37bb7bec885a16a7106348e5d7df5cc4b91d3 Mon Sep 17 00:00:00 2001 From: Luiz Lima Date: Sun, 18 Oct 2026 16:04:14 -0300 Subject: [PATCH 1/2] =?UTF-8?q?test(local-bin):=20cleanup=20retries=20a=20?= =?UTF-8?q?busy=20file=20=E2=80=94=20Windows=20may=20hold=20a=20just-run?= =?UTF-8?q?=20copy=20of=20node=20(EBUSY)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI failed in the file's afterAll hook: EBUSY on the temp .exe. Nothing the tests assert changes. --- test/local-bin.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/local-bin.test.ts b/test/local-bin.test.ts index 4b47402..8c07148 100644 --- a/test/local-bin.test.ts +++ b/test/local-bin.test.ts @@ -27,6 +27,9 @@ function withPath(value: string): NodeJS.ProcessEnv { return env; } +/** Windows may hold a just-run copy of node for a moment after it exits (EBUSY): Node's own retry, for cleanup only. */ +const removeQuietly = (p: string) => fs.rm(p, { recursive: true, force: true, maxRetries: 10, retryDelay: 200 }); + // Two copies of node for the whole file. LOCAL is the very entry npx adds for this process — the // `node_modules/.bin` of the directory the tests run in. PROJECT is a made-up project elsewhere: its // `node_modules/.bin` is skipped only when craftar is said to run inside it. @@ -45,8 +48,8 @@ beforeAll(async () => { await fs.chmod(path.join(PROJECT_BIN, TOOL + EXE), 0o755); }); afterAll(async () => { - await fs.rm(path.join(LOCAL, TOOL + EXE), { force: true }); - await fs.rm(PROJECT, { recursive: true, force: true }); + await removeQuietly(path.join(LOCAL, TOOL + EXE)); + await removeQuietly(PROJECT); }); describe("isLocalBin — the node_modules/.bin of a directory or of one above it", () => { @@ -118,7 +121,7 @@ describe("onPath — a program in such an entry is not found", () => { expect(await onPath(TOOL, env, PROJECT)).toBeNull(); expect(await onPath(TOOL, env, path.join(PROJECT, "packages", "app"))).toBeNull(); const elsewhere = await fs.realpath(await tmpDir("craftar-local-bin-elsewhere-")); - cleanups.push(() => fs.rm(elsewhere, { recursive: true, force: true })); + cleanups.push(() => removeQuietly(elsewhere)); expect(await onPath(TOOL, env, elsewhere)).toBe(path.join(PROJECT_BIN, TOOL + EXE)); }); From 8632c71d3a0ae976c1b30cb73959e72596c1fa38 Mon Sep 17 00:00:00 2001 From: Luiz Lima Date: Sun, 18 Oct 2026 16:10:36 -0300 Subject: [PATCH 2/2] test(proc): cleanup retries within the hook timeout, in one helper; proc.test.ts's planted node too (review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vitest's 10 s hookTimeout bounds the budget: 6 × 200 ms is 4.2 s, inside twice. proc.test.ts's afterAll had the same exposure. --- test/helpers/fs.ts | 9 +++++++++ test/local-bin.test.ts | 10 ++++------ test/proc.test.ts | 3 ++- 3 files changed, 15 insertions(+), 7 deletions(-) create mode 100644 test/helpers/fs.ts diff --git a/test/helpers/fs.ts b/test/helpers/fs.ts new file mode 100644 index 0000000..ca3abc7 --- /dev/null +++ b/test/helpers/fs.ts @@ -0,0 +1,9 @@ +import { promises as fs } from "node:fs"; + +/** + * Removes a file or a directory for a test's cleanup, retrying a busy one: Windows can report EBUSY on a copy of + * node a test has just run (a lingering handle, or a scan). Node retries only with `recursive: true` (harmless on a + * file); its backoff is linear, so 6 × 200 ms waits at most 4.2 s — inside vitest's 10 s hook timeout, twice. An + * error that outlasts the retries still throws. + */ +export const removeWithRetry = (p: string): Promise => fs.rm(p, { recursive: true, force: true, maxRetries: 6, retryDelay: 200 }); diff --git a/test/local-bin.test.ts b/test/local-bin.test.ts index 8c07148..4bfad2c 100644 --- a/test/local-bin.test.ts +++ b/test/local-bin.test.ts @@ -4,6 +4,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; import { execProgram, isLocalBin, onPath, pathEntries, spawnInherited } from "../src/core/proc.js"; import { dockerRunner, dockerVersion } from "../src/core/services.js"; import { tmpDir } from "./helpers/forge.js"; +import { removeWithRetry } from "./helpers/fs.js"; // 0.28.2 — `npx craftar` puts `/node_modules/.bin`, and every ancestor's, first on PATH. craftar runs in a // workspace or a Forge, so a program committed there would be the one it runs: those entries are skipped, in the @@ -27,9 +28,6 @@ function withPath(value: string): NodeJS.ProcessEnv { return env; } -/** Windows may hold a just-run copy of node for a moment after it exits (EBUSY): Node's own retry, for cleanup only. */ -const removeQuietly = (p: string) => fs.rm(p, { recursive: true, force: true, maxRetries: 10, retryDelay: 200 }); - // Two copies of node for the whole file. LOCAL is the very entry npx adds for this process — the // `node_modules/.bin` of the directory the tests run in. PROJECT is a made-up project elsewhere: its // `node_modules/.bin` is skipped only when craftar is said to run inside it. @@ -48,8 +46,8 @@ beforeAll(async () => { await fs.chmod(path.join(PROJECT_BIN, TOOL + EXE), 0o755); }); afterAll(async () => { - await removeQuietly(path.join(LOCAL, TOOL + EXE)); - await removeQuietly(PROJECT); + await removeWithRetry(path.join(LOCAL, TOOL + EXE)); + await removeWithRetry(PROJECT); }); describe("isLocalBin — the node_modules/.bin of a directory or of one above it", () => { @@ -121,7 +119,7 @@ describe("onPath — a program in such an entry is not found", () => { expect(await onPath(TOOL, env, PROJECT)).toBeNull(); expect(await onPath(TOOL, env, path.join(PROJECT, "packages", "app"))).toBeNull(); const elsewhere = await fs.realpath(await tmpDir("craftar-local-bin-elsewhere-")); - cleanups.push(() => removeQuietly(elsewhere)); + cleanups.push(() => removeWithRetry(elsewhere)); expect(await onPath(TOOL, env, elsewhere)).toBe(path.join(PROJECT_BIN, TOOL + EXE)); }); diff --git a/test/proc.test.ts b/test/proc.test.ts index 406fdbe..104e3a1 100644 --- a/test/proc.test.ts +++ b/test/proc.test.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; import { capture, execProgram, onPath, pathEntries, spawnInherited } from "../src/core/proc.js"; import { tmpDir } from "./helpers/forge.js"; +import { removeWithRetry } from "./helpers/fs.js"; // design D3 — the two ways craftar runs an external program besides git: no shell, an argument array. @@ -70,7 +71,7 @@ beforeAll(async () => { await fs.copyFile(process.execPath, path.join(PLANTED, TOOL + EXE)); await fs.chmod(path.join(PLANTED, TOOL + EXE), 0o755); }); -afterAll(() => fs.rm(PLANTED, { recursive: true, force: true })); +afterAll(() => removeWithRetry(PLANTED)); /** The process environment with PATH replaced, whatever the key's case (Windows spells it `Path`). */ function withPath(value: string): NodeJS.ProcessEnv {