Replace Playwright smoke tests with Selenium

This commit is contained in:
2026-05-04 20:54:10 +02:00
parent b97437fda3
commit c13a2ce7c7
13 changed files with 1313 additions and 620 deletions

View File

@@ -10,18 +10,19 @@ After this change, the browser URL will match the authenticated screen the user
This matters because the current authenticated workspace is still one large, structurally dynamic Blazor Server surface. `POSTMORTEM.md` shows that this architecture is fragile when browser extensions mutate form-related DOM during startup. The route-first rewrite reduces the amount of UI that wakes up at once, removes the dual-purpose `/` shell, and makes the authenticated shell easier to reason about, test, and evolve.
The change is complete when a human can run the app, open `/`, observe the correct redirect based on auth state, log in at `/login`, land on `/play`, navigate to `/campaigns` and `/admin` with real URLs, refresh any of those routes without being thrown back to `/`, and run the automated host and Playwright tests that prove the new behavior.
The change is complete when a human can run the app, open `/`, observe the correct redirect based on auth state, log in at `/login`, land on `/play`, navigate to `/campaigns` and `/admin` with real URLs, refresh any of those routes without being thrown back to `/`, and run the automated host and Selenium tests that prove the new behavior.
## Progress
- [x] (2026-05-04 17:52Z) Reviewed `POSTMORTEM.md`, the current app shell, workspace routing behavior, and the existing host and Playwright tests to define the rewrite around real routes instead of `sessionStorage` screen switching.
- [x] (2026-05-04 17:52Z) Reviewed `POSTMORTEM.md`, the current app shell, workspace routing behavior, and the existing host and frontend smoke tests to define the rewrite around real routes instead of `sessionStorage` screen switching.
- [x] (2026-05-04 17:52Z) Updated `README.md` so it accurately describes the current architecture and the approved rewrite direction.
- [x] (2026-05-04 18:29Z) Implemented a host-level `/` redirect to `/login` or `/play`, moved the static auth document to `/login`, switched login/logout targets to `/play` and `/login`, and updated the root-path host and smoke coverage to the new contract.
- [x] (2026-05-04 19:26Z) Replaced the checked-in Playwright smoke coverage with a geckodriver+Selenium smoke runner, including a Firefox DOM-wrap addon for extension-like startup mutations, and updated repo scripts/docs to the new browser verification path.
- [ ] Introduce real authenticated routes for `/play`, `/campaigns`, and `/admin` while preserving current behavior.
- [ ] Remove `screen` as a `sessionStorage` routing mechanism and replace menu actions with URL navigation.
- [ ] Split the large `Workspace` render tree so play, campaign management, and admin each own a smaller subtree.
- [ ] Reduce `OnAfterRenderAsync` to the smallest practical scope and keep staged startup out of the authenticated shell root.
- [ ] Update host tests, Playwright smoke tests, and docs so the new route model is the only documented and verified behavior.
- [ ] Update host tests, Selenium smoke tests, and docs so the new route model is the only documented and verified behavior.
## Surprises & Discoveries
@@ -29,7 +30,7 @@ The change is complete when a human can run the app, open `/`, observe the corre
Evidence: `RpgRoller/Components/RpgRollerApiClient.cs` calls `js.InvokeAsync("rpgRollerApi.request", ...)`, which means authenticated data fetches currently depend on an interactive render before they can run.
- Observation: the current smoke suite encodes the old dual-purpose `/` behavior and will fail as soon as `/` becomes a redirect entry point.
Evidence: `tests/e2e/smoke.spec.js` currently expects anonymous `GET /` to render static auth markup and authenticated `GET /` to render the Blazor workspace shell.
Evidence: the checked-in smoke coverage originally expected anonymous `GET /` to render static auth markup and authenticated `GET /` to render the Blazor workspace shell, so it had to be rewritten when `/` became a redirect entry point.
- Observation: the current host test also encodes an outdated assumption about `/`.
Evidence: `RpgRoller.Tests/Api/FrontendHostTests.cs` currently asserts that `GET /` returns HTTP 200 and a Blazor shell containing `_framework/blazor.web.js`.
@@ -40,8 +41,8 @@ The change is complete when a human can run the app, open `/`, observe the corre
- Observation: the repository-wide backend suite currently contains a missing-fixture failure unrelated to the route-first rewrite.
Evidence: `dotnet test RpgRoller.Tests/RpgRoller.Tests.csproj --collect:"XPlat Code Coverage" --settings RpgRoller.Tests/coverlet.runsettings` failed in `HostingCoverageTests.InitializeRpgRollerState_MigratesCopiedDevelopmentDatabaseAndPreservesD6Rolling` because `RpgRoller/App_Data/rpgroller.development.db` is not present in the worktree.
- Observation: the locally installed Snap Firefox build on this machine does not complete Playwrights Firefox control handshake.
Evidence: Playwright launched `/usr/bin/firefox` with `-juggler-pipe` and stalled before page automation began, so Milestone 1 browser verification was completed with `geckodriver` plus Selenium against the same temporary app instance instead.
- Observation: the locally installed Snap Firefox build on this machine is viable for Selenium through `geckodriver`, but not for Playwright protocol control.
Evidence: Playwright stalled during the `-juggler-pipe` handshake, while a `geckodriver` plus Selenium session against `/snap/firefox/current/usr/lib/firefox/firefox` completed the same Milestone 1 verification successfully.
## Decision Log
@@ -65,12 +66,18 @@ The change is complete when a human can run the app, open `/`, observe the corre
Rationale: the current workspace is dense and risk-prone. A staged rewrite keeps the app working while the route model changes, and it gives the test suite meaningful checkpoints.
Date/Author: 2026-05-04 / Codex
- Decision: standardize frontend smoke verification on geckodriver plus Selenium instead of Playwright in this repository.
Rationale: the user updated the repo instructions to make Selenium the required browser automation path, and the locally installed Firefox stack works reliably through geckodriver while Playwright cannot control the Snap Firefox build on this machine.
Date/Author: 2026-05-04 / Codex and user
## Outcomes & Retrospective
At plan creation time, the repository has an updated README and a concrete implementation plan, but no code for the route-first rewrite has been started yet. The immediate risk is not uncertainty about direction; it is carrying old assumptions about `/`, `Home.razor`, and `sessionStorage`-based screen switching into the first code changes. The milestones below are written to make those assumptions explicit and retire them in an observable order.
After Milestone 1, the dual-purpose `/` entry point is gone. Anonymous requests to `/` are now redirected before Blazor renders, the static auth document lives at `/login`, and successful login lands on `/play`. The main residual risk is that the authenticated shell is still monolithic behind the new `/play` route, so later milestones still need to replace in-memory screen switching with real route ownership.
After the Selenium migration iteration, the repositorys browser smoke coverage once again matches the documented verification path. The smoke suite now runs against Firefox through geckodriver, and the DOM-wrap regression coverage remains intact through a temporary test addon. The next risk is purely architectural again: the authenticated shell still uses in-memory screen switching, so Milestone 2 remains the next code change on the critical path.
This section must be updated after each major milestone. When the implementation is complete, summarize which parts of the old workspace architecture were fully removed, which compatibility constraints remain, and whether the final startup path still depends on any multi-batch structural rendering.
## Context and Orientation
@@ -151,7 +158,7 @@ Start by inspecting the current route and auth files before editing:
sed -n '1,260p' RpgRoller/Components/Pages/WorkspaceSessionCoordinator.cs
sed -n '1,260p' RpgRoller/wwwroot/js/rpgroller-api.js
sed -n '1,220p' RpgRoller.Tests/Api/FrontendHostTests.cs
sed -n '1,260p' tests/e2e/smoke.spec.js
sed -n '1,260p' tests/e2e/smoke.js
When implementing Milestone 1, update the host test first so the intended redirect behavior is explicit:
@@ -176,7 +183,7 @@ Then verify in a browser:
When implementing route pages and navigation, prefer running the focused smoke suite against a temporary database:
pwsh ./scripts/run-playwright.ps1
node ./scripts/run-selenium.js
If the app is already running and a faster inner loop is needed, run the checked-in smoke file directly:
@@ -186,7 +193,7 @@ After each milestone that touches C# files, run the relevant test suite and then
dotnet test RpgRoller.Tests/RpgRoller.Tests.csproj --collect:"XPlat Code Coverage" --settings RpgRoller.Tests/coverlet.runsettings
After major frontend milestones, repeat browser verification in Chromium and Firefox. If a Firefox profile with RoboForm is available, include that manual check and record the result in `Surprises & Discoveries` or `Outcomes & Retrospective`.
After major frontend milestones, repeat browser verification in Firefox. If a Firefox profile with RoboForm is available, include that manual check and record the result in `Surprises & Discoveries` or `Outcomes & Retrospective`.
## Validation and Acceptance
@@ -212,7 +219,7 @@ Automated coverage:
`dotnet test RpgRoller.Tests/RpgRoller.Tests.csproj --collect:"XPlat Code Coverage" --settings RpgRoller.Tests/coverlet.runsettings` passes.
`pwsh ./scripts/run-playwright.ps1` passes against a temporary SQLite database.
`node ./scripts/run-selenium.js` passes against a temporary SQLite database.
If any previous tests are deleted or renamed because they encoded the old `/` behavior, replace them with tests that prove the new route model instead of simply removing coverage.
@@ -222,9 +229,9 @@ This rewrite should be implemented as a sequence of additive, testable steps. Ea
The safest recovery strategy is to keep the current workspace internals temporarily while introducing the new route model. That means it is acceptable to reuse `Workspace` behind the new page routes during Milestone 2, as long as the route behavior is correct and clearly transitional. After that, extract route-specific subtrees in Milestone 3.
When changing redirects or login targets, update the host and Playwright assertions in the same commit as the code change so the repository never has code and tests describing different route contracts.
When changing redirects or login targets, update the host and Selenium assertions in the same commit as the code change so the repository never has code and tests describing different route contracts.
Use a temporary SQLite database for Playwright verification, as required by the repo instructions, so browser tests do not mutate the canonical development database.
Use a temporary SQLite database for Selenium verification, as required by the repo instructions, so browser tests do not mutate the canonical development database.
## Artifacts and Notes
@@ -234,10 +241,8 @@ Current evidence that must be retired by this rewrite:
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
Assert.Contains("_framework/blazor.web.js", html);
tests/e2e/smoke.spec.js
test("home page loads auth entry points", ...)
test("home document renders static auth markup without bootstrapping blazor", ...)
test("authenticated home document avoids prerendered workspace shell", ...)
tests/e2e/smoke.js
browser checks for anonymous `/`, static `/login`, authenticated `/`, and the authenticated workspace flows
Current evidence that explains the bootstrap constraint: