test(api-db): cover the retained_boot_interfaces store#2931
Conversation
|
@coderabbitai PTAL, thanks! |
|
✅ Action performedReview finished.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a test module for retained boot interface behavior, including helper utilities and coverage for upsert, lookup, consume, retention-window filtering, expiry deletion, and multi-MAC reads. ChangesRetained boot interface tests
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
🔍 Container Scan Summary
Per-CVE detail lives in the per-service |
retained_boot_interfaces is the store that lets a re-ingested machine recover its boot interface id after the live MAC is gone -- the piece that keeps the boot NIC resolvable by its interface id even when a DPU/NIC-mode flip (or an iDRAC partition) drops the MAC from Redfish. It had no direct tests; these lock in the round-trip and the retention-window semantics that bound a recycled MAC. - Round-trip: upsert then find_by_mac/take_by_mac recover the id, upsert keeps the newest id on conflict, and take_by_mac consumes the row. - Window: find_by_mac filters records aged past the window; take_by_mac withholds a stale id but still removes the row (so a recycled MAC can't inherit it); delete_expired sweeps only aged rows and no-ops without a window. - find_records_by_macs returns every match unfiltered (stale included) for the troubleshooting view. Tests added! This supports NVIDIA#2821 Signed-off-by: Chet Nichols III <chetn@nvidia.com>
|
@coderabbitai review |
6c0b386 to
561e3cc
Compare
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/api-db/src/retained_boot_interface.rs (1)
250-330: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd an exact cutoff case for the retention window.
These tests cover clearly fresh and clearly stale rows, but not the boundary itself. The production predicates use strict
>for reads and<=for sweeps, so a row recorded exactly atwindowage is a distinct contract worth locking in forfind_by_mac,take_by_mac, anddelete_expired. As per coding guidelines, "Add or update focused tests for bug fixes, shared behavior, API contracts, migrations, and cross-module changes."🤖 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 `@crates/api-db/src/retained_boot_interface.rs` around lines 250 - 330, Add a focused boundary test in retained_boot_interface.rs to cover a row aged exactly to the retention window, since the current tests only verify clearly fresh and stale cases. Use the existing helpers and symbols find_by_mac, take_by_mac, delete_expired, age_record, and upsert to assert the exact-cutoff contract: reads should still exclude the row at the boundary, take_by_mac should withhold the id but consume the row, and delete_expired should remove rows aged exactly at the window. Keep the new test aligned with the current retention-window behavior so the strict > and <= predicates are locked in.Source: Coding guidelines
🤖 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.
Nitpick comments:
In `@crates/api-db/src/retained_boot_interface.rs`:
- Around line 250-330: Add a focused boundary test in retained_boot_interface.rs
to cover a row aged exactly to the retention window, since the current tests
only verify clearly fresh and stale cases. Use the existing helpers and symbols
find_by_mac, take_by_mac, delete_expired, age_record, and upsert to assert the
exact-cutoff contract: reads should still exclude the row at the boundary,
take_by_mac should withhold the id but consume the row, and delete_expired
should remove rows aged exactly at the window. Keep the new test aligned with
the current retention-window behavior so the strict > and <= predicates are
locked in.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 6bf14ca8-3b84-4185-a0ab-5deda12c1907
📒 Files selected for processing (1)
crates/api-db/src/retained_boot_interface.rs
2 participants
Summary
retained_boot_interfacesis the store that lets a re-ingested machine recover its boot interface id after the live MAC is gone -- the piece that keeps the boot NIC resolvable by its interface id even when a DPU/NIC-mode flip (or an iDRAC partition, #2821) drops the MAC from Redfish. It had no direct tests; this adds deterministic, DB-backed coverage of its round-trip and retention-window behavior.Tests (all via the
#[crate::sqlx_test]harness against Postgres)upsertthenfind_by_mac/take_by_macrecover the id;upsertkeeps the newest id on conflict;take_by_macconsumes the row.find_by_macfilters records aged past the window;take_by_macwithholds a stale id but still removes the row (so a recycled MAC can't inherit it);delete_expiredsweeps only aged rows and no-ops without a window.find_records_by_macsreturns every match unfiltered (stale included).Test-only -- no production code changes.
This supports #2821