[rominf]

Follow-up to #659, covering juhovainio's two non-blocking notes there.

• Panic messages: when events_for_whole_body moved onto collect_events, its two expectations ("fixture lines parse" and "fixture stream finishes") became one message. Each step has its own message again, so a failure says whether the fixture failed to parse or failed to finish.
• Repeated shape: the two truncated-stream loopback tests each waited on the client within TRUNCATED_STREAM_BOUND, then on the server only if the client returned. That is now one client_then_server helper.

Checked by mutation:

• a client that hangs still fails within one bound;
• a smoke test that never reaches the server still fails, through the helper's server wait.

Test-only change: no user-observable CLI behaviour changes, so no Gherkin scenario is needed (AGENTS.md §3).

[jussielo-amd]

Full review of the whole change.
This is a follow-up PR (not tied to a tracker issue) that implements both non-blocking notes juhovainio left on predecessor PR #659 — confirmed against that PR's actual review body, which names exactly: "a slightly loosened panic-message distinction in a test helper" and "a small duplicated 3-line shape between the two edited test functions."

Blocking

None

Non-blocking

None

Decisions for the author

None

Positive signals

• client_then_server's doc comment states the reason for the conditional server wait ("a client that never returned already fails the test... so this does not spend a second bound waiting on a server it may never have reached") rather than restating what the code does.
• Splitting events_for_whole_body's single .expect("fixture stream parses and finishes") back into two distinct .expect() calls ("fixture lines parse" vs. "fixture stream finishes") is a real, verifiable improvement: a panic now says which of the two phases failed, where before both phases collapsed into one ambiguous message.

Deployment notes

None — test-only change, no shipped behavior affected.

What this covered

Both changed files in full (apps/rocm/src/providers.rs's mod tests, apps/rocm/src/providers/proptests.rs), plus the call sites and surrounding context: collect_events's closure signature, served_request_within_bound, TRUNCATED_STREAM_BOUND, and both client_then_server call sites (confirmed the generic T resolves correctly at both: the chunked-stream test's (Result<String,String>, Vec<ProviderStreamEvent>) tuple, and the smoke-test's SmokeMetrics). Reviewed against 4629d531…15024345.

Verified directly, not just read:

• Built an isolated worktree off the PR head and ran cargo test -p rocm providers:: — all 39 tests pass, including the two refactored tests and every proptests::* test that exercises events_for_whole_body through the fixture list.
• Ran cargo clippy -p rocm --all-targets in that worktree — clean.
• Read CI via gh pr checks 685 — every required check passes; only E2E tests (MI300X) is still pending, unrelated to this change's scope.
• Traced collect_events's closure contract against the new events_for_whole_body body to confirm the .expect() split still produces a Result<Vec<ProviderStreamEvent>> with finish()'s error reaching the outer .expect, exactly as the panic-message intent requires.
• Confirmed the description's claim against predecessor PR #659's actual, retrieved review body rather than trusting the PR text.

Did not run: mutation testing claimed in the PR description was not independently re-run; verified instead by reading the control flow (outcome.is_ok().then(...), served.transpose()) against both failure paths, which checks out.