From 7c4cf557155ad24f6934391ed8379c703b6350a1 Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 7 Jul 2026 15:51:57 +0200 Subject: [PATCH] test(web): make Libraries scan-polling tests deterministic (fixes #157) The five Libraries polling tests mock window.setInterval to capture the poll handler, but that handler is registered in a passive useEffect that only runs after an out-of-act state commit (the async fetch that flips hasActiveScans). findBy*/waitFor resolve on the DOM mutation via MutationObserver, which can win the race against React's passive-effect flush under CI load - leaving intervalHandlers empty when the test invokes it. The forEach was then a silent no-op: no poll fetch, so 'route delta' timed out at scan-status count > 1, and 'stops polling' never cleared the 75% label. Fix: new runPollTick helper flushes pending effects with an empty act(), asserts the poll handler is actually registered (turning any future regression into a clear failure instead of a timeout), and invokes the handlers inside act() so fetch microtasks and state updates are fully applied before assertions run. All post-tick waitFor blocks become plain synchronous expects - nothing depends on wall-clock scheduling anymore. Also aligned the 'stops polling' setInterval mock with its siblings' >=10000ms cadence filter so it captures exactly the poll handler, not waitFor's internal 50ms interval callback. Verified: 20x full App.test.tsx loop under CPU contention, 0 failures; full vitest suite 202 passed; tsc + eslint clean. Co-Authored-By: Claude Fable 5 --- web/src/App.test.tsx | 70 +++++++++++++++++++++++--------------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index 98019317f..17e7b2c7e 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -1,4 +1,4 @@ -import { cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { App } from './App'; import { Button, Checkbox, Input, ProgressBar, Switch, Tabs, Toast, Tooltip } from './components'; @@ -1274,19 +1274,17 @@ describe('ChicoryTV SPA scaffold', () => { expect(fetchCount('/api/libraries/scan-status')).toBe(1); const sourceFetchesBeforePoll = fetchCount('/api/media-sources'); - intervalHandlers.forEach((handler) => handler()); + await runPollTick(intervalHandlers); - await waitFor(() => { - expect(fetchCount('/api/libraries/scan-status')).toBeGreaterThan(1); - }); + expect(fetchCount('/api/libraries/scan-status')).toBeGreaterThan(1); expect(fetchCount('/api/media-sources')).toBe(sourceFetchesBeforePoll); }); it('stops Libraries scan polling and refreshes sources once when scans complete', async () => { const intervalHandlers: Array<() => void> = []; let clearCount = 0; - vi.spyOn(window, 'setInterval').mockImplementation((handler: TimerHandler) => { - if (typeof handler === 'function') { + vi.spyOn(window, 'setInterval').mockImplementation((handler: TimerHandler, timeout?: number) => { + if (typeof handler === 'function' && (timeout ?? 0) >= 10000) { intervalHandlers.push(handler as () => void); } @@ -1311,11 +1309,9 @@ describe('ChicoryTV SPA scaffold', () => { expect(await screen.findByText('75%')).toBeInTheDocument(); const sourceFetchesBeforeCompletion = fetchCount('/api/media-sources'); - intervalHandlers.forEach((handler) => handler()); + await runPollTick(intervalHandlers); - await waitFor(() => { - expect(screen.queryByText('75%')).not.toBeInTheDocument(); - }); + expect(screen.queryByText('75%')).not.toBeInTheDocument(); expect(fetchCount('/api/media-sources')).toBe(sourceFetchesBeforeCompletion + 1); expect(clearCount).toBeGreaterThan(0); }); @@ -1390,14 +1386,12 @@ describe('ChicoryTV SPA scaffold', () => { const sourceFetchesBeforeCompletion = fetchCount('/api/media-sources'); - intervalHandlers.forEach((handler) => handler()); - expect(await screen.findByText('30%')).toBeInTheDocument(); + await runPollTick(intervalHandlers); + expect(screen.getByText('30%')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeDisabled(); - intervalHandlers.forEach((handler) => handler()); - await waitFor(() => { - expect(screen.queryByText('30%')).not.toBeInTheDocument(); - }); + await runPollTick(intervalHandlers); + expect(screen.queryByText('30%')).not.toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Scan Movies' })).not.toBeDisabled(); expect(fetchCount('/api/media-sources')).toBe(sourceFetchesBeforeCompletion + 1); expect(clearCount).toBeGreaterThan(0); @@ -1440,17 +1434,13 @@ describe('ChicoryTV SPA scaffold', () => { expect(intervalHandlers.length).toBeGreaterThan(0); // The id never shows up in scan-status. It survives a couple of ticks... - intervalHandlers.forEach((handler) => handler()); - await waitFor(() => { - expect(fetchCount('/api/libraries/scan-status')).toBe(3); - }); + await runPollTick(intervalHandlers); + expect(fetchCount('/api/libraries/scan-status')).toBe(3); expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeDisabled(); // ...but expires once the grace window runs out, freeing the button and the poll. - intervalHandlers.forEach((handler) => handler()); - await waitFor(() => { - expect(screen.getByRole('button', { name: 'Scan Movies' })).not.toBeDisabled(); - }); + await runPollTick(intervalHandlers); + expect(screen.getByRole('button', { name: 'Scan Movies' })).not.toBeDisabled(); expect(clearCount).toBeGreaterThan(0); }); @@ -1496,18 +1486,14 @@ describe('ChicoryTV SPA scaffold', () => { expect(intervalHandlers.length).toBeGreaterThan(0); // scan-status keeps erroring on every poll tick... - intervalHandlers.forEach((handler) => handler()); - await waitFor(() => { - expect(fetchCount('/api/libraries/scan-status')).toBe(3); - }); + await runPollTick(intervalHandlers); + expect(fetchCount('/api/libraries/scan-status')).toBe(3); expect(screen.getByRole('button', { name: 'Scan Movies' })).toBeDisabled(); // ...but the same grace budget burns down on failures too, so persistent failure // eventually frees the button and stops the interval instead of polling forever. - intervalHandlers.forEach((handler) => handler()); - await waitFor(() => { - expect(screen.getByRole('button', { name: 'Scan Movies' })).not.toBeDisabled(); - }); + await runPollTick(intervalHandlers); + expect(screen.getByRole('button', { name: 'Scan Movies' })).not.toBeDisabled(); expect(clearCount).toBeGreaterThan(0); }); @@ -3805,6 +3791,24 @@ function mockDashboardApi({ }); } +// Deterministically fires one captured poll tick for the mocked-setInterval polling tests. +// +// The Libraries poll interval is registered in a passive useEffect that only runs after +// an out-of-act state commit (the async fetch that flips hasActiveScans), so `findBy*` +// queries - which resolve on the DOM mutation, via MutationObserver - can win the race +// against React's effect flush and observe an empty handler list (the #157 flake). +// The leading empty act() flushes those pending effects, the length assertion turns a +// still-missing registration into a clear failure instead of a silent no-op forEach, +// and the act() around the handlers applies the fetch microtasks + state updates before +// the caller asserts, so no assertion depends on wall-clock scheduling. +async function runPollTick(intervalHandlers: Array<() => void>): Promise { + await act(async () => {}); + expect(intervalHandlers.length).toBeGreaterThan(0); + await act(async () => { + intervalHandlers.forEach((handler) => handler()); + }); +} + function fetchCount(path: string): number { return vi.mocked(window.fetch).mock.calls.filter(([input]) => input.toString() === path).length; }