Compare commits

...
Author SHA1 Message Date
logan 2365806d18 fix(e2e): ask the fixture for a track that can navigate
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m29s
CI / e2e (pull_request) Successful in 8m4s
`queue-selection`'s name-click test failed on main on both engines,
having passed in its own PR and in two consecutive local suite runs. I
added it in #152; this is my defect and it had main red.

It staged a queue from the first few rows of `GetTracks(0)` and clicked
a track *name*, which `explore-link` routes to that track's **album**
page. Four tracks in the fixture library have no album — `01 Tone A`,
`02 Tone B`, `Title Only`, `no-tags-at-all` — and a name with nothing
to route to renders as plain text rather than as a link.

Which tracks arrive first is `audio_files.id` order, which is the order
the *scan* inserted them, which depends on concurrency and directory
traversal. Locally the first eight are all from two proper albums; CI
rebuilds its seed with a real scan and got a different eight. The
fixture had a requirement it did not state, so the queue now asks for
tracks that have an album.

A loose locator is what turned that into a mystery rather than a
message. The row was located with `.explore-link` and `first()`, and a
row has two — title and artist. With the title as plain text, `first()`
silently resolved to the *artist* link, so the click went somewhere
real and the assertion was about a destination the test had never
exercised. It names `.track-title .explore-link` now.

Reproduced before fixing, by staging the CI condition deliberately: a
no-album track at row 2 fails the test in 30s on this machine, and the
filtered fixture passes in 752ms.

The Direction's sweep found one other spec slicing `GetTracks` —
`queue-reorder`, which asserts on order alone and needs no property of
the tracks it gets, so it is left as it is.

Closes #156
2026-08-20 00:23:31 -04:00
logan 7d348f243a Merge pull request (#153) from fix/151-fuse-the-scroll-guard-and-the-write into main
CI / check (push) Successful in 2m35s
CI / e2e (push) Failing after 8m41s
Removes the window between the scroll guard and the write it guards.

Closes #151
2026-08-20 03:58:04 +00:00
2 changed files with 64 additions and 3 deletions
+33
View File
@@ -3928,3 +3928,36 @@ fused half and was missing the retry; it has both now.
Worth generalising: a spec that resizes and then measures is asserting
about a moving target for the next dozen frames. Fuse, then poll.
## "The first N tracks" is not a way to ask for an ordinary one (2026-08-20)
`queue-selection.spec.ts` staged its queue from the first few rows of
`library.Library.GetTracks(0)` and clicked a track *name*, which
`explore-link` routes to that track's **album** page. Four tracks in the
fixture library have no album at all — `01 Tone A`, `02 Tone B`,
`Title Only`, `no-tags-at-all` — and a name with nothing to route to
renders as **plain text**, not as a link.
Two things follow, and the second is the sharper one.
**The order is the scan's.** `GetTracks` returns `audio_files.id` order,
i.e. the order the scan inserted rows, which depends on concurrency and
directory traversal. Locally the first eight are all from two proper
albums, so the spec passed twice over; CI rebuilds its seed with a real
scan, got a different eight, and failed on both engines. This is the
same family as "a seed freezes every default it has already persisted" —
the fixture library is not a list, it is a *set* with an incidental
order, and no spec should depend on that order.
**A loose locator hid it.** The row was located with
`.locator('.explore-link').first()`, and a row has two — the title and
the artist. When the title is plain text, `first()` silently resolves to
the **artist** link, so the click went somewhere real and the assertion
was about a destination the test had not exercised. `.track-title
.explore-link` is the locator that says which one it means; the loose
one turned a fixture problem into a mystery.
The general rule for this repo's fixture library: it is deliberately
full of edge cases (untagged, unicode, duplicates, extremes), so a spec
that wants an *ordinary* track has to **say so** — filter on the
property it depends on rather than slicing.
+31 -3
View File
@@ -93,10 +93,31 @@ async function queueSixAndOpen(app: Page): Promise<void> {
'library.Library.GetTracks',
[0],
10_000,
)) as { FilePath: string; TrackName: string }[];
)) as { FilePath: string; TrackName: string; Album: string }[];
const long = tracks.find((t) => t.TrackName === longTitle);
const rest = tracks.filter((t) => t.TrackName !== longTitle).slice(0, 5);
/**
* **Tracks that have an album**, which is a requirement of one of
* the tests and was previously left to luck (#156).
*
* `explore-link` routes a track name to its *album's* page, so a
* track with no album renders a name that navigates nowhere — and
* the fixture library deliberately contains two (`01 Tone A`,
* `02 Tone B`). Which tracks arrive first is `audio_files.id`
* order, i.e. the order the **scan** inserted them, which depends
* on concurrency and directory traversal: locally the first eight
* all had albums and the spec passed twice over, and CI rebuilds
* its seed with a real scan and got a different eight.
*
* Asking for what the test needs is the fix. It is not a
* narrowing: every assertion here wants an ordinary track, and
* "the first five rows" was never a way to ask for one in a
* library whose whole purpose is edge cases.
*/
const rest = tracks
.filter((t) => t.TrackName !== longTitle && t.Album !== '')
.slice(0, 5);
// Index 3 is the long one: far enough down that a shift-extend has
// room either side of it.
@@ -223,7 +244,14 @@ test.describe('selecting in the queue with a mouse', () => {
.poll(() => selected(app), { timeout: HIGHLIGHT_MS })
.toEqual([1]);
await row(app, 2).locator('.explore-link').first().click();
// `.track-title .explore-link`, not `.explore-link` first(): a row
// has two, and which one `first()` finds depends on whether the
// *title* is a link at all. It is not, for a track with no album —
// `explore-link` renders plain text where it cannot route — so the
// loose locator silently clicked the **artist** instead and the
// assertion below was about a different destination than the one
// being exercised (#156).
await row(app, 2).locator('.track-title .explore-link').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',