From b4485bd3b6729a4177d95101e95fc71d13075929 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 5 Aug 2026 11:55:37 +0000 Subject: [PATCH 1/6] test(e2e): index scenarios per feature and qualify scenario ids Every feature file numbered its scenarios from 1, so the index identified nothing: '1', '2', '3' each appeared eight times across the suite. Some files had drifted further - examine numbered 1, 2, 5, 3, 4; model_serving had a '6b' and a '10' sitting in sixth place; install_lifecycle had no indexes at all. Renumber every scenario as - in declaration order, so an index names exactly one scenario suite-wide. Qualify the four scenario ids that did not carry their feature's key (help-* in examine, fix-* in diagnose) and rename the matching expectations.toml key. Record each scenario's feature and name in platform.json alongside its resolved expectation. A skipped scenario never reaches report.json, so this is the only place that identity survives - the report needs it to group the expectation grid by feature. Signed-off-by: Roman Inflianskas --- tests/e2e-cucumber/expectations.toml | 2 +- tests/e2e-cucumber/features/chat.feature | 14 +++--- tests/e2e-cucumber/features/dash.feature | 14 +++--- tests/e2e-cucumber/features/diagnose.feature | 18 +++---- tests/e2e-cucumber/features/examine.feature | 12 ++--- .../features/install_lifecycle.feature | 44 ++++++++--------- .../features/model_serving.feature | 28 +++++------ .../e2e-cucumber/features/networking.feature | 4 +- .../features/runtime_setup.feature | 6 +-- tests/e2e-cucumber/src/expectation.rs | 20 +++++++- tests/e2e-cucumber/tests/e2e.rs | 47 +++++++++++++++---- 11 files changed, 127 insertions(+), 82 deletions(-) diff --git a/tests/e2e-cucumber/expectations.toml b/tests/e2e-cucumber/expectations.toml index 0316bf62..4c298092 100644 --- a/tests/e2e-cucumber/expectations.toml +++ b/tests/e2e-cucumber/expectations.toml @@ -158,7 +158,7 @@ flaky = true # --- EAI-7383: `rocm help` lists subcommands in declaration order, not # alphabetically. Platform-independent (pure CLI help output). --- -[["help-lists-subcommands-alphabetically"]] +[["examine-help-lists-subcommands-alphabetically"]] when = {} bug = "EAI-7383" reason = "rocm help lists subcommands in declaration order, not alphabetically." diff --git a/tests/e2e-cucumber/features/chat.feature b/tests/e2e-cucumber/features/chat.feature index d0cc2609..50c96888 100644 --- a/tests/e2e-cucumber/features/chat.feature +++ b/tests/e2e-cucumber/features/chat.feature @@ -1,7 +1,7 @@ Feature: Chat and endpoint detection @id:chat-served-model-discoverable - Scenario: 1 - A served model is discoverable through the services list + Scenario: chat-01 - A served model is discoverable through the services list Given a model is being served And the model is registered with the CLI When the user checks for running services @@ -13,7 +13,7 @@ Feature: Chat and endpoint detection # offers it — the notice must precede any request. (Previously an # untestable-black-box gap.) @id:chat-privacy-notice-accurate @requires-os:linux - Scenario: 2 - The privacy notice is shown before using a local endpoint + Scenario: chat-02 - The privacy notice is shown before using a local endpoint Given a model is being served locally And the model is registered with the CLI When the user opens interactive chat @@ -23,7 +23,7 @@ Feature: Chat and endpoint detection Then interactive chat exits successfully @id:chat-managed-model-interactive @requires-os:linux - Scenario: 3 - Interactive chat uses a running managed model + Scenario: chat-03 - Interactive chat uses a running managed model Given a running managed model is available locally When the user opens interactive chat Then the local endpoint is shown for confirmation @@ -35,7 +35,7 @@ Feature: Chat and endpoint detection Then interactive chat exits successfully @id:chat-endpoint-shown-in-services - Scenario: 4 - A served model's endpoint is shown in the services list + Scenario: chat-04 - A served model's endpoint is shown in the services list Given a model is being served And the model is registered with the CLI When the user lists running services @@ -46,7 +46,7 @@ Feature: Chat and endpoint detection # assertion (a tools-bearing request is accepted) is engine-agnostic, so no GPU # is required — dropping @requires-gpu gives this per-PR mock-lane coverage. @id:chat-tool-definitions-accepted - Scenario: 5 - Chat requests that include tool definitions are accepted + Scenario: chat-05 - Chat requests that include tool definitions are accepted Given a managed runtime is active And a model is served in the background When a chat request with tool definitions is sent @@ -57,7 +57,7 @@ Feature: Chat and endpoint detection # reply, which is engine-agnostic — real generation is covered by the # @requires-gpu serve-*-inference scenarios. @id:chat-end-to-end-local-model - Scenario: 6 - End-to-end chat through a locally served model + Scenario: chat-06 - End-to-end chat through a locally served model Given a managed runtime is active And a model is served in the background And the served model has been detected @@ -70,7 +70,7 @@ Feature: Chat and endpoint detection # reports `rocm chat` as covered. Runs on mock (no GPU): the local provider # resolves the planted managed-service record and talks to the mock server. @id:chat-cli-oneshot-prompt - Scenario: 7 - The chat CLI answers a one-shot prompt against a local server + Scenario: chat-07 - The chat CLI answers a one-shot prompt against a local server Given a model is being served And the model is registered with the CLI When the user sends a one-shot chat prompt through the CLI diff --git a/tests/e2e-cucumber/features/dash.feature b/tests/e2e-cucumber/features/dash.feature index e9319ab0..604eb40c 100644 --- a/tests/e2e-cucumber/features/dash.feature +++ b/tests/e2e-cucumber/features/dash.feature @@ -6,7 +6,7 @@ Feature: Interactive dashboard # not yet promoted to a blocking contract (tracked as a follow-up). @id:dash-opens-and-navigates @requires-os:linux - Scenario: 1 - A user opens the dashboard and navigates to ROCm setup + Scenario: dash-01 - A user opens the dashboard and navigates to ROCm setup When the user opens the dashboard with demo data Then the dashboard home view is displayed When the user opens the ROCm view @@ -15,7 +15,7 @@ Feature: Interactive dashboard Then the dashboard exits successfully @id:dash-chat-offline-reply @requires-os:linux - Scenario: 2 - A user receives a response in interactive chat + Scenario: dash-02 - A user receives a response in interactive chat Given interactive chat uses an offline assistant When the user opens interactive chat And the user sends a message about GPU health @@ -24,7 +24,7 @@ Feature: Interactive dashboard Then interactive chat exits successfully @id:dash-loading-service-status @requires-os:linux - Scenario: 3 - The dashboard reports a model that is still loading as loading + Scenario: dash-03 - The dashboard reports a model that is still loading as loading Given a managed model is still loading When the user opens the dashboard And the user opens the Observe view @@ -33,7 +33,7 @@ Feature: Interactive dashboard Then the dashboard exits successfully @id:dash-managed-service-metrics @requires-os:linux - Scenario: 4 - Observe displays metrics from a managed model + Scenario: dash-04 - Observe displays metrics from a managed model Given a managed model exposes serving metrics When the user opens the dashboard And the user opens the Observe view @@ -42,7 +42,7 @@ Feature: Interactive dashboard Then the dashboard exits successfully @id:dash-help-guidance @requires-os:linux - Scenario: 5 - A user can discover dashboard help and next-step guidance + Scenario: dash-05 - A user can discover dashboard help and next-step guidance When the user opens the dashboard with demo data And the user opens dashboard help Then navigation and next-step guidance are displayed @@ -51,7 +51,7 @@ Feature: Interactive dashboard Then the dashboard exits successfully @id:dash-command-palette-navigation @requires-os:linux - Scenario: 6 - A user navigates to Serving through the command palette + Scenario: dash-06 - A user navigates to Serving through the command palette When the user opens the dashboard with demo data And the user opens the command palette Then dashboard destinations are displayed @@ -61,7 +61,7 @@ Feature: Interactive dashboard Then the dashboard exits successfully @id:dash-managed-service-visible @requires-os:linux - Scenario: 7 - A managed model is visible in the dashboard + Scenario: dash-07 - A managed model is visible in the dashboard Given a running managed model is available locally When the user opens the dashboard And the user opens the Observe view diff --git a/tests/e2e-cucumber/features/diagnose.feature b/tests/e2e-cucumber/features/diagnose.feature index db4db1af..08528ab7 100644 --- a/tests/e2e-cucumber/features/diagnose.feature +++ b/tests/e2e-cucumber/features/diagnose.feature @@ -11,38 +11,38 @@ Feature: Diagnosing failures and listing fixes # and a plan) and the query/refusal contracts. @id:diagnose-matches-known-symptom - Scenario: 1 - Diagnosing a recognised failure reports a likely cause and a fix + Scenario: diagnose-01 - Diagnosing a recognised failure reports a likely cause and a fix Given a user who hit a known ROCm failure When the user asks the CLI to diagnose that symptom Then the CLI reports a likely cause with a suggested fix @id:diagnose-always-offers-a-way-forward - Scenario: 2 - Diagnosing any failure always gives the user a way to escalate + Scenario: diagnose-02 - Diagnosing any failure always gives the user a way to escalate Given a user who hit a failure the CLI does not recognise When the user asks the CLI to diagnose that symptom in machine-readable form Then the CLI always points to somewhere the problem can be reported @id:diagnose-json-has-match-flag - Scenario: 3 - A diagnosis is available in machine-readable form for tooling + Scenario: diagnose-03 - A diagnosis is available in machine-readable form for tooling Given a user who hit a known ROCm failure When the user asks the CLI to diagnose that symptom in machine-readable form Then the result is machine-readable and identifies the matched cause - @id:fix-lists-known-recipes - Scenario: 4 - The user can see every fix the CLI knows how to apply + @id:diagnose-fix-lists-known-recipes + Scenario: diagnose-04 - The user can see every fix the CLI knows how to apply When the user asks the CLI which fixes it offers Then the CLI lists the fixes it can apply And each fix indicates whether the CLI can apply it automatically - @id:fix-dry-run-changes-nothing - Scenario: 5 - Previewing a fix explains the change without making it + @id:diagnose-fix-dry-run-changes-nothing + Scenario: diagnose-05 - Previewing a fix explains the change without making it Given a user who has chosen a known fix When the user previews that fix without applying it Then the CLI describes what the fix would change And nothing on the machine is changed - @id:fix-unknown-id-rejected - Scenario: 6 - Asking for a fix the CLI does not know is refused clearly + @id:diagnose-fix-unknown-id-rejected + Scenario: diagnose-06 - Asking for a fix the CLI does not know is refused clearly Given a user who names a fix the CLI does not offer When the user asks the CLI to apply that fix Then the CLI refuses and explains that the fix is not recognised diff --git a/tests/e2e-cucumber/features/examine.feature b/tests/e2e-cucumber/features/examine.feature index aca649e5..008f5b1f 100644 --- a/tests/e2e-cucumber/features/examine.feature +++ b/tests/e2e-cucumber/features/examine.feature @@ -1,32 +1,32 @@ Feature: GPU detection and system inspection @id:examine-version - Scenario: 1 - The CLI reports its version + Scenario: examine-01 - The CLI reports its version When the user asks for the version Then a version string is returned @id:examine-engines-list - Scenario: 2 - The CLI lists all supported engines + Scenario: examine-02 - The CLI lists all supported engines When the user lists available engines Then all supported engines are listed # Dogfooding #24: the `rocm help` subcommand list is in declaration order, not # alphabetical, which makes it harder to scan. Expected to FAIL until fixed — # surfaces the bug so it can be ticketed. - @id:help-lists-subcommands-alphabetically - Scenario: 5 - The help output lists subcommands in alphabetical order + @id:examine-help-lists-subcommands-alphabetically + Scenario: examine-03 - The help output lists subcommands in alphabetical order When the user asks for help Then the subcommands are listed in alphabetical order @id:examine-detects-gpu-and-driver @requires-gpu - Scenario: 3 - System inspection detects the GPU and driver + Scenario: examine-04 - System inspection detects the GPU and driver Given a machine with an AMD GPU When the user inspects the system Then the inspection reports which GPU is installed And the inspection reports that the driver is available @id:examine-distinguishes-unmanaged-rocm @requires-gpu - Scenario: 4 - System inspection distinguishes CLI-managed from pre-existing ROCm + Scenario: examine-05 - System inspection distinguishes CLI-managed from pre-existing ROCm Given a machine with a ROCm install that was not set up by the CLI When the user inspects the system Then the inspection reports the install as pre-existing diff --git a/tests/e2e-cucumber/features/install_lifecycle.feature b/tests/e2e-cucumber/features/install_lifecycle.feature index d5262710..f28a4cda 100644 --- a/tests/e2e-cucumber/features/install_lifecycle.feature +++ b/tests/e2e-cucumber/features/install_lifecycle.feature @@ -19,7 +19,7 @@ Feature: Release install lifecycle # ── Packaging + signature-verified install (Linux) ──────────────────── @id:lifecycle-linux-install-signed-keypath @lifecycle @requires-os:linux - Scenario: Linux - a bundle signed with a key file installs, verifies, and sets up the shell profile + Scenario: lifecycle-01 - Linux - a bundle signed with a key file installs, verifies, and sets up the shell profile Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -32,7 +32,7 @@ Feature: Release install lifecycle And the shell profile lists the install directory @id:lifecycle-linux-install-signed-pem @lifecycle @requires-os:linux - Scenario: Linux - a bundle signed with an inline PEM installs and verifies + Scenario: lifecycle-02 - Linux - a bundle signed with an inline PEM installs and verifies Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key PEM @@ -44,7 +44,7 @@ Feature: Release install lifecycle # ── Trust rejection: untrusted key, bad checksum, bad/missing signature ─ @id:lifecycle-linux-reject-untrusted-key @lifecycle @requires-os:linux - Scenario: Linux - an install with no public key falls back to the pinned trust root and rejects an untrusted signer + Scenario: lifecycle-03 - Linux - an install with no public key falls back to the pinned trust root and rejects an untrusted signer Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -53,7 +53,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-linux-reject-bad-checksum @lifecycle @requires-os:linux - Scenario: Linux - a tampered checksum is rejected before activation + Scenario: lifecycle-04 - Linux - a tampered checksum is rejected before activation Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -63,7 +63,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-linux-reject-bad-signature @lifecycle @requires-os:linux - Scenario: Linux - a tampered signature is rejected before activation + Scenario: lifecycle-05 - Linux - a tampered signature is rejected before activation Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -73,7 +73,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-linux-reject-missing-signature @lifecycle @requires-os:linux - Scenario: Linux - a missing signature is rejected when a signature is required + Scenario: lifecycle-06 - Linux - a missing signature is rejected when a signature is required Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -85,7 +85,7 @@ Feature: Release install lifecycle # ── Reinstall: stale-manifest purge, config preservation, PATH idempotency ─ @id:lifecycle-linux-reinstall-purges-stale @lifecycle @requires-os:linux - Scenario: Linux - reinstalling purges a stale prior entry and preserves config + Scenario: lifecycle-07 - Linux - reinstalling purges a stale prior entry and preserves config Given a freshly built release tree And a generated signing keypair And a signed bundle installed with the shell profile updated @@ -101,7 +101,7 @@ Feature: Release install lifecycle # ── Installed-binary PTY, shell-profile / XDG, and uninstall ──────────── @id:lifecycle-linux-installed-binary-pty @lifecycle @requires-os:linux - Scenario: Linux - the installed binary opens and exits interactive chat through a pseudo-terminal + Scenario: lifecycle-08 - Linux - the installed binary opens and exits interactive chat through a pseudo-terminal Given a freshly built release tree And a generated signing keypair And a signed bundle installed with the public key file @@ -113,7 +113,7 @@ Feature: Release install lifecycle And the installed config still selects the vllm default engine @id:lifecycle-linux-uninstall-full-purge @lifecycle @requires-os:linux - Scenario: Linux - uninstall removes binaries, manifest, and XDG state + Scenario: lifecycle-09 - Linux - uninstall removes binaries, manifest, and XDG state Given a freshly built release tree And a generated signing keypair And a signed bundle installed with the shell profile updated @@ -126,7 +126,7 @@ Feature: Release install lifecycle # ── Windows user-PATH restoration, loopback HTTP install, isolated smoke ─ @id:lifecycle-windows-install-signed @lifecycle @requires-os:windows - Scenario: Windows - a signed zip installs and verifies with native crypto + Scenario: lifecycle-10 - Windows - a signed zip installs and verifies with native crypto Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -136,7 +136,7 @@ Feature: Release install lifecycle And the install manifest is present @id:lifecycle-windows-key-rotation-fallback @lifecycle @requires-os:windows - Scenario: Windows - a malformed current trust root falls through to the valid next key + Scenario: lifecycle-11 - Windows - a malformed current trust root falls through to the valid next key Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -146,7 +146,7 @@ Feature: Release install lifecycle And the installed rocm binary is present @id:lifecycle-windows-all-pinned-keys-malformed @lifecycle @requires-os:windows - Scenario: Windows - all malformed pinned trust roots fail deterministically + Scenario: lifecycle-12 - Windows - all malformed pinned trust roots fail deterministically Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -156,7 +156,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-windows-updates-user-path @lifecycle @requires-os:windows - Scenario: Windows - a default install updates the user PATH and restores it afterwards + Scenario: lifecycle-13 - Windows - a default install updates the user PATH and restores it afterwards Given a freshly built release tree And a generated signing keypair And the user PATH is captured for restoration @@ -167,7 +167,7 @@ Feature: Release install lifecycle And the install directory is on the user PATH @id:lifecycle-windows-verifies-without-openssl @lifecycle @requires-os:windows - Scenario: Windows - the installer verifies with native crypto when openssl is absent from PATH + Scenario: lifecycle-14 - Windows - the installer verifies with native crypto when openssl is absent from PATH Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -177,7 +177,7 @@ Feature: Release install lifecycle And the install manifest is present @id:lifecycle-windows-rejects-bad-signature-without-openssl @lifecycle @requires-os:windows - Scenario: Windows - a bad signature is rejected with native crypto when openssl is absent from PATH + Scenario: lifecycle-15 - Windows - a bad signature is rejected with native crypto when openssl is absent from PATH Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -187,7 +187,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-windows-reinstall-purges-stale @lifecycle @requires-os:windows - Scenario: Windows - reinstalling purges a stale prior entry + Scenario: lifecycle-16 - Windows - reinstalling purges a stale prior entry Given a freshly built release tree And a generated signing keypair And a signed bundle installed with the public key file @@ -198,7 +198,7 @@ Feature: Release install lifecycle And the install manifest is present @id:lifecycle-windows-reject-untrusted-key @lifecycle @requires-os:windows - Scenario: Windows - an install with no public key rejects an untrusted signer + Scenario: lifecycle-17 - Windows - an install with no public key rejects an untrusted signer Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -207,7 +207,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-windows-reject-bad-checksum @lifecycle @requires-os:windows - Scenario: Windows - a tampered checksum is rejected before activation + Scenario: lifecycle-18 - Windows - a tampered checksum is rejected before activation Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -217,7 +217,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-windows-reject-bad-signature @lifecycle @requires-os:windows - Scenario: Windows - a tampered signature is rejected before activation + Scenario: lifecycle-19 - Windows - a tampered signature is rejected before activation Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -227,7 +227,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-windows-reject-missing-signature @lifecycle @requires-os:windows - Scenario: Windows - a missing signature is rejected when a signature is required + Scenario: lifecycle-20 - Windows - a missing signature is rejected when a signature is required Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -237,7 +237,7 @@ Feature: Release install lifecycle And no binaries are activated in the target directory @id:lifecycle-windows-http-install @lifecycle @requires-os:windows - Scenario: Windows - a signed bundle installs over a loopback HTTP download + Scenario: lifecycle-21 - Windows - a signed bundle installs over a loopback HTTP download Given a freshly built release tree And a generated signing keypair When the release is packaged and signed with the private key file @@ -247,7 +247,7 @@ Feature: Release install lifecycle And the install manifest is present @id:lifecycle-windows-installed-binary-smoke @lifecycle @requires-os:windows - Scenario: Windows - the installed binary honors isolated directories and keeps the running executable on uninstall + Scenario: lifecycle-22 - Windows - the installed binary honors isolated directories and keeps the running executable on uninstall Given a freshly built release tree And a generated signing keypair And a signed bundle installed with the public key file diff --git a/tests/e2e-cucumber/features/model_serving.feature b/tests/e2e-cucumber/features/model_serving.feature index 27fe87f3..03ae9868 100644 --- a/tests/e2e-cucumber/features/model_serving.feature +++ b/tests/e2e-cucumber/features/model_serving.feature @@ -6,24 +6,24 @@ Feature: Model serving # so the harness can skip a scenario whose engine can't start on this host. @id:serve-short-name-expansion - Scenario: 1 - Short model names are expanded to their full name + Scenario: serve-01 - Short model names are expanded to their full name When the user serves a model using its short name Then the output shows the full model name @id:serve-short-name-consistent-across-engines - Scenario: 2 - Short name expansion is consistent across engines + Scenario: serve-02 - Short name expansion is consistent across engines When the user serves the same short name with different engines Then all engines expand to the same full model name @id:serve-discoverable-by-name - Scenario: 3 - A running model server is discoverable by name + Scenario: serve-03 - A running model server is discoverable by name Given a model is being served on the default port And the model is registered with the CLI When the user lists running services Then the service appears with the correct model name and connection details @id:serve-connection-details - Scenario: 4 - Running services show the correct connection details + Scenario: serve-04 - Running services show the correct connection details Given a model is being served on a non-default port And the model is registered with the CLI When the user lists running services @@ -33,7 +33,7 @@ Feature: Model serving # deliberate vLLM half of a per-engine pair with `serve-lemonade-inference` # below, so it stays pinned to vLLM (the slug names the engine). @id:serve-vllm-inference @requires-gpu @requires-engine:vllm - Scenario: 5 - A served model responds to inference requests on vLLM + Scenario: serve-05 - A served model responds to inference requests on vLLM Given a managed runtime is active And a model is being served on GPU When the user sends a chat completion request @@ -47,7 +47,7 @@ Feature: Model serving # loads stay off the ordinary per-PR path, and the longer readiness timeout also # gives the first inference request enough time to complete. @id:serve-large-model-inference @requires-gpu @serve-timeout:2400 @nightly - Scenario: 10 - A large platform-specific model serves and responds to inference + Scenario: serve-06 - A large platform-specific model serves and responds to inference Given a managed runtime is active And a large model is being served on GPU When the user sends a chat completion request @@ -56,7 +56,7 @@ Feature: Model serving # Lemonade serve + inference (GGUF model). Engine coverage: Lemonade. @id:serve-lemonade-inference @requires-gpu @requires-engine:lemonade - Scenario: 7 - A model served on lemonade responds to inference requests + Scenario: serve-07 - A model served on lemonade responds to inference requests Given a managed runtime is active And a GGUF model is being served on lemonade When the user sends a chat completion request @@ -67,7 +67,7 @@ Feature: Model serving # default from the capability probe. xfail only where that resolves to vLLM # (EAI-7333) — see expectations.toml. @id:serve-default-engine-working-endpoint @requires-gpu - Scenario: 6 - Serving a model without specifying an engine produces a working endpoint + Scenario: serve-08 - Serving a model without specifying an engine produces a working endpoint Given a managed runtime is active When the user serves a model without specifying an engine Then an engine is selected automatically @@ -75,7 +75,7 @@ Feature: Model serving # The inference half of scenario 6. @id:serve-default-engine-inference @requires-gpu - Scenario: 6b - A default-engine served model responds to inference requests + Scenario: serve-09 - A default-engine served model responds to inference requests Given a managed runtime is active When the user serves a model without specifying an engine Then the model responds to inference requests @@ -87,7 +87,7 @@ Feature: Model serving # skips it on lemonade-default hosts (Strix Halo), where asserting a vLLM # default would be a guaranteed false failure. @id:serve-vllm-default-on-instinct @requires-gpu @requires-engine:vllm - Scenario: 9 - vLLM is the default serving engine on Instinct + Scenario: serve-10 - vLLM is the default serving engine on Instinct Given a managed runtime is active When the user serves a vLLM-capable model without specifying an engine Then vLLM is selected as the default engine @@ -97,7 +97,7 @@ Feature: Model serving # `a model is being served on GPU`), so this holds the contract on every GPU # platform. Where it resolves to vLLM, EAI-7333 makes it xfail (expectations.toml). @id:serve-readiness-contract @requires-gpu - Scenario: 8 - A service reported ready can immediately serve inference + Scenario: serve-11 - A service reported ready can immediately serve inference Given a managed runtime is active And a model is being served on GPU When the CLI reports the service as ready @@ -108,7 +108,7 @@ Feature: Model serving # launched — with an actionable message, never a CPU or device-0 fallback. Runs # on the no-GPU mock host, so it gates every PR (@requires-no-gpu, no GPU needed). @id:serve-no-gpu-fails-fast @requires-no-gpu - Scenario: 11 - Serving is refused on a host with no AMD GPU + Scenario: serve-12 - Serving is refused on a host with no AMD GPU When the user serves a model under the GPU-required default Then serving is refused before any engine starts And the user is told no AMD GPU was detected @@ -117,7 +117,7 @@ Feature: Model serving # GPU-required serve must treat it as "no GPU" and refuse, not fall back. Runs on # GPU hardware (Strix Halo / Instinct). @id:serve-masked-devices-fail @requires-gpu @requires-os:linux - Scenario: 12 - Serving is refused when every GPU is masked from view + Scenario: serve-13 - Serving is refused when every GPU is masked from view When the user serves a model with every GPU masked from view Then serving is refused before any engine starts And the user is told no AMD GPU was detected @@ -128,7 +128,7 @@ Feature: Model serving # refuses ("no usable AMD GPU") before the index is ever validated, so the # index-specific rejection can only be observed where a real device is present. @id:serve-absent-gpu-index-rejected @requires-gpu @requires-os:linux - Scenario: 13 - Serving pinned to a GPU that does not exist is refused + Scenario: serve-14 - Serving pinned to a GPU that does not exist is refused When the user serves a model pinned to a GPU index that does not exist Then serving is refused before any engine starts And the user is told that GPU index is unavailable diff --git a/tests/e2e-cucumber/features/networking.feature b/tests/e2e-cucumber/features/networking.feature index 4a6ca72f..5326c6eb 100644 --- a/tests/e2e-cucumber/features/networking.feature +++ b/tests/e2e-cucumber/features/networking.feature @@ -12,7 +12,7 @@ Feature: Native HTTP networking # native HTTP GET to `/v1/models` as its readiness probe (served by the mock). # Listing the model and its endpoint therefore exercises that native GET. @id:networking-native-http-endpoint-reachable - Scenario: 1 - The CLI reaches a served endpoint over the native HTTP stack + Scenario: networking-01 - The CLI reaches a served endpoint over the native HTTP stack Given a model is being served And the model is registered with the CLI When the user lists running services @@ -23,7 +23,7 @@ Feature: Native HTTP networking # to GET `/v1/models` and POST `/v1/chat/completions` over the native stack, then # prints the reply — proving the native HTTP client works end-to-end via the CLI. @id:networking-native-http-chat-round-trip - Scenario: 2 - A chat round-trip over a local endpoint uses the native HTTP stack + Scenario: networking-02 - A chat round-trip over a local endpoint uses the native HTTP stack Given a model is being served And the model is registered with the CLI When the user sends a one-shot chat prompt through the CLI diff --git a/tests/e2e-cucumber/features/runtime_setup.feature b/tests/e2e-cucumber/features/runtime_setup.feature index e81eb8fa..5de58fc0 100644 --- a/tests/e2e-cucumber/features/runtime_setup.feature +++ b/tests/e2e-cucumber/features/runtime_setup.feature @@ -1,7 +1,7 @@ Feature: Runtime configuration @id:runtime-install-sdk-active @requires-gpu @nightly - Scenario: 1 - Installing the SDK makes it the active runtime + Scenario: runtime-01 - Installing the SDK makes it the active runtime Given a machine with no CLI-managed runtimes When the user installs the SDK Then a runtime is registered @@ -14,7 +14,7 @@ Feature: Runtime configuration # runtime's folder path has no such recursive segment. GPU-gated (needs a real # install so the folder path is populated). @id:runtime-path-not-nested @requires-gpu - Scenario: 3 - The managed runtime path is not nested inside another runtime + Scenario: runtime-02 - The managed runtime path is not nested inside another runtime Given a managed runtime is active When the user inspects the system Then the managed runtime folder path is not recursively nested @@ -24,7 +24,7 @@ Feature: Runtime configuration # to a bogus `C:/usr/bin/python3` and errors on the missing path before it can # emit the install-type guidance), so the scenario's premise doesn't hold there. @id:runtime-adopt-preexisting-rejected @requires-os:linux - Scenario: 2 - Adopting a pre-existing ROCm install is rejected with guidance + Scenario: runtime-03 - Adopting a pre-existing ROCm install is rejected with guidance Given a machine with a standard ROCm install When the user tries to adopt the existing install Then the adoption is refused diff --git a/tests/e2e-cucumber/src/expectation.rs b/tests/e2e-cucumber/src/expectation.rs index c8b12e4a..736abf06 100644 --- a/tests/e2e-cucumber/src/expectation.rs +++ b/tests/e2e-cucumber/src/expectation.rs @@ -267,6 +267,16 @@ impl Expectation { #[derive(Debug, Clone, serde::Serialize)] pub struct ResolvedScenario { pub id: String, + /// The `Feature:` this scenario belongs to. Recorded here because a SKIPPED + /// scenario never reaches `report.json`, so the report has no other way to + /// place it under its feature in the grouped grid. + #[serde(default)] + pub feature: String, + /// The scenario's own name (`- - `). Carries the + /// per-feature index the report sorts rows by, and gives skipped scenarios a + /// human label they'd otherwise lack. + #[serde(default)] + pub scenario: String, pub effective_engine: String, /// "pass" | "xfail" | "skip". pub expected: String, @@ -279,7 +289,13 @@ pub struct ResolvedScenario { } impl ResolvedScenario { - pub fn new(id: &str, effective_engine: &str, expectation: &Expectation) -> Self { + pub fn new( + id: &str, + feature: &str, + scenario: &str, + effective_engine: &str, + expectation: &Expectation, + ) -> Self { let (bug, reason, flaky) = match expectation { Expectation::ExpectXfail { bug, reason, flaky } => { (Some(bug.clone()), Some(reason.clone()), *flaky) @@ -289,6 +305,8 @@ impl ResolvedScenario { }; Self { id: id.to_owned(), + feature: feature.to_owned(), + scenario: scenario.to_owned(), effective_engine: effective_engine.to_owned(), expected: expectation.label().to_owned(), bug, diff --git a/tests/e2e-cucumber/tests/e2e.rs b/tests/e2e-cucumber/tests/e2e.rs index 43be1645..305e38ae 100644 --- a/tests/e2e-cucumber/tests/e2e.rs +++ b/tests/e2e-cucumber/tests/e2e.rs @@ -77,6 +77,19 @@ pub struct E2eWorld { pub lifecycle: Option, } +/// One scenario's resolved expectation plus the identity needed to report it. +/// +/// Recorded by `filter_run`, which sees every scenario — including the ones it +/// filters OUT. A filtered (skipped) scenario never reaches `report.json`, so +/// this is the only place its feature and name survive to `platform.json`. +struct Resolution { + expectation: Expectation, + /// Effective serve engine for this scenario on this host. + engine: String, + feature: String, + scenario: String, +} + /// Resolve a CI-provided shared-directory env var to a validated, existing path. /// /// The value is CI-controlled, but validate it before it reaches a filesystem @@ -762,8 +775,11 @@ async fn main() { // Populated by `filter_run` (which sees every scenario, run or skipped) so // the post-run evaluation and platform.json can reconcile by id — including // skipped scenarios, which never appear in cucumber's report.json. - // id → (resolved expectation, effective engine for that scenario). - let resolutions: &'static Mutex> = + // id → (resolved expectation, effective engine, feature name, scenario name). + // The feature/scenario names travel with the resolution so platform.json can + // place even a SKIPPED scenario under its feature in the report's grouped + // grid — a skip never reaches report.json, which is the only other source. + let resolutions: &'static Mutex> = Box::leak(Box::new(Mutex::new(BTreeMap::new()))); // `.run()` records failures into the writers but never sets a non-zero exit @@ -819,17 +835,22 @@ async fn main() { // host — e.g. a required engine can't start) are filtered out and never // run; their resolution is still recorded so platform.json can show N/A. .filter_run(concat!(env!("CARGO_MANIFEST_DIR"), "/features/"), { - move |_feature, _rule, scenario| { + move |feature, _rule, scenario| { let decl = ScenarioDecl::from_tags(&scenario.tags); let expectation = resolve(&decl, cap, matrix, include_nightly, include_lifecycle); let run = (!only_lifecycle || decl.lifecycle) && !matches!(expectation, Expectation::Skip { .. }); if let Some(id) = &decl.id { let engine = decl.effective_engine(cap).to_owned(); - let prev = resolutions - .lock() - .expect("resolutions poisoned") - .insert(id.clone(), (expectation, engine)); + let prev = resolutions.lock().expect("resolutions poisoned").insert( + id.clone(), + Resolution { + expectation, + engine, + feature: feature.name.clone(), + scenario: scenario.name.clone(), + }, + ); // Two scenarios sharing an `@id` would silently overwrite each // other's resolution (e.g. a copy-paste with a forgotten id // change) — the report grid keys on @id, so the collision would @@ -884,8 +905,14 @@ async fn main() { versions, expectations: resolutions .iter() - .map(|(id, (exp, engine))| { - e2e_cucumber::expectation::ResolvedScenario::new(id, engine, exp) + .map(|(id, r)| { + e2e_cucumber::expectation::ResolvedScenario::new( + id, + &r.feature, + &r.scenario, + &r.engine, + &r.expectation, + ) }) .collect(), }; @@ -901,7 +928,7 @@ async fn main() { let mut unexpected_fail = Vec::new(); let mut xfail_count = 0u32; for (id, passed) in &actual { - match resolutions.get(id).map(|(exp, _)| exp) { + match resolutions.get(id).map(|r| &r.expectation) { Some(Expectation::ExpectXfail { bug, flaky, .. }) => { if *passed { let label = format!("{id} ({bug})"); From 15c9c08c9f01daee5d63be0113c7f31f152eb931 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 5 Aug 2026 11:58:39 +0000 Subject: [PATCH 2/6] feat(e2e-report): group the expectation grid by feature The grid rendered every scenario in the suite as one undivided ~60-row table, sorted alphabetically by scenario id. Nothing marked where one area of the CLI ended and the next began, and the ordering bore no relation to the order the scenarios are declared in. Render one sub-table per feature instead, under its own heading, with rows in feature-file order (by the - index now carried in each scenario name). The Scenario reference section follows the same grouping and order, so the links from the grid land in a layout that matches it. Feature and scenario name come from platform.json. For an artifact written before those fields existed, fall back to the feature name in report.json, then to the scenario id's leading segment - an older artifact still groups sensibly rather than collapsing into one bucket. Also give every grid row an anchor in the Scenario reference. A scenario that was n/a on every platform has no report.json entry, so its link previously dangled. Signed-off-by: Roman Inflianskas --- crates/e2e-report/src/lib.rs | 295 ++++++++++++++++++++++++++--------- 1 file changed, 222 insertions(+), 73 deletions(-) diff --git a/crates/e2e-report/src/lib.rs b/crates/e2e-report/src/lib.rs index c5867e76..f1c2c5d6 100644 --- a/crates/e2e-report/src/lib.rs +++ b/crates/e2e-report/src/lib.rs @@ -609,6 +609,14 @@ impl PlatformVersions { #[derive(Deserialize, Clone)] struct ManifestExpectation { id: String, + /// The `Feature:` this scenario belongs to. Absent in artifacts predating + /// the grouped grid — see [`feature_of`] for the fallback. + #[serde(default)] + feature: String, + /// The scenario's own name (`- - `), carrying the + /// per-feature index rows are sorted by. Absent in older artifacts. + #[serde(default)] + scenario: String, #[serde(default)] effective_engine: String, /// "pass" | "xfail" | "skip". @@ -726,19 +734,72 @@ struct GridColumn { details: std::collections::BTreeMap, } -/// The reconciled grid: ordered scenario ids × platform columns. Built from each -/// input's `platform.json` (expected) joined with its `report.json` (actual) by -/// stable `@id`. Inputs without a `platform.json` (pre-expectation artifacts) are -/// skipped here — they still appear in the legacy platform×tier matrix. +/// One row of the grid: a scenario, with the identity used to place and order it. +struct GridRow { + id: String, + /// The scenario's human name, or empty when unknown (an artifact predating + /// the `scenario` field whose scenario ran nowhere). + name: String, + /// Per-feature index parsed from the `-` name prefix. `None` when + /// the name is absent or unindexed — those rows sort last, by id. + index: Option, +} + +/// Scenarios of one `Feature:`, in display order. +struct FeatureGroup { + feature: String, + rows: Vec, +} + +/// The reconciled grid: scenario rows grouped by feature × platform columns. +/// Built from each input's `platform.json` (expected) joined with its +/// `report.json` (actual) by stable `@id`. Inputs without a `platform.json` +/// (pre-expectation artifacts) are skipped here — they still appear in the legacy +/// platform×tier matrix. struct Grid { - /// Scenario ids in first-seen order across all columns. - ids: Vec, + /// Feature groups, alphabetical by feature name; rows within a group ordered + /// by their `-` index (i.e. feature-file order). + groups: Vec, columns: Vec, } +/// The per-feature index in a scenario name like `serve-07 - Something happens`. +/// `None` for a name that doesn't carry one (older artifact, or a scenario +/// renamed out of the convention). +fn scenario_index(name: &str) -> Option { + let head = name.split(" - ").next()?; + let (_key, digits) = head.rsplit_once('-')?; + digits.parse().ok() +} + +/// The feature a scenario belongs to, best-effort. +/// +/// 1. `platform.json`'s own `feature` — the honest source, and the only one that +/// covers a scenario skipped on every platform. +/// 2. The feature name from a `report.json` that ran it. +/// 3. Failing both, the id's leading segment (`serve-vllm-inference` → `serve`), +/// so a pre-expectation artifact still groups sensibly instead of collapsing +/// into one bucket. +fn feature_of(exp: &ManifestExpectation, from_reports: Option<&str>) -> String { + if !exp.feature.is_empty() { + return exp.feature.clone(); + } + if let Some(name) = from_reports.filter(|n| !n.is_empty()) { + return name.to_owned(); + } + exp.id + .split_once('-') + .map_or_else(|| exp.id.clone(), |(key, _)| key.to_owned()) +} + impl Grid { fn build(inputs: &[(String, PathBuf)]) -> Self { - let mut ids: Vec = Vec::new(); + // Feature names for ids that ran somewhere, as a fallback for artifacts + // whose platform.json predates the `feature` field. + let features_from_reports = id_features(inputs); + // id → (feature, scenario name), merged across inputs. A later input can + // fill in identity an earlier (older) artifact lacked. + let mut identity: BTreeMap = BTreeMap::new(); let mut columns: Vec = Vec::new(); for (_label, json_path) in inputs { @@ -769,8 +830,15 @@ impl Grid { }); for exp in &manifest.expectations { - if !ids.contains(&exp.id) { - ids.push(exp.id.clone()); + let entry = identity + .entry(exp.id.clone()) + .or_insert_with(|| (String::new(), String::new())); + if entry.0.is_empty() { + entry.0 = + feature_of(exp, features_from_reports.get(&exp.id).map(String::as_str)); + } + if entry.1.is_empty() { + entry.1.clone_from(&exp.scenario); } let outcome = CellOutcome::reconcile(&exp.expected, exp.flaky, actual.get(&exp.id).copied()); @@ -788,8 +856,27 @@ impl Grid { } } - ids.sort(); - Self { ids, columns } + // Group by feature, then order rows by their per-feature index so the + // grid reads in feature-file order rather than the alphabetical-by-id + // mash the flat table used to show. Unindexed rows sort last, by id. + let mut by_feature: BTreeMap> = BTreeMap::new(); + for (id, (feature, name)) in identity { + let index = scenario_index(&name); + by_feature + .entry(feature) + .or_default() + .push(GridRow { id, name, index }); + } + let groups = by_feature + .into_iter() + .map(|(feature, mut rows)| { + rows.sort_by(|a, b| { + (a.index.is_none(), a.index, &a.id).cmp(&(b.index.is_none(), b.index, &b.id)) + }); + FeatureGroup { feature, rows } + }) + .collect(); + Self { groups, columns } } /// Every problem cell across the grid, as `(slug, id, outcome, detail)`. @@ -810,11 +897,28 @@ impl Grid { out } - const fn is_empty(&self) -> bool { - self.columns.is_empty() || self.ids.is_empty() + fn is_empty(&self) -> bool { + self.columns.is_empty() || self.groups.is_empty() } } +/// Map each scenario `@id` → the name of the feature that ran it, from every +/// input's `report.json`. The fallback used when a `platform.json` predates the +/// `feature` field. +fn id_features(inputs: &[(String, PathBuf)]) -> BTreeMap { + let mut map = BTreeMap::new(); + for (_label, json_path) in inputs { + for f in parse_features(json_path) { + for el in &f.elements { + if let Some(id) = scenario_id(el) { + map.entry(id).or_insert_with(|| f.name.clone()); + } + } + } + } + map +} + /// Map each scenario's stable `@id` → whether it passed, from a `report.json`. /// (Internal sibling of the public [`scenario_results_by_id`], returning a map.) fn id_pass_map(json_path: &Path) -> std::collections::HashMap { @@ -1223,31 +1327,39 @@ fn expectation_grid_html(inputs: &[(String, PathBuf)]) -> Markup { span.status-fail { "❌FAIL" } " regression · " "⚠️XPASS bug fixed here (stale entry) · · no data." } - table.stats { - thead { - tr { - th { "Scenario" } - @for col in &grid.columns { - @let versions = col.versions.summary(); - th { - (col.slug) - @if !col.engine.is_empty() { br; small { (col.engine) } } - @if !versions.is_empty() { br; small.versions { (versions) } } + // One table per feature, so a reader can scan a single area of the CLI + // instead of one undivided 60-row block. + @for group in &grid.groups { + h3.feature-heading { (group.feature) } + table.stats { + thead { + tr { + th { "Scenario" } + @for col in &grid.columns { + @let versions = col.versions.summary(); + th { + (col.slug) + @if !col.engine.is_empty() { br; small { (col.engine) } } + @if !versions.is_empty() { br; small.versions { (versions) } } + } } } } - } - tbody { - @for id in &grid.ids { - tr { - td { code { (id) } } - @for col in &grid.columns { - @let outcome = col.outcomes.get(id).copied().unwrap_or(CellOutcome::Missing); - // One combined class attr — `td.num class=(..)` would emit - // two `class` attributes, and the browser keeps only the - // first ("num"), dropping the status colour. - td class=(format!("num {}", outcome.grid_class())) { - (outcome.glyph()) + tbody { + @for row in &group.rows { + tr { + td { + @if !row.name.is_empty() { (row.name) br; } + code { (row.id) } + } + @for col in &grid.columns { + @let outcome = col.outcomes.get(&row.id).copied().unwrap_or(CellOutcome::Missing); + // One combined class attr — `td.num class=(..)` would emit + // two `class` attributes, and the browser keeps only the + // first ("num"), dropping the status colour. + td class=(format!("num {}", outcome.grid_class())) { + (outcome.glyph()) + } } } } @@ -1300,36 +1412,52 @@ fn expectation_grid_markdown( ❌FAIL regression · ⚠️XPASS bug fixed here (stale entry) · · no data._\n\n", ); - // Header: one column per platform, with its effective engine. Component - // versions live in the summary matrix above, not here. - out.push_str("| Scenario |"); - for col in &grid.columns { - let eng = if col.engine.is_empty() { - String::new() - } else { - format!("
{}", col.engine) - }; - let _ = write!(out, " {}{} |", col.slug, eng); - } - out.push('\n'); - out.push_str("|---|"); - for _ in &grid.columns { - out.push_str(":--:|"); - } - out.push('\n'); + // One table per feature, under its own heading — a single undivided table of + // every scenario in the suite is unreadable, and gives no clue where one area + // of the CLI ends and the next begins. + for group in &grid.groups { + let _ = writeln!(out, "#### {}\n", group.feature); - for id in &grid.ids { - // Scenario cell: human name on top, the @id below as a link to its entry - // in the Scenario reference section (GitHub anchors `#### ` to `#`). - let name = scenarios.get(id).map_or("", |(n, _)| n.as_str()); - let _ = write!(out, "| {name}
[`{id}`](#{id}) |"); + // Header: one column per platform, with its effective engine. Component + // versions live in the summary matrix above, not here. + out.push_str("| Scenario |"); for col in &grid.columns { - let g = col - .outcomes - .get(id) - .copied() - .unwrap_or(CellOutcome::Missing); - let _ = write!(out, " {} |", g.glyph()); + let eng = if col.engine.is_empty() { + String::new() + } else { + format!("
{}", col.engine) + }; + let _ = write!(out, " {}{} |", col.slug, eng); + } + out.push('\n'); + out.push_str("|---|"); + for _ in &grid.columns { + out.push_str(":--:|"); + } + out.push('\n'); + + for row in &group.rows { + // Scenario cell: human name on top, the @id below as a link to its + // entry in the Scenario reference section (GitHub anchors + // `#### ` to `#`). Prefer the name recorded in platform.json + // — it covers a scenario that was skipped everywhere and so has no + // report.json entry to read a name from. + let name = if row.name.is_empty() { + scenarios.get(&row.id).map_or("", |(n, _)| n.as_str()) + } else { + row.name.as_str() + }; + let id = &row.id; + let _ = write!(out, "| {name}
[`{id}`](#{id}) |"); + for col in &grid.columns { + let g = col + .outcomes + .get(id) + .copied() + .unwrap_or(CellOutcome::Missing); + let _ = write!(out, " {} |", g.glyph()); + } + out.push('\n'); } out.push('\n'); } @@ -1366,9 +1494,9 @@ fn expectation_grid_markdown( } /// Render the Scenario reference section: each scenario `@id` with its actual -/// Gherkin scenario (name + steps), anchored by the id (`#### ` → GitHub -/// anchor `#`) so the grid's id links resolve. Ordered by id for stability. -/// Empty when no scenarios are known. +/// Gherkin scenario (name + steps), anchored by the id (`##### ` → GitHub +/// anchor `#`) so the grid's id links resolve. Grouped by feature and ordered +/// like the grid. Empty when no scenarios are known. fn scenario_reference_markdown( inputs: &[(String, PathBuf)], scenarios: &std::collections::BTreeMap)>, @@ -1377,18 +1505,36 @@ fn scenario_reference_markdown( // Only emit when there is a grid to reference (platform.json sidecars present), // matching where the id links are generated. - if scenarios.is_empty() || Grid::build(inputs).is_empty() { + let grid = Grid::build(inputs); + if scenarios.is_empty() || grid.is_empty() { return String::new(); } + // Walk the grid's own order so the reference is laid out feature by feature, + // matching the tables that link into it. Every grid row gets an entry — a + // scenario skipped on every platform has no steps to show, but still needs + // its anchor or the grid's link to it dangles. let mut out = String::from("\n### Scenario reference\n\n"); - for (id, (name, steps)) in scenarios { - let _ = writeln!(out, "#### {id}\n"); - let _ = writeln!(out, "_{name}_\n"); - for step in steps { - let _ = writeln!(out, "- {step}"); + for group in &grid.groups { + let _ = writeln!(out, "#### {}\n", group.feature); + for row in &group.rows { + let entry = scenarios.get(&row.id); + let name = entry.map_or(row.name.as_str(), |(n, _)| n.as_str()); + let _ = writeln!(out, "##### {}\n", row.id); + if !name.is_empty() { + let _ = writeln!(out, "_{name}_\n"); + } + match entry { + Some((_, steps)) => { + for step in steps { + let _ = writeln!(out, "- {step}"); + } + } + // Ran nowhere (n/a on every platform), so no steps were recorded. + None => out.push_str("_Not run on any platform in this run._\n"), + } + out.push('\n'); } - out.push('\n'); } out } @@ -1901,6 +2047,9 @@ const STYLE: &str = r#" /* xfail: a known bug that failed as expected — a muted grey ✗, sibling to the red ✗. */ .status-xfail { color: #9e9e9e; font-weight: 600; } .grid-legend { font-size: 0.82rem; color: #555; margin: -0.5rem 0 0.75rem; } + /* Feature heading above each sub-table of the expectation grid. */ + .feature-heading { margin: 1.25rem 0 0.4rem; padding-bottom: 0.2rem; border-bottom: 1px solid #e0e0e0; + color: #1565c0; } table.stats { width: 100%; border-collapse: collapse; margin-bottom: 1.5rem; font-size: 0.9rem; } table.stats th { background: #f5f5f5; padding: 6px 12px; text-align: left; border: 1px solid #ddd; From 175e9c3f5c143e6132f67db747a7ea1904dc632b Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 5 Aug 2026 12:01:53 +0000 Subject: [PATCH 3/6] test: guard the feature-key naming convention against drift The report groups its grid by feature and orders rows by the - index in each scenario name, so the convention is load-bearing. It had already drifted badly before it was enforced: indexes restarting at 1 in every file, examine numbered 1, 2, 5, 3, 4, a stray '6b' in model_serving, and no indexes at all in install_lifecycle. Add a plain test target that parses the .feature files and asserts indexes are sequential per feature, unique suite-wide, and that every scenario carries a feature-qualified @id. It runs in the ordinary cargo test set, unlike the e2e target, so a mis-numbered scenario is caught without a full suite run. Verified it fails on each of those drift shapes before passing on the fixed files. Also cover the report side: grouping, index ordering (09 before 10, not lexically), the fallback for an artifact with no feature field, and the anchor for a scenario that ran nowhere. Signed-off-by: Roman Inflianskas --- crates/e2e-report/src/lib.rs | 126 ++++++++++++++++++- tests/e2e-cucumber/tests/feature_naming.rs | 136 +++++++++++++++++++++ 2 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 tests/e2e-cucumber/tests/feature_naming.rs diff --git a/crates/e2e-report/src/lib.rs b/crates/e2e-report/src/lib.rs index f1c2c5d6..041cc2ee 100644 --- a/crates/e2e-report/src/lib.rs +++ b/crates/e2e-report/src/lib.rs @@ -897,7 +897,7 @@ impl Grid { out } - fn is_empty(&self) -> bool { + const fn is_empty(&self) -> bool { self.columns.is_empty() || self.groups.is_empty() } } @@ -2473,6 +2473,130 @@ mod tests { (dir, report) } + /// The order the grid renders rows in, flattened across feature groups. + fn grid_order(inputs: &[(String, PathBuf)]) -> Vec<(String, String)> { + Grid::build(inputs) + .groups + .iter() + .flat_map(|g| g.rows.iter().map(|r| (g.feature.clone(), r.id.clone()))) + .collect() + } + + #[test] + fn scenario_index_parses_the_per_feature_number() { + assert_eq!(scenario_index("serve-07 - A model responds"), Some(7)); + assert_eq!(scenario_index("lifecycle-01 - Linux - a bundle"), Some(1)); + // Unindexed names (older artifacts) have no number to sort by. + assert_eq!(scenario_index("A model responds"), None); + assert_eq!(scenario_index(""), None); + } + + #[test] + fn grid_groups_by_feature_and_orders_by_index() { + // Declared deliberately out of order (and interleaved across features) + // so a passing assertion can only come from real grouping + sorting, + // not from the input order. + let report = feature_json(&[ + (&["id:serve-b"], &["passed"]), + (&["id:examine-a"], &["passed"]), + ]); + let platform = r#"{ + "platform_slug": "mock", + "capability": {"effective_serve_engine": "none"}, + "expectations": [ + {"id":"serve-b","feature":"Serving","scenario":"serve-02 - second","expected":"pass"}, + {"id":"examine-a","feature":"Examine","scenario":"examine-09 - ninth","expected":"pass"}, + {"id":"serve-a","feature":"Serving","scenario":"serve-01 - first","expected":"skip"}, + {"id":"examine-b","feature":"Examine","scenario":"examine-10 - tenth","expected":"skip"} + ] + }"#; + let (_d, path) = write_platform(&report, platform); + let inputs = vec![("mock".to_string(), path)]; + + assert_eq!( + grid_order(&inputs), + vec![ + ("Examine".to_string(), "examine-a".to_string()), + ("Examine".to_string(), "examine-b".to_string()), + ("Serving".to_string(), "serve-a".to_string()), + ("Serving".to_string(), "serve-b".to_string()), + ], + "rows must group by feature and sort by index (09 before 10, not \ + lexically), regardless of declaration order", + ); + + // Each feature gets its own table under its own heading. + let md = consolidated_summary_markdown(&inputs); + assert!( + md.contains("#### Examine"), + "missing feature heading:\n{md}" + ); + assert!( + md.contains("#### Serving"), + "missing feature heading:\n{md}" + ); + // The human name is shown alongside the id, sourced from platform.json — + // serve-a was skipped everywhere, so report.json has no name for it. + assert!( + md.contains("serve-01 - first
[`serve-a`](#serve-a)"), + "skipped scenario should still show its name:\n{md}" + ); + } + + #[test] + fn grid_falls_back_to_report_feature_then_id_prefix() { + // A platform.json predating the `feature`/`scenario` fields. `serve-x` + // ran (so report.json knows its feature); `dash-y` was skipped + // everywhere, leaving only its id to group by. + let report = feature_json(&[(&["id:serve-x"], &["passed"])]); + let platform = r#"{ + "platform_slug": "mock", + "capability": {"effective_serve_engine": "none"}, + "expectations": [ + {"id":"serve-x","effective_engine":"","expected":"pass"}, + {"id":"dash-y","effective_engine":"","expected":"skip"} + ] + }"#; + let (_d, path) = write_platform(&report, platform); + let inputs = vec![("mock".to_string(), path)]; + + // `feature_json` names its feature "F"; the id prefix covers the rest. + assert_eq!( + grid_order(&inputs), + vec![ + ("F".to_string(), "serve-x".to_string()), + ("dash".to_string(), "dash-y".to_string()), + ], + "an artifact with no feature field must still group, not vanish", + ); + } + + #[test] + fn scenario_reference_anchors_every_grid_row() { + // A scenario that is n/a everywhere has no report.json entry — it still + // needs an anchor, or the grid's link to it dangles. + let report = feature_json(&[(&["id:serve-x"], &["passed"])]); + let platform = r#"{ + "platform_slug": "mock", + "capability": {"effective_serve_engine": "none"}, + "expectations": [ + {"id":"serve-x","feature":"Serving","scenario":"serve-01 - ran","expected":"pass"}, + {"id":"serve-z","feature":"Serving","scenario":"serve-02 - skipped","expected":"skip"} + ] + }"#; + let (_d, path) = write_platform(&report, platform); + let md = consolidated_summary_markdown(&[("mock".to_string(), path)]); + assert!(md.contains("##### serve-x"), "missing anchor:\n{md}"); + assert!( + md.contains("##### serve-z"), + "a never-run scenario still needs its anchor:\n{md}" + ); + assert!( + md.contains("_Not run on any platform in this run._"), + "a never-run scenario should say so instead of showing no steps:\n{md}" + ); + } + #[test] fn grid_reconciles_xfail_and_pass_by_id() { // Scenario s0 tagged @id:serve-x, expected xfail, actually failed → xfail (good). diff --git a/tests/e2e-cucumber/tests/feature_naming.rs b/tests/e2e-cucumber/tests/feature_naming.rs new file mode 100644 index 00000000..220a06f3 --- /dev/null +++ b/tests/e2e-cucumber/tests/feature_naming.rs @@ -0,0 +1,136 @@ +// Copyright © Advanced Micro Devices, Inc., or its affiliates. +// +// SPDX-License-Identifier: MIT + +//! Drift guard for the `.feature` files' scenario naming and ids. +//! +//! The report groups its expectation grid by feature and orders rows by the +//! `-` index in each scenario's name, so that convention is +//! load-bearing, not cosmetic. It had already drifted once — indexes restarting +//! at 1 in every file, `examine` numbered 1, 2, 5, 3, 4, a stray `6b` in +//! `model_serving`, and no indexes at all in `install_lifecycle`. +//! +//! This runs in the ordinary `cargo test` set (unlike the `e2e` target, which +//! needs a real `rocm` binary), so a mis-numbered scenario is caught without a +//! full suite run. + +use std::collections::BTreeMap; +use std::path::{Path, PathBuf}; + +/// The short key each feature file's scenarios and ids are prefixed with. +/// Adding a `.feature` file means adding its key here — deliberately explicit, +/// so a new file can't quietly opt out of the convention. +const FEATURE_KEYS: &[(&str, &str)] = &[ + ("chat.feature", "chat"), + ("dash.feature", "dash"), + ("diagnose.feature", "diagnose"), + ("examine.feature", "examine"), + ("install_lifecycle.feature", "lifecycle"), + ("model_serving.feature", "serve"), + ("networking.feature", "networking"), + ("runtime_setup.feature", "runtime"), +]; + +fn features_dir() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("features") +} + +/// Every `.feature` file actually present, by file name. +fn feature_files() -> Vec { + let mut names: Vec = std::fs::read_dir(features_dir()) + .expect("features dir") + .map(|e| e.expect("dir entry").file_name().to_string_lossy().into()) + .filter(|n: &String| n.ends_with(".feature")) + .collect(); + names.sort(); + names +} + +/// The `@id:` tags and `Scenario:` names in one feature file, paired in +/// declaration order. Tags precede their scenario, so the most recent id seen +/// belongs to the next scenario line. +fn scenarios_of(file: &str) -> Vec<(Option, String)> { + let text = std::fs::read_to_string(features_dir().join(file)).expect("read feature"); + let mut pending_id = None; + let mut out = Vec::new(); + for line in text.lines() { + let line = line.trim(); + if let Some(rest) = line.strip_prefix('@') { + for tag in rest.split_whitespace() { + if let Some(id) = tag.strip_prefix("id:") { + pending_id = Some(id.to_owned()); + } + } + } else if let Some(name) = line.strip_prefix("Scenario: ") { + out.push((pending_id.take(), name.to_owned())); + } + } + out +} + +#[test] +fn every_feature_file_has_a_declared_key() { + let declared: Vec<&str> = FEATURE_KEYS.iter().map(|(f, _)| *f).collect(); + for file in feature_files() { + assert!( + declared.contains(&file.as_str()), + "{file} has no key in FEATURE_KEYS — add one so its scenarios are \ + indexed and its ids are qualified like every other feature", + ); + } +} + +#[test] +fn scenario_names_are_indexed_sequentially_per_feature() { + for (file, key) in FEATURE_KEYS { + for (n, (_id, name)) in scenarios_of(file).iter().enumerate() { + let expected = format!("{key}-{:02} - ", n + 1); + assert!( + name.starts_with(&expected), + "{file}: scenario {} is named {name:?} but must start with \ + {expected:?} — indexes are per-feature, sequential, and in \ + declaration order (the report sorts grid rows by them)", + n + 1, + ); + } + } +} + +#[test] +fn scenario_indexes_are_unique_across_the_suite() { + // The whole point of the feature key: an index must name exactly one + // scenario suite-wide. Before the key, "1" named eight different scenarios. + let mut seen: BTreeMap = BTreeMap::new(); + for (file, _key) in FEATURE_KEYS { + for (_id, name) in scenarios_of(file) { + let index = name + .split(" - ") + .next() + .expect("split always yields one part") + .to_owned(); + if let Some(prev) = seen.insert(index.clone(), (*file).to_owned()) { + panic!("index {index:?} is used by both {prev} and {file}"); + } + } + } +} + +#[test] +fn every_scenario_has_a_feature_qualified_id() { + let mut seen: BTreeMap = BTreeMap::new(); + for (file, key) in FEATURE_KEYS { + for (id, name) in scenarios_of(file) { + let id = id.unwrap_or_else(|| { + panic!("{file}: scenario {name:?} has no @id: tag — the report grid keys on it") + }); + assert!( + id.starts_with(&format!("{key}-")), + "{file}: @id:{id} must start with {key:?} so the id alone says \ + which feature it belongs to", + ); + if let Some(prev) = seen.insert(id.clone(), (*file).to_owned()) { + panic!("duplicate @id:{id} in both {prev} and {file}"); + } + } + } +} From c43c84a7beb699a28c12061294c7fb4f7910b674 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 5 Aug 2026 12:07:02 +0000 Subject: [PATCH 4/6] docs(e2e): document the feature-key naming convention The - scenario index and feature-qualified @id are now enforced by tests/feature_naming.rs, so the README should say what the convention is and where adding a feature file needs a matching entry. Also qualify Expectation in the Resolution struct - it sits above the function that imports the name. The e2e test target sets test = false, so cargo check --tests skips it and this only surfaced on cargo xtask e2e. Signed-off-by: Roman Inflianskas --- tests/e2e-cucumber/README.md | 19 ++++++++++++++++++- tests/e2e-cucumber/tests/e2e.rs | 2 +- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/tests/e2e-cucumber/README.md b/tests/e2e-cucumber/README.md index 77700135..3c1439f4 100644 --- a/tests/e2e-cucumber/README.md +++ b/tests/e2e-cucumber/README.md @@ -93,11 +93,28 @@ There is no tag-filter tiering. Each CI job runs the **whole** suite **pass / xfail / skip** at runtime from its capability tags plus the known-bug matrix, then reconciles the actual result against that expectation. +### Naming + +Each feature file has a short **key** (`chat`, `dash`, `diagnose`, `examine`, +`lifecycle`, `serve`, `networking`, `runtime`) that prefixes both its scenario +names and its ids: + +- **Scenario name** — `Scenario: - - `, numbered + sequentially in declaration order. The report sorts the grid's rows by this + index, so it must match the order in the file. Without the key the index names + nothing: every file used to number from 1, so "1" meant eight different + scenarios. +- **`@id:`** — `-`, so an id alone says which feature it belongs to. + +`tests/feature_naming.rs` enforces all of this (sequential, unique suite-wide, +feature-qualified ids) in the ordinary `cargo test` run. Adding a feature file +means adding its key to `FEATURE_KEYS` there. + Scenarios carry stable-id and capability tags: | Tag | Meaning | |---|---| -| `@id:` | Stable scenario id. Keys the expectation matrix and the report grid; every scenario has one. | +| `@id:-` | Stable scenario id, prefixed with its feature's key. Keys the expectation matrix and the report grid; every scenario has one. | | `@requires-gpu` | Needs a real AMD GPU. Resolves to **skip** (n/a) on a host with none (e.g. the mock job). | | `@requires-engine:` | Pins the serve engine. Resolves to skip where that engine can't start (e.g. vLLM on a lemonade-only Strix host). | | `@requires-os:` | Premise is OS-specific; skip on other OSes. | diff --git a/tests/e2e-cucumber/tests/e2e.rs b/tests/e2e-cucumber/tests/e2e.rs index 305e38ae..7dbf0e70 100644 --- a/tests/e2e-cucumber/tests/e2e.rs +++ b/tests/e2e-cucumber/tests/e2e.rs @@ -83,7 +83,7 @@ pub struct E2eWorld { /// filters OUT. A filtered (skipped) scenario never reaches `report.json`, so /// this is the only place its feature and name survive to `platform.json`. struct Resolution { - expectation: Expectation, + expectation: e2e_cucumber::expectation::Expectation, /// Effective serve engine for this scenario on this host. engine: String, feature: String, From 675c1c8f3037de9f09439f6eacbb8e2343d246d5 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Wed, 5 Aug 2026 12:17:31 +0000 Subject: [PATCH 5/6] fix(e2e-report): let a named feature override an older artifact's guess Grid::build only filled in a scenario's feature when the slot was still empty, but feature_of never returns empty - its last resort is the id's leading segment. So the first input to mention an id fixed its feature permanently, and a later artifact that actually names the feature was ignored. Consolidating a pre-expectation artifact with a current one then split a feature in two: 'serve' from the id prefix and 'Model serving' from the real name, sorted far apart. That is precisely the collapse the fallback chain exists to prevent. Treat an artifact that names the feature as authoritative and let it overwrite. Covered by a test that mixes both vintages in either input order; it fails on the previous logic. Also from review: match Scenario Outline in the drift guard (none today, but an outline would slip past all four checks silently), assert FEATURE_KEYS has no entry for a deleted file, and align the scenario-reference name preference with the grid's. Signed-off-by: Roman Inflianskas --- crates/e2e-report/src/lib.rs | 73 ++++++++++++++++++++-- tests/e2e-cucumber/tests/feature_naming.rs | 31 ++++++--- 2 files changed, 93 insertions(+), 11 deletions(-) diff --git a/crates/e2e-report/src/lib.rs b/crates/e2e-report/src/lib.rs index 041cc2ee..9f69e3ca 100644 --- a/crates/e2e-report/src/lib.rs +++ b/crates/e2e-report/src/lib.rs @@ -833,9 +833,18 @@ impl Grid { let entry = identity .entry(exp.id.clone()) .or_insert_with(|| (String::new(), String::new())); - if entry.0.is_empty() { - entry.0 = - feature_of(exp, features_from_reports.get(&exp.id).map(String::as_str)); + // An artifact that names the feature itself is authoritative and + // OVERWRITES whatever a fallback guessed — `feature_of` always + // yields something (worst case the id's prefix), so a mere + // is-empty check would let the first input's guess stick and the + // later, better-informed artifact be ignored. + if exp.feature.is_empty() { + if entry.0.is_empty() { + entry.0 = + feature_of(exp, features_from_reports.get(&exp.id).map(String::as_str)); + } + } else { + entry.0.clone_from(&exp.feature); } if entry.1.is_empty() { entry.1.clone_from(&exp.scenario); @@ -1514,12 +1523,23 @@ fn scenario_reference_markdown( // matching the tables that link into it. Every grid row gets an entry — a // scenario skipped on every platform has no steps to show, but still needs // its anchor or the grid's link to it dangles. + // + // Scoped to grid rows deliberately: this section exists to back the grid's + // links, so it documents exactly what the grid shows. A scenario present only + // in an artifact with no `platform.json` sidecar has no grid row and so gets + // no entry — nothing links to it either. let mut out = String::from("\n### Scenario reference\n\n"); for group in &grid.groups { let _ = writeln!(out, "#### {}\n", group.feature); for row in &group.rows { let entry = scenarios.get(&row.id); - let name = entry.map_or(row.name.as_str(), |(n, _)| n.as_str()); + // Prefer the platform.json name, as the grid does — it is the only + // source that covers a scenario which ran nowhere. + let name = if row.name.is_empty() { + entry.map_or("", |(n, _)| n.as_str()) + } else { + row.name.as_str() + }; let _ = writeln!(out, "##### {}\n", row.id); if !name.is_empty() { let _ = writeln!(out, "_{name}_\n"); @@ -2571,6 +2591,51 @@ mod tests { ); } + #[test] + fn a_named_feature_overrides_an_older_artifact_s_guess() { + // Mixing artifact vintages: the old one has no `feature` field, so the + // id prefix is all there is to go on; the new one names the feature. + // The named one must win regardless of input order, or the scenario + // splits across two groups sorted far apart. + let old = r#"{ + "platform_slug": "mi300x", + "capability": {"effective_serve_engine": "vllm"}, + "expectations": [{"id":"serve-x","expected":"skip"}] + }"#; + let new = r#"{ + "platform_slug": "mock", + "capability": {"effective_serve_engine": "none"}, + "expectations": [ + {"id":"serve-x","feature":"Model serving","scenario":"serve-01 - a","expected":"skip"} + ] + }"#; + let report = feature_json(&[]); + let (_d1, old_path) = write_platform(&report, old); + let (_d2, new_path) = write_platform(&report, new); + + // Old artifact first: its id-prefix guess must not stick. + let inputs = vec![ + ("mi300x".to_string(), old_path.clone()), + ("mock".to_string(), new_path.clone()), + ]; + assert_eq!( + grid_order(&inputs), + vec![("Model serving".to_string(), "serve-x".to_string())], + "a later artifact that names the feature must override the guess", + ); + + // And the reverse order must not regress it back to the guess. + let inputs = vec![ + ("mock".to_string(), new_path), + ("mi300x".to_string(), old_path), + ]; + assert_eq!( + grid_order(&inputs), + vec![("Model serving".to_string(), "serve-x".to_string())], + "an older artifact must not overwrite a named feature", + ); + } + #[test] fn scenario_reference_anchors_every_grid_row() { // A scenario that is n/a everywhere has no report.json entry — it still diff --git a/tests/e2e-cucumber/tests/feature_naming.rs b/tests/e2e-cucumber/tests/feature_naming.rs index 220a06f3..cd16c98c 100644 --- a/tests/e2e-cucumber/tests/feature_naming.rs +++ b/tests/e2e-cucumber/tests/feature_naming.rs @@ -46,11 +46,16 @@ fn feature_files() -> Vec { names } -/// The `@id:` tags and `Scenario:` names in one feature file, paired in -/// declaration order. Tags precede their scenario, so the most recent id seen -/// belongs to the next scenario line. +/// The `@id:` tags and scenario names in one feature file, paired in declaration +/// order. Tags precede their scenario, so the most recent id seen belongs to the +/// next scenario line. +/// +/// `Scenario Outline:` counts too — the suite has none today, but an outline +/// added later would otherwise slip past every check in this file silently. fn scenarios_of(file: &str) -> Vec<(Option, String)> { - let text = std::fs::read_to_string(features_dir().join(file)).expect("read feature"); + let path = features_dir().join(file); + let text = std::fs::read_to_string(&path) + .unwrap_or_else(|e| panic!("{}: {e} — stale FEATURE_KEYS entry?", path.display())); let mut pending_id = None; let mut out = Vec::new(); for line in text.lines() { @@ -61,7 +66,10 @@ fn scenarios_of(file: &str) -> Vec<(Option, String)> { pending_id = Some(id.to_owned()); } } - } else if let Some(name) = line.strip_prefix("Scenario: ") { + } else if let Some(name) = line + .strip_prefix("Scenario: ") + .or_else(|| line.strip_prefix("Scenario Outline: ")) + { out.push((pending_id.take(), name.to_owned())); } } @@ -69,15 +77,24 @@ fn scenarios_of(file: &str) -> Vec<(Option, String)> { } #[test] -fn every_feature_file_has_a_declared_key() { +fn feature_files_and_declared_keys_agree() { let declared: Vec<&str> = FEATURE_KEYS.iter().map(|(f, _)| *f).collect(); - for file in feature_files() { + let present = feature_files(); + for file in &present { assert!( declared.contains(&file.as_str()), "{file} has no key in FEATURE_KEYS — add one so its scenarios are \ indexed and its ids are qualified like every other feature", ); } + // The reciprocal: a key left behind after its file was deleted or renamed + // would otherwise surface as an unexplained read error deep in another test. + for file in declared { + assert!( + present.iter().any(|p| p == file), + "FEATURE_KEYS lists {file}, which does not exist — drop the entry", + ); + } } #[test] From 7c03df5ad82ffff80b285aa28d31f00998465324 Mon Sep 17 00:00:00 2001 From: Roman Inflianskas Date: Thu, 6 Aug 2026 12:32:39 +0000 Subject: [PATCH 6/6] fix(e2e): close robustness gaps in the naming guard From review of #186. The guard passed vacuously when a feature file yielded no parseable scenarios: every check is a loop over scenarios_of(), so an empty result meant zero loop bodies and a green test. Mangling the Scenario: keyword in chat.feature - the class of change this PR itself makes - hid all seven of its scenarios while the guard still reported 4 passed. Assert each file parses at least one scenario. The guard also read tags differently from the harness: it stripped the leading @ off the line rather than off each tag, so on a multi-tag line every token after the first kept its own @ and the id was invisible. Valid Gherkin the harness resolves fine (@requires-os:linux @id:examine-version) failed the guard. Strip per tag, as ScenarioDecl::from_tags does. Drop the two #[serde(default)] attributes added to ResolvedScenario: it derives Serialize only, so a deserialization attribute is dead there. The backward compatibility is real but comes entirely from ManifestExpectation on the consuming side; say so where the fields are declared. Give the display name the same last-non-empty-wins rule as the feature. Note that the reviewer's stated failure mode for this one does not hold - the previous is_empty() check already meant first NON-EMPTY wins, so a nameless artifact could not pin a stale sort position. The only case the rules differ on is two artifacts naming an id differently, which is what the new test pins. Also fix a comment left stale by this PR (#### -> ##### for the id anchors) and stop duplicating the FEATURE_KEYS list in the README, where nothing kept it in sync. Signed-off-by: Roman Inflianskas --- crates/e2e-report/src/lib.rs | 52 +++++++++++++++++++++- tests/e2e-cucumber/README.md | 7 +-- tests/e2e-cucumber/src/expectation.rs | 7 ++- tests/e2e-cucumber/tests/feature_naming.rs | 23 +++++++++- 4 files changed, 80 insertions(+), 9 deletions(-) diff --git a/crates/e2e-report/src/lib.rs b/crates/e2e-report/src/lib.rs index 9f69e3ca..7631399e 100644 --- a/crates/e2e-report/src/lib.rs +++ b/crates/e2e-report/src/lib.rs @@ -846,7 +846,15 @@ impl Grid { } else { entry.0.clone_from(&exp.feature); } - if entry.1.is_empty() { + // The display name needs no fallback — unlike `feature` it is + // never synthesized, so an absent one is simply empty and the + // first artifact that HAS a name supplies it either way. + // The only case the two rules differ on is two artifacts naming + // the same id differently (mixing vintages across a rename): + // take the last, matching `feature` above. The name carries the + // sort index, so the rule should at least be stated rather than + // falling out of map iteration order. + if !exp.scenario.is_empty() { entry.1.clone_from(&exp.scenario); } let outcome = @@ -1448,7 +1456,7 @@ fn expectation_grid_markdown( for row in &group.rows { // Scenario cell: human name on top, the @id below as a link to its // entry in the Scenario reference section (GitHub anchors - // `#### ` to `#`). Prefer the name recorded in platform.json + // `##### ` to `#`). Prefer the name recorded in platform.json // — it covers a scenario that was skipped everywhere and so has no // report.json entry to read a name from. let name = if row.name.is_empty() { @@ -2636,6 +2644,46 @@ mod tests { ); } + #[test] + fn the_last_naming_artifact_sets_the_sort_position() { + // Two artifacts naming the same ids differently — a `Scenario:` renamed + // between vintages. The name carries the sort index, so which one wins + // decides row order; pin the documented rule (last wins) rather than + // leaving it to fall out of iteration order. + let older = r#"{ + "platform_slug": "mi300x", + "capability": {"effective_serve_engine": "vllm"}, + "expectations": [ + {"id":"serve-a","feature":"Model serving","scenario":"serve-01 - a","expected":"skip"}, + {"id":"serve-b","feature":"Model serving","scenario":"serve-02 - b","expected":"skip"} + ] + }"#; + let newer = r#"{ + "platform_slug": "mock", + "capability": {"effective_serve_engine": "none"}, + "expectations": [ + {"id":"serve-a","feature":"Model serving","scenario":"serve-02 - a moved","expected":"skip"}, + {"id":"serve-b","feature":"Model serving","scenario":"serve-01 - b moved","expected":"skip"} + ] + }"#; + let report = feature_json(&[]); + let (_d1, older_path) = write_platform(&report, older); + let (_d2, newer_path) = write_platform(&report, newer); + + let inputs = vec![ + ("mi300x".to_string(), older_path), + ("mock".to_string(), newer_path), + ]; + assert_eq!( + grid_order(&inputs) + .into_iter() + .map(|(_, id)| id) + .collect::>(), + vec!["serve-b".to_string(), "serve-a".to_string()], + "the last artifact to name a scenario sets its sort position", + ); + } + #[test] fn scenario_reference_anchors_every_grid_row() { // A scenario that is n/a everywhere has no report.json entry — it still diff --git a/tests/e2e-cucumber/README.md b/tests/e2e-cucumber/README.md index 3c1439f4..1921593b 100644 --- a/tests/e2e-cucumber/README.md +++ b/tests/e2e-cucumber/README.md @@ -95,9 +95,10 @@ matrix, then reconciles the actual result against that expectation. ### Naming -Each feature file has a short **key** (`chat`, `dash`, `diagnose`, `examine`, -`lifecycle`, `serve`, `networking`, `runtime`) that prefixes both its scenario -names and its ids: +Each feature file has a short **key** that prefixes both its scenario names and +its ids. The key is usually the file's stem, but not always — `install_lifecycle` +uses `lifecycle` and `model_serving` uses `serve` — so `FEATURE_KEYS` in +`tests/feature_naming.rs` is the list, not this page: - **Scenario name** — `Scenario: - - `, numbered sequentially in declaration order. The report sorts the grid's rows by this diff --git a/tests/e2e-cucumber/src/expectation.rs b/tests/e2e-cucumber/src/expectation.rs index 736abf06..7f63ecf8 100644 --- a/tests/e2e-cucumber/src/expectation.rs +++ b/tests/e2e-cucumber/src/expectation.rs @@ -270,12 +270,15 @@ pub struct ResolvedScenario { /// The `Feature:` this scenario belongs to. Recorded here because a SKIPPED /// scenario never reaches `report.json`, so the report has no other way to /// place it under its feature in the grouped grid. - #[serde(default)] + /// + /// No `#[serde(default)]` here: this struct only derives `Serialize`, so a + /// deserialization attribute would be dead. Backward compatibility for + /// artifacts written before these fields existed lives entirely on the + /// consuming side — `ManifestExpectation` in the `e2e-report` crate. pub feature: String, /// The scenario's own name (`- - `). Carries the /// per-feature index the report sorts rows by, and gives skipped scenarios a /// human label they'd otherwise lack. - #[serde(default)] pub scenario: String, pub effective_engine: String, /// "pass" | "xfail" | "skip". diff --git a/tests/e2e-cucumber/tests/feature_naming.rs b/tests/e2e-cucumber/tests/feature_naming.rs index cd16c98c..51b09f53 100644 --- a/tests/e2e-cucumber/tests/feature_naming.rs +++ b/tests/e2e-cucumber/tests/feature_naming.rs @@ -60,8 +60,15 @@ fn scenarios_of(file: &str) -> Vec<(Option, String)> { let mut out = Vec::new(); for line in text.lines() { let line = line.trim(); - if let Some(rest) = line.strip_prefix('@') { - for tag in rest.split_whitespace() { + if line.starts_with('@') { + // Strip the `@` per TAG, not just off the head of the line: on a + // multi-tag line every token after the first keeps its own `@`, so + // `@requires-os:linux @id:x` would hide the id. This mirrors + // `ScenarioDecl::from_tags` in src/expectation.rs — the guard must + // read tags exactly as the harness does, or it rejects Gherkin the + // harness accepts. + for tag in line.split_whitespace() { + let tag = tag.strip_prefix('@').unwrap_or(tag); if let Some(id) = tag.strip_prefix("id:") { pending_id = Some(id.to_owned()); } @@ -95,6 +102,18 @@ fn feature_files_and_declared_keys_agree() { "FEATURE_KEYS lists {file}, which does not exist — drop the entry", ); } + // Every other check in this file is a `for … in scenarios_of(file)` loop, so + // a file the parser reads as having NO scenarios passes them all vacuously. + // A mangled `Scenario:` keyword — precisely what a bad bulk find-replace + // does — would then hide a whole feature from the guard while the report + // silently renders its rows unsorted. + for file in &present { + assert!( + !scenarios_of(file).is_empty(), + "{file}: no scenarios parsed — the naming checks would pass \ + vacuously. Is a `Scenario:` keyword malformed?", + ); + } } #[test]