From 680eee3cc73466d57da36defbfb4999f23591fb3 Mon Sep 17 00:00:00 2001
From: Mark <strategy155@gmail.com>
Date: Sat, 16 May 2026 10:34:03 +0200
Subject: [PATCH] fix(01-review): IN-04 delete decodeDryRun helper, retain only
 spawnSync-based decodeDryRunStrict

---
 tests/offscreen/webm-playback.test.ts | 84 +++++++++------------------
 1 file changed, 29 insertions(+), 55 deletions(-)

diff --git a/tests/offscreen/webm-playback.test.ts b/tests/offscreen/webm-playback.test.ts
index ebddc29..e82f373 100644
--- a/tests/offscreen/webm-playback.test.ts
+++ b/tests/offscreen/webm-playback.test.ts
@@ -36,7 +36,7 @@
 
 import { describe, it, expect } from 'vitest';
 import { existsSync, statSync } from 'node:fs';
-import { execFileSync } from 'node:child_process';
+import { spawnSync } from 'node:child_process';
 import { fileURLToPath } from 'node:url';
 import { dirname, resolve } from 'node:path';
 
@@ -63,52 +63,34 @@ interface DecodeResult {
   endedPrematurely: boolean;
 }
 
-function decodeDryRun(fixturePath: string): DecodeResult {
-  // `-f null -` swallows the decoded output but still surfaces every per-packet
-  // decoder error to stderr. `-nostdin` prevents ffmpeg from blocking on a TTY
-  // that vitest does not provide. `-v warning` filters the noise floor; the
-  // signals we care about (`Error submitting packet to decoder`,
-  // `File ended prematurely`) are emitted at warning level or above.
-  let stderr = '';
-  try {
-    execFileSync(
-      FFMPEG_BIN,
-      ['-nostdin', '-v', 'warning', '-i', fixturePath, '-f', 'null', '-'],
-      {
-        stdio: ['ignore', 'ignore', 'pipe'],
-        encoding: 'utf-8',
-        timeout: FFMPEG_TIMEOUT_MS,
-        maxBuffer: 4 * 1024 * 1024, // 4 MiB is comfortable for warning-level logs
-      },
-    );
-  } catch (err) {
-    // ffmpeg exits 0 even on per-packet decode errors with `-f null -`,
-    // so a thrown error usually means the binary is genuinely broken or the
-    // file is unreadable. Re-throw to fail loudly with full context.
-    const e = err as { stderr?: string; message?: string };
-    stderr = e.stderr ?? '';
-    if (!stderr) {
-      throw err;
-    }
-  }
-  // 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';
-
+/**
+ * Run ffmpeg in `-f null` mode to dry-decode a WebM fixture without writing
+ * any output. Returns the captured stderr plus parsed counters for the two
+ * signals we care about: per-packet decoder errors and the
+ * "File ended prematurely" tail-error.
+ *
+ * Why spawnSync (and not execFileSync):
+ *   execFileSync returns ONLY stdout — we cannot read the stderr pipe on the
+ *   success path. ffmpeg exits 0 even when it emitted per-packet decode
+ *   errors (with `-f null -`), so the diagnostic signal lives on stderr
+ *   regardless of exit code. spawnSync exposes both pipes uniformly.
+ *
+ * The IN-04 fix retired the parallel `decodeDryRun(execFileSync)` helper —
+ * the spawnSync path was always the actual code path used by the assertions
+ * below; the execFile variant existed only as a documentation foil and
+ * required a `void decodeDryRun` noUnusedLocals appeasement hack.
+ *
+ * Flags:
+ *   -nostdin    — never block on a TTY (vitest doesn't provide one)
+ *   -v warning  — drop the noise floor; signals we care about are emitted
+ *                 at warning level or above
+ *   -i <fix>    — input file
+ *   -f null -   — swallow decoded output; stderr still carries diagnostics
+ *
+ * @param fixturePath - Absolute path to the WebM file under test.
+ * @returns DecodeResult with `stderr`, `packetErrorCount`, `endedPrematurely`.
+ * @throws If ffmpeg was killed by a signal (not a clean exit).
+ */
 function decodeDryRunStrict(fixturePath: string): DecodeResult {
   const proc = spawnSync(
     FFMPEG_BIN,
@@ -168,12 +150,4 @@ describe('webm playback (RED — confirms webm-playback-freeze bug)', () => {
       ).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;
 });