From b3faeffc31bfcb683f51bb400352e34c011e9ac1 Mon Sep 17 00:00:00 2001 From: John McLear Date: Mon, 11 May 2026 19:35:16 +0100 Subject: [PATCH] fix(tests): retry rmdir to clear Windows EBUSY flake in updater-integration (#7728) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * 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) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- .../backend/specs/updater-integration.ts | 29 ++++++++++++++----- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/src/tests/backend/specs/updater-integration.ts b/src/tests/backend/specs/updater-integration.ts index e04a551c2..a6e9e8c99 100644 --- a/src/tests/backend/specs/updater-integration.ts +++ b/src/tests/backend/specs/updater-integration.ts @@ -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); } }); });