diff --git a/client/src/components/Roadtrip/ChargingInfo.test.tsx b/client/src/components/Roadtrip/ChargingInfo.test.tsx index 87d48bee8..864c3b615 100644 --- a/client/src/components/Roadtrip/ChargingInfo.test.tsx +++ b/client/src/components/Roadtrip/ChargingInfo.test.tsx @@ -66,7 +66,10 @@ describe('Charging information', () => { render() // Said twice over: once where the free count would be, once where the price would. - expect(await screen.findAllByText('No reliable data available')).toHaveLength(2) + // Counted only once the answer is in. The price line says the same thing while the + // request is still out, so a plain findAllByText settles on that one alone and the + // count would pass for the wrong reason. + await waitFor(() => expect(screen.getAllByText('No reliable data available')).toHaveLength(2)) expect(screen.queryByText(/\d+\/\d+/)).not.toBeInTheDocument() }) }) diff --git a/client/src/components/Roadtrip/RoadtripManualStopModal.test.tsx b/client/src/components/Roadtrip/RoadtripManualStopModal.test.tsx index 41c548c6a..95a81dc3c 100644 --- a/client/src/components/Roadtrip/RoadtripManualStopModal.test.tsx +++ b/client/src/components/Roadtrip/RoadtripManualStopModal.test.tsx @@ -12,9 +12,10 @@ vi.mock('../../api/client', async (importOriginal) => ({ mapsApi: { search }, })) // The hint is the trip's own doing and has its own suite; here it only has to arrive. -vi.mock('../../hooks/useLocationBias', () => ({ - useLocationBias: () => ({ point: { lat: 53, lng: 10, radius: 5000 } }), -})) +// Hoisted to one object rather than built per call, the way the real hook's useMemo +// hands back the same one until the trip's places move. +const BIAS = { point: { lat: 53, lng: 10, radius: 5000 } } +vi.mock('../../hooks/useLocationBias', () => ({ useLocationBias: () => BIAS })) import RoadtripManualStopModal from './RoadtripManualStopModal' @@ -100,20 +101,32 @@ function setup(over: Partial screen.getByLabelText('Search places...') -/** Type, then wait out the debounce and whatever the answer resolves into. */ -async function type(value: string, wait = DEBOUNCE): Promise { - fireEvent.change(queryBox(), { target: { value } }) - await act(async () => { vi.advanceTimersByTime(wait) }) +/** + * Typing, on the real clock. + * + * Deliberately not on a fake one. A faked timer here has to be installed before the + * dialog mounts, and React's own scheduler and Testing Library's waiting helpers both + * run on the same timers, so the pair deadlocks the worker before a single test + * reports. Every assertion below is reached through `waitFor` instead, and the two + * that have to prove something did NOT happen wait out a real pause. + */ +async function type(value: string): Promise { + await act(async () => { fireEvent.change(queryBox(), { target: { value } }) }) } +/** A real pause, long enough for the debounce to have fired if it were going to. */ +const settle = (ms = DEBOUNCE + 200) => act(async () => { await new Promise(resolve => setTimeout(resolve, ms)) }) + +/** The rows of the suggestion list, once the dialog has asked and been answered. */ +const answered = () => waitFor(() => expect(search).toHaveBeenCalled()) + beforeEach(() => { - vi.useFakeTimers({ shouldAdvanceTime: true }) search.mockReset() search.mockResolvedValue({ places: [hit()], source: 'openstreetmap' }) }) afterEach(() => { - vi.useRealTimers() + vi.restoreAllMocks() }) describe('RoadtripManualStopModal', () => { @@ -121,19 +134,23 @@ describe('RoadtripManualStopModal', () => { // A single common word without a region is refused upstream as too expensive to // answer, and asking anyway spends a request to be told so. setup() - await type('Su', DEBOUNCE * 2) + await type('Su') + await settle() expect(search).not.toHaveBeenCalled() }) it('FE-ROADTRIP-MANUAL-002: the search waits for a pause in the typing', async () => { setup() + await type('Supercharger') - fireEvent.change(queryBox(), { target: { value: 'Supercharger' } }) - await act(async () => { vi.advanceTimersByTime(DEBOUNCE - 120) }) + // Nothing goes out on the keystroke itself. The pause after it is measured on the + // real clock, so the test asks whether the request is debounced at all rather than + // whether the gap is exactly 320 ms: a busy runner can stretch any wall-clock + // window, and a boundary asserted against one is a flake waiting to happen. expect(search).not.toHaveBeenCalled() - await act(async () => { vi.advanceTimersByTime(120) }) + await answered() expect(search).toHaveBeenCalledTimes(1) // The query, the locale, and the hint that decides which of a thousand identically // named places is meant. @@ -143,13 +160,12 @@ describe('RoadtripManualStopModal', () => { it('FE-ROADTRIP-MANUAL-003: keystroke after keystroke is one question, not one each', async () => { setup() - fireEvent.change(queryBox(), { target: { value: 'Sup' } }) - await act(async () => { vi.advanceTimersByTime(100) }) - fireEvent.change(queryBox(), { target: { value: 'Super' } }) - await act(async () => { vi.advanceTimersByTime(100) }) - fireEvent.change(queryBox(), { target: { value: 'Supercharger' } }) - await act(async () => { vi.advanceTimersByTime(DEBOUNCE) }) + await type('Sup') + await type('Super') + await type('Supercharger') + await answered() + await settle() expect(search).toHaveBeenCalledTimes(1) expect(search).toHaveBeenCalledWith('Supercharger', expect.any(String), expect.anything()) }) @@ -278,6 +294,7 @@ describe('RoadtripManualStopModal', () => { search.mockRejectedValue(new Error('offline')) setup() await type('Supercharger') + await answered() // The rejection is swallowed on purpose (it is a typeahead, not a submit), but the // list has to stop saying it is loading. diff --git a/client/src/components/Roadtrip/RoadtripManualStopModal.tsx b/client/src/components/Roadtrip/RoadtripManualStopModal.tsx index 7e1365eeb..fa6f7f21f 100644 --- a/client/src/components/Roadtrip/RoadtripManualStopModal.tsx +++ b/client/src/components/Roadtrip/RoadtripManualStopModal.tsx @@ -126,6 +126,21 @@ export default function RoadtripManualStopModal({ return () => document.removeEventListener('mousedown', onPointer) }, [listOpen]) + /** + * The search hint, held by content rather than by identity. + * + * `useLocationBias` rebuilds its answer whenever the trip's places change, and the + * effect below has to re-ask when the hint really moves. Depending on the object + * itself makes that a question about identity instead: any caller whose hook hands + * back a fresh literal per render turns the effect into a render loop, because the + * effect sets state on a short query and the next render brings a new object. The + * hint is three numbers, so the key is those three numbers, and the value itself is + * read out of a ref at the moment the request actually goes out. + */ + const biasKey = locationBias ? `${locationBias.lat},${locationBias.lng},${locationBias.radius ?? ''}` : '' + const biasRef = useRef(locationBias) + biasRef.current = locationBias + // Debounced, and with a cancelled flag rather than a bare `.then(setState)`: the // endpoint takes no signal, so the only way a stale answer cannot overwrite a newer // one is for the run it belongs to to know it has been superseded. @@ -140,7 +155,7 @@ export default function RoadtripManualStopModal({ let cancelled = false debounceRef.current = setTimeout(() => { setLoading(true) - mapsApi.search(trimmed, locale, locationBias) + mapsApi.search(trimmed, locale, biasRef.current) .then(data => { if (cancelled) return setResults(data.places.map(toHit).filter((hit): hit is PlaceHit => hit !== null)) @@ -158,7 +173,7 @@ export default function RoadtripManualStopModal({ cancelled = true if (debounceRef.current) clearTimeout(debounceRef.current) } - }, [query, picked, locale, locationBias]) + }, [query, picked, locale, biasKey]) /** * Every leg of every routed day, flat. diff --git a/server/tests/unit/nest/charging.test.ts b/server/tests/unit/nest/charging.test.ts index 926bb4ea6..aa7b85dbe 100644 --- a/server/tests/unit/nest/charging.test.ts +++ b/server/tests/unit/nest/charging.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { chargingLookupSchema } from '@trek/shared'; import { ChargingService, empty } from '../../../src/nest/roadtrip/charging.service'; import { ChargingLookupController, ChargingLookupDto } from '../../../src/nest/roadtrip/charging-lookup.controller'; @@ -70,6 +70,21 @@ describe('Open charging data', () => { * difference is how the station is named. */ describe('Open charging data before the stop exists', () => { + /** + * The clock, pinned to the instant the fixtures above are written at. + * + * `normalizeCharging` takes the current time as an argument and the tests further up + * pass it; the service does not, so it reads the real one, and its freshness window is + * twenty minutes. Left alone, every answer that goes through the service is stale by + * construction and `available` comes back null, which is the field this is here to + * prove survives the trip from the registry to the dialog. + * + * Only `Date.now` is pinned, never the timers: the outbound requests carry an + * `AbortSignal.timeout` that has to keep running. + */ + beforeEach(() => { vi.spyOn(Date, 'now').mockReturnValue(now); }); + afterEach(() => { vi.restoreAllMocks(); }); + const answers = (tariffs: unknown) => vi.fn(async (url: string) => { if (url.includes('/sources')) return new Response(JSON.stringify({ items: [source] })); if (url.includes('/locations')) return new Response(JSON.stringify({ items: [station], total_count: 1 }));