Skip to content

test(core): pin three composition-length rules that no test covered - #5000

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/core-composition-length-pins
Oct 4, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/core-composition-length-pins

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

What

Tests only. Three rules moved unchanged into @hyperframes/core/runtime/composition-length by #4990 had no test that fails when the rule is removed. This adds one for each:

  • Direct sub-compositions only: the authored floor counts the root's own sub-compositions, not ones nested deeper.
  • One-frame skip: a media clip of one frame or less is skipped in the media window.
  • Pending Lottie: a declared data-lottie-src, or a loaded Lottie library, keeps the derived length pending until it registers.

Why

The review of #4990 found that removing any of these three rules left every test green. They were moved without change, so this is an older coverage gap, closed now so a later edit to the module cannot drop one silently.

Related work

Refs #4990.

How

Four cases in packages/core/src/runtime/compositionLength.test.ts, driving the exported functions directly on a linkedom document.

Test plan

  • Unit tests added: compositionLength.test.ts 20 passed, 3 runs in a row.
  • Each new test turns red when its rule is removed (direct-child filter, one-frame media skip, [data-lottie-src] check, the loaded-library flag), and only that test.
  • oxfmt --check clean; core tsc --noEmit exit 0.
  • Manual testing performed
  • Documentation updated (if applicable)
  • Comments follow CONTRIBUTING.md "Comments"

Size: test-only on purpose, the follow-up promised in #4990's review.

@miguel-heygen
miguel-heygen marked this pull request as ready for review October 4, 2026 08:40

@terencecho terencecho 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.

Approving c4b7418e. The four new tests pin the rules my #4990 review found unpinned, and each goes red when its rule is removed. The diff is one test file (+55, 0 deletions); no production code changes.

What I verified

  • compositionLength.test.ts passes 20/20, 3 runs in a row, with NODE_ENV=test.
  • I mutated compositionLength.ts six ways, and each mutant failed only the intended test:
    • Dropping the direct-child filter in resolveAuthoredCompositionFloorSeconds fails "counts only the root's own sub-compositions". The nested 50 s clip turns the 3 s floor into 51 s.
    • Changing <= to < in the media window fails "skips a media clip of one frame or less". The test feeds exactly MIN_VALID_TIMELINE_DURATION_SECONDS, so the boundary itself is pinned. Removing the skip altogether fails the same test.
    • Removing the [data-lottie-src] check fails the declared-Lottie test, and removing the unregisteredLottie flag fails the loaded-library test.
    • Forcing the Lottie branch to always push null fails two existing tests as well, so those pending-clip cases were already covered.
  • The tests call the exported functions on a parsed document with the real start resolver, not mocks, so they pin the rule and not an implementation detail. oxfmt --check and oxlint are clean on the file.

CI

  • The Build job was cancelled at its 10-minute cap: bun run build took 7m46s, so verify:packed-manifests was cut off. Studio: edit accuracy gate then failed because its shards need Build's CLI artifact and never ran ("Edit accuracy fell against the base branch" is that missing data, not a measured drop; nothing in a core test file can change it). Studio and player captures was also cancelled, on a superseded duplicate run.
  • Every other check at this head finished green (57 pass, 10 skipped). Build and the edit-accuracy gate need a rerun before this can merge; I did not rerun them.

Non-blocking: the one-frame test pins the boundary at exactly one frame; a threshold raised slightly above one frame would not be caught. Nothing in the module's contract calls for more.

I read the diff, ran the tests and mutants locally, and did not run the browser or Studio. This approval is on code merit and is not authorization to merge or deploy beyond what the gate already does.

— Review by tai (pr-review)

@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

Edit accuracy: accurate 1556 (base branch 1556), smooth 1435 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit 351d389 Oct 4, 2026
122 of 125 checks passed
@miguel-heygen
miguel-heygen deleted the fix/core-composition-length-pins branch October 4, 2026 09:32
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