NEO-119: Tighten quest-progress order and side-effect test coverage.

pull/158/head
VinPropane 2026-06-07 12:10:55 -04:00
parent 1565d2cde7
commit ad906fec40
3 changed files with 11 additions and 7 deletions

View File

@ -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");

View File

@ -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

View File

@ -58,6 +58,8 @@ public sealed class QuestProgressApiTests
// Arrange
await using var factory = new InMemoryWebApplicationFactory();
var client = factory.CreateClient();
var registry = factory.Services.GetRequiredService<IQuestDefinitionRegistry>();
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)