fix(test): migrate fs.rmSync to removeSyncWithRetries for EBUSY-safe cleanup

Export removeSyncWithRetries from @oh-my-pi/pi-utils as a standalone
function, then migrate the highest-impact fs.rmSync call sites:

- test/helpers/temp-home-cleanup.ts: 2 fs.rmSync → removeSyncWithRetries
  (affects all tests using the cleanupTempHome helper)
- test/core/apply-patch-regression.test.ts: 16 fs.rmSync → removeSyncWithRetries
  (the most fs.rmSync calls of any test file)

removeSyncWithRetries retries on EBUSY/EPERM/ENOTEMPTY (40x25ms on
Windows), matching TempDir.removeSync's retry logic. On Linux/macOS
(CI) the retries are a no-op — fs.rmSync succeeds immediately.
This commit is contained in:
oldschoola
2026-06-19 17:27:54 -07:00
parent 1a92b3f854
commit fa903b2b2c
4 changed files with 25 additions and 20 deletions
@@ -11,6 +11,7 @@ import * as fs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { applyPatch, findContextLine, seekSequence } from "@oh-my-pi/pi-coding-agent/edit";
import { removeSyncWithRetries } from "@oh-my-pi/pi-utils";
describe("regression: indentation adjustment for line-based replacements (2B)", () => {
let tempDir: string;
@@ -21,7 +22,7 @@ describe("regression: indentation adjustment for line-based replacements (2B)",
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("line-based patch adjusts indentation when fuzzy matching at different indent level", async () => {
@@ -102,7 +103,7 @@ describe("regression: ambiguity detection for context-less hunks (2C)", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("single-hunk simple diff rejects multiple occurrences", async () => {
@@ -166,7 +167,7 @@ describe("regression: context search uses line hints (2D)", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("unified diff line numbers help locate correct position", async () => {
@@ -251,7 +252,7 @@ describe("regression: insertion uses newStartLine fallback (2E)", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("pure addition with context uses context to find insertion point", async () => {
@@ -418,7 +419,7 @@ describe("plan: partial line matching for @@ context", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("@@ context matches when actual line contains it as substring", async () => {
@@ -482,7 +483,7 @@ describe("plan: unified diff format line numbers", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("@@ -10,6 +10,7 @@ is parsed as line numbers not literal text", async () => {
@@ -556,7 +557,7 @@ describe("plan: Codex-style wrapped patches", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("strips *** Begin Patch / *** End Patch wrapper", async () => {
@@ -697,7 +698,7 @@ describe("plan: strip + prefix from file creation", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("create file strips + prefix when all lines have it", async () => {
@@ -755,7 +756,7 @@ describe("regression: *** End of File marker handling (2A/2G)", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("*** End of File marker is preserved in hunk parsing", async () => {
@@ -814,7 +815,7 @@ describe("regression: model edit attempt - @@ line N syntax (session 2026-01-19)
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("@@ line 125 is parsed as line hint, not literal context search", async () => {
@@ -866,7 +867,7 @@ describe("regression: model edit attempt - nested @@ anchors (session 2026-01-19
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("@@ class X followed by @@ method on next line is parsed as nested anchors", async () => {
@@ -959,7 +960,7 @@ describe("regression: model edit attempt - space-separated anchors (session 2026
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("@@ class PatchTool constructor is parsed as hierarchical anchors", async () => {
@@ -1047,7 +1048,7 @@ describe("regression: model edit attempt - unique substring on long line (sessio
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("@@ class ClassName matches long export line when unique", async () => {
@@ -1127,7 +1128,7 @@ describe("regression: bench edit failures (2026-01-19)", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("@@ @@ is treated as empty context", async () => {
@@ -1559,7 +1560,7 @@ describe("regression: trailing context lines don't delete file content", () => {
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("context lines cannot cause collateral deletion via fuzzy match", async () => {
@@ -1806,7 +1807,7 @@ describe("regression: context-only hunks between @@ markers must not change inde
});
afterEach(() => {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
});
test("pure context hunk (no +/- lines) does not alter tab-indented file content", async () => {
@@ -1,4 +1,4 @@
import * as fs from "node:fs";
import { removeSyncWithRetries } from "@oh-my-pi/pi-utils";
export interface TempHomeState {
tempDir: string;
@@ -10,10 +10,10 @@ export function cleanupTempHome(getState: () => TempHomeState): () => void {
return () => {
const { tempDir, tempHomeDir, originalHome } = getState();
if (tempDir) {
fs.rmSync(tempDir, { recursive: true, force: true });
removeSyncWithRetries(tempDir);
}
if (tempHomeDir) {
fs.rmSync(tempHomeDir, { recursive: true, force: true });
removeSyncWithRetries(tempHomeDir);
}
if (originalHome === undefined) {
delete process.env.HOME;
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Added
- Exported `removeSyncWithRetries()` as a standalone function so tests that manage their own temp dirs can use the same retry-on-EBUSY cleanup logic as `TempDir.removeSync()`.
## [16.1.3] - 2026-06-19
### Changed
+1 -1
View File
@@ -95,7 +95,7 @@ async function removeWithRetries(target: string): Promise<void> {
}
}
function removeSyncWithRetries(target: string): void {
export function removeSyncWithRetries(target: string): void {
for (let attempt = 0; ; attempt++) {
try {
fs.rmSync(target, kRemoveOptions);