Conversation
.gitignore
Outdated
| **/.DS_Store | ||
|
|
||
| # Test artifacts | ||
| tests/fixtures/ |
There was a problem hiding this comment.
I think you are not generating any fixture with the tests, tell me if i am wrong but if not you should leave the gitignore untouched
📝 WalkthroughWalkthroughA new integration test suite is added for InitramfsBuilder with helper utilities and seven tokio-based async tests. Tests validate CPIO archive generation, metadata accuracy, init script injection, file injection, compression modes, exclude patterns, and reproducibility. Includes CPIO newc parser and gzip decompression utilities. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Evan-Paillard
force-pushed
the
feat/test-agent
branch
from
4cab882 to
1e891cb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration_test.rs
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration_test.rs`:
- Around line 250-252: The strict ordering assertion between none_size and
gzip_size is flaky; change the check on the sizes (the variables none_size and
gzip_size derived from sizes.iter()) to a tolerant assertion or weaker check:
either assert both sizes are non-zero (e.g., assert!(none_size > 0 && gzip_size
> 0)) or use a ratio/tolerance comparison (e.g., treat none_size >= gzip_size *
0.95) so gzip can be slightly larger without failing; update the assertion
accordingly to match the earlier metadata ratio style if you keep a ratio-based
check.
- Around line 91-94: The test currently hardcodes the floating image string
"debian:stable-slim" in the InitramfsBuilder chain
(InitramfsBuilder::new().image(...).build(&output)); add a helper function like
test_image() that reads INITRAMFS_TEST_IMAGE from the environment with a
fallback to "debian:stable-slim", then replace the literal in the image(...)
call with test_image(); this centralizes the image, allows CI to pin or override
the image, and improves reproducibility while keeping existing assertions
intact.
| let result = InitramfsBuilder::new() | ||
| .image("debian:stable-slim") | ||
| .compression(Compression::Gzip) | ||
| .build(&output) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the integration test file
find . -name "integration_test.rs" -type fRepository: DO-2K24-27/rusty-initramfs-builder
Length of output: 105
🏁 Script executed:
# Read the integration test file to see the context around lines 91-94
wc -l ./tests/integration_test.rsRepository: DO-2K24-27/rusty-initramfs-builder
Length of output: 109
🏁 Script executed:
# Read the area around lines 91-94
sed -n '85,100p' ./tests/integration_test.rsRepository: DO-2K24-27/rusty-initramfs-builder
Length of output: 557
🏁 Script executed:
# Check the entire test file to see all test functions and image usage patterns
grep -n "\.image(" ./tests/integration_test.rsRepository: DO-2K24-27/rusty-initramfs-builder
Length of output: 449
🏁 Script executed:
# Check for existing env var patterns in the test file
grep -n "env::" ./tests/integration_test.rsRepository: DO-2K24-27/rusty-initramfs-builder
Length of output: 60
🏁 Script executed:
# Read the full test file to understand what assertions are being made
cat -n ./tests/integration_test.rsRepository: DO-2K24-27/rusty-initramfs-builder
Length of output: 12226
Consider parameterizing the base image via environment variable for better test reproducibility and CI efficiency.
debian:stable-slim is a floating tag and will pull different builds over time. While current assertions are robust (checking for basic directory structures that exist in all stable Debian releases), pinning or centralizing the image string with env overrides improves build reproducibility and avoids unnecessary re-pulls across CI runs.
🔧 Suggested change (within this call site)
- .image("debian:stable-slim")
+ .image(&test_image())Add a helper near the top of the file:
fn test_image() -> String {
std::env::var("INITRAMFS_TEST_IMAGE")
.unwrap_or_else(|_| "debian:stable-slim".to_string())
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration_test.rs` around lines 91 - 94, The test currently hardcodes
the floating image string "debian:stable-slim" in the InitramfsBuilder chain
(InitramfsBuilder::new().image(...).build(&output)); add a helper function like
test_image() that reads INITRAMFS_TEST_IMAGE from the environment with a
fallback to "debian:stable-slim", then replace the literal in the image(...)
call with test_image(); this centralizes the image, allows CI to pin or override
the image, and improves reproducibility while keeping existing assertions
intact.
| let none_size = sizes.iter().find(|(l, _)| l == "none").unwrap().1; | ||
| let gzip_size = sizes.iter().find(|(l, _)| l == "gzip").unwrap().1; | ||
| assert!(none_size > gzip_size); |
There was a problem hiding this comment.
Compression size ordering isn’t guaranteed.
Gzip can be slightly larger for certain inputs, which can make this flaky. Consider adding a tolerance or only asserting non‑zero sizes. If you keep a ratio check, align the earlier metadata test for consistency.
🔧 Example tolerance-based assertion
- assert!(none_size > gzip_size);
+ assert!(
+ gzip_size <= none_size + 4096,
+ "gzip output larger than uncompressed by >4KiB"
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/integration_test.rs` around lines 250 - 252, The strict ordering
assertion between none_size and gzip_size is flaky; change the check on the
sizes (the variables none_size and gzip_size derived from sizes.iter()) to a
tolerant assertion or weaker check: either assert both sizes are non-zero (e.g.,
assert!(none_size > 0 && gzip_size > 0)) or use a ratio/tolerance comparison
(e.g., treat none_size >= gzip_size * 0.95) so gzip can be slightly larger
without failing; update the assertion accordingly to match the earlier metadata
ratio style if you keep a ratio-based check.
2 participants
Add integration tests for agent injection
Description
Added a proper integration test suite to verify the builder actually generates bootable archives.
Key changes:
tests/integration_test.rs: Downloads a kernel, builds an initramfs with a custom agent, and boots it in QEMU.tests/agent/: A dummy agent to verify reliable communication from inside the VM.This proves that injected binaries run correctly on boot.
How Has This Been Tested?
Ran
cargo test --test integration_testlocally.Checklist:
Summary by CodeRabbit