Skip to content

test(small crates): adopt carbide-test-support across five parser/serde suites#2745

Merged
chet merged 1 commit into
NVIDIA:mainfrom
chet:gh-issue-2731
Jun 22, 2026
Merged

test(small crates): adopt carbide-test-support across five parser/serde suites#2745
chet merged 1 commit into
NVIDIA:mainfrom
chet:gh-issue-2731

Conversation

@chet

@chet chet commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Wave 2 of the carbide-test-support integration (#2692) -- the small-crate bundle.

Five crates each proved a single parser/serde mapping one #[test] per case; each folds into one table over a named row:

  • carbide-dns: gRPC-code → DNS-response-code classifier, extended to the full 17-variant tonic::Code set.
  • bms-dsx-exchange: metadata-topic parsing over a Message { topic, payload } row.
  • carbide-dsx-exchange-consumer: point-path extraction + FaultValue serde, plus new LeakPointType probe-id/description tables.
  • carbide-kms-provider: config-file key-source loader over a KeySource row.
  • carbide-rvs: eval_sotpath + resolve_for_scenario (now asserting the full (output_path, url) vec).

Errors compare as Fails (these error types aren't PartialEq); env-mutating and different-shape tests stay separate. Adds the dev-dep to the four crates that lacked it. No production change. Verified: --lib tests pass in all five, clippy --locked --all-targets --all-features clean, nightly rustfmt clean.

Addresses #2731.

…de suites

Five crates each proved a single parser or serde mapping one `#[test]` per case;
this folds each into one `carbide-test-support` table over a named row:

  - `carbide-dns`: the gRPC-code -> DNS-response-code classifier, extended from a
    hand-rolled loop to the full 17-variant `tonic::Code` set.
  - `bms-dsx-exchange`: metadata-topic parsing (rack / heartbeat / value-topic
    derivation) over a `Message { topic, payload }` row.
  - `carbide-dsx-exchange-consumer`: point-path extraction and `FaultValue` serde,
    plus newly-added `LeakPointType` probe-id / description tables.
  - `carbide-kms-provider`: the config-file key-source loader (valid, bad-base64,
    wrong-length) over a `KeySource` row.
  - `carbide-rvs`: `eval_sotpath` and `resolve_for_scenario`, the latter now
    asserting the full `(output_path, url)` vec rather than a length and an index.

Errors compare as `Fails` (these error types aren't `PartialEq`, so each run
closure maps to a string); the expected variant or reason is named in the group
label. Tests that mutate process env or exercise a different input shape stay on
their own. Adds `carbide-test-support` as a dev-dependency to the four crates that
lacked it. No production change; the tests run green.

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@chet

chet commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai PTAL, thanks!

@copy-pr-bot

copy-pr-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

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

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 1dcab2d0-462a-4d1c-8b4b-5d2a288a8192

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Walkthrough

Five crates (bms-dsx-exchange, dns, dsx-exchange-consumer, kms-provider, rvs) gain carbide-test-support as a dev dependency. Their existing collections of per-function unit tests are refactored into table-driven tests using the scenarios!, value_scenarios!, and Outcome::* constructs provided by that shared crate.

Changes

carbide-test-support Wave 2: Table-driven test migration

Layer / File(s) Summary
Dev dependency additions
crates/bms-dsx-exchange/Cargo.toml, crates/dsx-exchange-consumer/Cargo.toml, crates/kms-provider/Cargo.toml, crates/rvs/Cargo.toml
Each manifest gains a [dev-dependencies] section (or entry) declaring carbide-test-support via path = "../test-support".
bms-dsx-exchange metadata parsing
crates/bms-dsx-exchange/src/lib.rs
Replaces four standalone parsing tests with a single parses_supported_metadata test using scenarios!, asserting derived point_type, integration, and value_topic on success and rejection on empty-field or non-metadata-topic inputs.
dns gRPC classification
crates/dns/src/lib.rs
Rewrites classify_failure_maps_grpc_codes_to_dns_response_codes with value_scenarios! and an Expected { code, transient } struct; expands transient-failure coverage to additional tonic::Code variants.
dsx-exchange-consumer health_updater and messages
crates/dsx-exchange-consumer/src/health_updater.rs, crates/dsx-exchange-consumer/src/messages.rs
Consolidates extract_point_path and ValueMessage parsing tests into value_scenarios!/scenarios! tables; adds new scenario-driven tests for LeakPointType::probe_id() and LeakPointType::description().
kms-provider IntegratedKmsProvider from_config
crates/kms-provider/src/providers/integrated.rs
Merges file/value/invalid-base64/wrong-key-length tests into one table-driven test asserting can_decrypt_kek for valid material and errors for malformed inputs; updates from_config_empty_errors with explanatory comments.
rvs scenario resolver
crates/rvs/src/scenario/resolver.rs
Adds carbide_test_support::Outcome::* and scenarios imports; replaces standalone eval_sotpath and resolve_for_scenario test functions with two scenarios!-driven tests using Query/Case structs and Yields/Fails assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Possibly related PRs

  • NVIDIA/infra-controller#2533: Directly precedes this PR in the same migration wave, applying the identical scenarios!/value_scenarios! pattern to a different set of crates.
  • NVIDIA/infra-controller#2568: Applies the same table-driven DSL refactoring approach using carbide_test_support macros without altering production logic.
  • NVIDIA/infra-controller#2710: Converts per-variant unit tests to scenarios!/value_scenarios! style and adds carbide-test-support as a dev dependency, mirroring this PR's pattern in different crates.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the primary changeset: adopting carbide-test-support across five parser/serde test suites, which is the main objective of this PR.
Description check ✅ Passed The description is directly related to the changeset, providing a comprehensive Wave 2 integration narrative that references all five affected crates and details the specific test migrations performed.
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

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

@chet: Certainly — I will conduct a thorough review of the PR now. I'll examine the table-driven test migrations across all five crates (carbide-dns, bms-dsx-exchange, carbide-dsx-exchange-consumer, carbide-kms-provider, and carbide-rvs), verify correctness of the Fails comparisons, coverage completeness against the original inline tests, and consistency of the new carbide-test-support integration. Thank you for the heads-up on Wave 2.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chet

chet commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@chet chet marked this pull request as ready for review June 22, 2026 06:35
@chet chet requested a review from a team as a code owner June 22, 2026 06:35
@chet

chet commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

@chet

chet commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions

Copy link
Copy Markdown

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

@chet chet merged commit 608f0a1 into NVIDIA:main Jun 22, 2026
55 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants