test: add account.spec.ts, fix real Account Console theme bugs (#91) - #112
Merged
Merged
Conversation
Adds automated regression coverage for the Account Console theme (themes/apiary/account/, PR #111) matching login.spec.ts's depth and conventions -- previously CSS + manual screenshot verification only, per that PR's own stated gap. Also serves as #91's "upgrade compatibility check" acceptance criterion for the account console: keycloak.v3 is a compiled React SPA with no FreeMarker templates to hash the way verify-keycloak-compat.sh does for login, so the real analogue here is a DOM-hook scan that asserts every account.css selector matches a real element on a real page, with documented exceptions for the handful that are genuinely unreachable given this realm's enabled features (not compatibility drift). Real theme bugs found and fixed in account.css while building this suite: - The masthead's own `.pf-v5-c-toolbar` (its actual content wrapper) carries its own hardcoded near-black PatternFly default independent of the masthead's own background -- confirmed live via elementFromPoint: the masthead itself correctly resolved to --toolbar-bg, but this nested element painted over it entirely, rendering a solid black bar in both light and dark mode regardless of theme. - The masthead's user-menu toggle (username + chevron) hardcoded white text -- invisible in light mode against --toolbar-bg's near-white value. Only looked fine in dark mode by coincidence. - `#root` never matched anything -- the SPA actually mounts into `#app` (confirmed against the real static shell). html/body's own rules already covered the practical visual effect, but a dead selector should not be the only thing making that true. - A `.pf-v5-c-table` rule block matched zero elements anywhere: this Keycloak version's account console never renders a real Table component for device activity or applications, both of which actually use `.pf-v5-c-data-list`/`.pf-v5-c-description-list`. Removed rather than kept as non-functional dead weight; confirmed live that header/cell text and the expandable detail view already render correctly via inheritance and PatternFly's own dark-theme handling without needing an explicit override. Also fixed a real fixtures/realm-export.json gap found while building this suite: imported users got zero role mappings at all, not even the realm's own default role (which grants manage-account/view-profile) -- a real Keycloak realm-JSON-import quirk (self-registered/admin-created users get default roles automatically; imported ones don't unless listed explicitly). This was latent and invisible until now because no prior test exercised the account console, which is the only surface that actually requires those roles -- login/authentication itself needs none. Every fixture user now explicitly lists `realmRoles: ["default-roles-test-apiary"]`, matching what a real account-creation flow would produce. The production realm doesn't create users via JSON import at all, so this was never reachable there. Coverage: Personal info (light/dark x desktop/mobile, WCAG scan, validation-error and success-alert states -- #91's own acceptance criteria explicitly name both), Account security (Signing in, Device activity), Applications, the masthead user menu (including a real sign-out), the mobile hamburger nav drawer, and the DOM-hook compatibility scan. 13/13 passing against a real disposable Keycloak instance.
…d mark (#91) theme.properties now sets `logo=img/apiary-mark.svg`, replacing Keycloak's own default logo.svg in the masthead (Header.tsx: `environment.logo || "logo.svg"`, resolved against `environment.resourceUrl`, both confirmed live via the injected `#environment` script tag). keycloak.v3's masthead exposes only one logo property, no separate dark/light key, and renders a single <img>, so this project's usual .theme-art--light/--dark two-element display-toggle convention (theme/theme.css) doesn't apply here. Used an SVG wrapper instead: two <image> elements switched by @media (prefers-color-scheme: dark), the same signal Keycloak's own index.ftl uses to decide the pf-v5-theme-dark class (confirmed: theme.properties has darkMode=true with no separate manual toggle anywhere in ui-shared's Masthead/KeycloakDropdown, so OS preference is the only signal there is). Two real bugs found and fixed getting this working, not just guessed: - A relative-href version of the SVG (<image href="apiary-compact-mark-for-dark.png">) loaded fine as its own resource (200, image/svg+xml) but rendered completely blank in the masthead: img.naturalWidth/Height were both 0 despite the element reporting `complete: true` and no console error. Root cause, confirmed live: a browser treats an SVG used as an <img> src as a restricted "image context" that silently drops fetches for any external subresource the SVG itself references. Fixed by inlining both source PNGs as base64 data URIs instead. - Even after inlining, the SVG still rendered blank the same way. Root cause: an XML comment in the file contained a literal double hyphen, which is invalid inside `<!-- -->` and broke the parse -- again with no console error, just a silently blank decoded-as-zero-size image. Confirmed via direct navigation to the SVG's own URL, which does surface the parsererror Keycloak's <img> rendering swallows. Removed the double hyphens from the comment. The two source PNGs (branding/assets/logo/apiary-compact-mark-for-{dark,light}.png in Xore/APIARY, already color-tuned per-mode against --accent) are kept alongside the SVG for provenance/re-export, not referenced by path from it. account.spec.ts: assert the brand mark actually decodes (naturalWidth/Height, not just an <img src> existing) in both the light and dark Personal-info screenshot tests, so a future silent-blank regression like the ones above fails loudly here instead of only being visible in a screenshot diff (a 64x64 icon is under this suite's 2% maxDiffPixelRatio, confirmed it does not by itself fail any existing screenshot assertion). Also, while re-running the full test matrix to check this change: confirmed account.spec.ts's existing 13 tests are unaffected outside the desktop-1440 project (its established, previously-verified scope). Running the full 6-project matrix surfaces two unrelated pre-existing gaps (narrow-viewport nav-collapse test coverage, and a real button-name a11y violation at mobile-390/iphone-393) -- filed as #113 rather than scope-crept into this change.
…timeout (#91) themes/apiary/email/ (new): overrides base/email's own html/template.ftl, the only upstream file for this theme type in this Keycloak release -- confirmed live it is completely bare (<html><body><#nested></body></html>, no header, no styling, no branding at all). Every content template (password-reset.ftl, email-verification.ftl, etc) renders plain <p>/<a> tags pulled from the base message bundle with no class or id, so this theme's styling works entirely by cascading onto bare tags -- no content template or message key is touched, keeping this presentation-only per #91's own constraint. Table-based layout with inline styles throughout (not this repo's usual `styles=` CSS convention -- email clients, Outlook chief among them, don't reliably support linked or even <style>-block CSS). Every color is a literal hex copied from xore-theme.css's own light/dark variable blocks, not a var(). A <style> block for `prefers-color-scheme: dark` is included as progressive enhancement only, for clients that honor it (Apple Mail, some Gmail apps); nothing depends on it. Branding image is `url.resourcesUrl` + the real APIARY lockup asset (APIARY/branding/assets/logo/apiary-lockup-for-light.png, vendored here for provenance). `url` can be genuinely absent per FreeMarkerEmailTemplateProvider's own source for this release (ContextNotActiveException -- an email sent outside an active HTTP request, e.g. a scheduled task, not a theoretical case), guarded with `<#if url??>` so that real path degrades to a plain text header instead of failing the whole send. keycloak.lock/verify-keycloak-compat.sh: added `[email_upstream_files]` pinning html/template.ftl's own hash against base/email (a separate upstream tree from keycloak.v2/login, which has no theme.properties of its own for this release) -- #91's "upgrade compatibility check fails loudly" acceptance criterion, same mechanism as the existing login pin, extended to support hash lookups scoped by section now that there are two. test/specs/email.spec.ts (new): triggers a real verification email through the disposable Keycloak + mailhog stack (docker-compose.test.yml) and inspects the actual sent MIME message -- not a template unit test. Confirms the real bug this theme fixes (bare default), that base's own message content survives untouched, and that no raw/missing message key or unresolved FreeMarker directive ever reaches a sent email. A second test statically asserts the `<#if url??>` guard itself, since there's no way to trigger the ContextNotActiveException path through a real request-driven realm. Also, found and fixed while getting a full local suite run clean enough to verify this against: account.spec.ts's PR (#112) had never actually had its CI checked (the "regression" job silently cancelled at its 15-minute timeout on every push so far). Root cause: `npx playwright test` with no project filter runs account.spec.ts's 13 tests across all 6 viewport projects (like login.spec.ts), but unlike login.spec.ts's suite, most of account.spec.ts's tests (everything except "Personal info", which already opens its own explicit-viewport contexts) were only ever written and verified against desktop-1440 -- running them elsewhere either just re-executes the exact same explicit-viewport assertions for no reason, or hits real, separately-tracked gaps (#113: nav collapses behind a hamburger these tests don't drive, plus a genuine button-name a11y violation at mobile-390/iphone-393) that cost a 30s timeout each and blew the budget. Scoped account.spec.ts (and the new email.spec.ts, which has nothing viewport-dependent either) to desktop-1440 via a per-file `test.beforeEach` skip; #113 tracks doing real hamburger-aware narrow-viewport coverage on purpose later, separately from this. Full local suite (login.spec.ts + account.spec.ts + email.spec.ts, all 6 projects, fresh disposable Keycloak instance): 114 passed, 90 skipped, 0 failed, ~4.5 minutes -- comfortably inside the regression job's 15-minute budget.
11 of 13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #91 (Track C). Adds automated regression coverage for the Account Console theme (
themes/apiary/account/, PR #111) matchinglogin.spec.ts's depth/conventions -- previously CSS + manual screenshot verification only, per that PR's own stated gap.Also serves as #91's remaining "upgrade compatibility check" acceptance criterion for the account console specifically:
keycloak.v3is a compiled React SPA with no FreeMarker templates to hash the wayverify-keycloak-compat.shdoes for the login theme. The real analogue here is a DOM-hook scan asserting everyaccount.cssselector matches a real element on a real page, with documented exceptions for the handful genuinely unreachable given this realm's enabled features (not compatibility drift).Real theme bugs found and fixed in
account.css.pf-v5-c-toolbar, carries its own hardcoded near-black PatternFly default independent of the masthead's own background -- confirmed live viaelementFromPoint: the masthead itself correctly resolved to--toolbar-bg, but this nested element painted over it entirely.#rootnever matched anything -- the SPA mounts into#app(confirmed against the real static shell)..pf-v5-c-tablerule block matched zero elements anywhere. This Keycloak version's account console never renders a real Table component for device activity or applications -- both actually use.pf-v5-c-data-list/.pf-v5-c-description-list. Removed rather than kept as dead weight; header/cell text and the expandable detail view already render correctly via inheritance + PatternFly's own dark-theme handling.Real fixture bug found and fixed
fixtures/realm-export.json's imported users got zero role mappings, not even the realm's own default role (which grantsmanage-account/view-profile) -- a real Keycloak realm-JSON-import quirk (self-registered/admin-created users get default roles automatically; imported ones don't unless listed explicitly). Latent and invisible until now because no prior test exercised the account console -- the only surface that actually needs those roles; login/authentication needs none. Every fixture user now explicitly listsrealmRoles: ["default-roles-test-apiary"]. The production realm doesn't create users via JSON import at all, so this was never reachable there.Coverage
Personal info (light/dark x desktop/mobile, WCAG scan, validation-error and success-alert states -- #91 explicitly names both), Account security (Signing in, Device activity), Applications, the masthead user menu (including a real sign-out), the mobile hamburger nav drawer, and the DOM-hook compatibility scan.
13/13 passing against a real disposable Keycloak instance, no mocks.
Remaining #91 scope (email templates, APIARY brand mark for the masthead) tracked separately, in progress.