fix(web): timeUntil rounds, so a 4h wait doesn't read as 3h — #2527
test-web / test (push) Successful in 34s
test-web / test (push) Successful in 34s
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.
This commit is contained in:
@@ -57,6 +57,15 @@ describe('timeUntil', () => {
|
|||||||
expect(timeUntil(new Date(NOW.getTime() + delta).toISOString())).toBe(expected);
|
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" —
|
// 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.
|
// the operator wants to know it is imminent, not how late it is.
|
||||||
test('a moment already passed reads as imminent', () => {
|
test('a moment already passed reads as imminent', () => {
|
||||||
|
|||||||
@@ -38,12 +38,15 @@ export function relativeTime(iso: string): string {
|
|||||||
*/
|
*/
|
||||||
export function timeUntil(iso: string): string {
|
export function timeUntil(iso: string): string {
|
||||||
const ms = new Date(iso).getTime() - Date.now();
|
const ms = new Date(iso).getTime() - Date.now();
|
||||||
if (ms <= 0) return 'any moment';
|
if (ms < 60_000) return 'any moment';
|
||||||
const days = Math.floor(ms / (24 * 3_600_000));
|
// Rounds where relativeTime floors, and the difference matters. Flooring an
|
||||||
if (days >= 1) return `in ${days}d`;
|
// elapsed time is honest — "3h ago" for 3h59m means "at least three hours".
|
||||||
const hours = Math.floor(ms / 3_600_000);
|
// Flooring a countdown is not: it would show "in 3h" for something 3h59m
|
||||||
if (hours >= 1) return `in ${hours}h`;
|
// away, so the operator comes back an hour early and finds nothing has
|
||||||
const minutes = Math.floor(ms / 60_000);
|
// happened. Caught by CI rendering a 4h-away attempt as "in 3h".
|
||||||
if (minutes >= 1) return `in ${minutes}m`;
|
const minutes = Math.round(ms / 60_000);
|
||||||
return 'any moment';
|
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`;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -66,6 +66,13 @@ const response: AdminMissingResponse = {
|
|||||||
|
|
||||||
afterEach(() => vi.clearAllMocks());
|
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 = {}) {
|
function renderWith(data: AdminMissingResponse | undefined, extra = {}) {
|
||||||
vi.mocked(createMissingFilesQuery).mockReturnValue(
|
vi.mocked(createMissingFilesQuery).mockReturnValue(
|
||||||
mockQuery({ data, ...extra }) as ReturnType<typeof createMissingFilesQuery>
|
mockQuery({ data, ...extra }) as ReturnType<typeof createMissingFilesQuery>
|
||||||
@@ -134,12 +141,14 @@ describe('admin missing files', () => {
|
|||||||
gave_up_at: null
|
gave_up_at: null
|
||||||
};
|
};
|
||||||
renderWith(withState);
|
renderWith(withState);
|
||||||
const line = screen.getByTestId('reacquisition-state');
|
// Collapsed: the markup wraps mid-sentence, so textContent carries the
|
||||||
expect(line.textContent).toMatch(/asked lidarr twice/i);
|
// template's newlines and indentation between the words.
|
||||||
expect(line.textContent).toMatch(/last 2d ago/i);
|
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
|
// Forward-looking, not relativeTime — which would say "just now" for a
|
||||||
// future timestamp and read as nonsense.
|
// 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', () => {
|
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()
|
gave_up_at: new Date(Date.now() - 5 * DAY).toISOString()
|
||||||
};
|
};
|
||||||
renderWith(withState);
|
renderWith(withState);
|
||||||
const line = screen.getByTestId('reacquisition-state');
|
const line = text('reacquisition-state');
|
||||||
expect(line.textContent).toMatch(/gave up/i);
|
expect(line).toMatch(/gave up/i);
|
||||||
expect(line.textContent).toMatch(/after 3 times/i);
|
expect(line).toMatch(/after 3 times/i);
|
||||||
expect(line.textContent).toMatch(/tried again/i);
|
expect(line).toMatch(/tried again/i);
|
||||||
});
|
});
|
||||||
|
|
||||||
// Paging only appears when it can do something: a single page of results
|
// Paging only appears when it can do something: a single page of results
|
||||||
|
|||||||
Reference in New Issue
Block a user