test(rpc): cover hardware discovery conversions by chet · Pull Request #4060 · NVIDIA/infra-controller

chet · 2026-07-24T00:12:56Z

Summary

Add direction-specific table-driven coverage for architecture, network and InfiniBand interfaces, LLDP data, discovery payloads, machine inventory, and NVLink conversions.
Exercise sparse, populated, invalid, and compatibility cases across every production conversion implementation in hardware_info.rs.
Keep downstream integration coverage intact; this changes no production behavior or dependencies.

Coverage

Production code in crates/rpc/src/model/hardware_info.rs:

Metric	Before	After
Functions	16 / 43 (37.21%)	43 / 43 (100%)
Lines	153 / 400 (38.25%)	394 / 400 (98.50%)
Regions	136 / 397 (34.26%)	368 / 397 (92.70%)

The model-feature RPC suite grows from 117 to 128 tests. The new coverage is organized as 11 table-driven tests with 29 named cases.

The six remaining uncovered production lines are Some(Err(error)) arms behind nested converters that are currently infallible. The surrounding optional and successful paths are covered directly.

Validation

cargo test -p carbide-rpc --features model hardware_info::tests --lib
cargo test -p carbide-rpc --features model --lib
cargo test -p carbide-rpc --all-features --lib
cargo make format-nightly
cargo make clippy
cargo make carbide-lints
Independent agent review, local CodeRabbit review, and local Claude review

Closes #4037

coderabbitai · 2026-07-24T00:13:16Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 5bdf9951-95a7-477c-933d-71acea6eacce

📥 Commits

Reviewing files that changed from the base of the PR and between e588786 and 2296c27.

📒 Files selected for processing (1)

crates/rpc/src/model/hardware_info.rs

Summary by CodeRabbit

Tests
- Added comprehensive validation for hardware information conversion behavior.
- Covered CPU architecture selection with legacy precedence and invalid-legacy fallback handling.
- Verified network interface inbound/outbound conversions, including MAC parsing, Mellanox sentinel MAC rewriting, and rejection of malformed MAC strings.
- Added LLDP switch and discovery conversion checks for IPv4/IPv6 handling, invalid management address rejection, TPM EK base64 validation, and propagation of nested validation failures.
- Validated machine inventory and GPU/NVLink conversions, preserving ordering and enforcing required identifiers.

Walkthrough

Adds comprehensive tests for hardware discovery conversion boundaries, including compatibility handling, parsing and validation, nested discovery data, inventory mappings, and NVLink conversions in both directions.

Changes

Hardware discovery conversion coverage

Layer / File(s)	Summary
Conversion projections and fixtures `crates/rpc/src/model/hardware_info.rs`	Adds projection types, conversion helpers, and sparse or populated fixtures for comparing internal and RPC hardware-discovery data.
Discovery parsing and validation tests `crates/rpc/src/model/hardware_info.rs`	Tests architecture precedence, MAC and LLDP address handling, TPM certificate decoding, nested discovery errors, and reverse discovery conversion.
Inventory and NVLink conversion tests `crates/rpc/src/model/hardware_info.rs`	Tests inventory field mapping and ordering, NVLink domain validation, GPU ordering, and bidirectional conversion behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the added RPC hardware discovery conversion tests.
Description check	✅ Passed	The description is clearly related to the changeset and matches the added test coverage.
Linked Issues check	✅ Passed	The tests cover the conversion boundary cases and representative fixtures requested in `#4037`.
Out of Scope Changes check	✅ Passed	The changes stay within the requested test coverage for hardware_info.rs and add no unrelated scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands.}

chet · 2026-07-24T00:14:31Z

@coderabbitai full_review, thanks!

coderabbitai · 2026-07-24T00:14:39Z

@chet Understood — I’ll perform a complete re-review of the PR. 🐇

✅ Action performed

Full review finished.

coderabbitai

🧹 Nitpick comments (1)

crates/rpc/src/model/hardware_info.rs (1)
1054-1068: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consolidate the #[allow(deprecated)] LLDP helpers. Both helpers suppress a lint the repo enforces as an error and depend on the deprecated ip_address field; verify the conversion still targets that field and, if required, scope/document the allow rather than leaving it broad. As per coding guidelines: "avoid #[allow(...)]".

crates/rpc/src/model/hardware_info.rs#L1054-L1068: justify or migrate rpc_lldp off the deprecated ip_address field; narrow the allow if the field is genuinely needed.

crates/rpc/src/model/hardware_info.rs#L1096-L1106: apply the same treatment to project_rpc_lldp, which reads the deprecated ip_address field.
🤖 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/rpc/src/model/hardware_info.rs` around lines 1054 - 1068, Consolidate
the deprecated LLDP handling in rpc_lldp and project_rpc_lldp: verify both
helpers still construct the deprecated ip_address field, migrate to the
replacement field when available, or retain a narrowly scoped, documented allow
only around the required field usage. Remove the broad helper-level
#[allow(deprecated)] attributes and apply the same treatment at both sites in
crates/rpc/src/model/hardware_info.rs:1054-1068 and 1096-1106.
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/rpc/src/model/hardware_info.rs`:
- Around line 1054-1068: Consolidate the deprecated LLDP handling in rpc_lldp
and project_rpc_lldp: verify both helpers still construct the deprecated
ip_address field, migrate to the replacement field when available, or retain a
narrowly scoped, documented allow only around the required field usage. Remove
the broad helper-level #[allow(deprecated)] attributes and apply the same
treatment at both sites in crates/rpc/src/model/hardware_info.rs:1054-1068 and
1096-1106.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e181c600-d177-40d0-87ba-024116aef2ca

📥 Commits

Reviewing files that changed from the base of the PR and between e588786 and 499ef54.

📒 Files selected for processing (1)

crates/rpc/src/model/hardware_info.rs

Exercise the RPC hardware-discovery conversion surface through direction-specific table-driven cases. Cover sparse, populated, invalid, compatibility, and nested projection behavior without changing production code. Signed-off-by: Chet Nichols III <chetn@nvidia.com>

chet · 2026-07-24T00:51:08Z

@coderabbitai full_review, thanks!

coderabbitai · 2026-07-24T00:51:18Z

@chet Understood — I’ll perform a complete re-review of #4060.

🐇🔍

✅ Action performed

Full review finished.

#4060 added comprehensive RPC hardware-discovery conversion tests using the original three-field `NvmeDevice`. #3717 independently added the optional `size_mb` and `pci_path` fields. Its tested branch did not contain #4060. When #3717 was subsequently squash-merged after #4060, Git combined the changes without a textual conflict, but the resulting test struct initializers were incomplete. This caused `make verify` to fail while compiling the `carbide-rpc` library tests. Update the RPC and model fixtures with representative NVMe capacity and PCI-path values. Include both fields in the test projections so the existing tests verify that they survive RPC-to-model and model-to-RPC conversion. ## Related issues #4060 #3717 ## Type of Change - [ ] **Add** - New feature or capability - [ ] **Change** - Changes in existing functionality - [x] **Fix** - Bug fixes - [ ] **Remove** - Removed features or deprecated functionality - [ ] **Internal** - Internal changes (refactoring, tests, docs, etc.) ## Breaking Changes - [ ] **This PR contains breaking changes** ## Testing - [x] Unit tests added/updated - [ ] Integration tests added/updated - [ ] Manual testing performed - [ ] No testing required (docs, internal refactor, etc.) ## Additional Notes Signed-off-by: Dmitry Porokh <dporokh@nvidia.com>

chet requested a review from a team as a code owner July 24, 2026 00:12

coderabbitai Bot reviewed Jul 24, 2026

View reviewed changes

chet enabled auto-merge (squash) July 24, 2026 00:25

chet force-pushed the gh-issue-4037 branch from 499ef54 to 2296c27 Compare July 24, 2026 00:50

poroh approved these changes Jul 24, 2026

View reviewed changes

chet merged commit 95ea7cd into NVIDIA:main Jul 24, 2026
108 of 110 checks passed

chet deleted the gh-issue-4037 branch July 24, 2026 03:29

poroh mentioned this pull request Jul 24, 2026

fix(build): NVMe inventory fields in conversions #4133

Merged

10 tasks

Navigation Menu

Uh oh!

test(rpc): cover hardware discovery conversions - #4060

test(rpc): cover hardware discovery conversions#4060
chet merged 1 commit into
NVIDIA:mainfrom
chet:gh-issue-4037

chet commented Jul 24, 2026

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

chet commented Jul 24, 2026

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot left a comment

Uh oh!

chet commented Jul 24, 2026

Uh oh!

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chet commented Jul 24, 2026

Summary

Coverage

Validation

Uh oh!

coderabbitai Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Walkthrough

Changes

Uh oh!

chet commented Jul 24, 2026

Uh oh!

coderabbitai Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

chet commented Jul 24, 2026

Uh oh!

coderabbitai Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading

coderabbitai Bot commented Jul 24, 2026 •

edited

Loading