Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
25 changes: 25 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions tests/e2e-cucumber/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <id>`") 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.
Loading