Skip to content

feat: add comprehensive test coverage for 7 modules (150 tests)#798

Open
photsathonspd1-create wants to merge 1 commit into
OpenCut-app:mainfrom
photsathonspd1-create:feat/comprehensive-test-coverage
Open

feat: add comprehensive test coverage for 7 modules (150 tests)#798
photsathonspd1-create wants to merge 1 commit into
OpenCut-app:mainfrom
photsathonspd1-create:feat/comprehensive-test-coverage

Conversation

@photsathonspd1-create
Copy link
Copy Markdown

@photsathonspd1-create photsathonspd1-create commented May 17, 2026

Summary

Adds 150 passing tests across 7 modules that previously lacked test coverage.

New Test Files

File Tests Coverage
subtitles/__tests__/srt.test.ts 22 Timestamps, multi-line, malformed blocks, line endings
subtitles/__tests__/ass.test.ts 17 Styles, alignment, override tags, play resolution
subtitles/__tests__/parse.test.ts 8 Dispatch, case-insensitive, unsupported extensions
utils/__tests__/math-extended.test.ts 35 clamp, snapToStep, isNearlyEqual, formatNumberForDisplay
fps/__tests__/fps-extended.test.ts 16 frameRate conversion, NTSC, GCD reduction
rendering/__tests__/rendering.test.ts 15 Transform builder, opacity, blend modes
params/__tests__/params.test.ts 32 coerceParamValue, valueKind, interpolation, numeric range

Also includes

  • HANDOFF.md with full documentation of workflow, completed work, and next steps

All tests pass: bun test → 150 pass, 0 fail, 292 expect() calls, 60ms

Summary by CodeRabbit

  • Tests
    • Added comprehensive unit test coverage across 7 critical modules including subtitle format parsing (SRT/ASS), frame rate utilities, parameter handling, math operations, and rendering functionality.
    • New test suite comprises 150 passing tests covering edge cases, format conversions, validation, and error handling.

Review Change Stack

New test files (150 tests, 0 failures):
- subtitles/srt: 22 tests (timestamps, multi-line, malformed, line endings)
- subtitles/ass: 17 tests (styles, alignment, override tags, resolution)
- subtitles/parse: 8 tests (dispatch, case-insensitive, unsupported ext)
- utils/math-extended: 35 tests (clamp, snapToStep, isNearlyEqual, format)
- fps-extended: 16 tests (frameRate conversion, NTSC, GCD reduction)
- rendering: 15 tests (transform builder, opacity, blend modes)
- params: 32 tests (coerce, valueKind, interpolation, numeric range)

Also includes HANDOFF.md with full documentation of workflow,
completed work, and next steps for other agents.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 17, 2026

Someone is attempting to deploy a commit to the OpenCut OSS Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

📝 Walkthrough

Walkthrough

This pull request adds comprehensive unit test coverage for seven previously untested modules in apps/web/src/, introducing 150 passing Bun-based tests across subtitle parsing (SRT and ASS formats with format dispatcher), math utilities, FPS conversion, parameter system operations, and rendering helpers, documented in a handoff guide.

Changes

Unit Test Coverage for Core Utilities and Parsers

Layer / File(s) Summary
Subtitle Parsing Tests
apps/web/src/subtitles/__tests__/parse.test.ts, apps/web/src/subtitles/__tests__/srt.test.ts, apps/web/src/subtitles/__tests__/ass.test.ts
Dispatcher validates format detection, routing to format-specific parsers. SRT parser covers timestamp formats (comma/dot separators, 1–2 digit milliseconds), multi-line cues, sequence numbers, malformed blocks, line-ending normalization, and whitespace handling. ASS parser validates style mapping, play-resolution defaults and overrides, escape sequences, alignment conversions, event robustness, and effect handling.
Utility Function Tests
apps/web/src/fps/__tests__/fps-extended.test.ts, apps/web/src/utils/__tests__/math-extended.test.ts, apps/web/src/params/__tests__/params.test.ts, apps/web/src/rendering/__tests__/rendering.test.ts
FPS conversion between float and fractional representations for integer and NTSC-like rates. Math utilities cover clamping, rounding, snapping to steps, fraction-digit derivation, equality tolerance, and number formatting. Parameter module tests coerce values across number/boolean/color/text/select types with validation, extract value kind and interpolation type, and compute numeric ranges. Rendering helpers test transform parameter extraction, opacity parsing, and blend mode validation.
Test Coverage Documentation
HANDOFF.md
Enumerates test module locations, coverage scope for each of the seven test suites, issues encountered and fixed during test authoring (heredoc backticks, -0 handling, ASS comma splitting), remaining work priorities by category, test-run command documentation, and list of created test files.

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 Seven test files bloom with care,
Math and parsing, handled fair,
Subtitles dance in SRT and ASS,
Parameters coerced with class,
Coverage springs from thoughtful mass!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete. It lacks required sections from the template, specifically the checkboxes indicating whether this is a bug fix or feature, and required approvals. Complete the template by selecting whether this is a bug fix or feature, and include required checkboxes. If this is a feature, note that the template states PRs will be closed until discussed in an issue first.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: adding comprehensive test coverage for 7 modules totaling 150 tests.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
apps/web/src/fps/__tests__/fps-extended.test.ts (1)

96-99: ⚡ Quick win

Fix the GCD-reduction test to use a true fractional input.

This case currently uses 25, so it only re-validates integer mapping and does not exercise fraction reduction logic as the test name states.

Suggested test adjustment
-	test("reduces arbitrary fractional fps with GCD", () => {
-		const result = floatToFrameRate(25);
-		expect(result).toEqual({ numerator: 25, denominator: 1 });
-	});
+	test("reduces arbitrary fractional fps with GCD", () => {
+		const result = floatToFrameRate(12.5);
+		expect(result).toEqual({ numerator: 25, denominator: 2 });
+	});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/fps/__tests__/fps-extended.test.ts` around lines 96 - 99, The
test named "reduces arbitrary fractional fps with GCD" is using an integer (25)
so it doesn't exercise fraction reduction; update the test to pass a true
fractional input (e.g., call floatToFrameRate with 25.5) and assert the reduced
fraction (for 25.5 expect { numerator: 51, denominator: 2 }) so
floatToFrameRate's GCD reduction logic is actually validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@HANDOFF.md`:
- Around line 99-107: Update the fenced code block under the "Files Created"
section to include a language identifier so markdownlint MD040 is satisfied;
replace the opening fence ``` with ```text (i.e., change the triple backticks
before the file list to ```text) and keep the closing ``` unchanged to mark it
as a text block.

---

Nitpick comments:
In `@apps/web/src/fps/__tests__/fps-extended.test.ts`:
- Around line 96-99: The test named "reduces arbitrary fractional fps with GCD"
is using an integer (25) so it doesn't exercise fraction reduction; update the
test to pass a true fractional input (e.g., call floatToFrameRate with 25.5) and
assert the reduced fraction (for 25.5 expect { numerator: 51, denominator: 2 })
so floatToFrameRate's GCD reduction logic is actually validated.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8bc22a60-f703-45eb-8703-2ea23b8741ec

📥 Commits

Reviewing files that changed from the base of the PR and between 6fdb155 and f247641.

📒 Files selected for processing (8)
  • HANDOFF.md
  • apps/web/src/fps/__tests__/fps-extended.test.ts
  • apps/web/src/params/__tests__/params.test.ts
  • apps/web/src/rendering/__tests__/rendering.test.ts
  • apps/web/src/subtitles/__tests__/ass.test.ts
  • apps/web/src/subtitles/__tests__/parse.test.ts
  • apps/web/src/subtitles/__tests__/srt.test.ts
  • apps/web/src/utils/__tests__/math-extended.test.ts

Comment thread HANDOFF.md
Comment on lines +99 to +107
```
apps/web/src/subtitles/__tests__/srt.test.ts
apps/web/src/subtitles/__tests__/ass.test.ts
apps/web/src/subtitles/__tests__/parse.test.ts
apps/web/src/utils/__tests__/math-extended.test.ts
apps/web/src/fps/__tests__/fps-extended.test.ts
apps/web/src/rendering/__tests__/rendering.test.ts
apps/web/src/params/__tests__/params.test.ts
```
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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to the fenced code block.

The block under “Files Created” is missing a fence language, which triggers markdownlint MD040.

Suggested fix
-```
+```text
 apps/web/src/subtitles/__tests__/srt.test.ts
 apps/web/src/subtitles/__tests__/ass.test.ts
 apps/web/src/subtitles/__tests__/parse.test.ts
 apps/web/src/utils/__tests__/math-extended.test.ts
 apps/web/src/fps/__tests__/fps-extended.test.ts
 apps/web/src/rendering/__tests__/rendering.test.ts
 apps/web/src/params/__tests__/params.test.ts
</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.22.1)</summary>

[warning] 99-99: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @HANDOFF.md around lines 99 - 107, Update the fenced code block under the
"Files Created" section to include a language identifier so markdownlint MD040
is satisfied; replace the opening fence withtext (i.e., change the
triple backticks before the file list to text) and keep the closing
unchanged to mark it as a text block.


</details>

<!-- fingerprinting:phantom:poseidon:hawk -->

<!-- This is an auto-generated comment by CodeRabbit -->

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.

1 participant