Stop manually inlining test files. #90

kixelated · 2026-01-07T03:58:26Z

No description provided.

coderabbitai · 2026-01-07T04:01:30Z

Walkthrough

Multiple test files in src/test/ have been refactored to replace inline hard-coded byte arrays with include_bytes!() macro calls that load test data from external files at compile time. Affected test files include av1.rs, bbb.rs, esds.rs, flac.rs, h264.rs, hevc.rs, uncompressed.rs, vp9.rs, and image.rs. In image.rs, two separate byte arrays for HEIF and AVIF test data are replaced with file inclusions. The module declaration in mod.rs is updated from mod heif; to mod image;. Test logic, decoding flow, and assertion logic remain unchanged across all modifications.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, only the title and metadata.	Consider adding a brief description explaining the motivation for using include_bytes! instead of inlined test data.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: replacing hard-coded, inlined test data with include_bytes! to load from external files.
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 e53a67b and 5dbd20c.

📒 Files selected for processing (4)

src/test/image.avif
src/test/image.heif
src/test/image.rs
src/test/mod.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/mod.rs
src/test/image.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/mod.rs
src/test/image.rs

src/test/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

src/test/mod.rs
src/test/image.rs

🧠 Learnings (2)

📓 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 **/*.rs : Write unit tests inline within modules using #[cfg(test)]

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/test/** : Put integration tests under src/test/ with sample MP4 files for different codecs

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 : Use the custom Buf/BufMut traits for contiguous byte slices instead of ad-hoc buffer handling

Applied to files:

src/test/image.rs

🔇 Additional comments (2)

src/test/mod.rs (1)

7-7: LGTM!

The module rename from heif to image appropriately reflects the expanded scope, as the module now contains tests for both HEIF and AVIF image formats.

src/test/image.rs (1)

6-6: Approved: Good refactoring to external test fixtures.

Replacing inline byte arrays with include_bytes!() is the idiomatic Rust approach for embedding test data. The fixture files exist at the expected locations and the relative paths in both calls (lines 6 and 283) are correct.

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.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/test/heif.rs (1)
283-307: Consider applying the same pattern to the avif() test.

The heif() test now uses include_bytes!(), but the avif() test still uses an inline byte array. For consistency with the PR objective ("Stop manually inlining test files"), consider moving this test data to an external avif.mp4 file as well.
🔎 Proposed refactor

Create src/test/avif.mp4 with the test data, then update the test:
 #[test]
 fn avif() {
     // Complete image from libavif
-    const ENCODED: &[u8] = &[
-        0x00, 0x00, 0x00, 0x20, 0x66, 0x74, 0x79, 0x70, 0x61, 0x76, 0x69, 0x66, 0x00, 0x00, 0x00,
-        0x00, 0x61, 0x76, 0x69, 0x66, 0x6d, 0x69, 0x66, 0x31, 0x6d, 0x69, 0x61, 0x66, 0x4d, 0x41,
-        0x31, 0x41, 0x00, 0x00, 0x01, 0x10, 0x6d, 0x65, 0x74, 0x61, 0x00, 0x00, 0x00, 0x00, 0x00,
-        ...
-        0x61, 0x84, 0x00, 0x01, 0x2a, 0xbe, 0xff, 0x80,
-    ];
+    const ENCODED: &[u8] = include_bytes!("avif.mp4");

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6632462 and e53a67b.

⛔ Files ignored due to path filters (9)

src/test/av1.mp4 is excluded by !**/*.mp4
src/test/bbb.mp4 is excluded by !**/*.mp4
src/test/esds.mp4 is excluded by !**/*.mp4
src/test/flac.mp4 is excluded by !**/*.mp4
src/test/h264.mp4 is excluded by !**/*.mp4
src/test/heif.mp4 is excluded by !**/*.mp4
src/test/hevc.mp4 is excluded by !**/*.mp4
src/test/uncompressed.mp4 is excluded by !**/*.mp4
src/test/vp9.mp4 is excluded by !**/*.mp4

📒 Files selected for processing (9)

src/test/av1.rs
src/test/bbb.rs
src/test/esds.rs
src/test/flac.rs
src/test/h264.rs
src/test/heif.rs
src/test/hevc.rs
src/test/uncompressed.rs
src/test/vp9.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/heif.rs
src/test/vp9.rs
src/test/flac.rs
src/test/uncompressed.rs
src/test/hevc.rs
src/test/av1.rs
src/test/esds.rs
src/test/bbb.rs
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/heif.rs
src/test/vp9.rs
src/test/flac.rs
src/test/uncompressed.rs
src/test/hevc.rs
src/test/av1.rs
src/test/esds.rs
src/test/bbb.rs
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/heif.rs
src/test/vp9.rs
src/test/flac.rs
src/test/uncompressed.rs
src/test/hevc.rs
src/test/av1.rs
src/test/esds.rs
src/test/bbb.rs
src/test/h264.rs

🧠 Learnings (4)

📓 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/test/** : Put integration tests under src/test/ with sample MP4 files for different codecs

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)]

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

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()

📚 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/heif.rs
src/test/vp9.rs
src/test/flac.rs
src/test/uncompressed.rs
src/test/av1.rs
src/test/esds.rs
src/test/bbb.rs
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/esds.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/esds.rs
src/test/h264.rs

🔇 Additional comments (9)

src/test/esds.rs (1)

5-5: LGTM! Consistent pattern applied.

The change correctly uses include_bytes!() to load test data from an external file, matching the pattern used in other test files in this PR.

src/test/heif.rs (1)

6-6: LGTM! External test data pattern applied.

The change correctly loads test data from an external file, improving code maintainability.

src/test/vp9.rs (1)

5-5: LGTM! Test data successfully externalized.

The change correctly applies the include_bytes!() pattern to load test data from an external file, consistent with the other test files in this PR.

src/test/uncompressed.rs (1)

8-8: LGTM! Clean refactor to external test data.

This change successfully moves the test data from inline bytes to an external file using include_bytes!(), which improves maintainability and readability. The macro loads the file contents at compile time, so there's no runtime overhead. The external test data file exists and is properly referenced.

src/test/av1.rs (1)

5-5: LGTM! Clean refactor to external test data.

The change correctly uses include_bytes!() to load test data at compile time from an external file, improving code maintainability and consistency across the test suite. Test data files are properly organized in src/test/.

src/test/bbb.rs (1)

5-5: LGTM! Consistent with the refactoring pattern.

The change follows the same clean pattern as the other test files, replacing the large inline byte array with a compile-time file include.

src/test/hevc.rs (1)

5-5: LGTM! Refactor improves code clarity.

This change is consistent with the broader refactoring effort and removes a large inline array (note the extensive HEVC configuration data that would have been inlined).

src/test/h264.rs (1)

6-6: LGTM! Refactor follows established pattern.

The change maintains consistency with other test files while preserving all test logic and assertions. Based on learnings, this aligns with the guideline to put integration tests under src/test/ with sample MP4 files.

src/test/flac.rs (1)

5-5: LGTM! Clean refactor to external test data.

This change improves maintainability by moving test data out of source code, aligning with the coding guidelines to use sample MP4 files for integration tests. The binary file flac.mp4 exists and the test structure properly validates decode/encode round-trips.

bradh

A couple of nits, but otherwise LGTM.

src/test/image.rs

src/test/heif.rs

kixelated · 2026-01-07T05:22:32Z

Also I briefly considered Git LFS. I think we should do it if we want to use larger video files for tests.

bradh · 2026-01-07T08:32:16Z

Also I briefly considered Git LFS. I think we should do it if we want to use larger video files for tests.

I'd prefer to try really hard for smaller size files. Since we're really about box parsing, there doesn't need to be a lot of frames.

bradh · 2026-01-07T21:43:02Z

Also I briefly considered Git LFS. I think we should do it if we want to use larger video files for tests.

Another option would be a git submodule for https://github.com/MPEGGroup/FileFormatConformance

bradh · 2026-01-08T00:41:32Z

@coderabbitai review

coderabbitai · 2026-01-08T00:41:37Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

Stop manually inlining test files.

e53a67b

kixelated requested a review from bradh January 7, 2026 03:58

coderabbitai bot reviewed Jan 7, 2026

View reviewed changes

bradh approved these changes Jan 7, 2026

View reviewed changes

src/test/image.rs Show resolved Hide resolved

src/test/heif.rs Outdated Show resolved Hide resolved

rename image file format tests

5dbd20c

bradh force-pushed the include_bytes branch from 279acf0 to 5dbd20c Compare January 8, 2026 00:38

bradh merged commit 68e6260 into main Jan 8, 2026
1 check passed

bradh deleted the include_bytes branch January 8, 2026 01:01

github-actions bot mentioned this pull request Jan 8, 2026

chore: release v0.10.0 #79

Open

bradh added a commit that referenced this pull request Jan 8, 2026

avoid inlining a couple more test files

81281db

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

bradh mentioned this pull request Jan 8, 2026

avoid inlining a couple more test files #95

Merged

kixelated pushed a commit that referenced this pull request Jan 8, 2026

avoid inlining a couple more test files (#95)

9c11c1d

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Stop manually inlining test files. #90

Stop manually inlining test files. #90

Uh oh!

kixelated commented Jan 7, 2026

Uh oh!

coderabbitai bot commented Jan 7, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

bradh left a comment

Uh oh!

Uh oh!

Uh oh!

kixelated commented Jan 7, 2026

Uh oh!

bradh commented Jan 7, 2026

Uh oh!

bradh commented Jan 7, 2026

Uh oh!

bradh commented Jan 8, 2026

Uh oh!

coderabbitai bot commented Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Stop manually inlining test files. #90

Stop manually inlining test files. #90

Uh oh!

Conversation

kixelated commented Jan 7, 2026

Uh oh!

coderabbitai bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

bradh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kixelated commented Jan 7, 2026

Uh oh!

bradh commented Jan 7, 2026

Uh oh!

bradh commented Jan 7, 2026

Uh oh!

bradh commented Jan 8, 2026

Uh oh!

coderabbitai bot commented Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Jan 7, 2026 •

edited

Loading