From 3ba830065206a7e01b5d7c56b952012a962ac2f3 Mon Sep 17 00:00:00 2001 From: VinPropane Date: Sun, 17 May 2026 20:21:42 -0400 Subject: [PATCH] =?UTF-8?q?NEO-48:=20address=20code=20review=20=E2=80=94?= =?UTF-8?q?=20Bruno=20deny,=20API=20deny=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add self-contained branch_already_chosen Bruno request, HTTP deny smoke for unknown track/tier/branch and null-body 400, folder docs for level_too_low. --- ...anch select deny branch already chosen.bru | 72 ++++++++++++++ .../neon-sprawl-server/perk-state/folder.bru | 6 ++ docs/manual-qa/NEO-48.md | 1 + docs/reviews/2026-05-17-NEO-48.md | 11 ++- .../Game/Mastery/PerkStateApiTests.cs | 93 ++++++++++++++++++- 5 files changed, 175 insertions(+), 8 deletions(-) create mode 100644 bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru diff --git a/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru new file mode 100644 index 0000000..c64f45d --- /dev/null +++ b/bruno/neon-sprawl-server/perk-state/Post branch select deny branch already chosen.bru @@ -0,0 +1,72 @@ +meta { + name: POST branch select deny branch already chosen + type: http + seq: 4 +} + +docs { + NEO-48: self-contained deny — pre-request grants salvage to level 2 and picks scrap_efficiency; + main request conflicts with bulk_haul on the same tier. Expects selected false + branch_already_chosen. +} + +script:pre-request { + const baseUrl = bru.getVar("baseUrl"); + const grant = await bru.sendRequest({ + method: "POST", + url: `${baseUrl}/game/players/dev-local-1/skill-progression`, + headers: { "Content-Type": "application/json" }, + data: { + schemaVersion: 1, + skillId: "salvage", + amount: 100, + sourceKind: "activity", + }, + }); + if (grant.status !== 200) { + throw new Error(`salvage grant failed: ${grant.status}`); + } + const firstPick = await bru.sendRequest({ + method: "POST", + url: `${baseUrl}/game/players/dev-local-1/perk-state`, + headers: { "Content-Type": "application/json" }, + data: { + schemaVersion: 1, + skillId: "salvage", + tierIndex: 1, + branchId: "scrap_efficiency", + }, + }); + if (firstPick.status !== 200 || firstPick.data?.selected !== true) { + throw new Error(`tier-1 pick failed: ${firstPick.status} ${JSON.stringify(firstPick.data)}`); + } +} + +post { + url: {{baseUrl}}/game/players/dev-local-1/perk-state + body: json + auth: none +} + +body:json { + { + "schemaVersion": 1, + "skillId": "salvage", + "tierIndex": 1, + "branchId": "bulk_haul" + } +} + +tests { + test("denies second pick with branch_already_chosen", function () { + expect(res.getStatus()).to.equal(200); + const body = res.getBody(); + expect(body.schemaVersion).to.equal(1); + expect(body.selected).to.equal(false); + expect(body.reasonCode).to.equal("branch_already_chosen"); + expect(body.unlockedEvents).to.be.an("array").that.is.empty; + const salvage = body.perkState.branchPicks.find((t) => t.skillId === "salvage"); + expect(salvage).to.be.an("object"); + const pick = salvage.picks.find((p) => p.tierIndex === 1); + expect(pick.branchId).to.equal("scrap_efficiency"); + }); +} diff --git a/bruno/neon-sprawl-server/perk-state/folder.bru b/bruno/neon-sprawl-server/perk-state/folder.bru index 62bd4c9..3a755c2 100644 --- a/bruno/neon-sprawl-server/perk-state/folder.bru +++ b/bruno/neon-sprawl-server/perk-state/folder.bru @@ -1,3 +1,9 @@ meta { name: perk-state } + +docs { + NEO-48 perk-state HTTP smoke. Run folder in seq order for full coverage. + Post branch select deny level too low: passes if salvage is still level 1; if dev-local-1 already has salvage XP from other runs, restart the server (or reset DB in Postgres mode) before expecting level_too_low. + Post branch select deny branch already chosen: self-contained (pre-request grant + first pick); safe to re-run. +} diff --git a/docs/manual-qa/NEO-48.md b/docs/manual-qa/NEO-48.md index 21b1c3c..a6e4506 100644 --- a/docs/manual-qa/NEO-48.md +++ b/docs/manual-qa/NEO-48.md @@ -44,6 +44,7 @@ curl -sS -X POST "http://localhost:5253/game/players/dev-local-1/perk-state" \ ## Bruno - [ ] Run folder `bruno/neon-sprawl-server/perk-state/` (GET + POST happy + deny smoke). +- [ ] Run **`Post branch select deny branch already chosen`** (self-contained; pre-request grant + first pick). ## Regression diff --git a/docs/reviews/2026-05-17-NEO-48.md b/docs/reviews/2026-05-17-NEO-48.md index f25ae3f..b2a376f 100644 --- a/docs/reviews/2026-05-17-NEO-48.md +++ b/docs/reviews/2026-05-17-NEO-48.md @@ -2,7 +2,8 @@ **Date:** 2026-05-17 **Scope:** Branch `NEO-48-perk-state-get-branch-selection-post-bruno` · commits `5290044`–`39c54c3` vs `main` -**Base:** `main` +**Base:** `main` +**Follow-up:** Code-review suggestions 1–3 and nit 1 addressed in a follow-up commit on the same branch. ## Verdict @@ -31,13 +32,13 @@ None. ## Suggestions -1. **Bruno deny for `branch_already_chosen`** — Plan and manual QA describe a second POST to the same tier with a different branch; automated tests cover it (`PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick`), but Bruno only ships `Post branch select deny level too low.bru`. A fourth request (grant + first pick + conflicting second pick) would make the collection self-contained for CI/manual runs without relying on manual QA alone. -2. **Optional thin HTTP deny smoke** — Engine tests lock `unknown_track`, `unknown_tier`, and `unknown_branch`; one API test each (bad `skillId`, bad `tierIndex`, bad `branchId` on dev player after level gate) would guard JSON field mapping if DTOs change. Non-blocking given `PerkUnlockEngineTests` coverage. -3. **POST null body** — Handlers return **400** for `body is null` (same as skill-progression). Neither API suite asserts empty-body **400** today; add a one-liner in `PerkStateApiTests` if you want parity documentation in tests. +1. ~~**Bruno deny for `branch_already_chosen`** — Plan and manual QA describe a second POST to the same tier with a different branch; automated tests cover it (`PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick`), but Bruno only ships `Post branch select deny level too low.bru`. A fourth request (grant + first pick + conflicting second pick) would make the collection self-contained for CI/manual runs without relying on manual QA alone.~~ **Done.** Added `Post branch select deny branch already chosen.bru` with `script:pre-request` grant + first pick via `bru.sendRequest`. +2. ~~**Optional thin HTTP deny smoke** — Engine tests lock `unknown_track`, `unknown_tier`, and `unknown_branch`; one API test each (bad `skillId`, bad `tierIndex`, bad `branchId` on dev player after level gate) would guard JSON field mapping if DTOs change. Non-blocking given `PerkUnlockEngineTests` coverage.~~ **Done.** `PostPerkState_ShouldDenyUnknownTrack_*`, `UnknownTier_*`, `UnknownBranch_*` in `PerkStateApiTests`. +3. ~~**POST null body** — Handlers return **400** for `body is null` (same as skill-progression). Neither API suite asserts empty-body **400** today; add a one-liner in `PerkStateApiTests` if you want parity documentation in tests.~~ **Done.** `PostPerkState_ShouldReturnBadRequest_WhenBodyNull`. ## Nits -- Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often. +- ~~Nit: `Post branch select deny level too low.bru` intentionally passes when salvage is already leveled (restart note) — acceptable for shared dev player; document in Bruno folder `docs` if operators hit false greens often.~~ **Done.** Folder `docs` block in `perk-state/folder.bru`. - Nit: Path-auto tier-2 unlocks surfaced in **`unlockedEvents`** use **`source: level_up`** (not `branch_pick`) — consistent with NEO-47/NEO-49 notes; clients/telemetry should treat `source` as unlock channel, not HTTP verb. - Nit: `PostPerkState_ShouldSelectTier1_AndGetReflectsPick_AfterSalvageLevel2` issues a follow-up **GET** inside **Assert** for end-to-end verification — valid AAA (verification in Assert); grant status checks in **Arrange** match other API tests in the repo. diff --git a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs index 7ee0265..d83c843 100644 --- a/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Mastery/PerkStateApiTests.cs @@ -1,6 +1,8 @@ using System.Linq; using System.Net; +using System.Net.Http; using System.Net.Http.Json; +using System.Text; using NeonSprawl.Server.Game.Mastery; using NeonSprawl.Server.Game.Skills; using Xunit; @@ -67,6 +69,22 @@ public sealed class PerkStateApiTests Assert.Empty(body.UnlockedPerkIds); } + [Fact] + public async Task PostPerkState_ShouldReturnBadRequest_WhenBodyNull() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + + // Act + var response = await client.PostAsync( + $"/game/players/{DevPlayer}/perk-state", + new StringContent(string.Empty, Encoding.UTF8, "application/json")); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + } + [Fact] public async Task PostPerkState_ShouldReturnBadRequest_WhenSchemaVersionMismatch() { @@ -156,15 +174,84 @@ public sealed class PerkStateApiTests Assert.Equal("scrap_efficiency", Assert.Single(getTrack.Picks).BranchId); } + private static async Task GrantSalvageLevel2Async(HttpClient client) + { + var grant = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/skill-progression", + SalvageGrant(100)); + Assert.Equal(HttpStatusCode.OK, grant.StatusCode); + } + + [Fact] + public async Task PostPerkState_ShouldDenyUnknownTrack_WhenSkillIdInvalid() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await GrantSalvageLevel2Async(client); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("refine", 1, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.UnknownTrack, envelope.ReasonCode); + } + + [Fact] + public async Task PostPerkState_ShouldDenyUnknownTier_WhenTierIndexInvalid() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await GrantSalvageLevel2Async(client); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 99, "scrap_efficiency")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.UnknownTier, envelope.ReasonCode); + } + + [Fact] + public async Task PostPerkState_ShouldDenyUnknownBranch_WhenBranchIdInvalid() + { + // Arrange + await using var factory = new InMemoryWebApplicationFactory(); + var client = factory.CreateClient(); + await GrantSalvageLevel2Async(client); + + // Act + var response = await client.PostAsJsonAsync( + $"/game/players/{DevPlayer}/perk-state", + SelectBranch("salvage", 1, "not_a_branch")); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var envelope = await response.Content.ReadFromJsonAsync(); + Assert.NotNull(envelope); + Assert.False(envelope!.Selected); + Assert.Equal(PerkUnlockReasonCodes.UnknownBranch, envelope.ReasonCode); + } + [Fact] public async Task PostPerkState_ShouldDenyBranchAlreadyChosen_OnSecondPick() { // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); - Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( - $"/game/players/{DevPlayer}/skill-progression", - SalvageGrant(100))).StatusCode); + await GrantSalvageLevel2Async(client); Assert.Equal(HttpStatusCode.OK, (await client.PostAsJsonAsync( $"/game/players/{DevPlayer}/perk-state", SelectBranch("salvage", 1, "scrap_efficiency"))).StatusCode);