[rominf]

Summary

Lets an E2E scenario describe the GPU machine it needs (eight Instinct GPUs, a WSL2 distribution with GPU passthrough, a bare-metal host, a box with no GPU) instead of depending on the runner's hardware. Built on #510's ROCM_CLI_TEST_HOST_ROOT (now on main), scenarios that only need the CLI to see a GPU run on every Linux lane, including the GitHub-hosted E2E tests lane on every PR. Three commits:

1. E2E_HARDWARE=simulated|real and @requires-real-gpu.
   • @requires-real-gpu implies @requires-gpu and also skips unless the run sets E2E_HARDWARE=real.
   • The default is simulated. Any other value aborts the run, so a misspelt real can't quietly skip the real-GPU scenarios and still go green.
   • Every self-hosted GPU lane sets real: per-PR MI300X, Strix Ubuntu, Strix Windows, WSL and W7900; nightly also R9700 and MI350P. The WSL lanes forward it through WSLENV.
2. Simulated machines (tests/e2e-cucumber/src/simulated_host.rs).
   • A Given step plants a directory laid out the way the kernel lays out /dev, /sys and /proc: the KFD topology (gfx_target_version inside each node's properties), DRM cards, /dev/kfd//dev/dxg, /proc/version, /etc/os-release.
   • Commands get a PATH without the machine-describing tools (uname, lspci, ldconfig, rocminfo, amd-smi, mpirun, the kernel log, the WSL interop shells). The tools the simulated machine has come back as a fake-host-tool stand-in answering from canned output. The runner's GPU scoping (HIP_VISIBLE_DEVICES, ROCR_VISIBLE_DEVICES, ROCM_PATH, ...) is unset and HOME moves into the scenario.
   • Given a managed runtime is active on a simulated machine plants a runtime whose vllm is a fake-vllm stand-in, so a serve runs the CLI's real engine selection, runtime lookup, managed launch and readiness wait; only the process at the end is simulated.
   • Nineteen scenarios move onto simulated machines and @requires-os:linux: examine-04/10/11/17, diagnose-08/15/16/17/18, driver-install-01..03, serve-11/13/15/16/19/20/21.
   • Every scenario left on @requires-gpu needs a physical device or a runtime really installed for one, so those become @requires-real-gpu. examine-29 (@id:examine-detects-this-machines-gpu-and-driver) repeats examine-04 against the real machine on every GPU lane, the Windows one included.
3. A nightly drift check. examine-30 (bare-metal GPU host) and examine-31 (WSL2) hold every fixture assumption against the live /dev, /sys, /proc/version and lspci of a real machine, so a kernel or driver change that moves something fails there, naming the difference, instead of the simulated scenarios passing while the CLI fails on hardware.

Not simulated: package-manager state, discovered ROCm installs, and the identity of the user running the suite ($USER, id -Gn, stat on the planted device nodes). No simulated scenario asserts on them. Container detection does read under the host root, so a simulated machine is never in a container.

Risk: low for the product (no product code changes); medium for the suite, since it changes which lanes run 19 scenarios. Each moved scenario still asserts the same behaviour.

Scenario coverage (AGENTS.md §3)

No CLI behaviour changes. This PR changes which machines existing scenarios run against, and adds examine-29/30/31. The moved scenarios gain coverage: several used to run only on the WSL lane or multi-GPU hosts and now run on every Linux lane. The real-kernel cross-check is kept by examine-29 (every real GPU lane) and examine-30/31 (nightly).

Test plan

On this head, rebased on main at 1c33248:

• cargo xtask e2e, full default suite on a WSL2 machine with no GPU: 237 scenarios ran (72 resolved to skip: real GPU, nightly, lifecycle, WSL, Windows, docker), 2 expected xfails, 3 unexpected failures, each explained:

  • serve-11 failed because another process on that shared machine already listened on 127.0.0.1:11435. In a private network namespace it passes.
  • comfyui-04 (download progress frame) and automations-04 failed under load and pass when run alone.

• Every scenario this PR touches passed in that run except serve-11 (above). examine-29/30/31 need real hardware, so they skip there.

• cargo test -p e2e-cucumber --lib --bins --test feature_naming: 186 + 1 + 4 pass. cargo check -p e2e-cucumber --test e2e is clean.

• cargo test -p xtask: 317 pass.

• cargo clippy --workspace --all-targets and -p e2e-cucumber --test e2e with -D warnings, on Linux and on x86_64-pc-windows-gnu: clean. cargo fmt --check is clean.

• cargo test -p rocm / -p rocm-core were not run locally: this PR changes no product code. CI runs them.

• If this PR fixes a bug, searched tests/e2e-cucumber/expectations.toml for the fixed ticket ID and removed/narrowed any now-stale xfail rows. (n/a; none of the moved scenarios has an xfail row)

• If this PR adds a new subcommand or subsystem, its domain implementation lives in its own file per docs/architecture.md. (n/a)

• Every new or changed user-facing message was read against the code path that runs after it. (n/a, no user-facing message changes)

[rominf]

Rebased onto main at 1c33248. Head is now 124b181. What changed:

• Dropped the first commit ("let e2e builds re-root the hardware probes"). test(core): let e2e builds re-root the hardware probes #510 landed it on main in a later form.
• E2E_HARDWARE: real is set on main's current lane set: per-PR MI300X, Strix Ubuntu, Strix Windows, WSL and W7900, plus nightly W7900. The per-PR R9700 and MI350P lanes no longer exist.
• Scenarios main added with @requires-gpu since this PR was written are now @requires-real-gpu: examine-19, serve-23, serve-24 and runtime-lifecycle-12. runtime-16 keeps main's @requires-os:windows, and therock-next-09 keeps main's removal of @nightly.
• diagnose-13 is no longer moved. vllm: point import failures at rocm diagnose #646 made its checker apply on WSL too, so it already runs on every Linux lane without a simulated machine. That makes 19 moved scenarios, not 20.
• This PR's examine scenarios are renumbered examine-29/30/31, because main took 18-28. Cross-references now use @id: instead of numbers.
• Removed this PR's own copy of the fixture-binary lookup in favour of main's fixture_binary::fixture_binary.
• Corrected docs that main's changes had made wrong:
  • Container detection now reads under the host root, so a simulated machine is never in a container.
  • The user's identity ($USER, id -Gn, stat on device nodes) is not simulated.
  • docs/ci-hardware-testing.md and the WSL lane comments no longer say the WSL diagnose and driver scenarios run only on the WSL lane.
• When the simulated vLLM never becomes ready, the serve-11 failure message now includes the service log.

Local results are in the updated description. The full mock suite ran 237 scenarios. The 3 unexpected failures were all local: a port already held on the shared test machine, and two load flakes that pass on their own. The PR stays in draft until required CI is green.

[tomastola]

Review — needs work

Full review of the whole change (base 1c33248c, head 124b1815).

Blocking

• examine-17's Then step can pass without asserting anything — tests/e2e-cucumber/tests/e2e/examine_steps.rs:1014-1022 (assert_gpus_match_kfd_nodes)
  The step returns silently when kfd_gpu_node_versions(...) is None or empty, and again when host_has_lspci is false. The scenario now plants eight simulated GPUs, so those early returns are exactly what hides a planted tree the CLI no longer reads. If the planting, world.host_path or the CLI's probe root regress, the scenario stays green. The sibling assert_gpu_target_matches_kfd got a "nothing may quietly decline" guard in this PR; this step did not.
  Confidence 70 · logic · Fix: when world.simulated_host is Some, assert the node count equals host.gpus.len() and fail instead of returning. Skip the lspci bail-out for simulated hosts.

Non-blocking

• fake-host-tool answers plausibly for argv nobody planted — tests/e2e-cucumber/src/bin/fake-host-tool.rs:43-49
  Lookup falls back from the full command line to the bare tool name, so uname -n or lspci with different flags return the canned -a / -nn -D text with exit 0. If the CLI changes its lspci flags, the stand-in will not disagree, which weakens the drift protection this PR is built around.
  Confidence 70 · logic · Fix: fall back to the bare name only for argument-less tools (mpirun, lsmod); otherwise exit 127 with a loud stderr message.
• Stale tag name in comments — .github/workflows/ci.yml:1527 says "@requires-gpu scenarios resolve to skip here", and features/dash.feature:329 says "@requires-wsl rather than @requires-gpu"; the skipping tag is now @requires-real-gpu.
  Confidence 85 · mechanical · Fix: rename in both.
• plant_simulated_host panics on a second call into the same root — simulated_host.rs:437 (link(), symlink already exists). Loud, but undocumented; two Given steps in one scenario would trigger it.
  Confidence 70 · mechanical · Fix: document it, or fail with a message naming the double plant.
• Unused Platform::Wsl2 { rocdxg_ready: false } variant — simulated_host.rs:137, 247-250, 317 has ldconfig/rocdxg branches nothing constructs, so they are untested dead code.
  Confidence 70 · mechanical · Fix: drop the variant and branches, or add a scenario that uses it.
• README does not tell GPU-box users about E2E_HARDWARE — the tag table mentions it, but the environment-variable table and run examples do not. A plain cargo xtask e2e on a GPU machine now skips every @requires-real-gpu scenario without saying why where people look.
  Confidence 70 · documentation · Fix: add an E2E_HARDWARE row (default simulated) and an E2E_HARDWARE=real cargo xtask e2e example.

Decisions for the author

• Coverage moves off real hardware, and Windows/macOS lose scenarios — tradeoff
  • serve-13 (serve-no-gpu-fails-fast) was @requires-no-gpu, so it ran on any no-GPU lane including Windows and macOS; it is now @requires-os:linux on a simulated host. examine-10 went from @requires-bare-metal (skips only on WSL) to @requires-os:linux, so --no-frameworks is no longer checked on Windows or macOS.
  • serve-13/15/16/19/20/21, examine-10/11/17, diagnose-08/15/16/17/18 and driver-install-01..03 no longer run against a real GPU or WSL host. Only examine-04 has a real twin (examine-29); examine-30/31 check layout assumptions only. Real masked-device serve refusals and real-WSL diagnose/driver-plan paths run in no lane.
  • The simulated run is hermetic and runs on every PR, which is a real gain. Cheap mitigations: a @requires-real-gpu copy of serve-15 and a @requires-wsl copy of diagnose-17, as examine-29 does for examine-04; keep a non-simulated no-GPU variant for Windows/macOS. Note the description's "each moved scenario still asserts the same behaviour" and "gain coverage" do not mention the Windows/macOS loss.
• Fake vLLM ignores everything except --host and --port — tradeoff (src/bin/fake-vllm.rs:30-36)
  serve-11 proves selection, runtime lookup, launch and readiness, but not that the CLI passes sane vLLM flags; --port=N would be ignored and fall back to 8000. Recording argv under the root and asserting the expected flags would close this; acceptable if the flags are covered elsewhere.
• serve-11 failed in the test plan on a port conflict (127.0.0.1:11435) — non-blocking-improvement
  A fixed port in a hermetic harness invites this on shared runners; an ephemeral port would remove the flake.

[nowycondro]

🤖 Automated review (Claude Code) · 124b181

I read the full diff for this PR (simulated-host infrastructure, expectation/tag plumbing, CI workflow changes, and the migrated e2e feature/step files), cloned the repo at the PR head commit, and cross-checked the two concerns from @tomastola's existing CHANGES_REQUESTED review against the current code. No Rust toolchain is available in this environment, so nothing was compiled or executed — everything below is from static reading of the diff and surrounding code.

Confirmed still open (both from the existing review)

• examine-17's assertion can still pass without checking anything — tests/e2e-cucumber/tests/e2e/examine_steps.rs:1012-1016 (assert_gpus_match_kfd_nodes). The function returns silently if kfd_gpu_node_versions(...) is None/empty, before ever comparing against the planted host. Compare this to its sibling assert_gpu_target_matches_kfd (same file, lines 945-956), which was given an explicit if let Some(expected) = world.simulated_host....gfx_target() { assert_eq!(...) } guard so "nothing below may quietly decline to check it." assert_gpus_match_kfd_nodes has no equivalent direct comparison against world.simulated_host.as_ref().map(|s| s.host.gpus.len()). The feature comment at features/examine.feature:226-227 ("the step's premises always hold") documents the intent but the code doesn't enforce it — a regression in planting, world.host_path, or the CLI's probe root would make this scenario pass vacuously instead of failing. Confidence: high (70-80%), this is a real gap, not a false positive.

• Coverage-loss tradeoff is real and still present — verified via git grep at head: features/model_serving.feature:135 (serve-13) now carries @requires-os:linux only, down from @requires-no-gpu (which also ran on Windows/macOS). features/examine.feature:111 (examine-10) is similarly now @requires-os:linux, down from @requires-bare-metal (which only skipped WSL). Both comments in the feature files candidly explain the simulated premise, but the net effect for those two scenarios is that Windows/macOS lanes no longer exercise GPU-required-refusal and --no-frameworks behavior. This is a tradeoff for the author/reviewers to accept or mitigate (e.g. a non-simulated Windows/macOS variant), not something I'd block on myself, but it is accurately described in the prior review and still applies at this head commit.

Additional observation (non-blocking, low confidence)

• tests/e2e-cucumber/src/simulated_host.rs (filter_real_path/link_stand_ins, around line 402) rebuilds the filtered-PATH fixture from scratch on every scenario invocation — a fresh read_dir+stat+symlink pass over every PATH directory per scenario. With 19+ scenarios now going through this path per run, this adds I/O overhead to the per-PR suite. This is a performance nit, not a correctness issue, and I did not measure actual wall-clock impact.

Not independently re-verified

I did not re-derive every item in the prior review's non-blocking list (fake-host-tool argv fallback, stale @requires-gpu tag mentions in comments, plant_simulated_host panic-on-double-call, unused Wsl2 { rocdxg_ready: false } variant, README gap, fake-vllm flag handling, serve-11 port conflict) — a spot read of the code is consistent with those being accurate, but I did not independently trace each one line-by-line given the medium-effort budget for this pass. I found no additional correctness bugs beyond what's above and what's already flagged.

🤖 Generated with Claude Code

[nowycondro]

🔴 Automated review · pr-review-watcher · 124b181

Review — needs work

Full review of the whole change, run as five independent file-grouped passes plus consolidation.
Implements ROCMAI-677 ("E2E: describe GPU hosts with simulated machines so GPU-visibility scenarios run on every PR"); the PR description's claims (HardwareMode gating, simulated_host.rs layout, fake-vllm/fake-host-tool stand-ins, nineteen scenarios moved to simulated machines, examine-29/30/31 real-hardware/drift checks) match what the diff contains, verified across passes with no mismatch found.

Blocking

• CI check "E2E tests (MI300X, current)" is failing on the reviewed commit — .github/workflows/e2e-selfhosted.yml (self-hosted MI300X lane); run: https://github.com/ROCm/rocm-cli/actions/runs/40314879688/job/121545019785
  Confirmed via gh pr view 517 --json statusCheckRollup (conclusion: FAILURE) and the rendered Actions page, which shows the "Run E2E tests on GPU hardware" step exiting with code 1; every other check on the PR is green. Full job logs were not retrievable from this environment (the GitHub Actions REST log/run API endpoints returned 403/Access Denied through an egress proxy), so the specific failing scenario or assertion could not be identified. This PR's own e2e-selfhosted.yml changes are what first set E2E_HARDWARE: real on this exact lane, so the failure may be the first real exercise of the new @requires-real-gpu gating on that hardware rather than an unrelated flake — that is plausible, not established.
  Confidence 100 · architectural · Fix: obtain the job's actual log/annotation (needs sign-in or maintainer access), determine whether it's a genuine regression in the new real-GPU gating/hardware-state assumptions or a known self-hosted-runner flake, and resolve or re-run before merge.

Non-blocking

• ldconfig cache header reports a fixed count of 5 libraries that doesn't match the number actually written — tests/e2e-cucumber/src/simulated_host.rs:308-320. The canned ldconfig -p output always prints "5 libs found in cache" but only 4 unconditional entries are written (a 5th, librocdxg.so, is added only for Platform::Wsl2 { rocdxg_ready: true }), so on bare-metal and WSL2-not-ready machines the printed header disagrees with the printed list below it. No consumer currently parses the header count (rocm-core's checks match by soname substring), so nothing currently breaks, but it's a fidelity gap in a fixture whose whole point is to match real ldconfig output.
  Confidence 80 · mechanical · Fix: compute the header count from the number of lines actually emitted rather than hardcoding it.
• bare_metal_layout_drift's GPU-topology checks early-return when the simulated side reports zero GPUs — tests/e2e-cucumber/src/simulated_host.rs:520-557 (let Some((_, simulated_props)) = simulated_gpus.first() else { return drift; }). Two drift checks (non-zero gfx_target_version on the CPU KFD node; DRM card vendor/device id shape) never execute when comparing against a no-GPU simulated host. Every current call site only drives this with instinct_mi300x(1), so it's unreachable today, but it's a latent gap if this drift check is ever reused for a no-GPU scenario.
  Confidence 70 · logic · Fix: run those two checks unconditionally, independent of whether the simulated side has a GPU.
• fake-host-tool.rs redeclares TOOL_OUTPUT_DIR and TEST_HOST_ROOT_ENV instead of importing them from simulated_host.rs — tests/e2e-cucumber/src/bin/fake-host-tool.rs:22,24 vs tests/e2e-cucumber/src/simulated_host.rs:38,41. The two copies are held in sync only by a "must match" doc comment with no compiler enforcement; the sibling binary fake-vllm.rs:17 proves a [[bin]] target in this crate can use e2e_cucumber::... to reach the crate's own lib, so this duplication isn't architecturally forced (unlike the separate, legitimately-forced duplication against rocm_core::hardware_root::TEST_HOST_ROOT_ENV, which fake-host-tool.rs cannot reach).
  Confidence 75 · mechanical · Fix: import the two constants from e2e_cucumber::simulated_host instead of redefining them.

Decisions for the author

• @id:examine-detects-gpu-and-driver (examine-04) keeps its scenario id while changing what it verifies — tradeoff. Before this PR the id denoted "verified against the real installed GPU" (rare, @requires-gpu-gated, self-hosted-only runs); after, the same id denotes "verified against a simulated Instinct GPU" (frequent, @requires-os:linux, every Linux PR lane), with a new, narrower id (examine-detects-this-machines-gpu-and-driver, examine-29) now covering what the old id used to mean, and a nightly drift check (examine-30/31) mitigating simulated/real divergence. This is well-reasoned and documented in the feature-file comments, but if any external dashboard or flaky-test tracker keys history by scenario id, examine-04's historical baseline will now be read against a very different population of runs. Worth confirming no such tracker exists, or that the discontinuity is acceptable.

Positive signals

• bare_metal_layout_drift, wsl_layout_drift, and lspci_drift (simulated_host.rs) are explicit, tested mechanisms for catching simulation/reality divergence — e.g. a regression test pins the exact gfx_target_version-in-properties vs standalone-file bug that motivated simulating hardware in the first place — directly addressing the core risk that a simulated-host suite could drift from what real hardware actually does.
• HardwareMode::parse (expectation.rs) fails closed: an unrecognized E2E_HARDWARE value panics the run rather than silently defaulting to simulated, so a typo can't quietly skip every real-GPU scenario while CI stays green — and the one call site (tests/e2e.rs) is wired to actually propagate that panic rather than swallow it.
• assert_vllm_default's new simulated-only readiness assertion (serving_steps.rs) closes a real gap: rocm serve exits 0 regardless of readiness, so without this check a simulated vLLM that never came up would have passed silently.
• feature_body (expectation.rs) reads the actual .feature file text to assert a scenario's premise line is really present, so deleting the underlying Gherkin step would fail this test rather than silently certifying stale coverage.

Deployment notes

None — no product code changes; this PR only changes which CI lanes exercise which existing e2e scenarios and adds new test infrastructure.

What this covered

Every file in the diff, read completely at PR head 124b1815e0ef5359513dece166627e3f81f9e04b against base d59f596f6e08297c207f008021eecb37defc3558 (32 files, +1991/-261), split across five independent file-grouped passes: (1) tests/e2e-cucumber/src/simulated_host.rs; (2) tests/e2e-cucumber/src/expectation.rs + mock_server.rs + lib.rs; (3) tests/e2e-cucumber/src/bin/fake-host-tool.rs + fake-vllm.rs + tests/e2e/simulated_host_steps.rs + tests/e2e.rs; (4) the remaining tests/e2e/*_steps.rs files plus all ten changed .feature files; (5) docs/testing.md, docs/ci-hardware-testing.md, the three .github/workflows/*.yml files, tests/e2e-cucumber/Cargo.toml, tests/e2e-cucumber/README.md. Each pass also read relevant surrounding/consumer code outside its primary scope (e.g. crates/rocm-core/src/examine.rs, openmpi.rs, apps/rocm/src/service_launch.rs, engines/vllm/src/runtime.rs) to verify specific cross-file claims. Step 7 (design review) questions were answered from material the five passes already surfaced (scenario-id continuity, simulation fidelity, real-vs-simulated gating design) rather than a separate fan-out round.

CI: gh pr view 517 --json statusCheckRollup read in full; one check (E2E tests, MI300X, current) reports FAILURE, all others SUCCESS/SKIPPED — reflected above as a blocking finding. Job logs were not retrievable (GitHub Actions REST log/run endpoints returned 403 from this environment's egress proxy); only the rendered Actions page's step name and exit code were available.

Did not run: the history pass (git log/git blame on modified regions) and prior-changes pass (review comments on earlier changes to these files) were not run this round — simulated_host.rs, the fake binaries, and the simulated-host step definitions are new files with no history, and the remaining changed files' history was not separately mined given the depth already reached by the code and impact-analysis passes; re-run these specifically if deeper confidence is wanted. No cargo build/cargo test/clippy was executed in this environment; all findings were settled by reading source against source, not execution — the PR's own description reports cargo test, clippy, and a 237-scenario local run as green except three explained, non-blocking flakes (serve-11, comfyui-04, automations-04), consistent with what the diff shows but not independently re-run here.

Ticket ROCMAI-677 retrieved by key (title, status, issue type only — no comments): "E2E: describe GPU hosts with simulated machines so GPU-visibility scenarios run on every PR," status In Progress. Matches the change's own description of itself.

Not reconciled against any PR discussion, review comments, or earlier review rounds — none were read, per method.