fix(01-review): IN-04 delete decodeDryRun helper, retain only spawnSync-based decodeDryRunStrict

This commit is contained in:
2026-05-16 10:34:03 +02:00
parent cb23143ccf
commit 680eee3cc7

View File

@@ -36,7 +36,7 @@
import { describe, it, expect } from 'vitest'; import { describe, it, expect } from 'vitest';
import { existsSync, statSync } from 'node:fs'; import { existsSync, statSync } from 'node:fs';
import { execFileSync } from 'node:child_process'; import { spawnSync } from 'node:child_process';
import { fileURLToPath } from 'node:url'; import { fileURLToPath } from 'node:url';
import { dirname, resolve } from 'node:path'; import { dirname, resolve } from 'node:path';
@@ -63,52 +63,34 @@ interface DecodeResult {
endedPrematurely: boolean; endedPrematurely: boolean;
} }
function decodeDryRun(fixturePath: string): DecodeResult { /**
// `-f null -` swallows the decoded output but still surfaces every per-packet * Run ffmpeg in `-f null` mode to dry-decode a WebM fixture without writing
// decoder error to stderr. `-nostdin` prevents ffmpeg from blocking on a TTY * any output. Returns the captured stderr plus parsed counters for the two
// that vitest does not provide. `-v warning` filters the noise floor; the * signals we care about: per-packet decoder errors and the
// signals we care about (`Error submitting packet to decoder`, * "File ended prematurely" tail-error.
// `File ended prematurely`) are emitted at warning level or above. *
let stderr = ''; * Why spawnSync (and not execFileSync):
try { * execFileSync returns ONLY stdout — we cannot read the stderr pipe on the
execFileSync( * success path. ffmpeg exits 0 even when it emitted per-packet decode
FFMPEG_BIN, * errors (with `-f null -`), so the diagnostic signal lives on stderr
['-nostdin', '-v', 'warning', '-i', fixturePath, '-f', 'null', '-'], * regardless of exit code. spawnSync exposes both pipes uniformly.
{ *
stdio: ['ignore', 'ignore', 'pipe'], * The IN-04 fix retired the parallel `decodeDryRun(execFileSync)` helper —
encoding: 'utf-8', * the spawnSync path was always the actual code path used by the assertions
timeout: FFMPEG_TIMEOUT_MS, * below; the execFile variant existed only as a documentation foil and
maxBuffer: 4 * 1024 * 1024, // 4 MiB is comfortable for warning-level logs * required a `void decodeDryRun` noUnusedLocals appeasement hack.
}, *
); * Flags:
} catch (err) { * -nostdin — never block on a TTY (vitest doesn't provide one)
// ffmpeg exits 0 even on per-packet decode errors with `-f null -`, * -v warning — drop the noise floor; signals we care about are emitted
// so a thrown error usually means the binary is genuinely broken or the * at warning level or above
// file is unreadable. Re-throw to fail loudly with full context. * -i <fix> — input file
const e = err as { stderr?: string; message?: string }; * -f null - — swallow decoded output; stderr still carries diagnostics
stderr = e.stderr ?? ''; *
if (!stderr) { * @param fixturePath - Absolute path to the WebM file under test.
throw err; * @returns DecodeResult with `stderr`, `packetErrorCount`, `endedPrematurely`.
} * @throws If ffmpeg was killed by a signal (not a clean exit).
} */
// ffmpeg may also write its diagnostics directly when execFileSync succeeds.
// The captured stderr lives on the error path; on success we attach the
// pipe explicitly.
// execFileSync returns stdout-only by design — to also capture success-path
// stderr, repeat with stdio: ['ignore', 'ignore', 'pipe'] reading the
// returned Buffer is not possible. Use spawnSync semantics instead.
return {
stderr,
packetErrorCount: (stderr.match(/Error submitting packet to decoder/g) ?? []).length,
endedPrematurely: /File ended prematurely/.test(stderr),
};
}
// Variant that uses spawnSync so we can read stderr on the success path too.
// execFileSync above is intentionally kept for the documentation value, but
// the actual assertion uses spawnSync.
import { spawnSync } from 'node:child_process';
function decodeDryRunStrict(fixturePath: string): DecodeResult { function decodeDryRunStrict(fixturePath: string): DecodeResult {
const proc = spawnSync( const proc = spawnSync(
FFMPEG_BIN, FFMPEG_BIN,
@@ -168,12 +150,4 @@ describe('webm playback (RED — confirms webm-playback-freeze bug)', () => {
).toBe(false); ).toBe(false);
}, },
); );
// Touch the unused decodeDryRun symbol so the file's documentation block
// stays compilable under noUnusedLocals. The intent is to leave both
// helpers documented side-by-side: one shows the execFileSync semantics
// (succeeds quietly on decode errors) and the other shows the spawnSync
// approach actually used. Vitest will not execute the body.
// eslint-disable-next-line @typescript-eslint/no-unused-expressions
void decodeDryRun;
}); });