test(firmware): make shared artifact resolution coverage authoritative

[chet]

The shared firmware resolver owns the files[] selection and location rules, but machine-controller repeated most of those cases around a thin adapter. That split made the shared crate depend on downstream tests for complete coverage and left two test suites to keep aligned.

This moves the complete resolver matrix into carbide-firmware, keeps machine-controller focused on adapter and Scout-specific behavior, and adds explicit coverage for trimmed and blank artifact locations. Production behavior is unchanged.

Related issues

This supports #3974.

Part of #3914.

Type of Change

• Add - New feature or capability
• Change - Changes in existing functionality
• Fix - Bug fixes
• Remove - Removed features or deprecated functionality
• Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

• This PR contains breaking changes

Testing

• Unit tests added/updated

• Integration tests added/updated

• Manual testing performed

• No testing required (docs, internal refactor, etc.)

• cargo test --locked --package carbide-firmware artifact_resolution --lib

• cargo test --locked --package carbide-machine-controller firmware_artifact --lib

• Three cargo llvm-cov checkpoints for carbide-firmware and carbide-machine-controller

• cargo make format-nightly

• cargo make clippy

• cargo make carbide-lints

Additional Notes

Owner-only coverage for resolve_files_firmware_artifact:

Checkpoint	Lines	Regions
Original	34/41	44/46
Converted	41/41	46/46
Expanded	41/41	46/46

The converted table reaches full stable line and region coverage. The expanded checkpoint adds URL trimming, filename trimming, and whitespace-only location contracts without changing those already-complete counts.

Machine-controller production coverage remains 58/60 lines. The two uncovered lines are the non-UTF-8 error path behind a &str parameter.

[chet]

@coderabbitai full_review, thanks!

[coderabbitai]

@chet: Starting a complete review of the PR, including the updated test ownership and artifact-location edge cases.

✅ Action performed

Full review finished.

[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: e4b07f9a-095c-48ba-bfcc-4789fcb64f6f

📥 Commits

Reviewing files that changed from the base of the PR and between 2de970b and 7451c6d.

📒 Files selected for processing (2)

• crates/firmware/src/artifact_resolution.rs
• crates/machine-controller/src/handler/firmware_artifact.rs

---

Summary by CodeRabbit

• Tests
  • Refactored firmware artifact resolution tests into table-driven, parameterized scenarios for clearer coverage.
  • Added/expanded assertions for URL vs. filename precedence, whitespace trimming, correct indexed file selection, and missing-input error cases.
  • Covered remote, local, and legacy resolution outcomes, including invalid index handling.
  • For scout artifact resolution, strengthened checks for derived URL behavior and rejected unsafe filename paths (e.g., traversal or sibling escapes).
  • No end-user functionality or behavior changes were introduced.

Walkthrough

The PR refactors firmware and machine-controller artifact resolution tests into parameterized case tables, adds shared fixture and error-comparison helpers, and preserves coverage for local, remote, legacy, missing-input, precedence, trimming, and unsafe-path scenarios.

Changes

Firmware artifact test consolidation

Layer / File(s)	Summary
Firmware resolution case table crates/firmware/src/artifact_resolution.rs	Replaces standalone tests with parameterized cases covering artifact selection, URL precedence, whitespace trimming, index errors, missing inputs, and shared expected-value helpers.
Controller resolution case tables crates/machine-controller/src/handler/firmware_artifact.rs	Converts firmware and Scout artifact tests to table-driven cases using shared fixtures and normalized error comparisons, including legacy selection and unsafe-path rejection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

• NVIDIA/infra-controller issue 3974 — Directly tracks centralizing firmware artifact resolution coverage and reducing duplicated machine-controller tests.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the test-coverage shift to the shared firmware resolver and is specific enough for history scanning.
Description check	✅ Passed	The description matches the refactor and test-coverage goals described in the changeset.
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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