Compare commits

...
Author SHA1 Message Date
logan 772c71c49f test(ui): clear localStorage between component tests
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 9m41s
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
2026-08-24 06:47:05 -04:00
3 changed files with 50 additions and 0 deletions
+20
View File
@@ -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)
@@ -161,6 +161,15 @@ describe('double-clicking a row in the track list', () => {
localStorage.removeItem('track-list-column-widths');
el = await fixture<LitElement>('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();
+17
View File
@@ -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(() => {