diff --git a/bruno/neon-sprawl-server/quest-progress/Get quest progress default.bru b/bruno/neon-sprawl-server/quest-progress/Get quest progress default.bru index 25cc15f..164412e 100644 --- a/bruno/neon-sprawl-server/quest-progress/Get quest progress default.bru +++ b/bruno/neon-sprawl-server/quest-progress/Get quest progress default.bru @@ -32,7 +32,7 @@ tests { "prototype_quest_operator_chain", "prototype_quest_refine_intro", ]; - const actualIds = body.quests.map((row) => row.questId).sort(); + const actualIds = body.quests.map((row) => row.questId); expect(actualIds).to.eql(expectedIds); for (const row of body.quests) { expect(row.status).to.equal("not_started"); diff --git a/docs/reviews/2026-06-07-NEO-119.md b/docs/reviews/2026-06-07-NEO-119.md index bb4b0ef..76f3167 100644 --- a/docs/reviews/2026-06-07-NEO-119.md +++ b/docs/reviews/2026-06-07-NEO-119.md @@ -32,9 +32,9 @@ NEO-119 adds **`GET /game/players/{id}/quest-progress`** — a versioned v1 snap ## Suggestions -1. **Assert response array order explicitly** — `GetQuestProgress_ShouldReturnNotStartedForAllQuests_WhenNoProgress` and the Bruno default test compare **sorted** quest ids. The plan and DTO doc promise catalog **`id`** ordinal order (same as **`GetDefinitionsInIdOrder()`**). For the fixed four-quest prototype, sorted vs sequence is equivalent today, but asserting sequence (as in `QuestDefinitionRegistryTests`) would lock the contract if ids ever stop sorting identically to registry order. +1. ~~**Assert response array order explicitly** — `GetQuestProgress_ShouldReturnNotStartedForAllQuests_WhenNoProgress` and the Bruno default test compare **sorted** quest ids. The plan and DTO doc promise catalog **`id`** ordinal order (same as **`GetDefinitionsInIdOrder()`**). For the fixed four-quest prototype, sorted vs sequence is equivalent today, but asserting sequence (as in `QuestDefinitionRegistryTests`) would lock the contract if ids ever stop sorting identically to registry order.~~ **Done.** Test asserts `GetDefinitionsInIdOrder()` sequence; Bruno compares response order without sort. -2. **Side-effecting GET test store read-back** — `GetQuestProgress_ShouldCompleteChainTerminal_WhenTokenHeldAndInventoryRefreshRunsOnGet` asserts HTTP `completed` and pre-GET `Active`, but does not re-read **`IPlayerQuestStateStore`** after GET. Adding a post-GET store assertion would strengthen coverage of the documented mutating read semantics (mirrors the `beforeGet` guard already in Arrange). +2. ~~**Side-effecting GET test store read-back** — `GetQuestProgress_ShouldCompleteChainTerminal_WhenTokenHeldAndInventoryRefreshRunsOnGet` asserts HTTP `completed` and pre-GET `Active`, but does not re-read **`IPlayerQuestStateStore`** after GET. Adding a post-GET store assertion would strengthen coverage of the documented mutating read semantics (mirrors the `beforeGet` guard already in Arrange).~~ **Done.** Post-GET `TryGetProgress` asserts `Completed`, terminal step index, and `CompletedAt`. ## Nits diff --git a/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs b/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs index 0e37ca2..ef1d379 100644 --- a/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs +++ b/server/NeonSprawl.Server.Tests/Game/Quests/QuestProgressApiTests.cs @@ -58,6 +58,8 @@ public sealed class QuestProgressApiTests // Arrange await using var factory = new InMemoryWebApplicationFactory(); var client = factory.CreateClient(); + var registry = factory.Services.GetRequiredService(); + var expectedQuestOrder = registry.GetDefinitionsInIdOrder().Select(static d => d.Id).ToArray(); // Act var response = await client.GetAsync($"/game/players/{PlayerId}/quest-progress"); @@ -68,10 +70,8 @@ public sealed class QuestProgressApiTests Assert.NotNull(body); Assert.Equal(QuestProgressListResponse.CurrentSchemaVersion, body!.SchemaVersion); Assert.Equal(PlayerId, body.PlayerId); - Assert.Equal(PrototypeE7M1QuestCatalogRules.ExpectedQuestIds.Count, body.Quests.Count); - Assert.Equal( - PrototypeE7M1QuestCatalogRules.ExpectedQuestIds.Order(StringComparer.Ordinal).ToArray(), - body.Quests.Select(static row => row.QuestId).Order(StringComparer.Ordinal).ToArray()); + Assert.Equal(expectedQuestOrder.Length, body.Quests.Count); + Assert.Equal(expectedQuestOrder, body.Quests.Select(static row => row.QuestId).ToArray()); foreach (var row in body.Quests) { Assert.Equal(QuestProgressApi.StatusNotStarted, row.Status); @@ -191,6 +191,10 @@ public sealed class QuestProgressApiTests Assert.Equal(QuestProgressApi.StatusCompleted, row.Status); Assert.Equal(3, row.CurrentStepIndex); Assert.NotNull(row.CompletedAt); + Assert.True(deps.ProgressStore.TryGetProgress(PlayerId, ChainQuestId, out var afterGet)); + Assert.Equal(QuestProgressStatus.Completed, afterGet.Status); + Assert.Equal(3, afterGet.CurrentStepIndex); + Assert.NotNull(afterGet.CompletedAt); } private static void CompleteOperatorChain(QuestWiringTestDependencies deps)