test(e2e): show stderr when a step fails on a CLI exit code - #248
Conversation
CI status, with evidence
A varying failure set is a flaky lane, not a deterministic break — and Running the full mock suite locally on this exact commit (6708e74) is clean: The 3 failures are the expected xfails. I'll also note the diff cannot reach either scenario: both are driven entirely by
The GPU lane demonstrates what this PR is forThat previously-undiagnosable failure now reads: Root cause in one line, on the first run: the Lemonade embeddable 11.5.1 binary Note |
Steps that assert on rocm's exit code printed only stdout, and rocm reports its failures on stderr. A failed serve therefore panicked with "rocm serve failed:" followed by nothing at all, which is how the MI300X lane ended up with a red scenario nobody can diagnose from CI (EAI-8031). run_rocm returns stderr already -- the five assertion sites just bound it to `_`. Replace them with a run_rocm_ok helper that panics through a shared cli_failure_report: invocation, exit code, and both streams, each labelled, each marked "(empty)" rather than omitted. The distinction matters -- "the CLI said nothing" and "the harness dropped the output" are different diagnoses, and telling them apart is the whole point. Two serve steps defer their rc assertion to a later Then step, so they now carry stderr in the world alongside the rc they already store. The formatter lives in the library, where `cargo test -p e2e-cucumber --lib` covers it in CI; the harness target's own tests never run. Signed-off-by: Roman Inflianskas <Roman.Inflianskas@amd.com>
6708e74 to
0b8e0f3
Compare
Summary
E2E steps that assert on
rocm's exit code printed only stdout — androcmreports its failures on stderr. So a failed serve panicked with:
…and nothing after the colon. That is the real state of the MI300X lane today:
a red scenario that cannot be diagnosed from CI logs at all (#247).
run_rocmhas returned(stdout, stderr, rc)all along — the five assertionsites simply bound stderr to
_:They now go through a
run_rocm_okhelper that panics with a sharedcli_failure_report: the invocation, the exit code, and both streams, eachlabelled and each marked
(empty)rather than omitted. That last detail is thepoint — "the CLI said nothing" and "the harness dropped the output" are
different diagnoses, and the current message can't tell them apart.
The same failure now reads:
Two serve steps defer their
rcassertion to a laterThenstep(
assert_vllm_default), so they now carry stderr in the world next to thercthey already store —
cli_stderris an existingE2eWorldfield that severalother steps already populate, so this follows the established pattern rather
than inventing one.
Why the formatter lives in the library
tests/e2e-cucumber/src/lib.rs, not the harness target. Thee2etarget setstest = false, so unit tests written there would never run; the library iscovered by the existing
cargo test -p e2e-cucumber --libjob. Putting the pureformatting logic there is what makes it testable at all.
Scope
Only the sites that panic on a CLI failure. Steps that deliberately record
rcfor a later content assertion (diagnose,examine) are untouched — theyaren't failing on the exit code, and their
Thensteps have their ownstderr-capturing
Whencounterparts.This does not fix the MI300X or Strix Halo Windows failures in #247. It is the
prerequisite: right now there is no way to see why the MI300X serve exits
non-zero. I don't have the hardware to reproduce either failure, so this is
deliberately the part that can be done and verified without it.
Risk: low — test-harness only, no product code, no behavior change to any
passing scenario.
Test plan
Three unit tests on
cli_failure_report, the first pinning the exact regression(empty stdout, populated stderr → the reason survives into the panic):
Also run locally:
cargo clippy --locked --workspace --all-targets -- -D warnings,cargo clippy --locked -p e2e-cucumber --test e2e -- -D warnings(the harness isexcluded from the first, so both are needed), and
prek run --all-files --no-group local-tools.Being straight about one thing: these tests cover a new function, so there is no
"fails before, passes after" — the old code path had nothing to assert on. The
before/after evidence is the message shape shown above. The end-to-end proof is
the next MI300X failure actually printing a reason, which needs a self-hosted run.
Closes nothing on its own; unblocks #247.
tests/e2e-cucumber/expectations.tomlxfail rows to narrow.