|
| 1 | +--- |
| 2 | +id: TASK-339 |
| 3 | +title: >- |
| 4 | + Fix six WebKit Playwright regressions: lazy tauriInvoke, library-ready wait, |
| 5 | + stale visual baselines |
| 6 | +status: Done |
| 7 | +assignee: [] |
| 8 | +created_date: '2026-04-29 20:59' |
| 9 | +updated_date: '2026-04-30 19:06' |
| 10 | +labels: |
| 11 | + - bug |
| 12 | + - tests |
| 13 | + - webkit |
| 14 | + - regression |
| 15 | +dependencies: [] |
| 16 | +references: |
| 17 | + - app/frontend/js/api/shared.js |
| 18 | + - app/frontend/js/components/settings-view.js |
| 19 | + - app/frontend/tests/accessibility.spec.js |
| 20 | + - app/frontend/tests/settings.spec.js |
| 21 | + - app/frontend/tests/visual-regression.spec.js |
| 22 | + - app/frontend/tests/fixtures/helpers.js |
| 23 | + - app/frontend/views/library.html |
| 24 | + - app/frontend/js/mixins/context-menu-actions.js |
| 25 | +priority: high |
| 26 | +ordinal: 2750 |
| 27 | +--- |
| 28 | + |
| 29 | +## Description |
| 30 | + |
| 31 | +<!-- SECTION:DESCRIPTION:BEGIN --> |
| 32 | +Six WebKit Playwright tests are failing. They split into three distinct root causes — fix each. |
| 33 | + |
| 34 | +## Failure 1 — `accessibility.spec.js:48` › "previous button has accessible name" |
| 35 | + |
| 36 | +`beforeEach` at line 28 times out on `await page.waitForSelector('[data-track-id]', { state: 'visible' })`. Playwright finds 44 elements but the first one isn't deemed visible (intermittent on WebKit only). The test itself only inspects `[data-testid="player-prev"]` — it doesn't actually need a track row laid out, only the library data to have arrived. |
| 37 | + |
| 38 | +## Failures 2 & 3 — `settings.spec.js:718, 839` › Log Export tests |
| 39 | + |
| 40 | +Same root cause for both: `app/frontend/js/api/shared.js:11` captures `invoke` at module-load time: |
| 41 | +```js |
| 42 | +export const invoke = window.__TAURI__?.core?.invoke; |
| 43 | +``` |
| 44 | +Tests stub `window.__TAURI__` *after* `page.goto('/')`, so the cached `invoke` reference is permanently `undefined`. `tauriInvoke()` short-circuits to `null` and the test's `core.invoke` mock is never called. |
| 45 | +
|
| 46 | +Result for test 2 ("loading state"): success path runs synchronously (no 500 ms delay) — `isExportingLogs` flips false within one microtask, so `toBeDisabled()` never observes the disabled state. |
| 47 | +
|
| 48 | +Result for test 3 ("error toast"): the failure-injecting mock is dead code; `tauriInvoke` returns `null`, success toast `'Diagnostics exported successfully'` is shown instead of `'Failed to export diagnostics'`. |
| 49 | +
|
| 50 | +This regression was introduced by commit `4ba8be8` (gnhf #3 / tauriInvoke extraction). `tauriConfirm` two functions down already does the right thing (lazy lookup); `tauriInvoke` should match. |
| 51 | +
|
| 52 | +`exportLogs()` is at `app/frontend/js/components/settings-view.js:403-431` and uses `tauriInvoke('export_diagnostics', ...)`. The toast string at line 427 is exactly `'Failed to export diagnostics'` — no mismatch there. |
| 53 | +
|
| 54 | +## Failures 4, 5, 6 — `visual-regression.spec.js:59, 70, 126` |
| 55 | +
|
| 56 | +Stale local visual baselines. Snapshots at `app/frontend/tests/visual-regression.spec.js-snapshots/` are `.gitignore`d (per-developer) and the suite is `test.skip`-ped under `CI=true` (see `visual-regression.spec.js:7-8`). |
| 57 | +
|
| 58 | +- **`context-menu-track.png` (test 6):** baseline is 331 px tall, current render is 405 px. The 74 px delta = exactly two new menu items at ~37 px each. Items added by intentional commits `83e6cdc` ("Go to Artist") and `e93eaba` ("Go to Album") in `app/frontend/js/mixins/context-menu-actions.js:108-116`. |
| 59 | +- **`library-view-list*.png` (tests 4, 5):** ~3% pixel diff (~27,700 px). The paginated-loading rewrite (`cb6876d`) and FOUC fix (`f8f2e1c`) added a `transform: translateY(...)` virtual-scroll wrapper at `app/frontend/views/library.html:207`; consistent with subpixel AA shifts on transformed text across many rows. |
| 60 | +
|
| 61 | +These are intentional UI changes, not regressions — baselines just need to be regenerated locally. |
| 62 | +
|
| 63 | +## Critical files |
| 64 | +
|
| 65 | +- `app/frontend/js/api/shared.js` — line 11 export and `tauriInvoke` body at lines 67-75 |
| 66 | +- `app/frontend/tests/fixtures/helpers.js` — append a `waitForLibraryReady` helper |
| 67 | +- `app/frontend/tests/accessibility.spec.js` — line 2 import, line 28 wait |
| 68 | +- `app/frontend/tests/visual-regression.spec.js-snapshots/*-webkit-darwin.png` — regenerate |
| 69 | +
|
| 70 | +## Recommended fixes |
| 71 | +
|
| 72 | +### Fix A — Make `tauriInvoke` lazy (resolves tests 2 & 3) |
| 73 | +
|
| 74 | +In `app/frontend/js/api/shared.js`: |
| 75 | +- Drop the `export const invoke = window.__TAURI__?.core?.invoke;` line. Verified via grep that nothing imports it directly: `player.js` does its own `window.__TAURI__?.core` destructure; everything else goes through `tauriInvoke`. |
| 76 | +- Inline the lookup inside `tauriInvoke`: |
| 77 | + ```js |
| 78 | + export async function tauriInvoke(cmd, params = {}) { |
| 79 | + const invoke = window.__TAURI__?.core?.invoke; |
| 80 | + if (!invoke) return null; |
| 81 | + try { |
| 82 | + return await invoke(cmd, params); |
| 83 | + } catch (error) { |
| 84 | + console.error(`[api.tauriInvoke] Tauri error (${cmd}):`, error); |
| 85 | + throw new ApiError(500, error.toString()); |
| 86 | + } |
| 87 | + } |
| 88 | + ``` |
| 89 | +
|
| 90 | +### Fix B — Wait on library-ready signal, not DOM visibility (resolves test 1) |
| 91 | +
|
| 92 | +In `app/frontend/tests/fixtures/helpers.js`, append: |
| 93 | +```js |
| 94 | +export async function waitForLibraryReady(page) { |
| 95 | + await page.waitForFunction(() => { |
| 96 | + const lib = window.Alpine?.store?.('library'); |
| 97 | + return lib && lib.totalTracks > 0; |
| 98 | + }); |
| 99 | + await page.waitForSelector('[data-track-id]', { state: 'attached' }); |
| 100 | +} |
| 101 | +``` |
| 102 | +
|
| 103 | +In `app/frontend/tests/accessibility.spec.js`: |
| 104 | +- Import `waitForLibraryReady` alongside `waitForAlpine` (line 2). |
| 105 | +- Replace line 28 `await page.waitForSelector('[data-track-id]', { state: 'visible' });` with `await waitForLibraryReady(page);`. |
| 106 | +
|
| 107 | +Leave `visual-regression.spec.js:50` alone — those tests legitimately need rows visually laid out before screenshotting. |
| 108 | +
|
| 109 | +### Fix C — Refresh local visual baselines (resolves tests 4, 5, 6) |
| 110 | +
|
| 111 | +After Fix A lands, regenerate the WebKit baselines: |
| 112 | +```bash |
| 113 | +cd app/frontend && npx playwright test visual-regression.spec.js --update-snapshots --project=webkit |
| 114 | +``` |
| 115 | +Spot-check the regenerated PNGs: |
| 116 | +- `context-menu-track-webkit-darwin.png` shows the full menu including "Go to Artist" and "Go to Album" |
| 117 | +- `library-view-list*-webkit-darwin.png` show the current paginated-list layout |
| 118 | +Anything else odd in the screenshots is a separate regression to investigate. |
| 119 | +
|
| 120 | +## Out of scope |
| 121 | +
|
| 122 | +- Do NOT change `player.js` Tauri destructure — it has its own no-op fallback and isn't broken by tests. |
| 123 | +- Do NOT touch `visual-regression.spec.js:50` — only the failing assertions are downstream of that wait. |
| 124 | +- Don't try to make the WebKit subpixel AA diff disappear by removing the virtual-scroll transform — the transform is load-bearing for performance. |
| 125 | +<!-- SECTION:DESCRIPTION:END --> |
| 126 | +
|
| 127 | +## Acceptance Criteria |
| 128 | +<!-- AC:BEGIN --> |
| 129 | +- [x] #1 `tauriInvoke` in `app/frontend/js/api/shared.js` looks up `window.__TAURI__?.core?.invoke` at call time, not at module load |
| 130 | +- [x] #2 The unused `export const invoke = ...` declaration in `shared.js` is removed |
| 131 | +- [x] #3 `waitForLibraryReady(page)` helper exists in `app/frontend/tests/fixtures/helpers.js` and waits on `Alpine.store('library').totalTracks > 0` then `[data-track-id]` attached |
| 132 | +- [x] #4 `accessibility.spec.js` `beforeEach` uses `waitForLibraryReady` instead of waiting for `[data-track-id]` visibility |
| 133 | +- [x] #5 Local WebKit visual baselines regenerated for `context-menu-track`, `library-view-list`, and `library-view-list-selected` |
| 134 | +- [x] #6 `npx playwright test --project=webkit accessibility.spec.js settings.spec.js visual-regression.spec.js` passes for the six previously-failing tests |
| 135 | +- [x] #7 No new test failures introduced in `settings.spec.js` or `accessibility.spec.js` (run the full files on webkit to confirm) |
| 136 | +<!-- AC:END --> |
0 commit comments