You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
refactor(rocm-core): one PATH lookup for the summary and the examination stacks - #953
rocm-core had two PATH lookups that answer the same question, "is this program installed": tool_on_path in host_gpu/capture.rs (the summary stack, which feeds the text of rocm examine and the summary object of its JSON) and which in probe_exec.rs (the examination stack, about fifteen capability checks in examine/ and fix/runners.rs). On WSL both ask about rocminfo in one rocm examine --json run, as summary.wsl.rocminfo and wsl.rocminfo, and they could disagree. This is pair 1 of 6 in docs/host-probing.md: the survivor is the behaviour of tool_on_path (it follows PATHEXT), housed in probe_exec.rs under the existing name which, so the call sites do not change. tool_path_candidates moved there with it because the capture runner and the Windows diagnostics capture start each spelling in turn. detect_wsl_summary now asks the shared which for rocminfo and cargo, and tool_on_path is gone. There is no third copy.
The empty-entry question the note left open is decided as skip: an empty PATH entry is not read as the current directory. That keeps the examination's answer, which the fifteen or so checks (including sudo and usermod) already relied on, so a file called sudo in the directory the CLI was started from never reads as installed. It changes only the summary's answer on WSL. The cost is that a program the CLI later starts by bare name can still resolve from the current directory through the operating system when PATH has an empty entry, and the lookup, which starts nothing, no longer agrees with that. The doc comment on which says so. Reading the code gave no concrete reason to take the other branch: following PATH semantics would widen what the examination calls installed in all fifteen checks. Skipping also makes the open question of how Windows split_paths treats an empty entry moot, because the entry is dropped either way.
The answers that move, each with the test that pins it (the characterisation commit pins both old answers, the convergence commit flips them):
An empty PATH entry with a file of the program's name in the current directory: the summary used to say found, the examination not found; now both say not found. Pinned by which_does_not_read_an_empty_path_entry_as_the_current_directory (leading, middle, trailing and whole-PATH shapes, plus the control that the same file counts once a real entry names its directory), path_dirs_drop_every_empty_entry and scenario examine-36.
A PATH that is not valid Unicode: the summary searched it, the examination found nothing for every program; now both search it, split as an OsString. Pinned by which_searches_a_path_that_is_not_unicode, path_dirs_keep_an_entry_that_is_not_unicode and scenario examine-37.
Windows PATHEXT entries outside .exe, .bat and .cmd, in both cases: the examination now tries them. Pinned by candidates_on_windows_try_every_pathext_entry_in_both_cases, candidates_on_windows_fall_back_to_the_default_pathext_when_it_is_unset and candidates_on_windows_trim_pathext_entries_and_skip_empty_ones (all hosts) and which_tries_a_pathext_extension_outside_exe_bat_and_cmd (Windows job).
A quoted Windows PATH entry holding a ;: the examination used to split on every ;; now std::env::split_paths keeps it whole. Pinned by which_searches_a_quoted_path_entry_that_holds_a_semicolon (Windows job; the splitting is std's, so it can only be tested where it runs).
The System32 locations of cmd, pnputil, powershell and pwsh: the examination now finds them. No which call site asks for those four today (checked with grep), so nothing visible changes; they stay in the shared candidate list because the capture runner needs them to start those tools at all, and one list read by both the runner and the lookup cannot drift. A future caller of which("powershell") gets "would the runner find it", not "is it on PATH". An absolute candidate ignores the directory it is joined onto, so it is found whenever PATH lists at least one directory; this is how tool_on_path behaved and the doc comment says it. Pinned by candidates_on_windows_add_the_fixed_system32_locations, found_in_asks_about_an_absolute_candidate_as_it_is_whatever_the_directory and which_finds_cmd_at_its_system32_location_without_it_being_on_path (Windows job).
Unchanged on purpose, and pinned: a program with an extension is tried as given (candidates_on_windows_for_a_name_with_an_extension_are_the_name_alone, which also records that the cmd.exe arm of the System32 list is unreachable through the early return, as before), off Windows only the bare name is tried, and a directory named like the program is not the program.
The near copy of tool_path_candidates in apps/rocm (program_path_candidates) did not join; the note keeps it on its open list.
What moved
Before
After
tool_on_path in host_gpu/capture.rs
deleted; which in probe_exec.rs has its behaviour
tool_path_candidates, windows_absolute_tool_candidates in host_gpu/capture.rs
probe_exec.rs, as tool_path_candidates over a pure path_candidates(program, windows, pathext, system_root)
the directory walk inside tool_on_path / which
probe_exec.rs: path_dirs (split, drop empty entries) and found_in(dirs, candidates, is_file), both pure
capture.rs and diagnostics.rs imports
crate::probe_exec::tool_path_candidates
detect_wsl_summary calls
which("rocminfo"), which("cargo")
The Windows spellings take the platform, PATHEXT and the system root as arguments, and the search takes the directories and an existence test, so the logic that only matters on Windows is unit-tested on every host and the Windows job exercises it. The directory split itself is std::env::split_paths, which cannot be injected, so the ; and quote behaviour is covered by the two cfg(windows) tests.
Test plan
Commit order: (1) characterisation tests, green against the unchanged code, pinning what both copies return for the same input and each copy's own answer where they differ; (2) the convergence, which moves the tests into probe_exec.rs and flips exactly the assertions meant to change; (3) the scenarios and the harness change they need, with the working-directory control added after review; (4) the note records the decision (the analysis of the two old copies stays as the record of what changed).
Because neither lookup can be pointed elsewhere on a test thread without disturbing every test that spawns a program by name, and one case needs a different working directory, the cases that read the real PATH run in a child copy of the test binary started with the PATH, PATHEXT and working directory the case needs. No test mutates the process environment.
Run locally on Linux, in a dedicated target dir: cargo test -p rocm-core --no-fail-fast (1057 passed, 3 ignored, exit 0); cargo test -p rocm --bin rocm examine (25 passed) and system_packages (1 passed); cargo test -p e2e-cucumber --test feature_naming (4 passed); cargo fmt --all --check; cargo clippy --locked -p rocm-core --all-targets -- -D warnings and cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings; the same two clippy runs for --target x86_64-pc-windows-msvc (checked, not run; this compiles and lints the cfg(windows) tests); cargo xtask manifest --check; cargo xtask check-crate-edges; lychee --offline on the three docs; the leak scan against the merge base (the only hits are the word "private" in text that was already there) and a conflict-marker grep. The leaf test-name list of cargo test -p rocm-core --lib -- --list went from 1043 to 1060 names: no test disappeared, 17 were added (the characterisation tests of the first commit are replaced inside this PR, so they do not appear in the comparison).
Also run locally: the four new scenarios through the real harness, cargo test -p e2e-cucumber --test e2e -- -n 'examine-3[5-8] ' with ROCM_CLI_BINARY pointing at a debug rocm built with --features rocm/e2e-test-hooks (4 scenarios, 12 steps, all passed on this WSL box). Not run: the Windows job (the four cfg(windows) tests and every Windows spelling are exercised only there; the pure path_candidates and found_in tests run on every host) and the rest of the e2e suite. examine-35 to examine-38 run on the Linux lanes only, because they plant a simulated host root; the ids follow examine-34, and several open PRs that also add examine-NN scenarios will need renumbering by whichever merges second. No Windows-gated scenario drives rocm examine's framework selection with PATHEXT or quoted-entry fixtures: .ps1 is not something the probe could then run (Rust starts only .exe by bare name), such a scenario cannot be validated without a Windows lane, and the Windows behaviour is covered by the pure path_candidates and found_in tests plus the three cfg(windows) lookup tests, which the Windows job runs. What the Windows lane alone can confirm is the real PATHEXT and split_paths behaviour of those three tests.
Scenario answer. rocm examine --json on a simulated WSL host (the machine examine-32 uses) is drivable from the Linux lane, so the observable answer is covered: examine-35 (control: rocminfo in a PATH directory, both reports true), examine-36 (only in the working directory behind an empty entry, both false), examine-37 (in a directory whose name is not valid Unicode, both true) and examine-38 (the control for 36: the same machine with . as the first entry, both true, which can only happen if rocm really runs from that directory). They assert summary.wsl.rocminfo and wsl.rocminfo together. The planted PATH leaves out the runner's own entries that hold a rocminfo, so a runner with ROCm installed cannot be the reason a tool reads as found. The Windows rows cannot run on the Linux lane and are covered by the unit tests above. The harness gains command_dir, consumed like command_env. Through the harness the answers are true/true, false/false, true/true and true/true for 35 to 38, both with the runner's own PATH and with . first on the harness's PATH (the runner's relative entries are resolved against the harness directory before they are filtered, by planted_path_entries in the harness library, which has a unit test per case). With the harness ignoring command_dir, examine-38 fails with false/false where true/true is expected, so examine-36's answer cannot come from a harness that did not apply the working directory. With the lookup keeping empty entries, examine-36 fails with true/true and 35, 37 and 38 stay green; by hand, putting back both old behaviours (empty entries kept, PATH read as a String) fails 36 and 37 and leaves the control alone.
Mutation evidence. For the scenarios: ignoring command_dir in run_rocm_with_scenario_env turned examine-38 red; keeping empty entries in path_dirs turned examine-36 red. For the characterisation commit, against the old code: keeping empty entries out of which turned the empty-entry test red; making tool_on_path read PATH with env::var turned the non-Unicode test red; exists() for is_file() in tool_on_path turned the directory test red; making which find nothing turned the shared-answer test red. For the survivor's tests, one mutation at a time, each restored by a targeted edit and the file hash compared at the end: path_dirs keeping empty entries (path_dirs_drop_every_empty_entry and which_does_not_read_an_empty_path_entry_as_the_current_directory red); which reading PATH as Unicode (which_searches_a_path_that_is_not_unicode red); dropping the lower-case spelling, removing the sort, removing the trim, losing .BAT from the default PATHEXT, dropping the cmd System32 arm, applying PATHEXT to a name with an extension, applying it off Windows (each red in the matching candidates_* tests); found_in trying only the first candidate or only the first directory (found_in_asks_each_candidate_in_each_directory_in_order_and_stops_at_the_first_file red); path_dirs mangling a non-Unicode entry (path_dirs_keep_an_entry_that_is_not_unicode red); exists() for is_file() in which (which_does_not_take_a_directory_named_like_the_program_for_the_program red); and stripping the root off every candidate before the join (found_in_asks_about_an_absolute_candidate_as_it_is_whatever_the_directory red).
The summary stack's tool_on_path and the examination stack's `which` answer the same question, is this program installed, and are about to become one. Pin what each returns for the same input first, so the change that merges them shows exactly which answers it moves.
They agree on a PATH of plain directories, on an unset PATH and on a directory that has the program's name. They differ on an empty PATH entry (tool_on_path reads it as the current directory, `which` skips it) and on a PATH that is not valid Unicode (tool_on_path still searches it, `which` reads PATH with env::var and finds nothing). On Windows they also differ on PATHEXT spellings outside .exe, .bat and .cmd, on a quoted PATH entry holding a semicolon, and on the System32 location of cmd.
Each case runs in a child copy of the test binary with its own PATH, PATHEXT and working directory, because neither lookup can be pointed elsewhere on a test thread without disturbing the tests that spawn programs by name. The Windows cases run only on the Windows job.
Signed-off-by: Jussi Elo <jussi.elo@amd.com>
…ion stacks
The summary stack's tool_on_path (host_gpu/capture.rs) and the examination stack's `which` (probe_exec.rs) answered the same question and disagreed on an empty PATH entry, on a PATH that is not valid Unicode and, on Windows, on PATHEXT spellings, quoted PATH entries and the System32 locations. `which` now has the behaviour tool_on_path had, and tool_on_path is gone. tool_path_candidates moved to probe_exec.rs with it, because the capture runner and the Windows diagnostics capture start each spelling in turn; they import it from there. detect_wsl_summary asks `which` for rocminfo and cargo.
An empty PATH entry is skipped rather than read as the current directory. That keeps the examination's answer, so a file named sudo or usermod in the directory the CLI runs from still does not read as installed, and moves only the summary's WSL answer. A program the CLI later starts by bare name can still resolve from the current directory through the operating system; the lookup no longer agrees with that, and its doc comment says so.
Moves relative to the two old copies: the examination now searches a PATH that is not valid Unicode (it used to find nothing in any directory), follows every PATHEXT entry in both cases, keeps a quoted PATH entry holding a semicolon whole, and finds cmd, pnputil, powershell and pwsh at their System32 locations (no `which` caller asks for those four). The summary stops finding a file in the current directory through an empty entry. The Windows spellings are computed by path_candidates, which takes the platform, PATHEXT and the system root as arguments, and the search by found_in, which takes the directories and an existence test, so both are unit-tested on every host; the cases that need the real PATH, PATHEXT and working directory run in a child copy of the test binary.
The characterisation tests of the previous commit move to probe_exec.rs and now assert the one answer: the cases that differed flip to the survivor's.
Signed-off-by: Jussi Elo <jussi.elo@amd.com>
… rocminfo is installed
`rocm examine --json` on WSL reports whether rocminfo is installed twice, as summary.wsl.rocminfo and as wsl.rocminfo, and the two used to come from different PATH lookups. They share one now, so three scenarios on the simulated WSL host that examine-32 uses hold the two answers level: a rocminfo in a PATH directory is found by both (examine-35), a rocminfo only in the directory the command runs from, behind an empty PATH entry, is found by neither (examine-36), and a rocminfo in a PATH directory whose name is not valid Unicode is found by both (examine-37).
The runner's own PATH entries that hold a rocminfo are left out of the planted PATH, so a runner with ROCm installed cannot be why the tool reads as found. The scenario with the empty entry first shows that the stand-in rocminfo does resolve through that entry from the working directory, so its "not installed" cannot come from a fixture that never reached it. The harness gains command_dir, the working directory for the next rocm run, consumed like command_env.
Signed-off-by: Jussi Elo <jussi.elo@amd.com>
The PATH lookup is the first pair of the host-probing note to converge. Record what was decided: which survived, that an empty PATH entry is skipped and what that costs, every answer that moved with the test or scenario that pins it, and that the System32 candidates stay in the shared list. The analysis of the two old copies stays as the record of what the change moved; the entry for the empty-entry question leaves the open list, and the near copy in apps/rocm stays on it.
Signed-off-by: Jussi Elo <jussi.elo@amd.com>
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Windows lookup changes lack CLI-level Gherkin coverage, and the working-directory regression scenario needs a positive control to detect an ignored override.
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the diff at head 502ef7f against ROCMAI-905. No blocking correctness problems found; the points below are what I'd like addressed or consciously acknowledged before merge.
Should address
Scenario examine-36 (false/false for the empty PATH entry) would also pass if the new command_dir harness plumbing were not applied: the Given step shows the stand-in resolves through the empty entry, but nothing shows the rocm child actually runs from that directory. The PR says the live e2e harness was only exercised by hand, not via cargo xtask e2e. Please add a control that distinguishes the two cases, or confirm from the Linux e2e lane that the control and examine-36 differ for the right reason (inline comment).
Nit
The edited module-doc line in host_gpu.rs is about 140 characters; re-wrap it to match the neighbouring //! lines (inline comment).
For a maintainer's conscious ack
On Windows, which("cmd" | "pnputil" | "powershell" | "pwsh") now returns true when the System32 file exists, regardless of PATH. It is documented in the PR and docs, and the PR states no current caller asks for these four; worth a quick grep confirmation.
Verified from the diff
Duplicate tool_on_path/old which removed and call sites in wsl.rs, diagnostics.rs and capture.rs rewired; scenarios examine-35/36/37 and the docs/testing.md entry added; Windows logic split into pure path_candidates/found_in helpers tested on every host; docs consistent with the code; no internal names in the diff.
Not verified by me
I did not re-run tests, clippy or the mutation checks (author-reported). At review time CI had 0 failures, with build-and-test, clippy, Windows build/test, coverage and the e2e lanes still pending; those need to go green before merge.
The reason will be displayed to describe this comment to others. Learn more.
examine-36 (false/false) would also pass if command_dir were not applied by the harness: the Given step checks the stand-in resolves through the empty entry, but not that the rocm child really runs from work. Consider a control that fails when the cwd is not work, or confirm from the e2e lane that the control and examine-36 differ for the right reason.
The reason will be displayed to describe this comment to others. Learn more.
Agreed, and fixed in b6a3346. The Given step only proved the stand-in resolves from work through a separate Command, not that the rocm child runs there. I added examine-38, the same machine with . as the first PATH entry instead of an empty one, asserting both reports find the stand-in; the two Given steps now share one planting helper, so 36 and 38 differ in nothing but that entry. I ran 35 to 38 through the real harness (debug rocm built with e2e-test-hooks, ROCM_CLI_BINARY, -n 'examine-3[5-8] '): all four pass. With command_dir ignored in run_rocm_with_scenario_env, examine-38 fails with false/false where true/true is expected; with path_dirs keeping empty entries, examine-36 fails with true/true. Both mutations were restored by targeted edit.
… working directory
examine-36 expects both reports to say rocminfo is not installed when the only copy is in the directory the command runs from and PATH has an empty entry. That answer would also come out if the harness ignored command_dir and started rocm somewhere else, so nothing showed the working directory was applied.
examine-38 is the same machine with `.` as the first PATH entry instead of an empty one. A `.` entry is an ordinary directory, so both reports find the stand-in, and only if rocm really runs from that directory. The two scenarios now differ in nothing but that entry. With command_dir ignored in the harness, examine-38 fails with false/false where true/true is expected; with the lookup keeping empty entries, examine-36 fails with true/true.
The planting is shared between the two Given steps, and the testing guide and the host-probing note list the new scenario.
Signed-off-by: Jussi Elo <jussi.elo@amd.com>
The line that named the child modules had grown to about 140 characters when capture lost its mention of the PATH lookup. Wrap it like the neighbouring lines.
Signed-off-by: Jussi Elo <jussi.elo@amd.com>
Should address: done in b6a3346, see the inline replies. examine-38 is the control, and I ran 35 to 38 through the real harness locally (all pass) and mutated command_dir out to see 38 fail. The lane still has the final say on its own hosts. Nit: re-wrapped in 03bee1c. Conscious ack: I grepped which("cmd, which("pnputil, which("powershell, which("pwsh and which(& across crates, apps and engines. No call site passes any of the four. The literal callers are python, python3, mokutil, lspci, rocminfo, dpkg, rpm, wsl.exe, usermod, sudo and cargo, and the one indirect call, probe_llama_cpp, iterates a fixed list (llama-cli, llama-server, main); the only other non-literal use is the test child. So the System32 candidates are reachable by the capture runner but not by any which caller. Not verified: the Windows job and the rest of the e2e suite; what I ran is cargo test -p rocm-core --lib probe_exec, cargo test -p e2e-cucumber --test feature_naming, fmt, the two clippy runs, manifest --check and check-crate-edges, all green locally.
Fixed: the examine-36 control (examine-38) is added in b6a3346 and shown red when the harness ignores the working directory, the module-doc line is re-wrapped in 03bee1c, and the System32 grep and the Windows-coverage decision are answered on the PR.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Inherited relative PATH entries can resolve to the planted rocminfo after the child changes directories, causing examine-36 to fail even when empty entries are correctly skipped.
If the runner's PATH includes . and its working directory contains no rocminfo, this filter retains that relative entry. examine-36 then changes the child's working directory to work, where ./rocminfo is the planted executable, so both lookups return true and the scenario fails despite correctly skipping the empty entry. Resolve inherited relative entries against the runner's working directory before filtering and appending them, so they cannot become additional routes to the planted executable.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
Inherited relative PATH entries are filtered from the harness directory but interpreted from work, allowing the planted rocminfo to cause false failures in examine-36.
The reason will be displayed to describe this comment to others. Learn more.
Good catch, fixed in 84b6332. The filter checked inherited relative entries from the harness directory while the child runs from work, so a runner PATH with . first survived and exposed the planted stand-in. The inherited entries are now resolved against the harness's working directory, checked and kept in that absolute form, inherited empty entries are dropped, and the fixture's own entries stay as written; the logic is a pure function in the harness library (planted_path_entries) with a test per case, which runs on every host. I reproduced your case: with the old filter and PATH=.:$PATH in the harness environment, examine-36 fails with true/true; with the fix, examine-35 to examine-38 pass both normally and with . first, run through the real harness on a debug rocm built with e2e-test-hooks.
…s directory before filtering them
The scenarios that plant a rocminfo leave out the runner's own PATH entries that hold one, but they checked an inherited relative entry from the harness's directory while the rocm child runs from the scenario's command_dir. A runner whose PATH contains `.` and whose harness directory has no rocminfo kept that entry, and the child then read it as the planted work directory and found the stand-in, so examine-36 failed with true/true despite a correct lookup.
planted_path_entries now makes each non-empty inherited entry absolute against the harness's own working directory, checks it there and keeps it in that resolved form, drops inherited empty entries, and leaves the fixture's own entries exactly as written, since the empty entry and `.` are what two scenarios are about. It is a pure function in the harness library with a row per case, so it runs on every host. With the old filter and `.` first on the harness PATH, examine-36 fails with true/true; with this change examine-35 to examine-38 pass both with and without it.
Signed-off-by: Jussi Elo <jussi.elo@amd.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Human assessment is needed because working-directory contents can cause the test harness to select an unintended executable or attempt to execute a directory.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
juhovainio
left a comment
Closes ROCMAI-905.
Summary
rocm-corehad two PATH lookups that answer the same question, "is this program installed":tool_on_pathinhost_gpu/capture.rs(the summary stack, which feeds the text ofrocm examineand thesummaryobject of its JSON) andwhichinprobe_exec.rs(the examination stack, about fifteen capability checks inexamine/andfix/runners.rs). On WSL both ask aboutrocminfoin onerocm examine --jsonrun, assummary.wsl.rocminfoandwsl.rocminfo, and they could disagree. This is pair 1 of 6 indocs/host-probing.md: the survivor is the behaviour oftool_on_path(it followsPATHEXT), housed inprobe_exec.rsunder the existing namewhich, so the call sites do not change.tool_path_candidatesmoved there with it because the capture runner and the Windows diagnostics capture start each spelling in turn.detect_wsl_summarynow asks the sharedwhichforrocminfoandcargo, andtool_on_pathis gone. There is no third copy.The empty-entry question the note left open is decided as skip: an empty
PATHentry is not read as the current directory. That keeps the examination's answer, which the fifteen or so checks (includingsudoandusermod) already relied on, so a file calledsudoin the directory the CLI was started from never reads as installed. It changes only the summary's answer on WSL. The cost is that a program the CLI later starts by bare name can still resolve from the current directory through the operating system whenPATHhas an empty entry, and the lookup, which starts nothing, no longer agrees with that. The doc comment onwhichsays so. Reading the code gave no concrete reason to take the other branch: followingPATHsemantics would widen what the examination calls installed in all fifteen checks. Skipping also makes the open question of how Windowssplit_pathstreats an empty entry moot, because the entry is dropped either way.The answers that move, each with the test that pins it (the characterisation commit pins both old answers, the convergence commit flips them):
PATHentry with a file of the program's name in the current directory: the summary used to say found, the examination not found; now both say not found. Pinned bywhich_does_not_read_an_empty_path_entry_as_the_current_directory(leading, middle, trailing and whole-PATHshapes, plus the control that the same file counts once a real entry names its directory),path_dirs_drop_every_empty_entryand scenarioexamine-36.PATHthat is not valid Unicode: the summary searched it, the examination found nothing for every program; now both search it, split as anOsString. Pinned bywhich_searches_a_path_that_is_not_unicode,path_dirs_keep_an_entry_that_is_not_unicodeand scenarioexamine-37.PATHEXTentries outside.exe,.batand.cmd, in both cases: the examination now tries them. Pinned bycandidates_on_windows_try_every_pathext_entry_in_both_cases,candidates_on_windows_fall_back_to_the_default_pathext_when_it_is_unsetandcandidates_on_windows_trim_pathext_entries_and_skip_empty_ones(all hosts) andwhich_tries_a_pathext_extension_outside_exe_bat_and_cmd(Windows job).PATHentry holding a;: the examination used to split on every;; nowstd::env::split_pathskeeps it whole. Pinned bywhich_searches_a_quoted_path_entry_that_holds_a_semicolon(Windows job; the splitting is std's, so it can only be tested where it runs).cmd,pnputil,powershellandpwsh: the examination now finds them. Nowhichcall site asks for those four today (checked with grep), so nothing visible changes; they stay in the shared candidate list because the capture runner needs them to start those tools at all, and one list read by both the runner and the lookup cannot drift. A future caller ofwhich("powershell")gets "would the runner find it", not "is it on PATH". An absolute candidate ignores the directory it is joined onto, so it is found wheneverPATHlists at least one directory; this is howtool_on_pathbehaved and the doc comment says it. Pinned bycandidates_on_windows_add_the_fixed_system32_locations,found_in_asks_about_an_absolute_candidate_as_it_is_whatever_the_directoryandwhich_finds_cmd_at_its_system32_location_without_it_being_on_path(Windows job).candidates_on_windows_for_a_name_with_an_extension_are_the_name_alone, which also records that thecmd.exearm of the System32 list is unreachable through the early return, as before), off Windows only the bare name is tried, and a directory named like the program is not the program.The near copy of
tool_path_candidatesinapps/rocm(program_path_candidates) did not join; the note keeps it on its open list.What moved
tool_on_pathinhost_gpu/capture.rswhichinprobe_exec.rshas its behaviourtool_path_candidates,windows_absolute_tool_candidatesinhost_gpu/capture.rsprobe_exec.rs, astool_path_candidatesover a purepath_candidates(program, windows, pathext, system_root)tool_on_path/whichprobe_exec.rs:path_dirs(split, drop empty entries) andfound_in(dirs, candidates, is_file), both purecapture.rsanddiagnostics.rsimportscrate::probe_exec::tool_path_candidatesdetect_wsl_summarycallswhich("rocminfo"),which("cargo")The Windows spellings take the platform,
PATHEXTand the system root as arguments, and the search takes the directories and an existence test, so the logic that only matters on Windows is unit-tested on every host and the Windows job exercises it. The directory split itself isstd::env::split_paths, which cannot be injected, so the;and quote behaviour is covered by the twocfg(windows)tests.Test plan
Commit order: (1) characterisation tests, green against the unchanged code, pinning what both copies return for the same input and each copy's own answer where they differ; (2) the convergence, which moves the tests into
probe_exec.rsand flips exactly the assertions meant to change; (3) the scenarios and the harness change they need, with the working-directory control added after review; (4) the note records the decision (the analysis of the two old copies stays as the record of what changed).Because neither lookup can be pointed elsewhere on a test thread without disturbing every test that spawns a program by name, and one case needs a different working directory, the cases that read the real
PATHrun in a child copy of the test binary started with thePATH,PATHEXTand working directory the case needs. No test mutates the process environment.Run locally on Linux, in a dedicated target dir:
cargo test -p rocm-core --no-fail-fast(1057 passed, 3 ignored, exit 0);cargo test -p rocm --bin rocm examine(25 passed) andsystem_packages(1 passed);cargo test -p e2e-cucumber --test feature_naming(4 passed);cargo fmt --all --check;cargo clippy --locked -p rocm-core --all-targets -- -D warningsandcargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings; the same two clippy runs for--target x86_64-pc-windows-msvc(checked, not run; this compiles and lints thecfg(windows)tests);cargo xtask manifest --check;cargo xtask check-crate-edges; lychee--offlineon the three docs; the leak scan against the merge base (the only hits are the word "private" in text that was already there) and a conflict-marker grep. The leaf test-name list ofcargo test -p rocm-core --lib -- --listwent from 1043 to 1060 names: no test disappeared, 17 were added (the characterisation tests of the first commit are replaced inside this PR, so they do not appear in the comparison).Also run locally: the four new scenarios through the real harness,
cargo test -p e2e-cucumber --test e2e -- -n 'examine-3[5-8] 'withROCM_CLI_BINARYpointing at a debugrocmbuilt with--features rocm/e2e-test-hooks(4 scenarios, 12 steps, all passed on this WSL box). Not run: the Windows job (the fourcfg(windows)tests and every Windows spelling are exercised only there; the purepath_candidatesandfound_intests run on every host) and the rest of the e2e suite.examine-35toexamine-38run on the Linux lanes only, because they plant a simulated host root; the ids followexamine-34, and several open PRs that also addexamine-NNscenarios will need renumbering by whichever merges second. No Windows-gated scenario drivesrocm examine's framework selection withPATHEXTor quoted-entry fixtures:.ps1is not something the probe could then run (Rust starts only.exeby bare name), such a scenario cannot be validated without a Windows lane, and the Windows behaviour is covered by the purepath_candidatesandfound_intests plus the threecfg(windows)lookup tests, which the Windows job runs. What the Windows lane alone can confirm is the realPATHEXTandsplit_pathsbehaviour of those three tests.Scenario answer.
rocm examine --jsonon a simulated WSL host (the machineexamine-32uses) is drivable from the Linux lane, so the observable answer is covered:examine-35(control:rocminfoin aPATHdirectory, both reports true),examine-36(only in the working directory behind an empty entry, both false),examine-37(in a directory whose name is not valid Unicode, both true) andexamine-38(the control for 36: the same machine with.as the first entry, both true, which can only happen ifrocmreally runs from that directory). They assertsummary.wsl.rocminfoandwsl.rocminfotogether. The plantedPATHleaves out the runner's own entries that hold arocminfo, so a runner with ROCm installed cannot be the reason a tool reads as found. The Windows rows cannot run on the Linux lane and are covered by the unit tests above. The harness gainscommand_dir, consumed likecommand_env. Through the harness the answers are true/true, false/false, true/true and true/true for 35 to 38, both with the runner's ownPATHand with.first on the harness'sPATH(the runner's relative entries are resolved against the harness directory before they are filtered, byplanted_path_entriesin the harness library, which has a unit test per case). With the harness ignoringcommand_dir,examine-38fails with false/false where true/true is expected, soexamine-36's answer cannot come from a harness that did not apply the working directory. With the lookup keeping empty entries,examine-36fails with true/true and 35, 37 and 38 stay green; by hand, putting back both old behaviours (empty entries kept,PATHread as aString) fails 36 and 37 and leaves the control alone.Mutation evidence. For the scenarios: ignoring
command_dirinrun_rocm_with_scenario_envturnedexamine-38red; keeping empty entries inpath_dirsturnedexamine-36red. For the characterisation commit, against the old code: keeping empty entries out ofwhichturned the empty-entry test red; makingtool_on_pathreadPATHwithenv::varturned the non-Unicode test red;exists()foris_file()intool_on_pathturned the directory test red; makingwhichfind nothing turned the shared-answer test red. For the survivor's tests, one mutation at a time, each restored by a targeted edit and the file hash compared at the end:path_dirskeeping empty entries (path_dirs_drop_every_empty_entryandwhich_does_not_read_an_empty_path_entry_as_the_current_directoryred);whichreadingPATHas Unicode (which_searches_a_path_that_is_not_unicodered); dropping the lower-case spelling, removing the sort, removing the trim, losing.BATfrom the defaultPATHEXT, dropping thecmdSystem32 arm, applyingPATHEXTto a name with an extension, applying it off Windows (each red in the matchingcandidates_*tests);found_intrying only the first candidate or only the first directory (found_in_asks_each_candidate_in_each_directory_in_order_and_stops_at_the_first_filered);path_dirsmangling a non-Unicode entry (path_dirs_keep_an_entry_that_is_not_unicodered);exists()foris_file()inwhich(which_does_not_take_a_directory_named_like_the_program_for_the_programred); and stripping the root off every candidate before the join (found_in_asks_about_an_absolute_candidate_as_it_is_whatever_the_directoryred).