[jussielo-amd]

What

PR #675 added an OsRelease-aware _read seam (install_hint_for_read, build_driver_install_plan_read, plan_for_os_release_read) alongside the existing bare-text and _at functions in crates/rocm-core/src/openmpi.rs and apps/rocm/src/driver_install.rs, leaving three entry points per builder: a bare-text function (test-only), an _at function (path-aware, the real implementation), and a _read function (OsRelease-aware).

Now that tests can construct OsRelease::found(text) directly and call the _read seam, the bare-text middle layer is redundant. This migrates every bare-text test call site to the _read seam and deletes the two now-unreferenced bare functions.

• apps/rocm/src/driver_install.rs: migrated all 33 call sites of build_driver_install_plan to build_driver_install_plan_read, using a small #[cfg(test)] os_release_found helper (since rocm_core::hardware_root::OsRelease::found is pub(crate) inside rocm-core and not visible from this crate's tests). Deleted the now-dead build_driver_install_plan.
• crates/rocm-core/src/openmpi.rs: migrated all 12 call sites of install_hint_for to install_hint_for_read using crate::hardware_root::OsRelease::found. Deleted the now-dead install_hint_for.
• docs/testing.md: updated a passage that described the now-deleted bare functions as currently-existing test helpers.
• The _at functions are untouched — they remain the real implementation both _read and any other path-aware caller route through.

This is a pure test/internal-API cleanup with no behavior change.

Fixes #690

Verification

• cargo build --workspace --all-targets, cargo test --workspace --all-targets, cargo clippy --workspace --all-targets -- -D warnings, and python scripts/smoke_local.py all pass.
• Every existing test in both files passes with its assertions unchanged (confirmed by diffing test output before/after the migration) — this is a pure refactor, so no test's expected value needed to change.
• cargo xtask coverage passes with no floor regression; coverage-floors.toml is untouched.
• Leak scan of the diff against origin/main is clean.

[Copilot]

🔵 Needs a closer look

The reported verification could not be independently confirmed because the repository checkout was unavailable.

1 open finding

• Use relative Markdown links for source citations · New

What changed in this PR

Completes #690 by removing redundant test-only wrappers and using the existing OsRelease-aware entry points.

Changes:

• Migrates driver-install and OpenMPI tests without changing their assertions.
• Deletes bare-text wrappers while preserving _at implementations.
• Updates testing documentation and related comments.

File	Description
docs/​testing.md	Documents direct use of _read test entry points.
crates/​rocm-core/​src/​openmpi.rs	Migrates hint tests and removes the bare-text wrapper.
apps/​rocm/​src/​driver_install.rs	Migrates planner tests and adds an OsRelease fixture helper.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[tomastola]

Approving: the migration is mechanical, every assertion is unchanged, _read delegates to _at, and a grep of the head finds no remaining references to the deleted functions. CI is green.

Two stale comments, non-blocking. The first is an inline suggestion on openmpi.rs:935. The second is outside the diff, so GitHub can't take it inline: crates/rocm-core/src/openmpi.rs:1510-1512, in the doc comment on a_fallback_reads_malformed_line_names_the_fallback_path. It says malformed-line tests "went through either the bare, hardcoded-/etc functions or OsRelease::found", but the bare functions no longer exist. Suggest: "Every malformed-line test before this one went through [crate::hardware_root::OsRelease::found] (which hardcodes /etc/os-release), so ...".