Skip to content

test(engine): add ffprobe-unavailable fallback regression tests#379

Open
vanceingalls wants to merge 1 commit intovance/shader-midpoint-testsfrom
vance/ffprobe-fallback-tests
Open

test(engine): add ffprobe-unavailable fallback regression tests#379
vanceingalls wants to merge 1 commit intovance/shader-midpoint-testsfrom
vance/ffprobe-fallback-tests

Conversation

@vanceingalls
Copy link
Copy Markdown
Collaborator

@vanceingalls vanceingalls commented Apr 21, 2026

Summary

Mock node:child_process.spawn to surface ENOENT and verify ffprobe's three callers behave correctly when ffprobe is missing.

Why

Chunk 9B of plans/hdr-followups.md. The PNG cICP fallback in extractMediaMetadata was added to support environments without ffprobe, but no test pinned the behavior — silently regressing it would break HDR image support on any system without ffprobe installed.

What changed

packages/engine/src/utils/ffprobe.test.ts: mocks child_process.spawn to surface ENOENT and asserts:

  • extractMediaMetadata falls back to PNG cICP metadata for image inputs.
  • extractMediaMetadata rethrows for non-image inputs lacking a still-image fallback.
  • extractAudioMetadata + analyzeKeyframeIntervals propagate the install-hint error verbatim.

Test plan

  • All new tests pass.
  • No production code changes — pure regression coverage of existing fallback behavior.

Stack

Chunk 9B of plans/hdr-followups.md. Test-only change, independent of all other chunks.

Copy link
Copy Markdown
Collaborator Author

vanceingalls commented Apr 21, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link
Copy Markdown
Collaborator

@jrusso1020 jrusso1020 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ffprobe-unavailable regression tests are the right coverage for a fallback path that's easy to break silently. The fallback exists for environments where ffprobe isn't on PATH (some minimal Docker images, some CI runners pre-apt-install) — without test coverage, someone could refactor extractMediaMetadata and lose the fallback entirely, with the bug only showing up when the environment actually lacked ffprobe.

Pinning both the "ffprobe missing" branch and the "ffprobe returns garbage" branch (if covered) is the durable form. Approved.

Rames Jusso

@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch from 6881964 to 0483635 Compare April 22, 2026 03:03
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch 2 times, most recently from 18e3dcf to a588b30 Compare April 22, 2026 03:34
@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch from 1d14220 to 2f8e87f Compare April 22, 2026 04:45
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch from a588b30 to c0b8e85 Compare April 22, 2026 04:45
@vanceingalls
Copy link
Copy Markdown
Collaborator Author

Thanks for the approval @jrusso1020.

You're right that this is exactly the silent-breakage class — the fallback only matters in environments without ffprobe on PATH (slim Docker images, pre-apt install CI), so a refactor of extractMediaMetadata could remove the fallback and every test would still pass on a normal dev machine.

The pinned surface in packages/engine/src/utils/ffprobe.test.ts is:

  1. PNG cICP fallback succeeds when ffprobe is missing — confirms videoCodec, colorTransfer=smpte2084, colorPrimaries=bt2020.
  2. Non-image files rethrow the ffprobe not found error verbatim — i.e. the fallback isn't accidentally swallowing errors for inputs it can't handle.
  3. Sibling APIs (extractAudioMetadata, analyzeKeyframeIntervals) also rethrow verbatim — pins that the fallback is scoped to extractMediaMetadata and didn't accidentally bleed into other surfaces.
  4. Install hint ("Please install FFmpeg") is part of the error contract.

So the "ffprobe missing" branch is fully covered. The "ffprobe returns garbage" branch isn't pinned in this PR — it's a different failure mode (ffprobe runs but emits malformed JSON / unexpected schema) and would need a separate harness. Logged as a follow-up rather than blocking this one.

No outstanding action items from the review.

@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch from 2f8e87f to abe64f1 Compare April 22, 2026 18:01
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch from c0b8e85 to a543667 Compare April 22, 2026 18:01
@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch from abe64f1 to a8a7d7f Compare April 22, 2026 18:35
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch from a543667 to 3c1ae7c Compare April 22, 2026 18:35
@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch from a8a7d7f to e3ea8d6 Compare April 22, 2026 18:50
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch 2 times, most recently from 2b1edce to e5aa605 Compare April 22, 2026 18:59
@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch from 230c5df to aa5be19 Compare April 22, 2026 19:34
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch 2 times, most recently from 566c95b to bfa61ac Compare April 22, 2026 20:45
@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch from 456f47f to bb1d031 Compare April 22, 2026 20:48
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch 2 times, most recently from d0dd838 to de75bbd Compare April 22, 2026 22:13
@vanceingalls vanceingalls force-pushed the vance/shader-midpoint-tests branch 2 times, most recently from c1d8c12 to 1f9a361 Compare April 22, 2026 22:50
Mock node:child_process.spawn to surface ENOENT and verify that:
- extractMediaMetadata falls back to PNG cICP metadata for image inputs
- extractMediaMetadata rethrows for non-image inputs lacking a still-image fallback
- extractAudioMetadata + analyzeKeyframeIntervals propagate the install-hint error verbatim

Closed gap from hdr-followups Chunk 9B.
@vanceingalls vanceingalls force-pushed the vance/ffprobe-fallback-tests branch from de75bbd to 817ab68 Compare April 22, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants