chore: AAA rules — no required blank line after phase comments
parent
31f9b74d3f
commit
7050bc12da
|
|
@ -46,7 +46,7 @@ Work through what applies to the diff (skip irrelevant sections briefly).
|
|||
2. **APIs & contracts** — Breaking changes, versioning, serialization shapes, documented public surface.
|
||||
3. **Security** — Injection, secrets in repo, authz/authn assumptions, unsafe defaults.
|
||||
4. **Performance** — Obvious hot-path allocations or N+1 patterns in new code only; avoid speculative micro-optimization.
|
||||
5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. **C# (`*Tests.cs`):** full AAA per [csharp-style](csharp-style.md#unit-and-integration-tests-arrange-act-assert) — labels plus **blank line after each**, Act-only invocation(s), **verification reads in Assert**. **GdUnit (`client/test/`):** same AAA with **`# Arrange` / `# Act` / `# Assert`** per [gdscript-style](gdscript-style.md#gdunit-test-layout-aaa). Treat missing or partial AAA on **any touched** test method as **should fix** (or **blocking** if the change is test-heavy); only **nit** when the diff is a one-line fix inside a legacy method you did not own end-to-end **and** AAA was already complete.
|
||||
5. **Tests** — Happy path, failure modes, integration boundaries per [testing-expectations](testing-expectations.md); note gaps. **C# (`*Tests.cs`):** full AAA per [csharp-style](csharp-style.md#unit-and-integration-tests-arrange-act-assert) — phase labels, Act-only invocation(s), **verification reads in Assert**. **GdUnit (`client/test/`):** same AAA with **`# Arrange` / `# Act` / `# Assert`** per [gdscript-style](gdscript-style.md#gdunit-test-layout-aaa). Treat missing or partial AAA on **any touched** test method as **should fix** (or **blocking** if the change is test-heavy); only **nit** when the diff is a one-line fix inside a legacy method you did not own end-to-end **and** AAA was already complete.
|
||||
6. **Observability** — Logging/metrics where failures would be hard to diagnose (server/runtime code).
|
||||
7. **Docs** — README or plan updates when behavior or run instructions change.
|
||||
8. **Plan & decomposition alignment** — Per **Plan and decomposition documentation** above: relevant `docs/plans/` and module/policy docs cited; implementation checked against them; **Documentation checked** section in the saved review file.
|
||||
|
|
|
|||
|
|
@ -89,25 +89,23 @@ public async Task PostMove_ShouldReturnBadRequest_WhenSchemaVersionIsWrong() {
|
|||
|
||||
## Unit and integration tests (Arrange, Act, Assert)
|
||||
|
||||
**Mandatory for every new or changed test method** in `*Tests.cs` (`[Fact]` / `[Theory]` bodies). Do not ship partial AAA (“comments present but body read still in Act,” missing blank lines, etc.). Agents and humans must treat this as **merge-ready** layout, not a suggestion.
|
||||
**Mandatory for every new or changed test method** in `*Tests.cs` (`[Fact]` / `[Theory]` bodies). Do not ship partial AAA (e.g. comments present but body read still in Act). Agents and humans must treat this as **merge-ready** layout, not a suggestion.
|
||||
|
||||
### Required layout
|
||||
|
||||
1. **Phase labels** — Each test method contains **`// Arrange`**, **`// Act`**, and **`// Assert`** exactly once, in that order, each on its own line (same indentation as the test body). Use these exact words so grep and review stay consistent.
|
||||
1. **Phase labels** — Each test method contains **`// Arrange`**, **`// Act`**, and **`// Assert`** exactly once, in that order, each on its own line (same indentation as the test body). Use these exact words so grep and review stay consistent. **No blank line is required** immediately after those comments; the first statement of each phase may follow on the next line.
|
||||
|
||||
2. **Blank line after each label** — Insert a **blank line** immediately after `// Arrange`, after `// Act`, and after `// Assert` before the first statement of that phase. **Required** (not optional): it matches the template, satisfies [code-review-agent](code-review-agent.md) expectations, and makes phases obvious in diff view.
|
||||
2. **Arrange** — Factories, `HttpClient`, queued mock transports, DTOs, seeds, and other setup. Do **not** assert the **outcome under test** in Arrange (rare guards on arrange-only helpers are acceptable).
|
||||
|
||||
3. **Arrange** — Factories, `HttpClient`, queued mock transports, DTOs, seeds, and other setup. Do **not** assert the **outcome under test** in Arrange (rare guards on arrange-only helpers are acceptable).
|
||||
3. **Act** — Invoke the **behavior under test** only (e.g. one `PostAsJsonAsync`, `GetAsync`, or SUT call). If the scenario *is* a short sequence (e.g. POST then GET to prove persistence), keep the whole sequence in Act with **no** `Assert.*` between those calls.
|
||||
|
||||
4. **Act** — Invoke the **behavior under test** only (e.g. one `PostAsJsonAsync`, `GetAsync`, or SUT call). If the scenario *is* a short sequence (e.g. POST then GET to prove persistence), keep the whole sequence in Act with **no** `Assert.*` between those calls.
|
||||
4. **Assert** — All `Assert.*` (and any other outcome checks). **HTTP / deserialization:** when the next step is verifying the response (status already observed, body for assertions), perform **`ReadFromJsonAsync`** / similar **reads here**, not in Act beside the POST. Reads that **drive** the next call belong in Act, not Assert.
|
||||
|
||||
5. **Assert** — All `Assert.*` (and any other outcome checks). **HTTP / deserialization:** when the next step is verifying the response (status already observed, body for assertions), perform **`ReadFromJsonAsync`** / similar **reads here**, not in Act beside the POST. Reads that **drive** the next call belong in Act, not Assert.
|
||||
|
||||
6. **One AAA triple per test method** — Each `[Fact]` or `[Theory]` case gets its own Arrange/Act/Assert; do not share a single Assert block across unrelated scenarios in one method.
|
||||
5. **One AAA triple per test method** — Each `[Fact]` or `[Theory]` case gets its own Arrange/Act/Assert; do not share a single Assert block across unrelated scenarios in one method.
|
||||
|
||||
### Authoring defaults
|
||||
|
||||
- Use VS Code snippet **`xut`** (method) or **`xutc`** (class) from `.vscode/csharp.code-snippets`, or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`** (includes blank lines after labels).
|
||||
- Use VS Code snippet **`xut`** (method) or **`xutc`** (class) from `.vscode/csharp.code-snippets`, or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`**.
|
||||
|
||||
### Example (minimal HTTP integration)
|
||||
|
||||
|
|
@ -116,17 +114,14 @@ public async Task PostMove_ShouldReturnBadRequest_WhenSchemaVersionIsWrong() {
|
|||
public async Task PostExample_ShouldReturnOk_WhenBodyValid()
|
||||
{
|
||||
// Arrange
|
||||
|
||||
await using var factory = new InMemoryWebApplicationFactory();
|
||||
var client = factory.CreateClient();
|
||||
var request = new { schemaVersion = 1 };
|
||||
|
||||
// Act
|
||||
|
||||
var response = await client.PostAsJsonAsync("/game/example", request);
|
||||
|
||||
// Assert
|
||||
|
||||
var body = await response.Content.ReadFromJsonAsync<ExampleResponse>();
|
||||
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
|
||||
Assert.NotNull(body);
|
||||
|
|
|
|||
|
|
@ -66,17 +66,15 @@ CI also enforces **`gdlint`** and **`gdformat`** (see `gdlintrc` and `.github/wo
|
|||
|
||||
**Mandatory** for every **new or changed** test function in **`client/test/**/*.gd`** (GdUnit suites, `test_*` methods).
|
||||
|
||||
1. **`# Arrange`**, **`# Act`**, and **`# Assert`** — each appears **once** per test, in that order, as a full-line comment at the same indentation as the test body.
|
||||
1. **`# Arrange`**, **`# Act`**, and **`# Assert`** — each appears **once** per test, in that order, as a full-line comment at the same indentation as the test body. **No blank line is required** after those comments; the next statement may follow immediately.
|
||||
|
||||
2. **Blank line after each label** — Required: put a blank line immediately after `# Arrange`, after `# Act`, and after `# Assert` before the first line of that phase (same rule as [C# AAA](csharp-style.md#unit-and-integration-tests-arrange-act-assert)).
|
||||
2. **Arrange** — Transports, clients, `enqueue`/`connect` setup, test data.
|
||||
|
||||
3. **Arrange** — Transports, clients, `enqueue`/`connect` setup, test data.
|
||||
3. **Act** — The call(s) under test only (e.g. `request_cast`, `call` into SUT). No `assert_that` / `assert_*` in Act.
|
||||
|
||||
4. **Act** — The call(s) under test only (e.g. `request_cast`, `call` into SUT). No `assert_that` / `assert_*` in Act.
|
||||
4. **Assert** — All expectations; parse or inspect results here when parsing exists only to support assertions.
|
||||
|
||||
5. **Assert** — All expectations; parse or inspect results here when parsing exists only to support assertions.
|
||||
|
||||
`gdlint` / `gdformat` do not enforce AAA text; **review and agents** do. Match the spirit of server **`xut`** snippets: three labeled phases, blank lines, one scenario per test.
|
||||
`gdlint` / `gdformat` do not enforce AAA text; **review and agents** do. Match the spirit of server **`xut`** snippets: three labeled phases, one scenario per test.
|
||||
|
||||
## Comments
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ alwaysApply: true
|
|||
|
||||
- **Non-trivial logic** (validation, simulation rules, serialization boundaries, idempotency, security-sensitive paths) should have **automated tests** once a test project exists in the repo.
|
||||
- **Default framework:** **xUnit** for new test projects unless the repo already standardizes on something else—stay consistent with existing `*.Tests` projects.
|
||||
- **Layout (mandatory for new/changed tests):** every **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **AAA**: **`// Arrange`**, **`// Act`**, **`// Assert`** on their own lines, a **blank line after each label**, **Act** limited to the behavior under test, and **response-body reads for verification in Assert** (not mixed into Act with the HTTP call). See [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). Treat anything less as **incomplete** for merge.
|
||||
- **Layout (mandatory for new/changed tests):** every **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **AAA**: **`// Arrange`**, **`// Act`**, **`// Assert`** on their own lines, **Act** limited to the behavior under test, and **response-body reads for verification in Assert** (not mixed into Act with the HTTP call). A blank line after each phase comment is **not** required. See [C# style — Unit and integration tests](csharp-style.md#unit-and-integration-tests-arrange-act-assert). Treat anything less as **incomplete** for merge.
|
||||
- **At-write-time default:** start new tests with snippet **`xut`** / **`xutc`** from `.vscode/csharp.code-snippets` or copy **`server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt`** so blank-line AAA is present from the first keystroke.
|
||||
- **Names:** **`MethodName_ShouldExpectedOutcome_WhenScenario`**; see [C# style — Test method naming convention](csharp-style.md#test-method-naming-convention).
|
||||
- **No test project yet:** It is acceptable to ship a small change without a suite only while the server is mostly scaffolding. When you add the **first** testable module, **add a `NeonSprawl.Server.Tests` (or equivalent) project** and start covering that area; do not leave new core logic permanently untested.
|
||||
|
|
@ -26,7 +26,7 @@ alwaysApply: true
|
|||
## Godot client (GDScript)
|
||||
|
||||
- **Harness (NEO-12):** **GdUnit4** lives under **`client/addons/gdUnit4/`**; suites under **`client/test/`**. **`client/README.md`** documents local and headless runs; **`.github/workflows/gdscript.yml`** runs **gdlint**, **gdformat**, and **headless GdUnit** when `client/scripts/`, `client/test/`, `client/addons/`, or `client/project.godot` change.
|
||||
- **GdUnit tests (`client/test/**/*.gd`):** every **new or changed** test function must use the same **AAA** discipline as C#: **`# Arrange`**, **`# Act`**, **`# Assert`** each on its own line, in order, with a **blank line after each label** before that phase’s statements. Act holds only the invocation(s) under test; assertions (and any parsing done purely to assert) live under Assert. See [gdscript-style — GdUnit test layout (AAA)](gdscript-style.md#gdunit-test-layout-aaa).
|
||||
- **GdUnit tests (`client/test/**/*.gd`):** every **new or changed** test function must use the same **AAA** discipline as C#: **`# Arrange`**, **`# Act`**, **`# Assert`** each on its own line, in order. A blank line after each label is **not** required. Act holds only the invocation(s) under test; assertions (and any parsing done purely to assert) live under Assert. See [gdscript-style — GdUnit test layout (AAA)](gdscript-style.md#gdunit-test-layout-aaa).
|
||||
- **Production script changes:** Any PR that **adds or changes** GDScript under **`client/scripts/`** (and any other **application** `.gd` outside **`client/addons/`** and **`client/test/`**) must **add or update** tests under **`client/test/`** in the same change set. If a change is **not** reasonably testable, say so in the PR (or story plan) with a short reason—do not skip tests silently.
|
||||
- **Scenes / assets / manual checks:** Scene tweaks, visuals, and flows that unit tests do not cover still deserve **manual verification** notes in the PR when behavior could regress.
|
||||
|
||||
|
|
|
|||
|
|
@ -8,15 +8,12 @@
|
|||
"public async Task ${1:MethodName}_Should${2:ExpectedOutcome}_When${3:Scenario}()",
|
||||
"{",
|
||||
" // Arrange",
|
||||
"",
|
||||
" ${4:// setup}",
|
||||
"",
|
||||
" // Act",
|
||||
"",
|
||||
" ${5:// execute}",
|
||||
"",
|
||||
" // Assert",
|
||||
"",
|
||||
" ${6:// verify}",
|
||||
"}"
|
||||
]
|
||||
|
|
@ -36,15 +33,12 @@
|
|||
" public async Task ${3:MethodName}_Should${4:ExpectedOutcome}_When${5:Scenario}()",
|
||||
" {",
|
||||
" // Arrange",
|
||||
"",
|
||||
" ${6:// setup}",
|
||||
"",
|
||||
" // Act",
|
||||
"",
|
||||
" ${7:// execute}",
|
||||
"",
|
||||
" // Assert",
|
||||
"",
|
||||
" ${8:// verify}",
|
||||
" }",
|
||||
"}"
|
||||
|
|
|
|||
|
|
@ -13,6 +13,6 @@ Optional **personas** for Cursor. Enable by **@ mentioning** the rule in chat or
|
|||
| **Code review** | [`.cursor/rules/code-review-agent.md`](.cursor/rules/code-review-agent.md) | PR / diff / pre-merge review; **always writes** `docs/reviews/YYYY-MM-DD-{slug}.md` with **Documentation checked** vs `docs/plans/` and `docs/decomposition/modules/`; **when suggestions are fixed, strike through those bullets + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); short chat pointer |
|
||||
| **Docs review** | [`.cursor/rules/docs-review-agent.md`](.cursor/rules/docs-review-agent.md) | Coherence / links / dev-guide fitness for `docs/` (especially `docs/game-design/` + decomposition); **writes** `docs/reviews/YYYY-MM-DD-{slug}.md`; **when suggestions are fixed, strike through + `Done.` in that file** ([planning-implementation-docs](.cursor/rules/planning-implementation-docs.md)); use when working in **documents** or @ mention |
|
||||
|
||||
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Automated tests (AAA, mandatory):** **C#** — every new or changed **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **Arrange → Act → Assert**: the three **`// Arrange` / `// Act` / `// Assert`** labels, a **blank line after each label**, **Act** = behavior under test only, **Assert** = all expectations including **`ReadFromJsonAsync`** (etc.) for verification — [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), [testing-expectations](.cursor/rules/testing-expectations.md); snippets **`xut`** / **`xutc`**. **GdUnit (`client/test/`)** — same AAA with **`# Arrange` / `# Act` / `# Assert`** and blank lines after each — [gdscript-style](.cursor/rules/gdscript-style.md#gdunit-test-layout-aaa). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use the **GitHub MCP** **`create_pull_request`** tool (`user-github`) with a title starting **`NEO-123:`** ([commit-and-review](.cursor/rules/commit-and-review.md)); **do not use `gh` CLI for GitHub operations in this repo**. **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
|
||||
Project-wide conventions live under [`.cursor/rules/`](.cursor/rules/) (many are `alwaysApply`). **Planning / implementation decisions** must be written into `docs/plans/` (and related docs)—[`.cursor/rules/planning-implementation-docs.md`](.cursor/rules/planning-implementation-docs.md). **Manual QA checklists** for user-visible ticketed work are generated during implementation at `docs/manual-qa/{KEY}.md` (same rule). **Automated tests (AAA, mandatory):** **C#** — every new or changed **`[Fact]` / `[Theory]`** method in **`*Tests.cs`** must use full **Arrange → Act → Assert**: the three **`// Arrange` / `// Act` / `// Assert`** labels, **Act** = behavior under test only, **Assert** = all expectations including **`ReadFromJsonAsync`** (etc.) for verification (no blank line required after the phase comments) — [csharp-style](.cursor/rules/csharp-style.md#unit-and-integration-tests-arrange-act-assert), [testing-expectations](.cursor/rules/testing-expectations.md); snippets **`xut`** / **`xutc`**. **GdUnit (`client/test/`)** — same AAA with **`# Arrange` / `# Act` / `# Assert`** — [gdscript-style](.cursor/rules/gdscript-style.md#gdunit-test-layout-aaa). **Godot client layout** (thin `main.gd`, split by concern): [`.cursor/rules/godot-client-script-organization.md`](.cursor/rules/godot-client-script-organization.md). **Git:** agents may **commit** at discretion on story/ticket work; **never** `git push` unless the user asks — [`.cursor/rules/commit-and-review.md`](.cursor/rules/commit-and-review.md). **Open PR:** when the user asks, use the **GitHub MCP** **`create_pull_request`** tool (`user-github`) with a title starting **`NEO-123:`** ([commit-and-review](.cursor/rules/commit-and-review.md)); **do not use `gh` CLI for GitHub operations in this repo**. **Story lifecycle:** kickoff [`.cursor/rules/story-kickoff.md`](.cursor/rules/story-kickoff.md); after merge / end of story — `checkout main`, `pull`, delete local story branch — [`.cursor/rules/story-end.md`](.cursor/rules/story-end.md). **Linear:** on **end story**, use **`AskQuestion`** multiple choice for **In Test** / **Done** / **Skip** when Agent mode supports it ([story-end](.cursor/rules/story-end.md) §4a); otherwise ask in prose. Wait for the choice (or the same message naming the state) **before** `save_issue`; do not guess. **PR / push text:** no “Made-with: Cursor” boilerplate (same file).
|
||||
|
||||
**Commits tied to a Linear issue** must start the subject with the **issue id** and a colon (e.g. `NEO-8: …`). Branch naming and exceptions (`chore:` when there is no ticket) — [`.cursor/rules/linear-git-naming.md`](.cursor/rules/linear-git-naming.md).
|
||||
|
|
|
|||
|
|
@ -2,14 +2,11 @@
|
|||
public async Task MethodName_ShouldExpectedOutcome_WhenScenario()
|
||||
{
|
||||
// Arrange
|
||||
|
||||
// Create factory/client/dependencies and test data.
|
||||
|
||||
// Act
|
||||
|
||||
// Invoke exactly one behavior/scenario under test.
|
||||
|
||||
// Assert
|
||||
|
||||
// Verify outcome(s) with Assert.* calls; read HTTP/deserialized bodies here when verifying responses.
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue