diff --git a/tests/e2e-cucumber/expectations.toml b/tests/e2e-cucumber/expectations.toml index 3eda8618c..beaa49aa4 100644 --- a/tests/e2e-cucumber/expectations.toml +++ b/tests/e2e-cucumber/expectations.toml @@ -178,7 +178,6 @@ flaky = true when = { is_wsl = true, therock_family = "gfx*" } bug = "EAI-7998" reason = "`examine --json` reports has_amd_gpu:false on a WSL2 host whose own summary names a gfx target." - # --- EAI-8031 (public mirror: ROCm/rocm-cli#260): on a Strix Halo WINDOWS host, # serving a canonical Hugging Face checkpoint through the `owner/repo:variant` # direct-serve path (`unsloth/Qwen3-0.6B-GGUF:Q4_0`) exits 0 but `/v1/models` @@ -208,3 +207,23 @@ when = { os = "windows", therock_family = "gfx*", has_amd_gpu = true } bug = "EAI-8031" reason = "Managed lemonade serve of a Hugging Face `owner/repo:variant` checkpoint exits 0 but the endpoint never becomes ready on Windows." serve_timeout_secs = 240 + +# --- EAI-8053: `install driver --dkms` prefixes every plan command with a literal +# `sudo `, unconditionally. As root that prefix is unnecessary, and on a root host +# with no `sudo` binary it is fatal: `sh -c "sudo apt-get update"` dies with +# `sudo: not found` before any driver work. The fix is uid-aware (prepend `sudo` +# only when NOT root), so the scenario is @requires-root (it SKIPS off root, where +# the prefix is correct); on a root runner the "no sudo prefix" contract fails +# today. Unconditional there, so `when = {}`. Remove when the plan stops prefixing +# `sudo` while already root. --- +[["driver-install-as-root-does-not-require-sudo"]] +when = {} +bug = "EAI-8053" +reason = "install driver --dkms prefixes sudo even as root, so it fails with `sudo: not found` on a root host without sudo." + +# EAI-8051 had a row here — `comfyui install` pulling CUDA `nvidia-*` wheels into +# the managed ROCm runtime and displacing its ROCm torch. The fix landed in +# 3e59435a (#298): the install now pins the runtime's torch stack with a `uv +# --constraint` file, so a transitive dependency can no longer resolve torch from +# PyPI. `comfyui-install-preserves-the-rocm-runtime` therefore EXPECT-PASSES now; +# keeping the row would make it a stale XPASS and fail the lane. diff --git a/tests/e2e-cucumber/features/comfyui.feature b/tests/e2e-cucumber/features/comfyui.feature new file mode 100644 index 000000000..a97f5fea4 --- /dev/null +++ b/tests/e2e-cucumber/features/comfyui.feature @@ -0,0 +1,31 @@ +Feature: ComfyUI application management + + # EAI-8051: `rocm comfyui install` installs ComfyUI's dependencies INTO the + # machine's managed ROCm runtime. It filters torch/torchvision/torchaudio out of + # ComfyUI's requirements, but nothing scopes the package index, so a transitive + # dependency can still pull CUDA `nvidia-*` wheels into the runtime and displace + # its ROCm torch. The runtime the whole machine serves models with is then a CUDA + # build with no AMD GPU support — installing an optional app broke the base. + # + # The contract: after installing an optional app, the machine's ROCm runtime must + # still be a ROCm runtime — its torch stays a ROCm (HIP) build and no `nvidia-*` + # CUDA distributions appear in it. We assert the runtime's health, NOT that the + # ComfyUI install exits 0: the real install exits non-zero AND still leaves the + # damage, so an exit-code assertion would miss the defect. + # + # Genuinely destructive and expensive: it needs a real managed runtime (a + # multi-GiB SDK install) and mutates it, so it runs ONLY on a GPU host, behind + # @nightly, against this scenario's own isolated runtime prefix (it must never + # share a runtime tree with other scenarios — it may corrupt it). Gated + # @requires-gpu @nightly, matching runtime-install-sdk-active, the other scenario + # that does a real `install sdk`. NOT @lifecycle: that tag is for OS-mutating + # release scenarios and no lane sets E2E_INCLUDE_LIFECYCLE on a GPU host, so + # combining it with @nightly would make this scenario unreachable on every lane; + # this mutates only its own isolated runtime prefix, not the OS. + @id:comfyui-install-preserves-the-rocm-runtime @requires-gpu @nightly + Scenario: 1 - Installing ComfyUI does not replace the ROCm runtime with a CUDA one + Given an isolated machine with a managed ROCm runtime + And the runtime's torch is a ROCm build + When the user installs ComfyUI + Then the runtime's torch is still a ROCm build + And no CUDA nvidia packages were added to the runtime diff --git a/tests/e2e-cucumber/features/driver.feature b/tests/e2e-cucumber/features/driver.feature new file mode 100644 index 000000000..9fb29f022 --- /dev/null +++ b/tests/e2e-cucumber/features/driver.feature @@ -0,0 +1,19 @@ +Feature: Native driver installation + + # EAI-8053: `rocm install driver --dkms` builds its command plan with a literal + # `sudo ` prefix on every step, unconditionally. When the CLI already runs as + # root the prefix is unnecessary, and on a root host that has no `sudo` binary it + # is actively harmful: the very first command dies with `sudo: not found` before + # any driver work happens. The contract is that being root — the state where the + # commands could otherwise succeed — must not be the thing that breaks the run. + # + # Root-gated: the fix is uid-aware (prepend `sudo` only when NOT root), so the + # "no sudo prefix" contract only has a premise where the runner is actually root. + # Off root the sudo prefix is correct, so @requires-root skips there rather than + # letting the row falsely pass. Linux-only: the plan and its `sh -c` execution + # are the Linux DKMS path. + @id:driver-install-as-root-does-not-require-sudo @requires-os:linux @requires-root + Scenario: 1 - Installing the driver as root does not depend on sudo being present + Given a root machine with no sudo command available + When the user installs the native driver with dkms + Then the install does not fail merely because sudo is missing diff --git a/tests/e2e-cucumber/src/capability.rs b/tests/e2e-cucumber/src/capability.rs index 9ca0d6a0a..53b4f6912 100644 --- a/tests/e2e-cucumber/src/capability.rs +++ b/tests/e2e-cucumber/src/capability.rs @@ -99,6 +99,10 @@ pub struct HostCapability { pub os_family: String, /// `examine`'s `wsl:` line. pub is_wsl: bool, + /// Whether the test process runs as root (effective UID 0). Always false off + /// Unix. Gates `@requires-root` scenarios whose contract only holds when the + /// runner is root (see [`ScenarioDecl::requires_root`]). + pub is_root: bool, /// First AMD GPU's gfx target from `examine`'s `detected_gfx_target:` line /// (e.g. "gfx942", "gfx1151"), if a real one was reported. pub gfx_target: Option, @@ -348,6 +352,7 @@ fn probe_host_capability() -> HostCapability { HostCapability { os_family, is_wsl, + is_root: process_is_root(), gfx_target, has_amd_gpu, available_engines, @@ -356,6 +361,23 @@ fn probe_host_capability() -> HostCapability { } } +/// Whether the current test process runs as root (effective UID 0). Read from +/// `/proc/self/status` rather than `geteuid` so the check stays inside this +/// crate's `deny(unsafe_code)` policy. The `Uid:` line lists real, effective, +/// saved and filesystem uids; the effective uid (the second field) is the one the +/// driver-install `sudo`-prefix contract turns on. False on any non-Linux host or +/// if the status file cannot be read — off Linux the contract does not apply. +fn process_is_root() -> bool { + let Ok(status) = std::fs::read_to_string("/proc/self/status") else { + return false; + }; + status + .lines() + .find_map(|line| line.strip_prefix("Uid:")) + .and_then(|rest| rest.split_whitespace().nth(1)) + .is_some_and(|euid| euid == "0") +} + /// Run `rocm ` with an isolated config/data/cache root, returning stdout /// (empty string on any failure — the probe must never panic the suite). fn run_probe(root: &std::path::Path, args: &[&str]) -> String { @@ -600,6 +622,7 @@ mod tests { let strix = HostCapability { os_family: "windows".to_owned(), is_wsl: false, + is_root: false, gfx_target: Some("gfx1151".to_owned()), has_amd_gpu: true, available_engines: vec!["lemonade".to_owned(), "vllm".to_owned()], @@ -613,6 +636,7 @@ mod tests { let mi300x = HostCapability { os_family: "linux".to_owned(), is_wsl: false, + is_root: false, gfx_target: Some("gfx942".to_owned()), has_amd_gpu: true, available_engines: vec!["lemonade".to_owned(), "vllm".to_owned()], diff --git a/tests/e2e-cucumber/src/expectation.rs b/tests/e2e-cucumber/src/expectation.rs index 1c617ad29..87186943f 100644 --- a/tests/e2e-cucumber/src/expectation.rs +++ b/tests/e2e-cucumber/src/expectation.rs @@ -28,6 +28,7 @@ const REQUIRES_GPU_TAG: &str = "requires-gpu"; const REQUIRES_NO_GPU_TAG: &str = "requires-no-gpu"; const REQUIRES_BARE_METAL_TAG: &str = "requires-bare-metal"; const REQUIRES_WSL_TAG: &str = "requires-wsl"; +const REQUIRES_ROOT_TAG: &str = "requires-root"; const SERVE_TIMEOUT_PREFIX: &str = "serve-timeout:"; const NIGHTLY_TAG: &str = "nightly"; const LIFECYCLE_TAG: &str = "lifecycle"; @@ -82,6 +83,15 @@ pub struct ScenarioDecl { /// host, so it is skipped on native Linux, native Windows and everything /// else. Same reason `@requires-os:linux` cannot stand in for it. pub requires_wsl: bool, + /// `@requires-root`: the scenario's premise is a process running as root + /// (effective UID 0), so it is skipped where the test process is not root. + /// Needed by the driver-install-as-root contract (EAI-8053): the fix is + /// uid-aware (prepend `sudo` only when NOT root), so the "no `sudo` prefix" + /// contract only holds where the runner is actually root. None of the four + /// `expectations.toml` condition keys can express "running as root", so — like + /// `@requires-wsl` — this is a tag, not a row, keeping the xfail row honest on + /// non-root lanes (they SKIP rather than falsely PASS). + pub requires_root: bool, /// Engine the scenario pins via `@requires-engine:` (if any). pub requires_engine: Option, /// OS the scenario requires via `@requires-os:` (e.g. "linux"), if any — @@ -120,6 +130,7 @@ impl ScenarioDecl { let mut requires_no_gpu = false; let mut requires_bare_metal = false; let mut requires_wsl = false; + let mut requires_root = false; let mut requires_engine = None; let mut requires_os = None; let mut serve_timeout_secs = None; @@ -147,6 +158,8 @@ impl ScenarioDecl { requires_bare_metal = true; } else if tag == REQUIRES_WSL_TAG { requires_wsl = true; + } else if tag == REQUIRES_ROOT_TAG { + requires_root = true; } else if tag == NIGHTLY_TAG { nightly = true; } else if tag == LIFECYCLE_TAG { @@ -161,6 +174,7 @@ impl ScenarioDecl { requires_no_gpu, requires_bare_metal, requires_wsl, + requires_root, requires_engine, requires_os, serve_timeout_secs, @@ -363,8 +377,8 @@ pub struct PlatformManifest<'a> { /// 1. Not-applicable → `Skip`: a `@nightly` scenario when nightly isn't included, /// a `@merge-queue` scenario outside the merge queue, a `@requires-gpu` /// scenario on a host with no AMD GPU, a `@requires-bare-metal` scenario on -/// WSL2, a `@requires-os:` scenario on a different OS, or a scenario whose -/// effective engine can't start. +/// WSL2, a `@requires-root` scenario off root, a `@requires-os:` scenario +/// on a different OS, or a scenario whose effective engine can't start. /// 2. First matching `expectations.toml` condition → `ExpectXfail`. /// 3. Otherwise → `ExpectPass`. /// @@ -420,6 +434,11 @@ pub fn resolve( reason: "requires WSL; this host is not running under WSL".to_owned(), }; } + if decl.requires_root && !cap.is_root { + return Expectation::Skip { + reason: "requires the runner to be root; this process is not root".to_owned(), + }; + } if let Some(os) = &decl.requires_os && !os.eq_ignore_ascii_case(&cap.os_family) { @@ -500,6 +519,7 @@ mod tests { "mi300x" => HostCapability { os_family: "linux".into(), is_wsl: false, + is_root: false, gfx_target: Some("gfx942".into()), has_amd_gpu: true, available_engines: vec!["lemonade".into(), "vllm".into()], @@ -509,6 +529,7 @@ mod tests { "strix-ubuntu" => HostCapability { os_family: "linux".into(), is_wsl: false, + is_root: false, gfx_target: Some("gfx1151".into()), has_amd_gpu: true, available_engines: vec!["lemonade".into(), "vllm".into()], @@ -518,6 +539,7 @@ mod tests { "strix-windows" => HostCapability { os_family: "windows".into(), is_wsl: false, + is_root: false, gfx_target: Some("gfx1151".into()), has_amd_gpu: true, available_engines: vec!["lemonade".into(), "vllm".into()], @@ -531,6 +553,7 @@ mod tests { "wsl2" => HostCapability { os_family: "linux".into(), is_wsl: true, + is_root: false, gfx_target: Some("gfx1151".into()), has_amd_gpu: true, available_engines: vec!["lemonade".into(), "vllm".into()], @@ -543,6 +566,7 @@ mod tests { "wsl" => HostCapability { os_family: "linux".into(), is_wsl: true, + is_root: false, gfx_target: None, has_amd_gpu: false, available_engines: vec!["lemonade".into(), "vllm".into()], @@ -554,15 +578,29 @@ mod tests { "wsl-no-passthrough" => HostCapability { os_family: "linux".into(), is_wsl: true, + is_root: false, gfx_target: Some("gfx1151".into()), has_amd_gpu: false, available_engines: vec!["lemonade".into(), "vllm".into()], effective_serve_engine: "lemonade".into(), platform_slug: "strix-halo-wsl".into(), }, + // A no-GPU host running as root — the mock CI lane's shape, where the + // driver-install sudo-prefix contract (EAI-8053) has a premise. + "mock-root" => HostCapability { + os_family: "linux".into(), + is_wsl: false, + is_root: true, + gfx_target: None, + has_amd_gpu: false, + available_engines: vec!["lemonade".into(), "vllm".into()], + effective_serve_engine: "lemonade".into(), + platform_slug: "mock".into(), + }, _ => HostCapability { os_family: "other".into(), is_wsl: false, + is_root: false, gfx_target: None, has_amd_gpu: false, available_engines: vec!["lemonade".into(), "vllm".into()], @@ -622,6 +660,28 @@ serve_timeout_secs = 90 assert!(!decl(&["id:x", "requires-gpu"]).requires_bare_metal); } + #[test] + fn root_tag_parses_and_gates_on_root() { + // Parses in both shapes; absent by default so no existing scenario changes. + assert!(decl(&["id:x", "requires-root"]).requires_root); + assert!(decl(&["@id:x", "@requires-root"]).requires_root); + assert!(!decl(&["id:x", "requires-gpu"]).requires_root); + + let m = Expectations::default(); + let d = decl(&["id:driver-root", "requires-os:linux", "requires-root"]); + // Root host: the premise holds, so it resolves (here, expected-pass with + // an empty matrix — an xfail row is layered on separately). + assert_eq!( + resolve(&d, &cap("mock-root"), &m, false, false, false), + Expectation::ExpectPass + ); + // Non-root host: skipped, so a `when = {}` xfail row can't XPASS there. + assert!(matches!( + resolve(&d, &cap("mock"), &m, false, false, false), + Expectation::Skip { .. } + )); + } + #[test] fn serve_timeout_tag_parses_seconds() { let d = decl(&["id:serve-large-model-inference", "serve-timeout:2400"]); diff --git a/tests/e2e-cucumber/tests/e2e.rs b/tests/e2e-cucumber/tests/e2e.rs index ec3be17f1..5a318af77 100644 --- a/tests/e2e-cucumber/tests/e2e.rs +++ b/tests/e2e-cucumber/tests/e2e.rs @@ -21,10 +21,12 @@ mod e2e { pub mod automations_steps; pub mod bench_steps; pub mod chat_steps; + pub mod comfyui_steps; pub mod config_steps; pub mod dash_steps; pub mod dependency_guard_steps; pub mod diagnose_steps; + pub mod driver_steps; pub mod engines_steps; pub mod examine_steps; pub mod lifecycle_steps; @@ -86,6 +88,12 @@ pub struct E2eWorld { /// launch step knows to pass `--chat-mock` (deterministic offline agent, no /// endpoint detection) instead of driving the real detection/consent path. pub chat_use_mock: bool, + /// The managed runtime's torch distribution version captured BEFORE a ComfyUI + /// install (EAI-8051), so the post-install step can require it to be unchanged. + /// A ComfyUI install must not replace the runtime's torch at all; comparing the + /// exact version catches a swap to any other build (CUDA `+cu`, a plain + /// `2.13.0` CPU wheel, etc.), which a "not a CUDA build" check alone would miss. + pub comfyui_baseline_torch: Option, /// Per-scenario release-lifecycle state (packaging dirs, signing keys, install /// dir, captured logs). `Some` only for `@lifecycle` scenarios; all its paths /// are rooted in `isolated_root` so teardown removes them with the temp dir. @@ -195,6 +203,7 @@ impl Default for E2eWorld { expect_xfail: false, tui: None, chat_use_mock: false, + comfyui_baseline_torch: None, lifecycle: None, } } @@ -604,6 +613,65 @@ pub fn run_rocm_with_env( ) } +/// Run `rocm` with `PATH` pointed at a temp dir exposing only the given host tools. +/// +/// This lets a scenario prove how the CLI behaves when a tool it shells out to is +/// absent. Returns `(stdout, stderr, rc)` plus the `TempDir`, which the caller must +/// keep alive for the duration of the run. +/// +/// Used by the root-without-`sudo` driver-install scenario (EAI-8053): the driver +/// plan shells its commands through `sh -c`, so `sh` must stay reachable while +/// `sudo` must not. Each requested tool is symlinked from wherever it lives on the +/// current `PATH`; a tool that cannot be found is skipped (its absence is exactly +/// what some scenarios want to arrange). +/// +/// Compiles on every platform (cucumber step functions are registered regardless +/// of host), but the only scenario that uses it is `@requires-os:linux`, so its +/// Unix-only symlink path is the only one that runs; on Windows the temp dir is +/// created empty and the scenario is skipped before reaching this call. +pub fn run_rocm_with_only_tools( + world: &E2eWorld, + args: &[&str], + tools: &[&str], +) -> (String, String, i32, TempDir) { + let bin = TempDir::with_prefix("rocm-e2e-path-").expect("failed to create temp PATH dir"); + for tool in tools { + if let Some(real) = which_on_path(tool) { + #[cfg(unix)] + let _ = std::os::unix::fs::symlink(&real, bin.path().join(tool)); + #[cfg(not(unix))] + let _ = std::fs::copy(&real, bin.path().join(tool)); + } + } + let binary = rocm_binary(); + let mut cmd = std::process::Command::new(&binary); + cmd.args(args); + world.isolate_cmd(&mut cmd); + cmd.env("PATH", bin.path()); + let output = cmd + .output() + .unwrap_or_else(|e| panic!("failed to run {binary}: {e}")); + let rc = output.status.code().unwrap_or(-1); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + record_command(world.current_scenario.as_deref(), args, rc, &stdout); + ( + stdout, + String::from_utf8_lossy(&output.stderr).to_string(), + rc, + bin, + ) +} + +/// Resolve a bare tool name to its absolute path by scanning the current `PATH`, +/// following the same first-match rule a shell would. Returns `None` if the tool +/// is not on `PATH`. +fn which_on_path(tool: &str) -> Option { + let path = std::env::var_os("PATH")?; + std::env::split_paths(&path) + .map(|dir| dir.join(tool)) + .find(|candidate| candidate.is_file()) +} + /// Append one `rocm` invocation to `results/commands.jsonl` so the consolidated /// report can build a command × platform coverage table tied to real results. /// Best-effort: a recording failure must never fail a scenario. diff --git a/tests/e2e-cucumber/tests/e2e/comfyui_steps.rs b/tests/e2e-cucumber/tests/e2e/comfyui_steps.rs new file mode 100644 index 000000000..1c8ed6dbd --- /dev/null +++ b/tests/e2e-cucumber/tests/e2e/comfyui_steps.rs @@ -0,0 +1,244 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +use std::path::{Path, PathBuf}; + +use cucumber::{given, then, when}; + +use crate::E2eWorld; + +/// Report the installed torch distribution's version string via `importlib.metadata` +/// — WITHOUT importing torch. Emits JSON `{version}` (e.g. `2.7.0+rocm6.4` for a +/// ROCm build, `2.7.0+cu128` for a CUDA build) or `{error}` when no torch +/// distribution is installed. +/// +/// Deliberately does not `import torch`: importing it loads the ROCm/CUDA shared +/// libraries, which need the runtime's `LD_LIBRARY_PATH`/`ROCM_PATH` set up (the +/// product runs its own torch probe *with* that env, ours runs the interpreter +/// bare). The `+rocm` / `+cu` local-version label in the dist metadata is the +/// definitive ROCm-vs-CUDA discriminator and is readable with no native load — so +/// a bare interpreter suffices, and a runtime whose torch is present but whose +/// native libs aren't on our env no longer reads as "not a ROCm build". +const TORCH_DIST_PROBE: &str = "import json,sys\n\ + from importlib import metadata\n\ + out={}\n\ + try:\n\ + \x20 out['version']=metadata.version('torch')\n\ + except Exception as ex:\n\ + \x20 out['error']=type(ex).__name__+': '+str(ex)\n\ + sys.stdout.write(json.dumps(out))\n"; + +/// Locate the managed runtime's venv interpreter. `rocm runtimes list` prints an +/// `install_root: ` line for each installed runtime; the interpreter lives +/// under a `bin/python` (Unix) / `Scripts/python.exe` (Windows) inside that tree. +/// The exact env sub-layout is an internal detail, so search for the interpreter +/// rather than reconstruct the path — black-box, and tolerant of layout changes. +/// +/// Reads `runtimes list` rather than `examine`: examine only prints a `Folder:` +/// line for the *active* runtime and takes a different branch when none is marked +/// active, so it is not a reliable source for the install root (this cost a GPU +/// dispatch — the scenario panicked on a missing `Folder:` there). `runtimes list` +/// prints `install_root:` for every installed runtime unconditionally. +fn active_runtime_python(world: &E2eWorld) -> PathBuf { + let (listing, _, _) = crate::run_rocm(world, &["runtimes", "list"]); + let root = listing + .lines() + .find_map(|l| l.trim().strip_prefix("install_root:")) + .map_or_else( + || panic!("no 'install_root:' line in `runtimes list`:\n{listing}"), + str::trim, + ); + find_venv_python(Path::new(root)).unwrap_or_else(|| { + panic!("could not locate a venv python under the runtime install_root {root}") + }) +} + +/// Depth-limited search for a `bin/python` (Unix) or `Scripts/python.exe` +/// (Windows) under `root`. The managed runtime keeps its interpreter a few levels +/// down; cap the walk so a pathological tree can't hang the scenario. +fn find_venv_python(root: &Path) -> Option { + #[cfg(windows)] + let (bin, exe) = ("Scripts", "python.exe"); + #[cfg(not(windows))] + let (bin, exe) = ("bin", "python"); + let mut frontier = vec![(root.to_path_buf(), 0usize)]; + while let Some((dir, depth)) = frontier.pop() { + let candidate = dir.join(bin).join(exe); + if candidate.is_file() { + return Some(candidate); + } + if depth >= 6 { + continue; + } + let Ok(entries) = std::fs::read_dir(&dir) else { + continue; + }; + for entry in entries.flatten() { + if entry.file_type().is_ok_and(|t| t.is_dir()) { + frontier.push((entry.path(), depth + 1)); + } + } + } + None +} + +/// The installed torch distribution's version string, or `None` if no torch +/// distribution is installed. Reads dist metadata without importing torch (see +/// [`TORCH_DIST_PROBE`]), so it works against a bare interpreter. +/// +/// The local-version label identifies the build. A CUDA wheel is unmistakable: +/// `+cu` (e.g. `2.7.0+cu128`). A ROCm build is NOT reliably `+rocm`, though — +/// TheRock's managed torch labels the local version with a git hash +/// (`2.11.0+gitd0c8b1f`, measured on the MI300X lane), so callers judge "ROCm" as +/// "torch is present and is NOT a CUDA build", which is exactly the flip the +/// EAI-8051 corruption would cause. +fn torch_version(python: &Path) -> Option { + let output = std::process::Command::new(python) + .args(["-c", TORCH_DIST_PROBE]) + .output() + .unwrap_or_else(|e| panic!("failed to run runtime python {}: {e}", python.display())); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let data: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|_| { + panic!("torch version probe returned non-JSON:\nstdout: {stdout}\nstderr: {stderr}") + }); + data.get("version") + .and_then(serde_json::Value::as_str) + .map(str::to_owned) +} + +/// Whether `version` is a CUDA torch build (carries a `+cuNNN` local label). Used +/// only for the baseline PREMISE check — that the runtime didn't start out on a +/// CUDA torch. The post-install invariant is stronger: the torch version must be +/// byte-for-byte unchanged (a ComfyUI install must not replace the runtime's torch +/// at all), which also catches a swap to a plain non-`+cu` wheel (e.g. the observed +/// `2.11.0+gitd0c8b1f` → `2.13.0`) that this label check alone would miss. +fn is_cuda_torch(version: &str) -> bool { + version.to_ascii_lowercase().contains("+cu") +} + +/// Enumerate installed distributions via `importlib.metadata` and emit their names +/// as a JSON array. Used instead of `pip list` because uv-created managed runtimes +/// have no `pip` module — `python -m pip` there exits non-zero with empty stdout, +/// which a naive reader would misread as "no packages installed" and pass the +/// nvidia check while the runtime is actually corrupted. `importlib.metadata` is in +/// the stdlib, so it is always present; the probe emits `{names}` on success or +/// `{error}` on failure so the caller can fail loudly rather than treat a broken +/// probe as a clean result. +const DISTRIBUTIONS_PROBE: &str = "import json,sys\n\ + out={}\n\ + try:\n\ + \x20 from importlib import metadata\n\ + \x20 out['names']=sorted({(d.metadata['Name'] or '') for d in metadata.distributions()})\n\ + except Exception as ex:\n\ + \x20 out['error']=type(ex).__name__+': '+str(ex)\n\ + sys.stdout.write(json.dumps(out))\n"; + +/// The `nvidia-*` CUDA distributions installed in the interpreter's environment. +/// A ROCm runtime should have none; ComfyUI's install dragging any in is the +/// EAI-8051 defect. Panics if the interpreter cannot be run or the probe reports +/// an error — a probe that cannot enumerate packages must NOT read as "no nvidia +/// packages", which would pass the contract on a runtime it never actually checked. +fn nvidia_distributions(python: &Path) -> Vec { + let output = std::process::Command::new(python) + .args(["-c", DISTRIBUTIONS_PROBE]) + .output() + .unwrap_or_else(|e| panic!("failed to run runtime python {}: {e}", python.display())); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let data: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|_| { + panic!("distributions probe returned non-JSON:\nstdout: {stdout}\nstderr: {stderr}") + }); + if let Some(error) = data.get("error").and_then(serde_json::Value::as_str) { + panic!( + "could not enumerate installed distributions on {}: {error}", + python.display() + ); + } + let names = data + .get("names") + .and_then(serde_json::Value::as_array) + .unwrap_or_else(|| panic!("distributions probe returned no 'names' array:\n{stdout}")); + names + .iter() + .filter_map(serde_json::Value::as_str) + .filter(|name| name.to_ascii_lowercase().starts_with("nvidia-")) + .map(str::to_owned) + .collect() +} + +#[given("an isolated machine with a managed ROCm runtime")] +async fn setup_isolated_runtime(world: &mut E2eWorld) { + // DELIBERATELY do NOT call `world.use_shared_runtimes()`: this scenario may + // corrupt the runtime (that is the bug it pins), so it must own a private, + // throwaway runtime prefix. Each World already has isolated ROCM_CLI_* dirs, + // so a plain `install sdk` here lands in this scenario's own tree. + let (stdout, _, _) = crate::run_rocm(world, &["runtimes", "list"]); + if stdout.contains("installed: none") { + crate::run_rocm_ok(world, &["install", "sdk"]); + } + let (stdout, _, _) = crate::run_rocm(world, &["runtimes", "list"]); + assert!( + !stdout.contains("installed: none"), + "no managed runtime is active after install:\n{stdout}" + ); +} + +#[given("the runtime's torch is a ROCm build")] +async fn assert_baseline_rocm_torch(world: &mut E2eWorld) { + let python = active_runtime_python(world); + let version = torch_version(&python); + assert!( + version.as_deref().is_some_and(|v| !is_cuda_torch(v)), + "baseline runtime torch is absent or already a CUDA build; scenario premise absent \ + (torch version: {version:?}, python: {})", + python.display() + ); + assert!( + nvidia_distributions(&python).is_empty(), + "runtime already has nvidia-* distributions before ComfyUI install; premise absent" + ); + // Record the exact baseline version so the post-install step can require it to + // be unchanged (see `assert_torch_still_rocm`). + world.comfyui_baseline_torch = version; +} + +#[when("the user installs ComfyUI")] +async fn user_installs_comfyui(world: &mut E2eWorld) { + // Do NOT use run_rocm_ok: the real install exits non-zero while still leaving + // the runtime damaged, so the exit code is not the contract (see the feature + // comment). Capture the outcome for diagnostics only. + let (stdout, stderr, rc) = crate::run_rocm(world, &["comfyui", "install"]); + world.cli_output = Some(stdout); + world.cli_stderr = Some(stderr); + world.cli_rc = Some(rc); +} + +#[then("the runtime's torch is still a ROCm build")] +async fn assert_torch_still_rocm(world: &mut E2eWorld) { + let python = active_runtime_python(world); + let version = torch_version(&python); + let baseline = world + .comfyui_baseline_torch + .as_deref() + .expect("no baseline torch version was captured"); + assert_eq!( + version.as_deref(), + Some(baseline), + "ComfyUI install replaced the managed runtime's torch \ + (before: {baseline}, after: {version:?}, python: {})", + python.display() + ); +} + +#[then("no CUDA nvidia packages were added to the runtime")] +async fn assert_no_nvidia_packages(world: &mut E2eWorld) { + let python = active_runtime_python(world); + let nvidia = nvidia_distributions(&python); + assert!( + nvidia.is_empty(), + "ComfyUI install added CUDA nvidia-* distributions to the ROCm runtime: {}", + nvidia.join(", ") + ); +} diff --git a/tests/e2e-cucumber/tests/e2e/driver_steps.rs b/tests/e2e-cucumber/tests/e2e/driver_steps.rs new file mode 100644 index 000000000..458a11aa9 --- /dev/null +++ b/tests/e2e-cucumber/tests/e2e/driver_steps.rs @@ -0,0 +1,54 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +use cucumber::{given, then, when}; + +use crate::E2eWorld; + +/// The tools the driver plan legitimately shells out to on the DKMS path, minus +/// `sudo`. The scenario runs `rocm` with a `PATH` exposing only these, so the +/// plan can still execute its commands via `sh -c` while `sudo` is genuinely +/// absent — the exact state EAI-8053 reproduces. `sudo` is deliberately excluded; +/// `apt-get`/`curl` etc. are omitted too, so a fixed (uid-aware) CLI fails at a +/// real package step rather than at `sudo`, which is precisely the distinction the +/// contract asserts. +const PLAN_TOOLS_WITHOUT_SUDO: &[&str] = &["sh", "env"]; + +#[given("a root machine with no sudo command available")] +async fn setup_root_no_sudo(_world: &mut E2eWorld) { + // Applicability is enforced by the scenario's @requires-root tag (the runner + // must be root) and by the sanitized PATH the When step builds (no `sudo`); + // nothing to arrange on the World here. +} + +#[when("the user installs the native driver with dkms")] +async fn user_installs_driver(world: &mut E2eWorld) { + let (stdout, stderr, rc, _path) = crate::run_rocm_with_only_tools( + world, + &["install", "driver", "--dkms", "--yes"], + PLAN_TOOLS_WITHOUT_SUDO, + ); + world.cli_output = Some(stdout); + world.cli_stderr = Some(stderr); + world.cli_rc = Some(rc); +} + +#[then("the install does not fail merely because sudo is missing")] +async fn assert_not_broken_by_missing_sudo(world: &mut E2eWorld) { + let stdout = world.cli_output.as_deref().unwrap_or(""); + let stderr = world.cli_stderr.as_deref().unwrap_or(""); + let combined = format!("{stdout}\n{stderr}"); + // The bug's signature: the plan prefixed `sudo` even though the process is + // root, so `sh -c "sudo …"` dies with `sudo: not found` and the CLI reports + // the sudo-prefixed command as the failure. A root-aware plan would run the + // command without `sudo` and get to the actual package work. We assert the + // ABSENCE of the sudo-not-found failure, not a successful install (impossible + // in a container), so the row goes stale the day the prefix becomes uid-aware. + assert!( + !combined.contains("sudo: not found") + && !combined.contains("sudo: command not found") + && !combined.contains("driver command failed: sudo "), + "driver install as root failed because it invoked a missing `sudo`:\n{combined}" + ); +}