mirror of
https://github.com/agentclientprotocol/claude-agent-acp.git
synced 2026-10-27 04:30:59 +00:00
test: retry the acp-v2 teardown rm on Windows (#1373)
## What breaks
On Windows, `npm run test:run` fails inside `src/tests/acp-v2.test.ts`,
and every failure is raised by the teardown rather than by the behaviour
the test checks. Measured on Windows 11 build 26200, Node 22.22.3,
vitest 5.0.4, two runs:
```
$ node node_modules/vitest/vitest.mjs run src/tests/acp-v2.test.ts
Test Files 1 failed (1)
Tests 45 failed | 91 passed (136)
Duration 11.75s (tests 91%, transform 7%, import 2%)
```
```
Test Files 1 failed (1)
Tests 53 failed | 83 passed (136)
Duration 11.79s (tests 96%, import 2%, transform 2%)
```
Exit code 1 both times. Every failure is the same error, and it comes
from `afterEach`, after the test body has finished:
```
FAIL src/tests/acp-v2.test.ts > ACP v2 prompts > does not report the next turn as waiting for a question that its client never settled
Error: EBUSY: resource busy or locked, rmdir 'C:\Users\...\Temp\acp-v2-TNkkd2\project-J9PwxJ'
FAIL src/tests/acp-v2.test.ts > ACP v2 prompts > the tool calls that a cancel stops > ends a stopped tool call and its subagent's as cancelled, before it reports idle cancelled
Error: EBUSY: resource busy or locked, rmdir 'C:\Users\...\Temp\acp-v2-O3wrrr\project-NgBUMz'
```
The string `EBUSY` occurs 45 and 53 times in those two logs, matching
the two failure counts. The failing set moves between runs; which test
loses the race depends on when Windows releases each directory handle.
CI runs the suite on `ubuntu-latest` only
(`.github/workflows/ci.yml:20`, `:45`), so CI cannot show this. Anyone
who runs the documented `npm run test:run` on Windows gets a red file
and a non-zero exit.
## Why
The per-test teardown at `src/tests/acp-v2.test.ts:156` is:
```ts
await rm(tempDir, { recursive: true, force: true });
```
`force: true` suppresses ENOENT only; it does not retry EBUSY. Each
test's cwd is a child of `tempDir` (`cwd = await
mkdtemp(path.join(tempDir, "project-"))`, line 147), and Windows keeps a
directory busy briefly after the process holding it exits, so `rmdir`
returns EBUSY, `afterEach` rejects, and vitest marks the test failed.
## The fix
```diff
- await rm(tempDir, { recursive: true, force: true });
+ await rm(tempDir, { recursive: true, force: true, maxRetries: 10 });
```
`maxRetries` is Node's own retry loop for `fs.rm`, covering EBUSY,
EMFILE, ENFILE, ENOTEMPTY and EPERM with a backoff between attempts. The
repository already uses it for the same class of problem in
`src/tests/session-hooks.test.ts:150`:
```ts
// The CLI may still write the transcript of the last session after dispose.
if (root) await rm(root, { recursive: true, force: true, maxRetries: 10 });
```
## Measured after
```
$ node node_modules/vitest/vitest.mjs run src/tests/acp-v2.test.ts
Test Files 1 passed (1)
Tests 136 passed (136)
Duration 16.16s (tests 96%, transform 2%, import 2%)
```
A second run after restoring the patched file: `Tests 136 passed (136)`,
`Duration 16.56s`, exit code 0, `EBUSY` count 0. `npm run check` (eslint
plus `prettier --check .`) also passes, and `npm run build` (`tsc`,
whose `include` covers `src/**/*.ts` so the test file is typechecked
too) exits 0.
## Scope
1 file changed, 1 insertion, 1 deletion: `src/tests/acp-v2.test.ts`.
Deliberately not changed:
- No other test file, and no teardown other than this one call.
`session-hooks.test.ts` is untouched.
- No assertion, fixture, mock or production code, so the 136 tests still
check exactly what they checked before.
- No CI change. Adding a Windows job is a separate call about CI time
and runner budget, and this fix can be verified on Windows without one.
## Unproven
- I have one Windows 11 machine. On `ubuntu-latest` the teardown did not
fail here before or after the patch, and I did not run the job on Linux;
the Linux side is "no behaviour change" by inspection, not something I
measured.
- `maxRetries: 10` covers a handle that closes shortly after the process
exits. A handle held for longer than the whole retry window would still
surface EBUSY; I did not observe one.
- I ran only this file, not the full suite, so I cannot say whether
other files fail on Windows for other reasons.
This commit is contained in:
@@ -153,7 +153,7 @@ afterEach(async () => {
|
||||
await Promise.all(agents.splice(0).map((agent) => agent.dispose()));
|
||||
execFileSpy.mockReset();
|
||||
vi.unstubAllEnvs();
|
||||
await rm(tempDir, { recursive: true, force: true });
|
||||
await rm(tempDir, { recursive: true, force: true, maxRetries: 10 });
|
||||
});
|
||||
|
||||
/** Connects a fresh router to a client, and returns the client end of the stream. */
|
||||
|
||||
Reference in New Issue
Block a user