test(api-model,rpc,uuid): collapse per-variant clusters and drop redundant tests

[chet]

Wave 1 of the carbide-test-support integration (#2692).

api-model: is_power_shelf's five per-input tests → one value_scenarios! table; build_version's partial_cmp check folds into the ordering table (now asserts partial_cmp == Some(cmp) per row); drops the Display-equals-Debug check (can't fail), the AssignStaticResult PartialEq-derive table, and two IpAddr stdlib-formatting tests.

rpc: drops two identity_config_try_from_proto tests that duplicated exact rows of the existing error-case table. uuid: merges RackId/RackProfileId parse + serde tests behind a generic helper; drops two tautological transparent-newtype conversion tests.

~340 fewer test lines, no production change. Verified per crate (cargo test: api-model 320, rpc 10, uuid rack 2), clippy-clean.

Two items from #2692's plan intentionally left as-is, on review:

• Kept the seven MachineCapability* conversion tests separate rather than collapsing them into one table -- they are distinct type-pair From conversions (not duplicates), and a single table would require type erasure that erases the per-field "which field mismapped" failure diagnostic.
• Kept test_db_column_name -- it guards each type's DB_COLUMN_NAME, which feeds production DB queries via db_primary_uuid_name() (api-db), so it is not a pure tautology.

Addresses #2695.

[chet]

@coderabbitai PTAL, thanks!

[copy-pr-bot]

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

[coderabbitai]

Summary by CodeRabbit

• Tests

  • Refactored and consolidated test suites across multiple modules for improved maintainability and reduced duplication.
  • Strengthened validation tests to verify consistency between comparison implementations.
  • Converted individual test cases to table-driven testing approach for better coverage organization.

• Chores

  • Removed redundant test cases that were superseded by refactored test structures.

Walkthrough

This PR cleans up test suites across api-model, rpc, and uuid crates. It removes redundant or tautological tests, consolidates partial_cmp coverage into an existing scenario runner, refactors is_power_shelf tests into a value_scenarios! macro, and introduces generic parse_as<T>/deserialize_as<T> helpers to eliminate per-type duplication in rack ID tests.

Changes

Test Suite Cleanup and Consolidation

Layer / File(s)	Summary
Redundant test removal across api-model and rpc crates/api-model/src/allocation_type.rs, crates/api-model/src/controller_outcome.rs, crates/api-model/src/machine/network.rs, crates/rpc/src/model/tenant.rs	Removes AssignStaticResult equality rows from variants_compare_by_identity, the test_display_matches_debug test, two IpAddr formatting/parsing tests, and two identity_config_try_from_proto negative test cases.
Fold partial_cmp coverage into build_version_ordering crates/api-model/src/machine/upgrade_policy.rs	The build_version_ordering scenario runner now asserts partial_cmp agrees with cmp on every row; the standalone build_version_partial_cmp_matches_cmp test is removed and an explanatory comment is added.
Refactor is_power_shelf tests to value_scenarios! crates/api-model/src/site_explorer/mod.rs	Multiple standalone is_power_shelf_* functions are replaced by a single value_scenarios!-driven test with an inline ChassisInput struct covering all true/false cases; value_scenarios is added to imports.
Generic parse_as/deserialize_as helpers in rack test suite crates/uuid/src/rack/mod.rs	Four per-type parse/serde helpers are replaced by generic parse_as<T> and deserialize_as<T> functions; parsing and serde scenarios are consolidated into rack_id_types_parse and rack_id_types_serde, each running the same scenario table for both RackId and RackProfileId.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Collapse per-variant test clusters in api-model, rpc, and uuid #2695: This PR directly implements the refactoring work described: collapsing per-variant test clusters, removing tautological coverage, and consolidating parse/serde helpers across api-model, rpc, and uuid crates.
• carbide-test-support integration -- finish the migration, extend the coverage #2692: The changes implement Wave 1 cleanup work from the referenced Epic, specifically the value_scenarios! refactor in site_explorer/mod.rs and the generic helper introduction in uuid/src/rack/mod.rs.

Possibly related PRs

• NVIDIA/infra-controller#2503: Both PRs modify crates/rpc/src/model/tenant.rs by removing or consolidating identity_config_try_from_proto negative test cases into table-driven patterns.
• NVIDIA/infra-controller#2563: Both PRs restructure the crates/uuid/src/rack/mod.rs test suite, specifically how RackId/RackProfileId parse and serde cases are organized and asserted.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title precisely summarizes the primary change: consolidating redundant test clusters and removing duplicate test cases across three crates.
Description check	✅ Passed	The description comprehensively documents the changeset, detailing specific test consolidations and removals across crates with quantified impact.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

gh pr view 2713 --repo NVIDIA/infra-controller --json title,body,files,additions,deletions,commits

echo "--- DIFF ---"
gh pr diff 2713 --repo NVIDIA/infra-controller

Length of output: 29896

---

---

@chet: This is a clean, well-reasoned test hygiene pass. The changes are correct, the rationale for each removal is sound, and the consolidations are an improvement over the original structure. A few observations:

---

Strengths

• upgrade_policy.rs — partial_cmp fold is strictly stronger. The old standalone test checked 3 hand-picked cases; the new inline assert_eq!(l.partial_cmp(&r), Some(ordering), …) fires on every row of the ordering table, which is a wider and more principled regression net. Well done.

• uuid/src/rack/mod.rs — parse_as::<T> exhaustiveness. The closure map_err(|RackIdParseError::Empty| ParseFailure::Empty) is an irrefutable destructuring pattern today, but if a second variant is ever added to RackIdParseError the match will become non-exhaustive and fail to compile, surfacing the gap immediately. That is a useful side-effect.

• Removals are principled. Each dropped test falls into a well-defined category: #[derive] tautologies (PartialEq, Display-equals-Debug), stdlib behavior (IpAddr::{to_string, from_str}), or exact row duplication in an already-existing table. None of the removals touch project-specific logic.

---

Minor observation (non-blocking)

crates/uuid/src/rack/mod.rs — rack_id_types_parse and rack_id_types_serde each contain two back-to-back scenarios! blocks separated only by a blank line. The module-level comment above the test cluster explains the intent, but a one-line inline marker before each second block (e.g. // RackProfileId) would make it immediately obvious at a glance which block exercises which type without needing to read the run = type parameter. Entirely cosmetic — feel free to ignore.

---

Verdict

No correctness or coverage concerns. The production logic is untouched, and the stated test counts (api-model 320, rpc 10, uuid rack 2) are consistent with the scope. LGTM.

[coderabbitai]

🧹 Nitpick comments (1)

crates/uuid/src/rack/mod.rs (1)

354-368: 💤 Low value

Consider inline markers for consecutive scenario blocks.

The two scenarios! blocks (RackId at line 331, RackProfileId at line 356) are separated only by blank lines. A one-line comment marker (e.g., // RackProfileId) would immediately clarify which block exercises which type without requiring readers to inspect type parameters.

This is a minor readability suggestion and entirely optional.

🤖 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/uuid/src/rack/mod.rs` around lines 354 - 368, The consecutive
scenarios! blocks for RackId and RackProfileId lack clear inline markers that
indicate which block tests which type. Add a single-line comment marker (e.g.,
`// RackProfileId`) immediately before the scenarios! macro invocation that
tests parse_as::<RackProfileId> to improve code readability and make it
immediately clear which type each scenario block exercises without requiring
readers to inspect the type parameters.

🤖 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/uuid/src/rack/mod.rs`:
- Around line 354-368: The consecutive scenarios! blocks for RackId and
RackProfileId lack clear inline markers that indicate which block tests which
type. Add a single-line comment marker (e.g., `// RackProfileId`) immediately
before the scenarios! macro invocation that tests parse_as::<RackProfileId> to
improve code readability and make it immediately clear which type each scenario
block exercises without requiring readers to inspect the type parameters.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 923189c3-ea25-4d86-a505-28927614e7bd

📥 Commits

Reviewing files that changed from the base of the PR and between 79e54c0 and 3a493b8.

📒 Files selected for processing (7)

• crates/api-model/src/allocation_type.rs
• crates/api-model/src/controller_outcome.rs
• crates/api-model/src/machine/network.rs
• crates/api-model/src/machine/upgrade_policy.rs
• crates/api-model/src/site_explorer/mod.rs
• crates/rpc/src/model/tenant.rs
• crates/uuid/src/rack/mod.rs

💤 Files with no reviewable changes (3)

• crates/api-model/src/controller_outcome.rs
• crates/rpc/src/model/tenant.rs
• crates/api-model/src/machine/network.rs

[github-actions]

🔍 Container Scan Summary

Service	Total	Critical	High	Medium	Low	Other
boot-artifacts-aarch64	3	0	0	3	0	0
boot-artifacts-x86_64	3	0	0	3	0	0
forge-admin-cli-x86_64	264	6	23	99	6	130
machine-validation-runner	704	34	183	258	35	194
machine_validation	704	34	183	258	35	194
nvmetal-carbide	704	34	183	258	35	194
TOTAL	2382	108	572	879	111	712

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.