From a094d5f8b0b4a808f37ed2234de7f1a42b4645ce Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 6 Aug 2026 21:14:53 -0400 Subject: [PATCH] =?UTF-8?q?test(metrics):=20target=20deltas=20by=20test=20?= =?UTF-8?q?id,=20not=20by=20glyph=20=E2=80=94=20#2495?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two CI failures, both in my own new tests, both informative. settings: queryByText(/≈/) matched the LEGEND explaining the glyph rather than a delta, so the "no delta" case failed on the explanation being present. Delta spans now carry data-testid so a test can name what it means instead of pattern-matching prose that sits next to it. tuning: getByText("40%") found two elements. testing-library matches an element and its OWN direct text nodes, so the skip cell still matches "40%" despite the trailing play-count span — and discover late-week completion is also 40%. Genuinely ambiguous now; assert the count. --- web/src/routes/admin/tuning/tuning.test.ts | 9 ++++++--- web/src/routes/settings/+page.svelte | 4 ++++ web/src/routes/settings/settings.test.ts | 15 +++++++++------ 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/web/src/routes/admin/tuning/tuning.test.ts b/web/src/routes/admin/tuning/tuning.test.ts index 49a91bac..d0e2f63c 100644 --- a/web/src/routes/admin/tuning/tuning.test.ts +++ b/web/src/routes/admin/tuning/tuning.test.ts @@ -200,10 +200,13 @@ describe('Admin tuning page', () => { // Radio's latest week: 40% skip over 15 plays; Discover's: 60% over 5. expect(screen.getByText('/15')).toBeInTheDocument(); expect(screen.getByText('/5')).toBeInTheDocument(); - // Completion columns are unchanged and still bare percentages — radio 70%, - // discover 40%. + // Completion columns are unchanged and still bare percentages — radio 70%. expect(screen.getByText('70%')).toBeInTheDocument(); - expect(screen.getByText('40%')).toBeInTheDocument(); + // '40%' is now genuinely ambiguous: radio's latest SKIP rate and discover's + // latest COMPLETION are both 40%. testing-library matches an element's own + // direct text nodes, so the skip cell still matches despite its trailing + // play-count span. Assert the count rather than pretending it's unique. + expect(screen.getAllByText('40%')).toHaveLength(2); // The window/last-week distinction has to be visible in the headers, or the // Plays total reads as the denominator of the skip rate. That misreading is // what #2495 was filed over. diff --git a/web/src/routes/settings/+page.svelte b/web/src/routes/settings/+page.svelte index 96a950b6..17fbabc5 100644 --- a/web/src/routes/settings/+page.svelte +++ b/web/src/routes/settings/+page.svelte @@ -405,6 +405,7 @@ {pct(m.skip_rate)} {#if m.skip_delta} {deltaText(m.skip_delta)} {/if} @@ -412,6 +413,7 @@ {pct(m.avg_completion)} {#if m.completion_delta} {deltaText(m.completion_delta)} {/if} @@ -439,6 +441,7 @@ {pct(b.skip_rate)} {#if b.skip_delta} {deltaText(b.skip_delta)} {/if} @@ -446,6 +449,7 @@ {pct(b.avg_completion)} {#if b.completion_delta} {deltaText(b.completion_delta)} {/if} diff --git a/web/src/routes/settings/settings.test.ts b/web/src/routes/settings/settings.test.ts index 53179a6a..7923241b 100644 --- a/web/src/routes/settings/settings.test.ts +++ b/web/src/routes/settings/settings.test.ts @@ -269,16 +269,18 @@ describe('Settings page — Recommendation metrics card', () => { render(SettingsPage); await waitFor(() => expect(screen.getByText('Discover')).toBeInTheDocument()); - // The indistinguishable skip delta is prefixed and explained on hover. - const skip = screen.getByText('≈-13'); - expect(skip).toBeInTheDocument(); + // Targeted by test id rather than text: the legend below the table also + // contains a "≈", so matching on the glyph finds the explanation instead of + // the delta. (It did, on the first attempt at this test.) + const skip = screen.getByTestId('skip-delta-discover'); + expect(skip).toHaveTextContent('≈-13'); expect(skip).toHaveAttribute('title', expect.stringContaining('not distinguishable from zero')); // It must NOT be coloured as a real regression/improvement. expect(skip.className).toContain('opacity-60'); // The completion delta clears its margin, so it renders plainly. - const completion = screen.getByText('+28'); - expect(completion).toBeInTheDocument(); + const completion = screen.getByTestId('completion-delta-discover'); + expect(completion).toHaveTextContent('+28'); expect(completion.className).not.toContain('opacity-60'); // And the legend explains the glyph rather than leaving it a mystery. @@ -303,7 +305,8 @@ describe('Settings page — Recommendation metrics card', () => { }; render(SettingsPage); await waitFor(() => expect(screen.getByText('Radio')).toBeInTheDocument()); - expect(screen.queryByText(/≈/)).not.toBeInTheDocument(); + expect(screen.queryByTestId('skip-delta-radio')).not.toBeInTheDocument(); + expect(screen.queryByTestId('completion-delta-radio')).not.toBeInTheDocument(); }); test('surfaces without a breakdown render no toggle and no sub-rows', async () => {