Follow-up from PR #503, which added focused tests for buildVehiclePopupData (closing #501).
Two optional improvements were identified during review but are out of scope for that narrowly-scoped test PR:
-
Source-asymmetry test. buildVehiclePopupData sources nextDestination from activeTrip.tripHeadsign while vehicleId/lastUpdateTime/predicted come from vehicle. The current tests exercise this mapping but use all-distinct values, so a future source-swap bug (e.g. nextDestination: vehicle.tripHeadsign) would only be caught incidentally. Add a test where vehicle and activeTrip share field names with different values, asserting each field resolves from the correct argument.
-
Null activeTrip guard (code concern). buildVehiclePopupData assumes activeTrip, vehicle, and stopsMap are non-null. At the call sites (OpenStreetMapProvider.svelte.js and the Google equivalent), activeTrip comes from activeTripMap.get(...) and could in principle be undefined, in which case activeTrip.tripHeadsign throws. Consider adding a guard or documenting the precondition.
Neither is blocking; tracking here so they aren't lost.
Follow-up from PR #503, which added focused tests for
buildVehiclePopupData(closing #501).Two optional improvements were identified during review but are out of scope for that narrowly-scoped test PR:
Source-asymmetry test.
buildVehiclePopupDatasourcesnextDestinationfromactiveTrip.tripHeadsignwhilevehicleId/lastUpdateTime/predictedcome fromvehicle. The current tests exercise this mapping but use all-distinct values, so a future source-swap bug (e.g.nextDestination: vehicle.tripHeadsign) would only be caught incidentally. Add a test wherevehicleandactiveTripshare field names with different values, asserting each field resolves from the correct argument.Null
activeTripguard (code concern).buildVehiclePopupDataassumesactiveTrip,vehicle, andstopsMapare non-null. At the call sites (OpenStreetMapProvider.svelte.jsand the Google equivalent),activeTripcomes fromactiveTripMap.get(...)and could in principle beundefined, in which caseactiveTrip.tripHeadsignthrows. Consider adding a guard or documenting the precondition.Neither is blocking; tracking here so they aren't lost.