From ea76fc4bc07327b9d8297aa6b2663660ca0d5947 Mon Sep 17 00:00:00 2001 From: German Khokhlov Date: Fri, 6 Nov 2026 22:01:39 +0400 Subject: [PATCH] Fold duplicate coverage tests into their neighbours --- .../1576-transit-bikeshare-coverage-prompt.md | 8 +- src/data/manager.js | 12 -- src/layers/bikeshare/coverageStatus.test.mjs | 121 ------------------ src/layers/bikeshare/rowRepaint.test.mjs | 79 +++++++++++- src/layers/bikeshare/viewport.js | 4 +- src/layers/promptRepaint.test.mjs | 120 ----------------- src/layers/transit/gatePrompt.test.mjs | 64 --------- src/layers/transit/lifecycle.test.mjs | 12 ++ .../transit/refreshErrorRepaint.test.mjs | 2 + 9 files changed, 94 insertions(+), 328 deletions(-) delete mode 100644 src/layers/bikeshare/coverageStatus.test.mjs delete mode 100644 src/layers/promptRepaint.test.mjs delete mode 100644 src/layers/transit/gatePrompt.test.mjs diff --git a/changelog.d/1576-transit-bikeshare-coverage-prompt.md b/changelog.d/1576-transit-bikeshare-coverage-prompt.md index 660c1f30b..4d13d8d3e 100644 --- a/changelog.d/1576-transit-bikeshare-coverage-prompt.md +++ b/changelog.d/1576-transit-bikeshare-coverage-prompt.md @@ -1,6 +1,4 @@ - Say why Transit and Bike Share are empty outside coverage. Over a city with - no feed or system (London, Tokyo) their Layers rows read - `No feed here yet · 17 regions available` and - `No bike share here yet · 61 systems available`, or how low to fly, instead - of `never`; leaving coverage clears the last-update time (German, @gekh, - #1576). + no feed or system (London, Tokyo) their Layers rows say so and count the + covered regions or systems, or say how low to fly, instead of `never`; + leaving coverage clears the last-update time (German, @gekh, #1576). diff --git a/src/data/manager.js b/src/data/manager.js index 688fb94d2..5de10f665 100644 --- a/src/data/manager.js +++ b/src/data/manager.js @@ -15,18 +15,6 @@ export class DataLayerManager extends LayerLifecycle { this._presentation.refresh(); } - /** - * Repaint the toggle panel now. For layers whose data arrives outside their - * manager tick (camera-driven loads such as Transit's proximity polls), so a - * row shows its count when the data lands instead of at the next interval. - * One DOM pass; skipped while the document is hidden. - * @param {string} [layerId] - The layer whose row changed. - */ - refreshLayerStats(layerId) { - this._dropRecoveredRefreshError(layerId); - this._refreshTogglePanel(); - } - _buildMetaText(layer) { return this._presentation.panel._buildMetaText(layer); } diff --git a/src/layers/bikeshare/coverageStatus.test.mjs b/src/layers/bikeshare/coverageStatus.test.mjs deleted file mode 100644 index 004c75c94..000000000 --- a/src/layers/bikeshare/coverageStatus.test.mjs +++ /dev/null @@ -1,121 +0,0 @@ -// BIKESHARE COVERAGE STATUS — a view with no system says so. -// -// With no system in range (London, Tokyo) the layer says why instead of -// "never", and keeps no date for data it no longer draws. -// -// Run with: npm test -import { test } from 'node:test'; -import assert from 'node:assert/strict'; -import { createState } from './state.js'; -import { createControls } from './controls.js'; -import { createModel } from './model.js'; -import { createViewport } from './viewport.js'; -import { GBFS_CITY_REGISTRY } from './registry.js'; - -const LONDON = { lat: 51.5072, lon: -0.1276 }; - -function makeLayer() { - const services = { - overlays: { - setOverlayEntries() {}, - setOverlaySourceVisible() {}, - clearOverlaySource() {}, - }, - render: { governorRequestRender() {} }, - }; - const state = createState({ services }); - const parts = { - queries: { collectDetectableStations: () => [] }, - model: createModel({ state, services, parts: {}, source: null }), - ingestion: { abortInFlight() {} }, - rendering: { removeCityPoints() {} }, - }; - parts.viewport = createViewport({ state, services, parts, source: null }); - const { methods } = createControls({ state, services, parts, source: {} }); - state._enabled = true; - return { state, methods, viewport: parts.viewport }; -} - -/** A viewer looking straight down at `at` from `height` metres. */ -function viewerAt(at, height) { - const rad = Math.PI / 180; - return { - camera: { - positionCartographic: { - height, - latitude: at.lat * rad, - longitude: at.lon * rad, - }, - computeViewRectangle: () => undefined, - }, - scene: { globe: {} }, - }; -} - -test('above the altitude gate the row says how low to fly', async () => { - const { state, methods, viewport } = makeLayer(); - state._viewer = viewerAt(LONDON, 2_000_000); - await viewport.runProximityCheck(); - - const stats = methods.getStats(); - assert.equal(stats.status, 'zoom-in'); - assert.equal(stats.statusMessage, 'Fly below 48 km to a covered city'); - assert.equal(stats.error, undefined); -}); - -test('a city with no system says so and counts the ones there are', async () => { - const { state, methods, viewport } = makeLayer(); - state._viewer = viewerAt(LONDON, 8_000); - await viewport.runProximityCheck(); - - const stats = methods.getStats(); - assert.equal(stats.status, 'zoom-in'); - assert.equal( - stats.statusMessage, - `No bike share here yet · ${GBFS_CITY_REGISTRY.length} systems available`, - ); - assert.equal(stats.count, 0); -}); - -test('a city with a system in range is not given the prompt', () => { - const { state, methods } = makeLayer(); - state._altitudeGateEnabled = true; - state._activeCityIds.add('boston-bluebikes'); - - assert.equal(methods.getStats().status, undefined); -}); - -test('a loading or failed city reports itself, not the prompt', () => { - const { state, methods } = makeLayer(); - state._altitudeGateEnabled = true; - state._loading = true; - assert.equal(methods.getStats().status, undefined); - - // A city that failed is dropped from range, and still owns the row. - state._loading = false; - state._failedCityIds.add('boston-bluebikes'); - const stats = methods.getStats(); - assert.equal(stats.status, undefined); - assert.equal(stats.error, 'GBFS fetch error'); -}); - -test('a disabled layer reports no prompt', () => { - const { state, methods } = makeLayer(); - state._enabled = false; - assert.equal(methods.getStats().status, undefined); -}); - -test('leaving every system behind drops the date of the data it drew', async () => { - for (const height of [8_000, 2_000_000]) { - const { state, methods, viewport } = makeLayer(); - state._lastUpdate = Date.now(); - // Boston's stations were drawn and dated; the camera moves to London. - state._activeCityIds.add('boston-bluebikes'); - state._cityRuntime.set('boston-bluebikes', {}); - state._viewer = viewerAt(LONDON, height); - await viewport.runProximityCheck(); - - assert.equal(state._activeCityIds.size, 0); - assert.equal(methods.getStats().lastUpdate, null, `at ${height} m`); - } -}); diff --git a/src/layers/bikeshare/rowRepaint.test.mjs b/src/layers/bikeshare/rowRepaint.test.mjs index 5f5538168..4bfea999c 100644 --- a/src/layers/bikeshare/rowRepaint.test.mjs +++ b/src/layers/bikeshare/rowRepaint.test.mjs @@ -1,11 +1,13 @@ -// Bike Share: after the camera settles, the last row the panel painted must -// match what the layer reports now. Each case drives the real viewport, +// Bike Share's Layers row. With no system in range it says why instead of +// "never" and keeps no date for data it no longer draws. After the camera +// settles, the last row the panel painted must match what the layer reports +// now. Each case drives the real viewport, // ingestion, rendering and controls; the manager is a spy that records the // row text (LayerPanel._buildMetaText) at every refreshLayerStats() call. import { test } from 'node:test'; import assert from 'node:assert/strict'; import * as Cesium from 'cesium'; -import { CITY_BY_ID } from './registry.js'; +import { CITY_BY_ID, GBFS_CITY_REGISTRY } from './registry.js'; import { createState } from './state.js'; import { createControls } from './controls.js'; import { createIngestion } from './ingestion.js'; @@ -175,6 +177,7 @@ function makeLiveLayer() { source, painted, lastPainted: () => painted.at(-1), + stats: () => controls.getStats(), live: () => rowText(controls.getStats()), moveTo(place, height = LOW_M) { Object.assign(camera, place, { height }); @@ -291,3 +294,73 @@ test('leaving the failed city of two repaints DEGRADED away', async () => { assert.doesNotMatch(layer.live(), /^degraded/); assert.equal(layer.lastPainted(), layer.live()); }); + +test('above the altitude gate the row says how low to fly', async () => { + const layer = makeLiveLayer(); + await layer.enable(); + await layer.moveTo(LONDON, 2_000_000); + const stats = layer.stats(); + assert.equal(stats.status, 'zoom-in'); + assert.equal(stats.statusMessage, 'Fly below 48 km to a covered city'); + assert.equal(stats.error, undefined); +}); + +test('a city with no system says so and counts the ones there are', async () => { + const layer = makeLiveLayer(); + await layer.enable(); + const stats = layer.stats(); + assert.equal(stats.status, 'zoom-in'); + assert.equal( + stats.statusMessage, + `No bike share here yet · ${GBFS_CITY_REGISTRY.length} systems available`, + ); + assert.equal(stats.count, 0); +}); + +test('a city in range, loading, failed or a disabled layer gets no prompt', async () => { + const layer = makeLiveLayer(); + assert.equal(layer.stats().status, undefined, 'disabled'); + + const info = deferred(); + layer.source.gates.set(`${BOSTON_ID}:info`, info); + await layer.enable(); + const entering = layer.moveTo(BOSTON); + assert.equal(layer.stats().status, undefined, 'loading'); + info.resolve(); + await entering; + await settle(); + assert.equal(layer.stats().status, undefined, 'loaded'); + + // Left mid-load: range is empty, but the aborted load still owns the row. + const leaving = makeLiveLayer(); + leaving.source.gates.set(`${BOSTON_ID}:info`, deferred()); + await leaving.enable(); + const loading = leaving.moveTo(BOSTON); + await settle(); + const left = leaving.moveTo(LONDON); + assert.equal(leaving.state._activeCityIds.size, 0); + assert.equal(leaving.stats().loading, true); + assert.equal(leaving.stats().status, undefined, 'leaving mid-load'); + await Promise.all([loading, left]); + + const failed = makeLiveLayer(); + failed.source.dead.add(BOSTON_ID); + await failed.enable(); + await failed.moveTo(BOSTON); + await settle(); + assert.equal(failed.stats().status, undefined, 'failed'); + assert.equal(failed.stats().error, 'GBFS fetch error'); +}); + +test('leaving every system behind drops the date of the data it drew', async () => { + for (const height of [LOW_M, 2_000_000]) { + const layer = makeLiveLayer(); + await layer.enable(); + await layer.moveTo(BOSTON); + await settle(); + assert.ok(layer.stats().lastUpdate, 'Boston is dated'); + + await layer.moveTo(LONDON, height); + assert.equal(layer.stats().lastUpdate, null, `at ${height} m`); + } +}); diff --git a/src/layers/bikeshare/viewport.js b/src/layers/bikeshare/viewport.js index d4cc44cb6..46b6b7d12 100644 --- a/src/layers/bikeshare/viewport.js +++ b/src/layers/bikeshare/viewport.js @@ -243,11 +243,9 @@ export function createViewport({ state: layerState, services, parts, source }) { nextActive.size !== layerState._activeCityIds.size || [...nextActive].some((cityId) => !layerState._activeCityIds.has(cityId)); layerState._activeCityIds = nextActive; - if (layerState._activeCityIds.size === 0) { + if (nextActive.size === 0) { layerState._count = 0; layerState._lastUpdate = null; - if (changed) refreshRow(); - return; } if (changed) refreshRow(); diff --git a/src/layers/promptRepaint.test.mjs b/src/layers/promptRepaint.test.mjs deleted file mode 100644 index 38193ca72..000000000 --- a/src/layers/promptRepaint.test.mjs +++ /dev/null @@ -1,120 +0,0 @@ -// A camera move that changes a Transit or Bike Share coverage prompt repaints -// the row at once, not at the next manager tick (15 s, 60 s). -import { test } from 'node:test'; -import assert from 'node:assert/strict'; -import { createState as createBikeState } from './bikeshare/state.js'; -import { createControls as createBikeControls } from './bikeshare/controls.js'; -import { createModel as createBikeModel } from './bikeshare/model.js'; -import { createViewport as createBikeViewport } from './bikeshare/viewport.js'; -import { createState as createTransitState } from './transit/state.js'; -import { createViewport as createTransitViewport } from './transit/viewport.js'; -import { createQueries as createTransitQueries } from './transit/queries.js'; - -const rad = Math.PI / 180; -const LONDON = { lat: 51.5072, lon: -0.1276 }; -const BOSTON = { lat: 42.3601, lon: -71.0589 }; - -const services = () => ({ - overlays: { - setOverlayEntries() {}, - setOverlaySourceVisible() {}, - clearOverlaySource() {}, - }, - render: { governorRequestRender() {} }, -}); - -function viewer(at, height) { - return { - camera: { - positionCartographic: { - height, - latitude: at.lat * rad, - longitude: at.lon * rad, - }, - computeViewRectangle: () => undefined, - }, - scene: { globe: {} }, - }; -} - -function spyManager() { - const dm = { repaints: 0 }; - dm.refreshLayerStats = () => { - dm.repaints++; - }; - return dm; -} - -test('Bike Share: flying from London into Boston repaints the prompt row', async () => { - const svc = services(); - const state = createBikeState({ services: svc }); - const parts = { - queries: { collectDetectableStations: () => [] }, - model: createBikeModel({ state, services: svc, parts: {}, source: null }), - ingestion: { - abortInFlight() {}, - loadCityStationInfo: async () => new Map(), - loadCityStationStatus: async () => new Map(), - }, - rendering: { - removeCityPoints() {}, - ensureCityPoints() {}, - applyStatusToPoints() {}, - }, - }; - parts.viewport = createBikeViewport({ - state, - services: svc, - parts, - source: null, - }); - const { methods } = createBikeControls({ - state, - services: svc, - parts, - source: {}, - }); - state._enabled = true; - const dm = spyManager(); - methods.attachDataManager(dm); - - state._viewer = viewer(LONDON, 8_000); - await parts.viewport.runProximityCheck(); - assert.equal(methods.getStats().status, 'zoom-in'); - - const before = dm.repaints; - state._viewer = viewer(BOSTON, 3_000); - await parts.viewport.runProximityCheck(); - assert.equal(methods.getStats().status, undefined, 'Boston is covered'); - assert.ok(dm.repaints > before, 'the row was asked to repaint'); -}); - -test('Transit: leaving Boston for London repaints the row', () => { - const svc = services(); - const state = createTransitState({ services: svc }); - const parts = { - ingestion: { - feedStatus: () => ({ count: 0 }), - pollFeed() {}, - removeFeedVehicles() {}, - sweepAgedVehicles() {}, - }, - rendering: { requestVisibility() {}, syncRenderHold() {} }, - }; - parts.viewport = createTransitViewport({ state, services: svc, parts }); - const queries = createTransitQueries({ state, parts }).methods; - state._enabled = true; - const dm = spyManager(); - state._dataManager = dm; - - state._viewer = viewer(BOSTON, 4_000); - parts.viewport.runProximityCheck(); - assert.ok(state._activeFeeds.size > 0, 'Boston has a feed'); - state._lastUpdate = Date.now(); - - const before = dm.repaints; - state._viewer = viewer(LONDON, 4_000); - parts.viewport.runProximityCheck(); - assert.equal(queries.getStats().status, 'zoom-in'); - assert.ok(dm.repaints > before, 'the row was asked to repaint'); -}); diff --git a/src/layers/transit/gatePrompt.test.mjs b/src/layers/transit/gatePrompt.test.mjs deleted file mode 100644 index dc4c3e863..000000000 --- a/src/layers/transit/gatePrompt.test.mjs +++ /dev/null @@ -1,64 +0,0 @@ -// The Transit prompt names the altitude the gate opens at: at 2,900 km over -// Boston (under 3,000 km, above the 2,850 km enter threshold) nothing loads. -import { test } from 'node:test'; -import assert from 'node:assert/strict'; -import { createState } from './state.js'; -import { createViewport } from './viewport.js'; -import { createQueries } from './queries.js'; -import { ACTIVATION_ENTER_ALTITUDE_M } from './policy.js'; - -const BOSTON = { lat: 42.3601, lon: -71.0589 }; - -function makeLayer() { - const services = { - overlays: { - setOverlayEntries() {}, - setOverlaySourceVisible() {}, - clearOverlaySource() {}, - }, - render: { governorRequestRender() {} }, - }; - const state = createState({ services }); - const polled = []; - const parts = { - ingestion: { - feedStatus: () => ({ count: 0 }), - pollFeed: (feed) => polled.push(feed.id), - removeFeedVehicles() {}, - sweepAgedVehicles() {}, - }, - rendering: { requestVisibility() {}, syncRenderHold() {} }, - }; - parts.viewport = createViewport({ state, services, parts }); - const queries = createQueries({ state, parts }).methods; - state._enabled = true; - const rad = Math.PI / 180; - state._viewer = { - camera: { - positionCartographic: { - height: 0, - latitude: BOSTON.lat * rad, - longitude: BOSTON.lon * rad, - }, - computeViewRectangle: () => undefined, - }, - scene: { globe: {} }, - }; - return { state, queries, viewport: parts.viewport, polled }; -} - -test('below the prompted altitude but above the gate, the prompt names the gate', () => { - const { state, queries, viewport, polled } = makeLayer(); - state._viewer.camera.positionCartographic.height = 2_900_000; - viewport.runProximityCheck(); - - // Nothing loads at 2,900 km: the gate opens at the enter threshold. - assert.equal(state._altitudeGateOpen, false); - assert.deepEqual(polled, []); - - const km = Math.round(ACTIVATION_ENTER_ALTITUDE_M / 1000).toLocaleString( - 'en-US', - ); - const { statusMessage } = queries.getStats(); - assert.equal(statusMessage, `Fly below ${km} km to a covered region`); -}); diff --git a/src/layers/transit/lifecycle.test.mjs b/src/layers/transit/lifecycle.test.mjs index dc18fd6f0..4711eebdf 100644 --- a/src/layers/transit/lifecycle.test.mjs +++ b/src/layers/transit/lifecycle.test.mjs @@ -4021,3 +4021,15 @@ test('a view with no feed reads its coverage prompt, not an undated feed', async 'Fly below 2,850 km to a covered region', ); }); + +test('the prompt names the altitude the gate opens at, not 3,000 km', async (t) => { + // 2,900 km is under ACTIVATION_ALTITUDE_M but above the enter threshold. + const app = harness(t, { altitude: 2_900_000 }); + app.layer.enable(app.viewer); + await app.layer.update(); + assert.deepEqual(app.requested, [], 'nothing loads over Boston'); + assert.equal( + app.layer.getStats().statusMessage, + 'Fly below 2,850 km to a covered region', + ); +}); diff --git a/src/layers/transit/refreshErrorRepaint.test.mjs b/src/layers/transit/refreshErrorRepaint.test.mjs index d1a5c16b8..d85a4a375 100644 --- a/src/layers/transit/refreshErrorRepaint.test.mjs +++ b/src/layers/transit/refreshErrorRepaint.test.mjs @@ -91,8 +91,10 @@ test('leaving a feed whose tick failed repaints its error away', async () => { await lifecycle.refreshLayer('transit'); assert.match(row(), /feed unavailable/, 'the tick recorded the failure'); + const before = painted.length; state._viewer = viewer(LONDON, 4_000); parts.viewport.runProximityCheck(); assert.equal(queries.getStats().status, 'zoom-in'); + assert.ok(painted.length > before, 'leaving Boston repainted the row'); assert.doesNotMatch(painted.at(-1), /feed unavailable/); });