test(cli): prove detached refresh by ordering, not by wall clock (#3221)

`lets the parent exit without waiting for a detached refresh child` bet
twice on absolute wall-clock budgets and lost both bets on loaded CI
runners:

- `expect(elapsed).toBeLessThan(1_800)` bounded the *parent's* cold
  `node --import tsx` boot, inferring "did not wait" from a 1050ms margin
  over the mock fetch's 750ms sleep. Reproduced failing on Linux under
  40-way load at 1904ms.
- The 15s poll for the cache file had to cover the whole detached child:
  node boot, a tsx transpile of 22 source files, an `acquireFileLock`
  that shells out to `ps` (POSIX) or `powershell.exe -Command
  Get-CimInstance Win32_Process` (Windows), the mocked fetch, and the
  atomic write. That chain measures ~1.0s locally but has no bounded
  upper limit on a contended runner, and it is what timed out in CI.

Replace both budgets with an ordering proof. The preloaded mock fetch now
parks the refresh child until the test releases it, so the sequence
asserted is: parent exited -> child provably still mid-refresh (started
marker present, cache absent) -> release -> cache written. That is
strictly stronger than the old elapsed-time inference, and it holds at any
machine speed. The remaining `expect.poll` timeouts no longer carry the
assertion's meaning; they are only "is the child dead" safety nets.

The mock's wait is bounded at 60s so an abandoned child (test failed
before releasing, temp home already removed) still exits instead of
spinning forever.

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergő Magyar 2026-09-08 09:14:47 +01:00 committed by GitHub
parent d1463977c8
commit bddbb0ff9f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -127,12 +127,26 @@ describe('CLI update notice subprocess behavior', () => {
'dir',
);
// The refresh child parks inside fetch() until this test releases it. That
// orders the parent's exit against work that is provably still in flight,
// instead of racing it against a wall-clock budget: the child's real cost
// (node boot, tsx transpile, a lock acquisition that shells out to
// ps/powershell) has no bounded upper limit on a loaded CI runner.
const started = path.join(home, 'refresh-started');
const release = path.join(home, 'refresh-release');
const preload = path.join(home, 'mock-refresh.mjs');
fs.writeFileSync(
preload,
`Object.defineProperty(process.stderr, 'isTTY', { value: true, configurable: true });
`import fs from 'node:fs';
Object.defineProperty(process.stderr, 'isTTY', { value: true, configurable: true });
globalThis.fetch = async () => {
await new Promise((resolve) => setTimeout(resolve, 750));
fs.writeFileSync(${JSON.stringify(started)}, '');
// Bounded so an abandoned child (test failed before releasing, temp home
// already deleted) still exits instead of spinning forever.
const deadline = Date.now() + 60_000;
while (!fs.existsSync(${JSON.stringify(release)}) && Date.now() < deadline) {
await new Promise((resolve) => setTimeout(resolve, 25));
}
return new Response(JSON.stringify({ version: '99.0.0' }), {
status: 200,
headers: { 'content-type': 'application/json' },
@ -141,7 +155,6 @@ globalThis.fetch = async () => {
`,
);
const startedAt = Date.now();
await new Promise<void>((resolve, reject) => {
const parent = spawn(
process.execPath,
@ -162,17 +175,20 @@ globalThis.fetch = async () => {
else reject(new Error(`notifier parent exited ${String(code)}`));
});
});
const elapsed = Date.now() - startedAt;
expect(elapsed).toBeLessThan(1_800);
// The parent already exited above, so reaching a still-parked child proves
// the refresh outlived it and was never awaited.
const cache = path.join(home, 'update-check.json');
await expect.poll(() => fs.existsSync(started), { timeout: 30_000, interval: 50 }).toBe(true);
expect(fs.existsSync(cache)).toBe(false);
await expect.poll(() => fs.existsSync(cache), { timeout: 15_000, interval: 100 }).toBe(true);
fs.writeFileSync(release, '');
await expect.poll(() => fs.existsSync(cache), { timeout: 30_000, interval: 50 }).toBe(true);
expect(JSON.parse(fs.readFileSync(cache, 'utf8'))).toMatchObject({
latestVersion: '99.0.0',
registry: 'https://registry.npmjs.org',
});
}, 20_000);
}, 90_000);
it('prints the localized notice on a forced-TTY stderr and keeps stdout clean', () => {
const home = tempHome();