Skip to content

Conversation

@bradh
Copy link
Collaborator

@bradh bradh commented Jan 8, 2026

Follow-up for #90, two more that got missed first time around.

Follow-up for #90, two more that got missed first time around.
@coderabbitai
Copy link

coderabbitai bot commented Jan 8, 2026

Walkthrough

The pull request refactors test data handling in src/test/h264.rs by replacing large inlined byte arrays with include_bytes! macro references. Two test fixtures—h264_avcc_ext_2.mp4 and h264_encrypted_segment.mp4—are now loaded from external binary files at compile time instead of being hardcoded in the source. Test logic, control flow, and assertions remain unchanged. This reduces source code size while maintaining identical test behavior.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: replacing inlined test data with external file references to avoid large byte arrays in the source code.
Description check ✅ Passed The description clearly relates to the changeset by referencing PR #90 and explaining this is a follow-up that addresses additional test files that were missed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a26a585 and 81281db.

⛔ Files ignored due to path filters (2)
  • src/test/h264_avcc_ext_2.mp4 is excluded by !**/*.mp4
  • src/test/h264_encrypted_segment.mp4 is excluded by !**/*.mp4
📒 Files selected for processing (1)
  • src/test/h264.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Code must be clippy-clean (cargo clippy --all-targets --all-features -- -D warnings)
Code must be formatted with rustfmt (cargo fmt) and pass formatting checks (cargo fmt -- --check)
Write unit tests inline within modules using #[cfg(test)]

Files:

  • src/test/h264.rs
src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.rs: Each MP4 atom type must implement the Atom trait with KIND, decode_body(), and encode_body()
Use Header::read_from() to handle large atoms without loading entire contents into memory
All operations should return Result using custom error types defined in error.rs
Use the custom Buf/BufMut traits for contiguous byte slices instead of ad-hoc buffer handling

Files:

  • src/test/h264.rs
src/test/**

📄 CodeRabbit inference engine (CLAUDE.md)

Put integration tests under src/test/ with sample MP4 files for different codecs

Files:

  • src/test/h264.rs
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/**/*.rs : Use the custom Buf/BufMut traits for contiguous byte slices instead of ad-hoc buffer handling
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/**/*.rs : Each MP4 atom type must implement the Atom trait with KIND, decode_body(), and encode_body()

Applied to files:

  • src/test/h264.rs
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/**/*.rs : Use the custom Buf/BufMut traits for contiguous byte slices instead of ad-hoc buffer handling

Applied to files:

  • src/test/h264.rs
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to **/*.rs : Write unit tests inline within modules using #[cfg(test)]

Applied to files:

  • src/test/h264.rs
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/buf.rs : Define and maintain contiguous byte-slice buffer behavior via custom Buf and BufMut traits in buf.rs

Applied to files:

  • src/test/h264.rs
🔇 Additional comments (2)
src/test/h264.rs (2)

328-328: LGTM! Consistent refactoring pattern.

The change follows the same pattern as the other test, correctly externalizing the encrypted segment test fixture data.


306-306: LGTM! Clean refactoring to externalize test data.

The change correctly uses include_bytes! to load test fixture data from an external file at compile time, reducing source code bloat while maintaining identical test behavior. Both required test fixture files (h264_avcc_ext_2.mp4 and h264_encrypted_segment.mp4) are present in the repository.


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.

@kixelated kixelated merged commit 9c11c1d into main Jan 8, 2026
1 check passed
@kixelated kixelated deleted the additional_inline_removal_2026-01-08 branch January 8, 2026 20:26
@github-actions github-actions bot mentioned this pull request Jan 8, 2026
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.

3 participants