fix(roadtrip): the manual-stop dialog looped on a hint rebuilt per render

Three things the first CI run found, all of them in the new tests rather
than in what they cover.

The dialog's search effect depended on the location hint object. Its own
hook memoises, so nothing looped in the app, but the effect sets state on a
query too short to ask about, and a caller whose hook hands back a fresh
literal per render turns that pair into a render loop that never yields. The
suite mocked it exactly that way, and the worker hung before a single test
reported, which is what took the client coverage job past half an hour. The
hint is three numbers, so the effect now turns on those and reads the value
out of a ref when the request goes out. The mock hands back one object, the
way the real hook does.

The suite also installed fake timers in a beforeEach, which deadlocks
against React's scheduler and Testing Library's own waiting. Every wait is
a waitFor now, and the two cases that have to prove something did not
happen sit out a real pause. The debounce boundary is no longer asserted
against a wall clock, because a busy runner stretches any such window.

The charging fixtures are pinned to a fixed instant and the service reads
the real clock against a twenty-minute freshness window, so every answer
through it was stale by construction and available came back null. Date.now
is pinned for those tests, never the timers, since the requests carry an
AbortSignal.timeout that has to keep running.

And the unidentified-station test counted its two "no reliable data" lines
before the answer landed: the price line already says it while the request
is out, so the count settled on one and passed for the wrong reason.
This commit is contained in:
Maurice
2026-09-14 13:20:21 +02:00
parent 00d00be1c0
commit b058b63f6e
4 changed files with 73 additions and 23 deletions
@@ -66,7 +66,10 @@ describe('Charging information', () => {
render(<TranslationProvider><ChargingInfo lat={48.137} lng={11.575} name="Ladepark Nord" /></TranslationProvider>)
// 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()
})
})
@@ -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<React.ComponentProps<typeof RoadtripManualStopModal
/** The place box, by the placeholder every place search in TREK carries. */
const queryBox = () => screen.getByLabelText('Search places...')
/** Type, then wait out the debounce and whatever the answer resolves into. */
async function type(value: string, wait = DEBOUNCE): Promise<void> {
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<void> {
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.
@@ -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.
+16 -1
View File
@@ -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 }));