mirror of
https://github.com/4gray/iptvnator.git
synced 2026-10-10 18:36:15 -08:00
fix(e2e): reap data directories abandoned by earlier runs (#1296)
`removeDataDir` tolerates a locked directory rather than failing the run, but then abandons it, and nothing collects it on our behalf: Windows never clears %TEMP% on process exit, and the Unix equivalents only run on a schedule. Every teardown that lost that race leaked a database and user-data tree on developer machines and long-lived runners, invisibly, while CI stayed green. Sweeps leftover `iptvnator-electron-e2e-*` directories once per run, before the first one is created. Ownership is settled by pid rather than age: each run records its pid and the sweep asks the OS via `process.kill(pid, 0)`. - A live owner is kept, so a concurrent suite is never collected — this repo is routinely checked out into several worktrees at once. Age cannot answer this: writes land under `databases/` and `user-data/`, which never refreshes the root's mtime, so a run paused in a debugger looks arbitrarily old. - A dead owner is collected immediately. - An undeterminable owner (missing, empty or malformed marker) falls back to a 24h cutoff. The marker is published via rename so a half-written file cannot bypass that guard. - A live-looking owner past a week is collected anyway, since the OS recycles pids and a stranger inheriting one would otherwise pin the directory forever. Covered by a 10-test spec running on Linux, macOS and Windows, since `process.kill(pid, 0)` semantics are platform-specific. Each behaviour was verified to fail against the preceding implementation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c637a0520e
commit
72f8cebd2e
3 files changed
+340
-1
No files matched your search
@@ -0,0 +1,204 @@
|
||||
import { test, expect } from '@playwright/test';
|
||||
import { spawn, ChildProcess } from 'child_process';
|
||||
import {
|
||||
existsSync,
|
||||
mkdirSync,
|
||||
mkdtempSync,
|
||||
rmSync,
|
||||
utimesSync,
|
||||
writeFileSync,
|
||||
} from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import {
|
||||
dataDirHardMaxAgeMs,
|
||||
dataDirOwnerMarker,
|
||||
dataDirPrefix,
|
||||
orphanDataDirMaxAgeMs,
|
||||
readDataDirOwner,
|
||||
reapOrphanedDataDirs,
|
||||
writeDataDirOwnerMarker,
|
||||
} from './data-dir-reaper';
|
||||
|
||||
/**
|
||||
* Coverage for the orphaned-data-directory sweep.
|
||||
*
|
||||
* These are filesystem-level rather than app-level, but they belong in the
|
||||
* Electron suite: it runs on Linux, macOS and Windows, and the sweep turns on
|
||||
* `process.kill(pid, 0)` semantics that differ per platform.
|
||||
*/
|
||||
const roots: string[] = [];
|
||||
const children: ChildProcess[] = [];
|
||||
|
||||
function makeRoot(): string {
|
||||
const root = mkdtempSync(join(tmpdir(), 'reaper-spec-'));
|
||||
roots.push(root);
|
||||
return root;
|
||||
}
|
||||
|
||||
/** A data directory shaped like a real one, optionally aged and/or owned. */
|
||||
function makeDataDir(
|
||||
root: string,
|
||||
name: string,
|
||||
options: { ageHours?: number; pid?: number | string } = {}
|
||||
): string {
|
||||
const dir = join(root, `${dataDirPrefix}${name}`);
|
||||
mkdirSync(join(dir, 'databases'), { recursive: true });
|
||||
writeFileSync(join(dir, 'databases', 'iptvnator.db'), 'db');
|
||||
if (options.pid !== undefined) {
|
||||
writeFileSync(join(dir, dataDirOwnerMarker), String(options.pid));
|
||||
}
|
||||
if (options.ageHours !== undefined) {
|
||||
const when = new Date(Date.now() - options.ageHours * 3600_000);
|
||||
utimesSync(dir, when, when);
|
||||
}
|
||||
return dir;
|
||||
}
|
||||
|
||||
function spawnLiveProcess(): ChildProcess {
|
||||
const child =
|
||||
process.platform === 'win32'
|
||||
? spawn('cmd', ['/c', 'ping -n 30 127.0.0.1 > NUL'], {
|
||||
stdio: 'ignore',
|
||||
})
|
||||
: spawn('sleep', ['30'], { stdio: 'ignore' });
|
||||
children.push(child);
|
||||
return child;
|
||||
}
|
||||
|
||||
test.afterAll(() => {
|
||||
for (const child of children) {
|
||||
child.kill();
|
||||
}
|
||||
for (const root of roots) {
|
||||
rmSync(root, { force: true, recursive: true });
|
||||
}
|
||||
});
|
||||
|
||||
test.describe('orphaned data directory reaper', () => {
|
||||
test('keeps a live owner well past the unmarked cutoff', () => {
|
||||
const root = makeRoot();
|
||||
const child = spawnLiveProcess();
|
||||
// Age is deliberately past the cutoff: writes land under databases/ and
|
||||
// never refresh the root's mtime, so a running suite can look this old.
|
||||
const dir = makeDataDir(root, 'live', {
|
||||
ageHours: 48,
|
||||
pid: child.pid,
|
||||
});
|
||||
|
||||
reapOrphanedDataDirs(root);
|
||||
|
||||
expect(existsSync(dir)).toBe(true);
|
||||
});
|
||||
|
||||
test('reaps a live-looking owner once past the hard cap, since the pid must be recycled', () => {
|
||||
const root = makeRoot();
|
||||
// A pid that resolves to a live process, but on a directory far older
|
||||
// than any suite could run — so the pid belongs to a stranger now.
|
||||
const child = spawnLiveProcess();
|
||||
const dir = makeDataDir(root, 'recycled', {
|
||||
ageHours: 24 * 8,
|
||||
pid: child.pid,
|
||||
});
|
||||
|
||||
expect(readDataDirOwner(dir)).toBe('alive');
|
||||
|
||||
reapOrphanedDataDirs(root);
|
||||
|
||||
expect(existsSync(dir)).toBe(false);
|
||||
});
|
||||
|
||||
test('reaps a dead owner immediately, without waiting out the cutoff', async () => {
|
||||
const root = makeRoot();
|
||||
const child = spawnLiveProcess();
|
||||
const dir = makeDataDir(root, 'dead', {
|
||||
ageHours: 0,
|
||||
pid: child.pid,
|
||||
});
|
||||
|
||||
child.kill('SIGKILL');
|
||||
// The pid only leaves the table once the parent reaps the exit status,
|
||||
// so wait for that rather than assuming kill() is synchronous.
|
||||
await expect
|
||||
.poll(() => readDataDirOwner(dir), { timeout: 10_000 })
|
||||
.toBe('dead');
|
||||
|
||||
reapOrphanedDataDirs(root);
|
||||
|
||||
expect(existsSync(dir)).toBe(false);
|
||||
});
|
||||
|
||||
test('reaps an unmarked directory once it is past the cutoff', () => {
|
||||
const root = makeRoot();
|
||||
const dir = makeDataDir(root, 'legacy', { ageHours: 48 });
|
||||
|
||||
reapOrphanedDataDirs(root);
|
||||
|
||||
expect(existsSync(dir)).toBe(false);
|
||||
});
|
||||
|
||||
test('keeps a fresh unmarked directory', () => {
|
||||
const root = makeRoot();
|
||||
const dir = makeDataDir(root, 'fresh', { ageHours: 1 });
|
||||
|
||||
reapOrphanedDataDirs(root);
|
||||
|
||||
expect(existsSync(dir)).toBe(true);
|
||||
});
|
||||
|
||||
test('treats an empty or malformed marker as undeterminable, not dead', () => {
|
||||
const root = makeRoot();
|
||||
// A half-written marker must not bypass the age guard and take out a
|
||||
// suite that is still starting up.
|
||||
const empty = makeDataDir(root, 'empty', { ageHours: 1, pid: '' });
|
||||
const partial = makeDataDir(root, 'partial', {
|
||||
ageHours: 1,
|
||||
pid: 'not-a-pid',
|
||||
});
|
||||
|
||||
expect(readDataDirOwner(empty)).toBe('unknown');
|
||||
expect(readDataDirOwner(partial)).toBe('unknown');
|
||||
|
||||
reapOrphanedDataDirs(root);
|
||||
|
||||
expect(existsSync(empty)).toBe(true);
|
||||
expect(existsSync(partial)).toBe(true);
|
||||
});
|
||||
|
||||
test('publishes the owner marker atomically', () => {
|
||||
const root = makeRoot();
|
||||
const dir = makeDataDir(root, 'atomic', { ageHours: 0 });
|
||||
|
||||
writeDataDirOwnerMarker(dir);
|
||||
|
||||
// No `.pending` residue, and the marker resolves to this live process.
|
||||
expect(existsSync(`${join(dir, dataDirOwnerMarker)}.pending`)).toBe(
|
||||
false
|
||||
);
|
||||
expect(readDataDirOwner(dir)).toBe('alive');
|
||||
});
|
||||
|
||||
test('ignores directories that do not carry the suite prefix', () => {
|
||||
const root = makeRoot();
|
||||
const foreign = join(root, 'some-other-tool-XYZ');
|
||||
mkdirSync(foreign, { recursive: true });
|
||||
const when = new Date(Date.now() - 99 * 3600_000);
|
||||
utimesSync(foreign, when, when);
|
||||
|
||||
reapOrphanedDataDirs(root);
|
||||
|
||||
expect(existsSync(foreign)).toBe(true);
|
||||
});
|
||||
|
||||
test('survives a missing root instead of throwing', () => {
|
||||
expect(() =>
|
||||
reapOrphanedDataDirs(join(tmpdir(), 'reaper-spec-does-not-exist'))
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
test('cutoffs are a day and a week, and the hard cap is the looser one', () => {
|
||||
expect(orphanDataDirMaxAgeMs).toBe(24 * 60 * 60 * 1000);
|
||||
expect(dataDirHardMaxAgeMs).toBe(7 * 24 * 60 * 60 * 1000);
|
||||
expect(dataDirHardMaxAgeMs).toBeGreaterThan(orphanDataDirMaxAgeMs);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,123 @@
|
||||
import {
|
||||
readdirSync,
|
||||
readFileSync,
|
||||
renameSync,
|
||||
rmSync,
|
||||
statSync,
|
||||
writeFileSync,
|
||||
} from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
|
||||
export const dataDirPrefix = 'iptvnator-electron-e2e-';
|
||||
export const dataDirOwnerMarker = '.e2e-owner-pid';
|
||||
/** Fallback cutoff, used only for leftovers whose owner cannot be determined. */
|
||||
export const orphanDataDirMaxAgeMs = 24 * 60 * 60 * 1000;
|
||||
/**
|
||||
* Backstop for pid reuse. A pid that looks alive is normally decisive, but the
|
||||
* OS recycles pids — aggressively so on the long-lived Windows runners this
|
||||
* sweep exists for — and an unrelated service inheriting an abandoned run's pid
|
||||
* would otherwise pin that directory forever, defeating the whole point. No
|
||||
* suite survives a week, so past this age a live-looking pid is a stranger.
|
||||
*/
|
||||
export const dataDirHardMaxAgeMs = 7 * 24 * 60 * 60 * 1000;
|
||||
|
||||
export type DataDirOwner = 'alive' | 'dead' | 'unknown';
|
||||
|
||||
/**
|
||||
* Records the current process as the owner of `dataDir`.
|
||||
*
|
||||
* Written to a temporary name and renamed into place, because `writeFileSync`
|
||||
* creates the file before its bytes land: a concurrent sweep could otherwise
|
||||
* observe an empty marker and misjudge a starting run. `rename` is atomic
|
||||
* within a filesystem, so the marker is either absent or complete.
|
||||
*/
|
||||
export function writeDataDirOwnerMarker(dataDir: string): void {
|
||||
const markerPath = join(dataDir, dataDirOwnerMarker);
|
||||
const pendingPath = `${markerPath}.pending`;
|
||||
writeFileSync(pendingPath, String(process.pid));
|
||||
renameSync(pendingPath, markerPath);
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves whether the run that created `dataDir` is still alive.
|
||||
*
|
||||
* Directory age cannot answer this. The suite writes beneath `databases/` and
|
||||
* `user-data/`, which never refreshes the root's mtime, so a run paused in a
|
||||
* debugger or blocked on a native process looks arbitrarily old while still
|
||||
* using its data — and Unix would let a sweep unlink files the live Electron
|
||||
* still has open. So the owner is asked of the OS directly.
|
||||
*
|
||||
* `unknown` covers a missing, unreadable, empty or malformed marker. Callers
|
||||
* must treat it as "cannot tell", never as "dead".
|
||||
*/
|
||||
export function readDataDirOwner(dataDir: string): DataDirOwner {
|
||||
let raw: string;
|
||||
try {
|
||||
raw = readFileSync(join(dataDir, dataDirOwnerMarker), 'utf8').trim();
|
||||
} catch {
|
||||
return 'unknown';
|
||||
}
|
||||
|
||||
const pid = Number.parseInt(raw, 10);
|
||||
if (!raw || !Number.isInteger(pid) || pid <= 0 || String(pid) !== raw) {
|
||||
return 'unknown';
|
||||
}
|
||||
|
||||
try {
|
||||
// Signal 0 runs the existence/permission check without delivering.
|
||||
process.kill(pid, 0);
|
||||
return 'alive';
|
||||
} catch (error) {
|
||||
// EPERM means the pid exists but belongs to another user — still alive.
|
||||
return (error as NodeJS.ErrnoException).code === 'EPERM'
|
||||
? 'alive'
|
||||
: 'dead';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort sweep of data directories abandoned by earlier runs.
|
||||
*
|
||||
* `removeDataDir` tolerates a locked directory rather than failing the run, but
|
||||
* then abandons it, and nothing collects it on our behalf: Windows never clears
|
||||
* %TEMP% on process exit, and the Unix equivalents only run on a schedule. So
|
||||
* every teardown that loses that race leaks a database and user-data tree on a
|
||||
* developer machine or a long-lived self-hosted runner, invisibly.
|
||||
*
|
||||
* A live owner is kept — this repo is routinely checked out into several
|
||||
* worktrees at once — up to `dataDirHardMaxAgeMs`, past which the pid is
|
||||
* assumed recycled rather than still ours. A dead owner is collected at once,
|
||||
* since the pid settles what age only guesses at. An undeterminable owner
|
||||
* falls back to `orphanDataDirMaxAgeMs`.
|
||||
*/
|
||||
export function reapOrphanedDataDirs(root: string = tmpdir()): void {
|
||||
const now = Date.now();
|
||||
let entries: string[];
|
||||
try {
|
||||
entries = readdirSync(root);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const entry of entries) {
|
||||
if (!entry.startsWith(dataDirPrefix)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const candidate = join(root, entry);
|
||||
try {
|
||||
const owner = readDataDirOwner(candidate);
|
||||
const age = now - statSync(candidate).mtimeMs;
|
||||
if (owner === 'alive' && age < dataDirHardMaxAgeMs) {
|
||||
continue;
|
||||
}
|
||||
if (owner === 'unknown' && age < orphanDataDirMaxAgeMs) {
|
||||
continue;
|
||||
}
|
||||
rmSync(candidate, { force: true, recursive: true, maxRetries: 3 });
|
||||
} catch {
|
||||
// Still locked, or owned by another user — leave it for next time.
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -22,6 +22,11 @@ import {
|
||||
} from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { dirname, join, resolve } from 'path';
|
||||
import {
|
||||
dataDirPrefix,
|
||||
reapOrphanedDataDirs,
|
||||
writeDataDirOwnerMarker,
|
||||
} from './data-dir-reaper';
|
||||
|
||||
export const workspaceRoot = resolve(__dirname, '../../..');
|
||||
export const electronMainPath = join(
|
||||
@@ -154,10 +159,17 @@ function removeDataDir(dataDir: string): void {
|
||||
}
|
||||
}
|
||||
|
||||
let reapedOrphanedDataDirs = false;
|
||||
|
||||
export const test = base.extend<ElectronFixtures>({
|
||||
dataDir: async ({ browserName }, use) => {
|
||||
void browserName;
|
||||
const dataDir = mkdtempSync(join(tmpdir(), 'iptvnator-electron-e2e-'));
|
||||
if (!reapedOrphanedDataDirs) {
|
||||
reapedOrphanedDataDirs = true;
|
||||
reapOrphanedDataDirs();
|
||||
}
|
||||
const dataDir = mkdtempSync(join(tmpdir(), dataDirPrefix));
|
||||
writeDataDirOwnerMarker(dataDir);
|
||||
|
||||
await use(dataDir);
|
||||
|
||||
|
||||
Reference in new issue
Block a user