test(mqttea,dpf): fold per-case error and node-label tests into tables

[chet]

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

mqttea's ~38 near-identical error tests collapse into two tables -- category predicates per error, and Display/source rendering -- keeping the distinct constructor, equality, and Default tests. Drops the Send/Sync compile-checks, the Result/Option stdlib-ergonomics tests, and two timing assertions that belong in a bench. dpf's six verify_node_labels_* tests become one check_cases_async table seeded by a small Seeded enum; two redundant init_config tests and a Display-matches-as_ref duplicate go.

~230 fewer test lines, no production change; the predicate table asserts the full category vector per row, a touch stronger than the originals. Verified per crate (cargo test: mqttea 13 + 22, dpf sdk 40, dpf types 7).

Addresses #2697.

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

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: 0ac013f0-ac88-4dc1-b02c-5c12395cc99b

📥 Commits

Reviewing files that changed from the base of the PR and between 7d459ea and 8c0a0e4.

📒 Files selected for processing (4)

• crates/dpf/src/sdk.rs
• crates/dpf/src/types.rs
• crates/mqttea/tests/errors.rs
• crates/mqttea/tests/stats.rs

💤 Files with no reviewable changes (1)

• crates/mqttea/tests/stats.rs

🚧 Files skipped from review as they are similar to previous changes (3)

• crates/dpf/src/types.rs
• crates/dpf/src/sdk.rs
• crates/mqttea/tests/errors.rs

---

Summary by CodeRabbit

• Tests
  • Consolidated and reorganized unit tests across modules using table-driven patterns for clearer, more systematic coverage.
  • Improved label-verification coverage with seeded node scenarios and fixed terminating-time handling for repeatable results.
  • Refactored error-suite assertions to centralize categorization, formatting (Display/Debug), and error source expectations; removed redundant/standalone checks.
  • Removed the performance-oriented stats test previously used to validate low-contention behavior.

Walkthrough

Test suites across the dpf and mqttea crates are refactored to consolidate redundant coverage into table-driven tests. In dpf, the verify_node_labels_* family of tests is replaced by a single table-driven async test, and the DpuPhase display test is folded into the existing as_ref scenario runner. In mqttea, error categorization, display, debug, and source tests are consolidated into two structured tests, redundant per-constructor assertions are removed, and a performance-timing test is deleted from stats.rs.

Changes

DPF and mqttea test suite consolidation

Layer / File(s)	Summary
DpuPhase Display/AsRef test merge crates/dpf/src/types.rs	dpu_phase_as_ref_renders_each_variant now asserts Display equivalence inline with message "Display must match AsRef", and the standalone dpu_phase_display_matches_as_ref test is removed.
Node-label tests consolidated into table-driven async test crates/dpf/src/sdk.rs	InitDpfResourcesConfig unit tests removed; terminating_timestamp() helper added for reusable test fixtures; multiple verify_node_labels_* tests replaced with a single carbide_test_support::check_cases_async-driven test covering all label seeding scenarios (Absent, RegisteredByLabeler, WithLabels).
Predicates struct and consolidated categorization test crates/mqttea/tests/errors.rs	Predicates struct introduced to aggregate all five MqtteaClientError category flags; compile-time Send + Sync trait assertion added; piecemeal categorization tests replaced by a single error_categorization_predicates table-driven test covering From conversions, constructors, and variant-specific cases.
Consolidated display/debug/source test and redundancy removal crates/mqttea/tests/errors.rs	error_display_debug_and_source replaces many separate formatting and source assertions; redundant is_* categorization assertions removed from constructor tests; superseded standalone source-chaining tests and trailing test blocks deleted; only long-message edge-case retained.
Performance timing test removal crates/mqttea/tests/stats.rs	test_stats_performance_no_contention deleted, eliminating its atomic-operation wall-clock timing assertion and post-run totals consistency check.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

• Slim the mqttea error tests and table-ify dpf's node-label checks #2697: This PR directly implements the test refactoring described in that issue, including table-driven node-label tests, merged DpuPhase display assertions, and collapsed mqttea error tests.
• carbide-test-support integration -- finish the migration, extend the coverage #2692: This PR corresponds to Wave 1 cleanup work in the referenced Epic, migrating hand-written tests to carbide_test_support table-driven patterns across dpf and mqttea.

Possibly related PRs

• NVIDIA/infra-controller#2530: Directly modifies DpuPhase tests in crates/dpf/src/types.rs with overlapping as_ref()/Display equivalence coverage.
• NVIDIA/infra-controller#2565: Modifies DpuPhase AsRef<str>/Display unit tests in crates/dpf/src/types.rs, overlapping directly with the assertion changes in this PR.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: consolidating per-case error and node-label tests into table-driven tests across mqttea and dpf crates.
Description check	✅ Passed	The description comprehensively relates to the changeset, detailing the test consolidation across both crates, quantifying the reduction in test lines, and addressing the specific issue #2697.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% 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 2708 --repo NVIDIA/infra-controller --json title,body,files,additions,deletions,commits

echo "---FILES---"
gh pr diff 2708 --repo NVIDIA/infra-controller

Length of output: 44210

---

---

@chet: The refactoring is clean and well-reasoned. The Predicates struct — asserting the entire categorization vector per row — is a notably stronger invariant than the original piecemeal per-predicate assertions, and the Seeded enum parameterizes async setup in an idiomatic, readable way. The only() + struct-update syntax for topic/registry rows is elegant. A few observations:

---

Substantive concern — Send + Sync compile-checks removed

crates/mqttea/tests/errors.rs previously contained:

fn test_error_send_sync() {
    fn assert_send<T: Send>() {}
    fn assert_sync<T: Sync>() {}

    assert_send::<MqtteaClientError>();
    assert_sync::<MqtteaClientError>();
}

These are zero-cost compile-time guards. They do not test standard-library behaviour — they assert a property of MqtteaClientError itself that is safety-critical for async callers. If a future change adds a !Send field (e.g., a Rc<_> or a non-Send inner error type), the suite currently loses that signal entirely.

Consider replacing with static_assertions:

// compile-time: MqtteaClientError must be Send + Sync for async callers
static_assertions::assert_impl_all!(MqtteaClientError: Send, Sync);

or retaining the original two-liner if static_assertions isn't a desired dependency.

---

Minor — Display gap for JsonSerializationError / YamlSerializationError

The new error_display_debug_and_source table covers deserialization and all constructor-based variants, but JsonSerializationError and YamlSerializationError display rendering is absent. This was also absent before (not a regression), but given the table now serves as the canonical rendering spec, a note or two rows for "from serde_json::Error -> serialization display" / "from serde_yaml::Error -> serialization display" would close the gap.

---

Nit — error_display_debug_and_source loop style

The hand-rolled for case in cases { … } loop is a conscious, justified choice (substring containment and has_source don't map cleanly to check_values' equality contract), and the code comment documents this clearly. No change needed — just acknowledging the intentional deviation from the module's table convention.

---

Overall the test-collapse is a net improvement in readability, coupling, and coverage density. The single actionable recommendation is restoring the Send + Sync bound enforcement.

[chet]

@coderabbitai Thanks for the eyes. Updated -- restored the Send + Sync bound as a compile-time guard (error_is_send_and_sync); you are right that it pins a real property of MqtteaClientError for async callers, not stdlib behavior. Also added the two Json/Yaml serialization rows to the render table so it stands as the complete rendering spec. PTAL. Resolve if good, thanks!

[coderabbitai]

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

gh pr diff 2708 --repo NVIDIA/infra-controller | grep -A 30 "send_and_sync\|error_is_send\|assert_impl_all\|Send.*Sync\|JsonSerialization\|YamlSerialization" | head -80

Length of output: 3809

---

---

@chet: Both items verified against the diff:

1. error_is_send_and_sync — present and well-documented. The inline comment precisely articulates the async-safety contract (!Send/!Sync field would silently break cross-task holds), which is exactly the right level of documentation for a compile-time guard.
2. Serialization render rows — JsonSerializationError and YamlSerializationError are in the table with display_contains and has_source: true, closing the spec gap cleanly.

No outstanding concerns. Resolving.

(∧ᵕ᷄˳ᕗ᷅∧)

[resolve]

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