From 772c71c49fbf47b330ebdf3f4f4b226ec14b9ca4 Mon Sep 17 00:00:00 2001 From: Logan Date: Mon, 24 Aug 2026 06:47:05 -0400 Subject: [PATCH] test(ui): clear localStorage between component tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A test file does not get its own origin. `@vitest/browser-playwright` opens one BrowserContext per session and runs several files in it, one after another, so everything a component persists survives from file to file. `track-list` restores its sort in `connectedCallback` and `aria-tail.test.ts` activates the Title column header, so any file that mounts a track list later in that tab opens sorted by title — where `track-11` precedes `track-3`, which is #138's failure exactly. Nothing about it is specific to that pair: a probe that throws when a test starts with a non-empty `localStorage` failed 24 test-starts in one full run (11 with the two sort keys, 8 with `cover-grid-size`, 5 with `track-list-column-widths`) and cascaded into 248 failures. Which files share a tab, and in what order, changes run to run, which is the whole of why this reads as a 1-in-3 flake and passes in isolation. The clear belongs in `setup.ts` rather than in the specs that write, because the spec that reads is never the one that knows — and it is safe for the same reason the leak exists: files within a session are sequential, so it cannot wipe storage a concurrent file is using. The spec now also states the order it asserts rather than inheriting a default, and checks the row it is about to double-click carries the path it expects, so a stray sort fails by naming itself instead of as an off-by-eight file path. Closes #138 --- CLAUDE.md | 20 +++++++++++++++++++ .../test/components/play-in-context.test.ts | 13 ++++++++++++ frontend/test/setup.ts | 17 ++++++++++++++++ 3 files changed, 50 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 97eb7a5..b36f34b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -262,6 +262,26 @@ real store code. A binding carries an **ID**, not a name `yellowjacket/backend/home.Service.GetShelves`), so the fake derives that map from the generated tree rather than writing it down. +**A test file does not get its own origin, so `setup.ts` clears +`localStorage` between tests.** `@vitest/browser-playwright` opens one +BrowserContext per session and runs several files in it one after +another, so everything a component persists — the track list's sort and +column widths, the cover size, `now-playing`'s scroll mode — is still +there when the next file mounts the same component. Which files share a +tab, and in what order, changes run to run, so the symptom is a spec +that fails about one test in three and passes every time it is run on +its own: #138 cost three scheduled runs, one of them a PR whose diff +held no frontend code at all. Measured on the build before the fix, a +single full run started **24** tests with storage already set. Two +things follow. The clear is safe precisely because the leak is +sequential — files in a session do not overlap, so it cannot wipe +storage a concurrently-running file is in the middle of using — and it +belongs in `setup.ts` rather than in the specs that write, because the +spec that *reads* is never the one that knows. And a spec whose +assertion depends on an order still **states that order** rather than +inheriting a default, or the next change to a default is the same +mystery again. + **`frontend/bindings/` is generated by `wails3`, not `go generate`**, so the pre-commit codegen check does not cover it. `make bindings-check` (~3.5 s warm, ~20 s on a cold build cache, also a pre-commit hook) diff --git a/frontend/test/components/play-in-context.test.ts b/frontend/test/components/play-in-context.test.ts index 457b3aa..4c9676b 100644 --- a/frontend/test/components/play-in-context.test.ts +++ b/frontend/test/components/play-in-context.test.ts @@ -161,6 +161,15 @@ describe('double-clicking a row in the track list', () => { localStorage.removeItem('track-list-column-widths'); el = await fixture('track-list', { externalTracks: LIST }); + + // Say which order is being asserted rather than inheriting one. + // `restoreSortPreferences()` runs in `connectedCallback`, so the + // list opens in whatever sort was last persisted -- and + // `track-11` sorts before `track-3` by title, which is the shape + // of #138. The row below is the fixture's third track only while + // nothing is sorting the list. + (el as unknown as { sortField: string | null }).sortField = null; + el.style.display = 'block'; el.style.height = '600px'; await flush(); @@ -172,6 +181,10 @@ describe('double-clicking a row in the track list', () => { const rows = shadowAll(el, '.track-row'); const row = rows.find((r) => r.getAttribute('data-index') === '3'); + // Stated first, so a list that is not in the order this asserts + // fails by saying so rather than as an off-by-eight file path. + expect(row?.getAttribute('data-file-path')).toBe('/music/track-3.mp3'); + dblclick(row!); await flush(); diff --git a/frontend/test/setup.ts b/frontend/test/setup.ts index 6850897..a2c2fef 100644 --- a/frontend/test/setup.ts +++ b/frontend/test/setup.ts @@ -72,6 +72,23 @@ document.body.style.margin = '0'; beforeEach(() => { resetHarness(); + + // A test file does not get its own origin. `@vitest/browser-playwright` + // opens one BrowserContext per session and runs several files in it, + // one after another, so everything a component persists — the track + // list's sort and column widths, the cover size, `now-playing`'s + // scroll mode — is still there when the next file mounts the same + // component. That is invisible until it is intermittent, because + // which files share a tab and in what order changes run to run: it + // cost #138 three scheduled runs, one of them a PR with no frontend + // code in it at all. + // + // Clearing here rather than in the specs that write is deliberate — + // the spec that *reads* is never the one that knows. It is safe for + // the same reason the leak exists: files in a session are + // sequential, so this cannot wipe storage a concurrent file is in + // the middle of using. + localStorage.clear(); }); afterEach(() => {