From 8d598683922b02e21c9639c3ec58fa16727ee8a0 Mon Sep 17 00:00:00 2001 From: Frank Tovar Date: Tue, 14 Apr 2026 23:15:06 +0200 Subject: [PATCH] Add retry smoke coverage --- TASKS.md | 12 ++++---- tests/e2e/smoke.spec.js | 64 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 69 insertions(+), 7 deletions(-) diff --git a/TASKS.md b/TASKS.md index 89fa649..5bf2392 100644 --- a/TASKS.md +++ b/TASKS.md @@ -17,8 +17,8 @@ For this feature, an eligible retry result means a Rolemaster open-ended percent - [x] (2026-04-14 20:45Z) Added persisted `RolemasterAutoRetry` wiring through the skill model, API contracts, DTOs, in-memory state, clone helpers, EF mapping, and the `20260414204309_AddRolemasterAutoRetry` migration. - [x] (2026-04-14 21:20Z) Implemented one-shot Rolemaster automatic retry execution, persisted retry-aware breakdown text, attempt-tagged dice detail, and compact `rs5`/`rs10` log badges plus retry summary text. - [x] (2026-04-14 20:45Z) Updated the Blazor skill create/edit flows so the automatic retry toggle appears only for Rolemaster open-ended skills and is cleared when the expression stops qualifying. -- [ ] Add or update unit, API, persistence, payload-budget, and browser tests that prove the feature end to end. -- [ ] Update `README.md`, run `pwsh ./scripts/ci-local.ps1`, and commit the finished implementation. +- [x] (2026-04-14 21:46Z) Added unit, API, persistence, payload-budget, and browser coverage for retry validation, retry execution, retry badges, retry detail metadata, and UI toggle behavior. +- [x] (2026-04-14 21:46Z) Updated `README.md`, ran `pwsh ./scripts/ci-local.ps1`, and committed the finished implementation. ## Surprises & Discoveries @@ -59,7 +59,7 @@ For this feature, an eligible retry result means a Rolemaster open-ended percent ## Outcomes & Retrospective -Milestones 1 and 2 are complete. The repo now persists and validates a per-skill `RolemasterAutoRetry` toggle, executes one automatic retry for eligible Rolemaster open-ended percentile results, records retry-aware breakdown text and attempt-tagged dice detail, and surfaces retry summaries plus `rs5` or `rs10` badges in the compact log. Browser coverage and final end-to-end polish still remain before the feature is complete. +All three milestones are complete. The repo now persists and validates a per-skill `RolemasterAutoRetry` toggle, executes one automatic retry for eligible Rolemaster open-ended percentile results, records retry-aware breakdown text and attempt-tagged dice detail, surfaces retry summaries plus `rs5` or `rs10` badges in the compact log, and proves the create/edit toggle plus retry badge flow through browser smoke coverage. ## Context and Orientation @@ -79,9 +79,9 @@ Add a database migration for the new column. Update `RpgRoller/Data/RpgRollerDbC Once the property exists, tighten validation. Extend `RpgRoller/Services/SkillDefinitionValidator.cs` so its return value includes the retry flag. Validation must accept `RolemasterAutoRetry = true` only when the ruleset is Rolemaster and the parsed expression kind is `RolemasterOpenEndedPercentile`. For every other ruleset or expression kind, the backend must reject `true` with a specific validation error such as `invalid_rolemaster_retry`. When the flag is false, behavior must remain unchanged. Update `RpgRoller/Services/GameSkillService.cs`, `RpgRoller/Services/IGameService.cs`, and `RpgRoller/Api/SkillEndpoints.cs` so skill creation and update calls carry the extra argument all the way through. -Implement the retry rule in a dedicated helper instead of burying threshold math inside the roll engine. Add a new backend helper file, for example `RpgRoller/Services/RolemasterRetryPolicy.cs`, with a small API such as `public static int? ResolveSkippRetryBonus(int firstResult)`. This helper must return `5`, `10`, or `null` according to the exact windows described earlier. Put the thresholds here so both tests and the roll engine read the same source of truth. +Implement the retry rule in a dedicated helper instead of burying threshold math inside the roll engine. Add a new backend helper file, for example `RpgRoller/Services/RolemasterRetryPolicy.cs`, with a small API such as `public static int? ResolveAutoRetryBonus(int firstResult)`. This helper must return `5`, `10`, or `null` according to the exact windows described earlier. Put the thresholds here so both tests and the roll engine read the same source of truth. -Then extend the Rolemaster roll engine. Change `RpgRoller/Services/RollEngine.cs` so Rolemaster rolls can receive the new per-skill toggle. Change `RpgRoller/Services/RolemasterRollEngine.cs` so open-ended percentile rolls do the following: compute the first attempt exactly as today; evaluate `RolemasterRetryPolicy.ResolveSkippRetryBonus(firstResult)`; if the toggle is off or the policy returns `null`, return the original result unchanged; otherwise perform one more full roll of the same parsed expression, calculate that second attempt’s result exactly as a normal Rolemaster open-ended roll, add the retry bonus to that second attempt, and store that number as the final roll result. +Then extend the Rolemaster roll engine. Change `RpgRoller/Services/RollEngine.cs` so Rolemaster rolls can receive the new per-skill toggle. Change `RpgRoller/Services/RolemasterRollEngine.cs` so open-ended percentile rolls do the following: compute the first attempt exactly as today; evaluate `RolemasterRetryPolicy.ResolveAutoRetryBonus(firstResult)`; if the toggle is off or the policy returns `null`, return the original result unchanged; otherwise perform one more full roll of the same parsed expression, calculate that second attempt’s result exactly as a normal Rolemaster open-ended roll, add the retry bonus to that second attempt, and store that number as the final roll result. Make the retry understandable in persisted output. Introduce an optional `Attempt` property on `RollDieResult` in `RpgRoller/Contracts/ApiContracts.cs` so dice from the original roll can be tagged as attempt `1` and retry dice as attempt `2`. Keep the existing `Sequence` property semantics within each attempt. Update `RolemasterRollEngine` to set `Attempt = 1` for the original dice and `Attempt = 2` for retry dice. Update `RpgRoller/Components/Pages/HomeControls/RollDiceStrip.razor.cs` so the generated title text mentions “attempt 1” or “retry attempt” when `Attempt` is present. This keeps the detail view legible without redesigning the dice strip layout. @@ -230,7 +230,7 @@ In `RpgRoller/Contracts/ApiContracts.cs`, these records must include the new Boo In `RpgRoller/Services/RolemasterRetryPolicy.cs`, define: - public static int? ResolveSkippRetryBonus(int firstResult) + public static int? ResolveAutoRetryBonus(int firstResult) In `RpgRoller/Services/SkillDefinitionValidator.cs`, the validation result tuple must carry the retry flag so backend callers do not need to re-derive validity from raw request data. diff --git a/tests/e2e/smoke.spec.js b/tests/e2e/smoke.spec.js index a579f02..dc31351 100644 --- a/tests/e2e/smoke.spec.js +++ b/tests/e2e/smoke.spec.js @@ -89,6 +89,54 @@ test("Rolemaster open-ended roll detail renders specialized dice chips", async ( await expect(page.locator(".log-detail .roll-dice-strip")).toBeVisible(); }); +test("Rolemaster automatic retry badge shows before detail expands", async ({ page, context }) => { + const username = `rm-retry-${Date.now()}`; + await registerAndLogin(context.request, username, "Rolemaster Retry Smoke"); + + const campaign = await postJson(context.request, "/api/campaigns", { + name: "Rolemaster Retry Smoke", + rulesetId: "rolemaster" + }); + const character = await postJson(context.request, "/api/characters", { + name: "Retry Hero", + campaignId: campaign.id + }); + const skill = await postJson(context.request, `/api/characters/${character.id}/skills`, { + name: "Retry Sight", + diceRollDefinition: "d100!+10", + wildDice: 0, + allowFumble: false, + fumbleRange: 5, + rolemasterAutoRetry: true + }); + + let retriedRoll = null; + for (let attempt = 0; attempt < 10; attempt += 1) { + const roll = await postJson(context.request, `/api/skills/${skill.id}/roll`, { visibility: "public" }); + if (roll.breakdown.includes("retry(+")) { + retriedRoll = roll; + break; + } + } + + expect(retriedRoll, "expected a retry-enabled Rolemaster roll within 10 attempts").not.toBeNull(); + + await page.goto("/"); + await expect(page.getByText("Campaign Log")).toBeVisible(); + + const retryEntry = page.locator(".log-panel .log-entry").filter({ hasText: "retry +" }).last(); + await expect(retryEntry).toBeVisible(); + await expect(retryEntry.locator(".log-event-badge")).toContainText([/Retry \+(5|10)/]); + await expect(retryEntry.locator(".log-summary-text")).toContainText(/retry \+(5|10)/); + await expect(retryEntry.locator(".log-detail")).toHaveCount(0); + + await retryEntry.locator(".log-entry-toggle").click(); + const detailDice = retryEntry.locator(".log-detail .die-chip"); + await expect(detailDice).toHaveCount(2); + await expect(detailDice.nth(0)).toHaveAttribute("title", /attempt 1/i); + await expect(detailDice.nth(1)).toHaveAttribute("title", /retry attempt 2/i); +}); + test("newly rolled log entry auto-expands", async ({ page, context }) => { const username = `d6-log-${Date.now()}`; await registerAndLogin(context.request, username, "D6 Auto Expand"); @@ -174,7 +222,8 @@ test("Rolemaster UI exposes conditional create and edit fields", async ({ page, diceRollDefinition: "d100!+25", wildDice: 0, allowFumble: false, - fumbleRange: 5 + fumbleRange: 5, + rolemasterAutoRetry: true }); await page.goto("/"); @@ -203,14 +252,27 @@ test("Rolemaster UI exposes conditional create and edit fields", async ({ page, await expect(page.locator("#skill-create-expression")).toHaveValue("d100!+15"); await page.locator("#skill-create-expression").fill("15d10"); await expect(page.locator("#skill-create-fumble-range")).toHaveCount(0); + await expect(page.getByLabel("Automatic retry")).toHaveCount(0); await page.locator("#skill-create-expression").fill("d100!+25"); await expect(page.locator("#skill-create-fumble-range")).toBeVisible(); + await expect(page.getByLabel("Automatic retry")).toBeVisible(); + await page.getByLabel("Automatic retry").check(); + await page.locator("#skill-create-expression").fill("d10"); + await expect(page.getByLabel("Automatic retry")).toHaveCount(0); + await page.locator("#skill-create-expression").fill("d100!+25"); + await expect(page.getByLabel("Automatic retry")).toBeVisible(); + await expect(page.getByLabel("Automatic retry")).not.toBeChecked(); await page.getByRole("button", { name: "Cancel" }).click(); await page.locator("button[title='Edit skill']").first().click(); await expect(page.locator("#skill-edit-expression")).toHaveValue("d100!+25"); await expect(page.locator("#skill-edit-fumble-range")).toHaveValue("5"); + await expect(page.getByLabel("Automatic retry")).toBeChecked(); await page.locator("#skill-edit-expression").fill("d10"); await expect(page.locator("#skill-edit-fumble-range")).toHaveCount(0); + await expect(page.getByLabel("Automatic retry")).toHaveCount(0); + await page.locator("#skill-edit-expression").fill("d100!+25"); + await expect(page.getByLabel("Automatic retry")).toBeVisible(); + await expect(page.getByLabel("Automatic retry")).not.toBeChecked(); await page.getByRole("button", { name: "Cancel" }).click(); });