diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 7fb8e45ae..a5f60e1a2 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -6,3 +6,4 @@ SPDX-License-Identifier: MIT - [ ] If this PR fixes a bug, searched `tests/e2e-cucumber/expectations.toml` for the fixed ticket ID and removed/narrowed any now-stale xfail rows. - [ ] If this PR adds a new subcommand or subsystem, its domain implementation lives in its own file per `docs/architecture.md` (the clap declaration and dispatch wiring staying in `main.rs`/`lib.rs` is expected, not a violation). +- [ ] Every new or changed user-facing message was read against the code path that runs after it, and its test asserts the resulting state — not only the wording — per AGENTS.md §3. Covers claims of an outcome, remediation advice naming a command, and promises that something will *not* happen. diff --git a/AGENTS.md b/AGENTS.md index 72c03279a..842f2ab19 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -101,6 +101,31 @@ definitions when no existing scenario already covers it: - purely internal changes (refactors, CI plumbing, docs) do not need one; say why in the PR text rather than leaving it unexplained +**A message about the CLI's own behavior is asserted together with the behavior.** When a +change adds or edits a line the CLI prints about what it just did, what it will do next, +or what the user must do to recover, the covering test asserts the message *and* the +resulting state in the same test. Three shapes need this: + +- claims of an outcome ("this becomes the active default runtime", "nothing was saved") +- remediation advice naming a command — the named command must exist, accept those flags, + and actually clear the condition that printed it +- promises that something will *not* happen ("no driver commands will be executed", + "never stops servers automatically"), which no happy-path test exercises + +A test that only pins the wording certifies the string, not the truth of it, and a pin +over a false claim holds the claim in place. Where the text genuinely has to be pinned on +its own, the assertion carries a comment naming the test that proves the behavior — see +`setup_reset_cli_output_is_plain_and_persists_first_time_prompt` in `apps/rocm/src/main.rs`, +which pins the onboarding line and points at +`startup_focus_gate_only_opens_onboarding_for_explicit_setup_focus` for the behavior +itself. + +The message and the code it describes are usually in different functions and often +different files, so nothing links them by construction. Where the printed line can be +derived from the same value the branch is taken on — as `preapproved_install_line` in +`apps/rocm/src/therock.rs` derives it from the approval source — prefer that: a message +computed from the decision cannot disagree with it. + ## 4) Live State Verification Before Any External Claim Before each stateful decision or public status update: diff --git a/tests/e2e-cucumber/README.md b/tests/e2e-cucumber/README.md index 751f7b0cf..06acf0848 100644 --- a/tests/e2e-cucumber/README.md +++ b/tests/e2e-cucumber/README.md @@ -241,3 +241,4 @@ The `.feature` file is both the spec and the test input — cucumber reads it at - **Isolated state.** Each scenario uses isolated config, data, and cache directories. Tests never touch `~/.rocm`. - **Behavioral language.** Feature files describe what users care about, not implementation details. How steps are implemented (mock vs real, which port, which API) stays in the step functions. - **OS-assigned ports.** The mock server binds to `127.0.0.1:0` to avoid port conflicts between tests. +- **A message is asserted with the state it describes.** A `Then` step matching printed output ("no existing SDK was found", "becomes the active default runtime", "stop them with `rocm services stop `") asserts the resulting state in the same scenario — the file that was or was not written, the runtime that is or is not active, the advice's named command actually clearing the condition. Matching the text alone certifies the wording, not the truth of it, which is how a scenario passes over a CLI that is wrong about what it did. See AGENTS.md §3.