mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-11-07 06:19:43 -10:00
test(integration): pin removeScratchDir's retry budget with a witness (#15488)
removeScratchDir's rm() budget (maxRetries: 5 / retryDelay: 200) was unpinned: mutating it to 0 kept every test green, so a regression that removed retrying entirely would ship silently. The existing ENOTDIR case cannot catch it -- ENOTDIR is outside rm's retryable set -- and asserting only that the promise resolves is vacuous, because the catch swallows every rejection. Add a success-path case with a self-limiting straggler refill writer. Seeded entries widen the removal walk so a refill lands between readdir and rmdir (ENOTEMPTY, retryable), and the writer is capped by both a file ceiling and a wall-clock cutoff so it goes quiet well inside the shipped budget. The load-bearing assertions are that the tree is actually gone and that no warning was logged. Red witness: setting maxRetries to 0 in scratch-dir.ts fails this case on 2/2 runs while the ENOTDIR case still passes; scratch-dir.ts is restored byte-identical (md5 3cdb5f2f504ea6cc7663e2708ae133f4). Refs #11310 Patrol-Run: qwen-issue-patrol/jmvn0b1rrjt Co-authored-by: yiliang114 <[email protected]> Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
1 file changed
+69
-1
@@ -4,7 +4,8 @@
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { mkdtemp, rm, writeFile } from 'node:fs/promises';
|
||||
import { existsSync } from 'node:fs';
|
||||
import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
@@ -39,4 +40,71 @@ describe('removeScratchDir', () => {
|
||||
expect.anything(),
|
||||
);
|
||||
});
|
||||
|
||||
it('removes a dir a straggler writer keeps refilling', async () => {
|
||||
// rm(recursive) only retries on EBUSY/EMFILE/ENFILE/ENOTEMPTY, so the
|
||||
// retry budget is exactly what absorbs a CLI child still writing under its
|
||||
// scratch dir. This refill writer outlives a single removal attempt but
|
||||
// stops well inside the shipped 5x200ms budget, which makes that budget
|
||||
// load-bearing: drop maxRetries to 0 and the first ENOTEMPTY reaches the
|
||||
// catch, the tree survives, and this case goes red. The ENOTDIR case above
|
||||
// cannot do this -- it is outside the retryable set, so it is equally red
|
||||
// or green whatever the budget is. The prefix is deliberately not
|
||||
// qwen-e2e-home-*, so globalSetup's sweeper cannot race this tree.
|
||||
const warn = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
const root = join(tmpRoot, 'straggler-root');
|
||||
const target = join(root, 'artifacts');
|
||||
await mkdir(target, { recursive: true });
|
||||
|
||||
// ENOTEMPTY only lands when a file appears between readdir and rmdir, so
|
||||
// seed enough entries that the removal walk spans several refill ticks.
|
||||
const seedFiles = 1500;
|
||||
await Promise.all(
|
||||
Array.from({ length: seedFiles }, (_, i) =>
|
||||
writeFile(join(target, `seed-${i}.txt`), 'x'),
|
||||
),
|
||||
);
|
||||
|
||||
// Both caps are real: the file ceiling normally fires first (~450ms), the
|
||||
// wall-clock cutoff backstops it. Either way the writer is quiet long
|
||||
// before the shipped budget runs out, so no straggler can outlive the case
|
||||
// or outrun the retries.
|
||||
const REFILL_TICK_MS = 5;
|
||||
const REFILL_CEILING = 90;
|
||||
const REFILL_BUDGET_MS = 600;
|
||||
let written = 0;
|
||||
let stopRefill = () => {};
|
||||
const refillStopped = new Promise<void>((resolve) => {
|
||||
const timer = setInterval(() => {
|
||||
written += 1;
|
||||
if (written > REFILL_CEILING) {
|
||||
stopRefill();
|
||||
return;
|
||||
}
|
||||
// Once the tree is gone these reject with ENOENT; a successful removal
|
||||
// must not be turned into an unhandled rejection.
|
||||
writeFile(join(target, `refill-${written}.txt`), 'x').catch(() => {});
|
||||
}, REFILL_TICK_MS);
|
||||
const cutoff = setTimeout(() => stopRefill(), REFILL_BUDGET_MS);
|
||||
// The executor is synchronous, so both timers exist before either
|
||||
// callback can fire and stopRefill is the real stop by then.
|
||||
stopRefill = () => {
|
||||
clearInterval(timer);
|
||||
clearTimeout(cutoff);
|
||||
resolve();
|
||||
};
|
||||
});
|
||||
|
||||
try {
|
||||
await removeScratchDir(root);
|
||||
await refillStopped;
|
||||
// The load-bearing assertion: removeScratchDir swallows every rejection,
|
||||
// so a resolved promise alone is equally green at maxRetries: 0.
|
||||
expect(existsSync(root)).toBe(false);
|
||||
expect(warn).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
stopRefill();
|
||||
await rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user