fix(tests): retry rmdir to clear Windows EBUSY flake in updater-integration (#7728)

* fix(tests): retry rmdir to clear Windows EBUSY flake in updater-integration

The Windows backend-test job has been intermittently red on `crash-loop
guard: bootCount=3 forces immediate rollback` (and other cases in
`updater-integration.ts`) with:

  Error: EBUSY: resource busy or locked, rmdir
  'C:\Users\RUNNER~1\AppData\Local\Temp\updater-it-...'

Each `it()` builds a temp git repo via `execSync('git ...')` and cleans
up in a `try…finally` with `fs.rm(dir, {recursive: true, force: true})`.
On Windows, git child processes can briefly hold file handles after
exit (NTFS lazy-release / antivirus scan / pack-file handles), so the
first rmdir attempt hits EBUSY. `fs.rm`'s default `maxRetries` is 0, so
there is no recovery and the test errors out.

Hoist the cleanup to a single `cleanupTmp()` helper that passes
`maxRetries: 10, retryDelay: 100` (a built-in `fs.rm` capability since
Node 14.14). On Linux/macOS this is a no-op — there's nothing to retry.
On Windows it absorbs the transient lock.

No production code touched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(tests): poll for rollback terminal state instead of 250ms sleep

Windows CI failure on this branch surfaced a *second* flake in the same
file (`crash-loop guard: bootCount=3 forces immediate rollback`):

  TypeError: Cannot read properties of undefined (reading 'execution')
    at updater-integration.ts:230

`checkPendingVerification` kicks off `performRollback` as fire-and-forget
(`void performRollback(s, deps).catch(...)`), and the test waits a flat
250 ms before asserting `states.at(-1)!.execution.status === 'rolled-back'`.
On Linux 250 ms is plenty. On Windows, git checkout + spawned-process
bookkeeping regularly push past that — so `saveState` hasn't fired yet
and `states` is empty.

This race was previously masked: the test's `finally` ran `fs.rm`,
which threw EBUSY against handles still held by the in-flight rollback,
and JS's "finally-throws-override-try-throws" semantics meant mocha
reported the EBUSY rather than the underlying TypeError. The retry-rm
patch on this branch unmasked it.

Replace the flat sleep with condition-based polling (25 ms tick, 10 s
ceiling) for a terminal state (`rolled-back` | `rollback-failed`). The
existing `assert.equal(... 'rolled-back')` still runs, giving a clean
diff if rollback landed on the failure side instead. Linux runtime
drops 329 ms → 104 ms because the poll exits as soon as the state
lands.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
John McLear 2026-05-11 19:35:16 +01:00 committed by GitHub
parent c7100e0ba4
commit b3faeffc31
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -12,6 +12,12 @@ import {EMPTY_STATE, UpdateState} from '../../../node/updater/types';
const sh = (cmd: string, opts: any = {}) =>
execSync(cmd, {stdio: 'pipe', ...opts}).toString().trim();
// On Windows, git's child processes can briefly hold file handles after exit
// (NTFS lazy-release / antivirus / pack files), so an immediate rmdir on the
// temp repo hits EBUSY. fs.rm's built-in retry clears the flake.
const cleanupTmp = (dir: string) =>
fs.rm(dir, {recursive: true, force: true, maxRetries: 10, retryDelay: 100});
const buildTmpRepo = async (): Promise<{dir: string; v1Sha: string; v2Sha: string}> => {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), 'updater-it-'));
sh('git init -b main', {cwd: dir});
@ -94,7 +100,7 @@ describe(__filename, function () {
// The fromSha recorded in state matches the v0.0.1 SHA.
assert.equal((states.at(-1)!.execution as {fromSha: string}).fromSha, v1Sha);
} finally {
await fs.rm(dir, {recursive: true, force: true});
await cleanupTmp(dir);
}
});
@ -140,7 +146,7 @@ describe(__filename, function () {
const lock = await fs.readFile(path.join(dir, 'pnpm-lock.yaml'), 'utf8');
assert.match(lock, /lockfileVersion: x/);
} finally {
await fs.rm(dir, {recursive: true, force: true});
await cleanupTmp(dir);
}
});
@ -182,7 +188,7 @@ describe(__filename, function () {
assert.equal(states.at(-1)!.execution.status, 'rolled-back');
assert.equal(sh('git rev-parse HEAD', {cwd: dir}), v1Sha);
} finally {
await fs.rm(dir, {recursive: true, force: true});
await cleanupTmp(dir);
}
});
@ -219,13 +225,22 @@ describe(__filename, function () {
rollbackHealthCheckSeconds: 60,
});
assert.equal(r.armed, false);
// Wait for the fire-and-forget rollback to finish.
await new Promise((resolve) => setTimeout(resolve, 250));
// Poll for the fire-and-forget rollback to land in its terminal state.
// A flat sleep here was racy on Windows (git checkout + spawned-process
// bookkeeping can push past several hundred ms).
const deadline = Date.now() + 10_000;
while (
states.at(-1)?.execution.status !== 'rolled-back' &&
states.at(-1)?.execution.status !== 'rollback-failed' &&
Date.now() < deadline
) {
await new Promise((resolve) => setTimeout(resolve, 25));
}
assert.equal(states.at(-1)!.execution.status, 'rolled-back');
assert.equal(sh('git rev-parse HEAD', {cwd: dir}), v1Sha);
assert.equal(exitedWith, 75);
} finally {
await fs.rm(dir, {recursive: true, force: true});
await cleanupTmp(dir);
}
});
@ -259,7 +274,7 @@ describe(__filename, function () {
assert.equal(states.at(-1)!.lastResult!.outcome, 'rollback-failed');
assert.equal(exitedWith, 75);
} finally {
await fs.rm(dir, {recursive: true, force: true});
await cleanupTmp(dir);
}
});
});