From 6d729d15120ba17e67e68b438584ebce90aae660 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Mon, 17 Aug 2026 00:25:40 -0400 Subject: [PATCH] =?UTF-8?q?fix(web):=20timeUntil=20rounds,=20so=20a=204h?= =?UTF-8?q?=20wait=20doesn't=20read=20as=203h=20=E2=80=94=20#2527?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught a real bug, not just a brittle test. The page rendered an attempt four hours away as "in 3h", because timeUntil floored the way relativeTime does. Flooring an elapsed time is honest: "3h ago" means at least three hours have passed. Flooring a countdown is not -- 3h59m away became "in 3h", so the operator comes back an hour early and finds nothing has happened. It rounds now, with the boundary cases pinned: sub-minute is "any moment", 59.6m is "in 1h", 24h is "in 1d". That divergence is now the fourth documented difference between the two formatters, all deliberate, all in snippet #2699 with a test asserting they disagree so nobody unifies them later. The test assertions were also genuinely wrong: they read raw textContent from a template that wraps mid-sentence, so "last 2d ago" arrived as "last\n 2d ago". Added a whitespace-normalising helper — asserting on raw textContent makes a test fail when the markup reflows, which says nothing about the behaviour. --- web/src/lib/utils/relativeTime.test.ts | 9 +++++++ web/src/lib/utils/relativeTime.ts | 19 ++++++++------ .../admin/missing-files/missing-files.test.ts | 25 +++++++++++++------ 3 files changed, 37 insertions(+), 16 deletions(-) diff --git a/web/src/lib/utils/relativeTime.test.ts b/web/src/lib/utils/relativeTime.test.ts index 72d2184e..3e3c3f63 100644 --- a/web/src/lib/utils/relativeTime.test.ts +++ b/web/src/lib/utils/relativeTime.test.ts @@ -57,6 +57,15 @@ describe('timeUntil', () => { expect(timeUntil(new Date(NOW.getTime() + delta).toISOString())).toBe(expected); }); + // The bug CI caught: flooring rendered a 4h-away attempt as "in 3h", + // sending the operator back an hour early. + test('rounds rather than floors, so nearly-4h reads as 4h', () => { + vi.useFakeTimers(); + vi.setSystemTime(NOW); + const almost4h = new Date(NOW.getTime() + 4 * 3_600_000 - 2_000).toISOString(); + expect(timeUntil(almost4h)).toBe('in 4h'); + }); + // A due-or-overdue attempt is the sweeper's next tick away, not "3h ago" — // the operator wants to know it is imminent, not how late it is. test('a moment already passed reads as imminent', () => { diff --git a/web/src/lib/utils/relativeTime.ts b/web/src/lib/utils/relativeTime.ts index a70a8276..cc7eb5be 100644 --- a/web/src/lib/utils/relativeTime.ts +++ b/web/src/lib/utils/relativeTime.ts @@ -38,12 +38,15 @@ export function relativeTime(iso: string): string { */ export function timeUntil(iso: string): string { const ms = new Date(iso).getTime() - Date.now(); - if (ms <= 0) return 'any moment'; - const days = Math.floor(ms / (24 * 3_600_000)); - if (days >= 1) return `in ${days}d`; - const hours = Math.floor(ms / 3_600_000); - if (hours >= 1) return `in ${hours}h`; - const minutes = Math.floor(ms / 60_000); - if (minutes >= 1) return `in ${minutes}m`; - return 'any moment'; + if (ms < 60_000) return 'any moment'; + // Rounds where relativeTime floors, and the difference matters. Flooring an + // elapsed time is honest — "3h ago" for 3h59m means "at least three hours". + // Flooring a countdown is not: it would show "in 3h" for something 3h59m + // away, so the operator comes back an hour early and finds nothing has + // happened. Caught by CI rendering a 4h-away attempt as "in 3h". + const minutes = Math.round(ms / 60_000); + if (minutes < 60) return `in ${minutes}m`; + const hours = Math.round(ms / 3_600_000); + if (hours < 24) return `in ${hours}h`; + return `in ${Math.round(ms / (24 * 3_600_000))}d`; } diff --git a/web/src/routes/admin/missing-files/missing-files.test.ts b/web/src/routes/admin/missing-files/missing-files.test.ts index 03cbea44..7e069762 100644 --- a/web/src/routes/admin/missing-files/missing-files.test.ts +++ b/web/src/routes/admin/missing-files/missing-files.test.ts @@ -66,6 +66,13 @@ const response: AdminMissingResponse = { afterEach(() => vi.clearAllMocks()); +// Whitespace-normalised text of a testid'd element. Svelte templates wrap +// mid-sentence, so raw textContent has newlines where the rendered page has +// single spaces — asserting on it directly makes tests fail on reflow. +function text(testId: string): string { + return (screen.getByTestId(testId).textContent ?? '').replace(/\s+/g, ' ').trim(); +} + function renderWith(data: AdminMissingResponse | undefined, extra = {}) { vi.mocked(createMissingFilesQuery).mockReturnValue( mockQuery({ data, ...extra }) as ReturnType @@ -134,12 +141,14 @@ describe('admin missing files', () => { gave_up_at: null }; renderWith(withState); - const line = screen.getByTestId('reacquisition-state'); - expect(line.textContent).toMatch(/asked lidarr twice/i); - expect(line.textContent).toMatch(/last 2d ago/i); + // Collapsed: the markup wraps mid-sentence, so textContent carries the + // template's newlines and indentation between the words. + const line = text('reacquisition-state'); + expect(line).toMatch(/asked lidarr twice/i); + expect(line).toMatch(/last 2d ago/i); // Forward-looking, not relativeTime — which would say "just now" for a // future timestamp and read as nonsense. - expect(line.textContent).toMatch(/next in 4h/i); + expect(line).toMatch(/next in 4h/i); }); test('a given-up album says so and says it can come back', () => { @@ -151,10 +160,10 @@ describe('admin missing files', () => { gave_up_at: new Date(Date.now() - 5 * DAY).toISOString() }; renderWith(withState); - const line = screen.getByTestId('reacquisition-state'); - expect(line.textContent).toMatch(/gave up/i); - expect(line.textContent).toMatch(/after 3 times/i); - expect(line.textContent).toMatch(/tried again/i); + const line = text('reacquisition-state'); + expect(line).toMatch(/gave up/i); + expect(line).toMatch(/after 3 times/i); + expect(line).toMatch(/tried again/i); }); // Paging only appears when it can do something: a single page of results