From 2365806d1873a4940500f05ffc427014194f4267 Mon Sep 17 00:00:00 2001 From: Logan Date: Thu, 20 Aug 2026 00:23:31 -0400 Subject: [PATCH] fix(e2e): ask the fixture for a track that can navigate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 --- .planning/NOTES.md | 33 ++++++++++++++++++++++++++++++ e2e/specs/queue-selection.spec.ts | 34 ++++++++++++++++++++++++++++--- 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/.planning/NOTES.md b/.planning/NOTES.md index 1cf0812..caab6bf 100644 --- a/.planning/NOTES.md +++ b/.planning/NOTES.md @@ -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. diff --git a/e2e/specs/queue-selection.spec.ts b/e2e/specs/queue-selection.spec.ts index 6c37eae..56f5254 100644 --- a/e2e/specs/queue-selection.spec.ts +++ b/e2e/specs/queue-selection.spec.ts @@ -93,10 +93,31 @@ async function queueSixAndOpen(app: Page): Promise { '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', -- 2.54.0