From 0fe182565e6e4460b46932dd3d9ec68032a375d3 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Tue, 16 Jun 2026 17:26:40 -0400 Subject: [PATCH] =?UTF-8?q?NEO-139:=20address=20review=20=E2=80=94=20trim-?= =?UTF-8?q?path=20tests=20and=20Bruno=20folder=20note.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add whitespace-only 404 and padded-path playerId echo tests; document seq 4 standing side effect in faction-standing folder docs. --- .../faction-standing/folder.bru | 5 +++ docs/plans/NEO-139-implementation-plan.md | 2 +- docs/reviews/2026-06-16-NEO-139.md | 8 +++-- .../Game/Factions/FactionStandingApiTests.cs | 32 +++++++++++++++++++ 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/bruno/neon-sprawl-server/faction-standing/folder.bru b/bruno/neon-sprawl-server/faction-standing/folder.bru index 99cdae0..89b2cb6 100644 --- a/bruno/neon-sprawl-server/faction-standing/folder.bru +++ b/bruno/neon-sprawl-server/faction-standing/folder.bru @@ -1,3 +1,8 @@ meta { name: faction-standing } + +docs { + Run seq 1–3 on a fresh dev player for neutral standing defaults. + Seq 4 (operator chain) leaves Grid Operators standing at 15 — re-run seq 2 only after server restart or standing reset, or expect non-zero values. +} diff --git a/docs/plans/NEO-139-implementation-plan.md b/docs/plans/NEO-139-implementation-plan.md index 1495e78..0124c95 100644 --- a/docs/plans/NEO-139-implementation-plan.md +++ b/docs/plans/NEO-139-implementation-plan.md @@ -58,7 +58,7 @@ Client-readable standing snapshot for all frozen factions. - **API:** `FactionStandingApi.MapFactionStandingApi` — `GET /game/players/{id}/faction-standing`. - **DTOs:** `FactionStandingSnapshotResponse`, `FactionStandingRowJson` (`schemaVersion` 1). - **Gate:** `IPositionStateStore.TryGetPosition` → **404** for unknown player. -- **Tests:** `FactionStandingApiTests` (3 cases); **819** tests green. +- **Tests:** `FactionStandingApiTests` (5 cases: 404 unknown/whitespace-only, trim-path echo, defaults, post-rep); **821** tests green. - **Bruno:** `Get faction standing default.bru`, `unknown player.bru`, `after operator chain.bru`. - **Docs:** `server/README.md` Faction standing read section; E7.M3 module + alignment register updated. diff --git a/docs/reviews/2026-06-16-NEO-139.md b/docs/reviews/2026-06-16-NEO-139.md index b51c9bb..1eef8d4 100644 --- a/docs/reviews/2026-06-16-NEO-139.md +++ b/docs/reviews/2026-06-16-NEO-139.md @@ -4,6 +4,8 @@ **Scope:** Branch `NEO-139-e7m3-get-faction-standing-http-read` — commits `5fc780c` … `3f08812` vs `main` **Base:** `main` +Follow-up: suggestion + applicable nits below are **done** (strikethrough + **Done.**). + ## Verdict **Approve with nits** @@ -29,14 +31,14 @@ None. ## Suggestions -1. **Add trim-path integration test.** The plan §4 prototype scenario table lists “GET with trimmed id → **200**; body **`playerId`** echoes trimmed path” (skill/gig snapshot precedent). The handler trims (`id.Trim()`) but no test exercises whitespace-padded path segments or asserts echoed **`playerId`**. A small fourth `[Fact]` would lock the contract. +1. ~~**Add trim-path integration test.** The plan §4 prototype scenario table lists “GET with trimmed id → **200**; body **`playerId`** echoes trimmed path” (skill/gig snapshot precedent). The handler trims (`id.Trim()`) but no test exercises whitespace-padded path segments or asserts echoed **`playerId`**. A small fourth `[Fact]` would lock the contract.~~ **Done.** Added `GetFactionStanding_ShouldReturnOk_WhenPathIdHasLeadingAndTrailingWhitespace` and `GetFactionStanding_ShouldReturnNotFound_WhenPathIdIsWhitespaceOnly` in `FactionStandingApiTests.cs`. ## Nits -- Nit: Empty or whitespace-only `{id}` returns **404** (trim → empty) but is not covered by tests — low risk given identical gate to other snapshot APIs. +- ~~Nit: Empty or whitespace-only `{id}` returns **404** (trim → empty) but is not covered by tests — low risk given identical gate to other snapshot APIs.~~ **Done.** Whitespace-only path covered by `GetFactionStanding_ShouldReturnNotFound_WhenPathIdIsWhitespaceOnly`. - Nit: **`BuildSnapshot`** (throws on failure) is exposed for reuse while HTTP uses **`TryBuildSnapshot`** — fine; no direct unit tests call **`BuildSnapshot`** today. - Nit: **`FactionStandingApiTests`** duplicates a **`RewardRouterTestDependencies`** helper record also seen in reward-router tests — acceptable for now; extract only if a third caller appears. -- Nit: Bruno **`Get faction standing default.bru`** (seq 2) documents Postgres/restart caveats; re-running the folder on a warm in-memory server after seq 4 can leave standing at **15** — docs already warn; optional one-line note in folder README if authors hit flaky local runs. +- ~~Nit: Bruno **`Get faction standing default.bru`** (seq 2) documents Postgres/restart caveats; re-running the folder on a warm in-memory server after seq 4 can leave standing at **15** — docs already warn; optional one-line note in folder README if authors hit flaky local runs.~~ **Done.** `faction-standing/folder.bru` docs note seq 4 side effect and seq 2 re-run caveat. ## Verification diff --git a/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingApiTests.cs index 4b7025a..bffc18b 100644 --- a/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Factions/FactionStandingApiTests.cs @@ -33,6 +33,38 @@ public sealed class FactionStandingApiTests Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); } + [Fact] + public async Task GetFactionStanding_ShouldReturnNotFound_WhenPathIdIsWhitespaceOnly() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.GetAsync("/game/players/%20%20/faction-standing"); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Fact] + public async Task GetFactionStanding_ShouldReturnOk_WhenPathIdHasLeadingAndTrailingWhitespace() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.GetAsync("/game/players/%20dev-local-1%20/faction-standing"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(body); + Assert.Equal("dev-local-1", body!.PlayerId); + Assert.Equal(FactionStandingSnapshotResponse.CurrentSchemaVersion, body.SchemaVersion); + } + [Fact] public async Task GetFactionStanding_ShouldReturnSchemaV1_WithNeutralStanding_ForPrototypeFactions() {