From 7050bc12da6cb916e38728c66c4cd0ab8ff3049c Mon Sep 17 00:00:00 2001 From: VinPropane Date: Mon, 27 Apr 2026 21:54:02 -0400 Subject: [PATCH] =?UTF-8?q?chore:=20AAA=20rules=20=E2=80=94=20no=20require?= =?UTF-8?q?d=20blank=20line=20after=20phase=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .cursor/rules/code-review-agent.md | 2 +- .cursor/rules/csharp-style.md | 19 +++++++------------ .cursor/rules/gdscript-style.md | 12 +++++------- .cursor/rules/testing-expectations.md | 4 ++-- .vscode/csharp.code-snippets | 6 ------ AGENTS.md | 2 +- .../AAA_TEST_METHOD_TEMPLATE.cs.txt | 3 --- 7 files changed, 16 insertions(+), 32 deletions(-) diff --git a/.cursor/rules/code-review-agent.md b/.cursor/rules/code-review-agent.md index 8b67df4..a2ec9de 100644 --- a/.cursor/rules/code-review-agent.md +++ b/.cursor/rules/code-review-agent.md @@ -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. diff --git a/.cursor/rules/csharp-style.md b/.cursor/rules/csharp-style.md index 8aeb37e..4f3e4b9 100644 --- a/.cursor/rules/csharp-style.md +++ b/.cursor/rules/csharp-style.md @@ -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(); Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.NotNull(body); diff --git a/.cursor/rules/gdscript-style.md b/.cursor/rules/gdscript-style.md index aff34ee..9106fb0 100644 --- a/.cursor/rules/gdscript-style.md +++ b/.cursor/rules/gdscript-style.md @@ -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 diff --git a/.cursor/rules/testing-expectations.md b/.cursor/rules/testing-expectations.md index 5eb2f1f..51106c2 100644 --- a/.cursor/rules/testing-expectations.md +++ b/.cursor/rules/testing-expectations.md @@ -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. diff --git a/.vscode/csharp.code-snippets b/.vscode/csharp.code-snippets index 950f8b7..106f6f5 100644 --- a/.vscode/csharp.code-snippets +++ b/.vscode/csharp.code-snippets @@ -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}", " }", "}" diff --git a/AGENTS.md b/AGENTS.md index af78752..854468d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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). diff --git a/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt b/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt index 0c5122f..c00c10a 100644 --- a/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt +++ b/server/NeonSprawl.Server.Tests/TestTemplates/AAA_TEST_METHOD_TEMPLATE.cs.txt @@ -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. }