Skip to content

test(engine): cover held tails in HDR preflight - #3963

Open
heygengenesis[bot] wants to merge 1 commit into
mainfrom
test/3958-held-tail-regressions
Open

heygengenesis[bot] wants to merge 1 commit into
mainfrom
test/3958-held-tail-regressions

Conversation

@heygengenesis

@heygengenesis heygengenesis Bot commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #3958.

Summary

  • Add a mixed SDR-in-HDR-timeline regression fixture for finite past-EOF held-tail extraction.
  • Make the existing lint exclusion case use a successful ffprobe mock, so the exclusions—not probe failure—produce no finding.

Behaviour is unchanged: this PR changes tests only.

Validation

  • bunx vitest run src/services/videoFrameExtractor.test.ts -t 'keeps a finite SDR past-EOF slot in a mixed HDR timeline'
  • bunx vitest run src/project.test.ts -t 'silently skips open, looping, remote, variable, missing, and unprobeable slots'
  • bunx vitest run src/project.test.ts — 37 passed
  • bun run --cwd packages/engine typecheck
  • bun run --cwd packages/lint typecheck
  • bunx oxlint and bunx oxfmt --check on the changed tests
  • bunx fallow audit --base origin/main --fail-on-issues — no new issues in changed files

Mutation verification:

  • Reverting the mixed-HDR preflight guard to mediaStart >= playableDuration makes the new fixture fail with media_start_out_of_range for sdr-past-eof.
  • Removing the loop exclusion makes the successful-probe lint fixture emit video_media_start_at_or_past_eof.

Simplify: built-in /simplify was attempted after CI; sandbox error: Not logged in · Please run /login. No reductions were applied.

Commit-pinned references

The guarded mixed-HDR preflight path on main is videoFrameExtractor.ts:1752-1764. The test changes at this head are the mixed HDR fixture and the successful-probe exclusion fixture.

Add regression coverage for finite SDR video slots that start past EOF in mixed HDR timelines, and verify lint exclusions with successful local duration probes.

Co-authored-by: miguel.sierra <229591595+miguel-heygen@users.noreply.github.com>

@miga-heygen miga-heygen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed head 7aadf147 in an isolated worktree. Test-only follow-up to #3962; the diff is exactly two test files and touches no production code.

Strengths

  • packages/engine/src/services/videoFrameExtractor.test.ts:2241-2263 — the mixed SDR-in-HDR fixture is the missing pin for the canHoldFinalFramePastEof half of the preflight guard (videoFrameExtractor.ts:1755). Reverting the guard to mediaStart >= playableDuration locally makes it fail with media_start_out_of_range for sdr-past-eof, exactly as the PR body states.
  • packages/lint/src/project.test.ts:657-659 — switching the exclusion case to mockDurationProbe(2) turns a vacuous test into a real one. Locally, deleting each of the loop, data-var-src, and data-duration exclusions in readFiniteVideoSlot (hevcPreviewLint.ts:176-179) now makes this test emit video_media_start_at_or_past_eof; on the previous head all three survived.

Verified

  • Both tests pass on this head (engine fixture 1/1 with ffmpeg present; project.test.ts 37/37).
  • Four mutants killed: guard revert, drop loop exclusion, drop data-var-src exclusion, drop open-slot (data-duration) exclusion.
  • Fixture files land under the per-suite FIXTURE_DIR temp dir that afterAll removes; no new cleanup surface.
  • All nine required checks green on 7aadf147.

Nit

  • packages/lint/src/project.test.ts:649 — the title still says "unprobeable", but with a succeeding probe mock no row in this fixture exercises that path any more (probe failure is covered separately by "silently skips the finding when ffprobe errors or times out"). Either drop the word or add a row whose probe fails, so the name matches what the assertion proves.

Verdict: APPROVE
Reasoning: Test-only change with no production diff; both new assertions are mutation-verified locally to fail when the guard or any of the three lint exclusions is removed, closing the coverage gaps noted on #3962.

— Miga

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