From e5c74c695edf75de2503a9f363305d5e213cb176 Mon Sep 17 00:00:00 2001 From: Sameh Khamis Date: Fri, 16 Oct 2026 15:31:08 -0700 Subject: [PATCH] Move the #971 tests into the director suite and add the changelog fragment The standalone director971.test.mjs rebuilt the browser, storage and viewer doubles that director.test.mjs already provides. Its cases now live in the director suite on makeDirector/installSceneRuntime, which gains optional seeded storage entries and a quota-full switch; existing tests are unchanged. The checkpoint and failed-save-status cases are split so each fails on its own. --- .../1135-director-save-status-run-results.md | 5 + scripts/format-scope.json | 1 - src/scenes/director.test.mjs | 86 ++++++- src/scenes/director971.test.mjs | 210 ------------------ 4 files changed, 85 insertions(+), 217 deletions(-) create mode 100644 changelog.d/1135-director-save-status-run-results.md delete mode 100644 src/scenes/director971.test.mjs diff --git a/changelog.d/1135-director-save-status-run-results.md b/changelog.d/1135-director-save-status-run-results.md new file mode 100644 index 000000000..aa3c6f0e6 --- /dev/null +++ b/changelog.d/1135-director-save-status-run-results.md @@ -0,0 +1,5 @@ +- Scenes keep the "Scene not saved" status when a capture, update, append or + import cannot be saved, and an unreadable saved project no longer overwrites + the recovery checkpoint. `startScene()` resolves to its documented result, + `afterShotId` works without an explicit scene, and `scenesRun` counts the + distinct scenes queued (djabhi31, #1135). diff --git a/scripts/format-scope.json b/scripts/format-scope.json index c097ee5de..a2b3e01f8 100644 --- a/scripts/format-scope.json +++ b/scripts/format-scope.json @@ -38,7 +38,6 @@ "scripts/build-admin-packs.mjs", "scripts/qa-admin-outlines.mjs", "src/data/inputOwnership.test.mjs", - "src/scenes/director971.test.mjs", "docs/CODE-BOUNDARIES.md", "src/main.js", "docs/APPLICATION.md", diff --git a/src/scenes/director.test.mjs b/src/scenes/director.test.mjs index 95552025f..f72b43dc7 100644 --- a/src/scenes/director.test.mjs +++ b/src/scenes/director.test.mjs @@ -601,8 +601,12 @@ const PROJECT_FIXTURE = { }], }; -/** Stub the browser globals the director touches, headlessly. */ -function installSceneRuntime(project = PROJECT_FIXTURE) { +/** + * Stub the browser globals the director touches, headlessly. `storage` replaces + * the seeded entries (key -> raw string); setting `localStorage.full` makes + * every write throw a quota error. + */ +function installSceneRuntime(project = PROJECT_FIXTURE, { storage } = {}) { const originalDocument = globalThis.document; const originalLocalStorage = globalThis.localStorage; const noopClassList = { add() {}, remove() {}, toggle() {}, contains: () => false }; @@ -614,10 +618,17 @@ function installSceneRuntime(project = PROJECT_FIXTURE) { removeEventListener() {}, body: { classList: noopClassList, appendChild() {} }, }; - const stored = new Map([['godsEyeView.sceneProject.v2', JSON.stringify(project)]]); + const stored = new Map(storage + ? Object.entries(storage) + : [['godsEyeView.sceneProject.v2', JSON.stringify(project)]]); globalThis.localStorage = { + stored, + full: false, getItem: (key) => stored.get(key) ?? null, - setItem: (key, value) => stored.set(key, value), + setItem(key, value) { + if (this.full) throw new DOMException('quota exceeded', 'QuotaExceededError'); + stored.set(key, value); + }, removeItem: (key) => stored.delete(key), }; @@ -718,7 +729,8 @@ async function settle(turns = 8) { /** Build a director over doubles, with the fixture project loaded. */ function makeDirector(options = {}) { - const restore = installSceneRuntime(options.project); + const restore = installSceneRuntime(options.project, { storage: options.storage }); + const storage = globalThis.localStorage; const viewer = fakeViewer(); const styleManager = fakeStyleManager(options.style); const dataManager = fakeDataManager(options.data); @@ -728,7 +740,7 @@ function makeDirector(options = {}) { // Telemetry is only accumulated during a run; observable-failure assertions // need the accumulator without driving a whole run. director._activeRun = { events: [] }; - return { director, viewer, styleManager, dataManager, restore }; + return { director, viewer, styleManager, dataManager, storage, restore }; } test('only Play Shot or a scene run grants transient media authority; LOAD, Stop and replacement revoke it', async () => { @@ -1584,3 +1596,65 @@ test('camera refusal starts no authored frame or playback clock', async () => { assert.deepEqual(viewer.flights, []); } finally { restore(); } }); + +test('an unreadable saved project does not overwrite its recovery checkpoint (#971)', async () => { + const unreadable = '{"version":99,"scenes":[]}'; + const warn = console.warn; + console.warn = () => {}; + const { director, storage, restore } = makeDirector({ storage: { + 'godsEyeView.sceneProject.v2': unreadable, + 'godsEyeView.sceneProject.checkpoint.v1': 'EARLIER-CHECKPOINT', + } }); + try { + assert.equal(storage.stored.get('godsEyeView.sceneProject.v2'), unreadable); + assert.equal(storage.stored.get('godsEyeView.sceneProject.checkpoint.v1'), 'EARLIER-CHECKPOINT'); + } finally { console.warn = warn; await director.destroy(); restore(); } +}); + +test('a capture, update or import that cannot be saved keeps the "Scene not saved" status (#971)', async () => { + const warn = console.warn; + console.warn = () => {}; + const { director, storage, restore } = makeDirector(); + let status = ''; + director.subscribe(({ state }) => { status = state.status; }); + try { + storage.full = true; + director._selectedSceneId = 'scene-1'; + director._selectedShotId = 'shot-a'; + director.captureShot(); + assert.match(status, /^Scene not saved/, 'failed capture keeps the save failure'); + director.updateSelectedShot(); + assert.match(status, /^Scene not saved/, 'failed update keeps the save failure'); + assert.equal(await director.importProjectFile( + { name: 'fixture.json' }, + { prepared: { project: structuredClone(PROJECT_FIXTURE), assets: [] } }, + ), true); + assert.match(status, /^Scene not saved/, 'failed import keeps the save failure'); + } finally { console.warn = warn; await director.destroy(); restore(); } +}); + +test('startScene resolves its result, resumes after a shot without a scene id, and counts distinct scenes (#971)', async () => { + const { director, restore } = makeDirector(); + try { + const options = { single: true, preview: false }; + assert.deepEqual(await director.startScene('scene-1', options), { started: true, shots: 2 }); + assert.equal(director._lastRun.scenesRun, 1, 'one scene with two shots is one scene run'); + assert.deepEqual( + await director.startScene(undefined, { ...options, afterShotId: 'shot-a' }), + { started: true, shots: 1 }, + ); + } finally { await director.destroy(); restore(); } +}); + +test('a caught run failure resolves as run-failed, not success (#971)', async () => { + const { director, styleManager, restore } = makeDirector(); + const warn = console.warn; + console.warn = () => {}; + styleManager.applyVisualState = async () => { throw new Error('visual failed'); }; + try { + assert.deepEqual( + await director.startScene('scene-1', { single: true, preview: false }), + { started: false, reason: 'run-failed' }, + ); + } finally { console.warn = warn; await director.destroy(); restore(); } +}); diff --git a/src/scenes/director971.test.mjs b/src/scenes/director971.test.mjs deleted file mode 100644 index 5328fd568..000000000 --- a/src/scenes/director971.test.mjs +++ /dev/null @@ -1,210 +0,0 @@ -import assert from 'node:assert/strict'; -import test from 'node:test'; -import { SceneDirector } from './director.js'; - -const STORAGE_KEY = 'godsEyeView.sceneProject.v2'; -const CHECKPOINT_KEY = 'godsEyeView.sceneProject.checkpoint.v1'; -const pose = { - lat: 10, - lon: 20, - alt: 500000, - heading: 0, - pitch: -40, - roll: 0, -}; -const shot = (id) => ({ - id, - title: id, - durationSec: 0.2, - holdSec: 0, - camera: pose, - visual: { style: 'normal' }, - layers: {}, -}); -const project = JSON.stringify({ - version: 3, - scenes: [{ id: 's', title: 'S', shots: [shot('a'), shot('b')] }], -}); - -function classList() { - return { - add() {}, - remove() {}, - toggle() {}, - contains: () => false, - }; -} - -function installDocument() { - globalThis.document = { - getElementById: () => null, - createElement: () => ({ - classList: classList(), - style: {}, - dataset: {}, - appendChild() {}, - remove() {}, - click() {}, - addEventListener() {}, - removeEventListener() {}, - setAttribute() {}, - }), - addEventListener() {}, - removeEventListener() {}, - body: { classList: classList(), appendChild() {} }, - }; -} - -function make(initial, { styleOverrides = {}, dataOverrides = {} } = {}) { - const stored = new Map(Object.entries(initial)); - const storage = { - stored, - full: false, - getItem: (key) => stored.get(key) ?? null, - setItem(key, value) { - if (this.full) - throw new DOMException('quota exceeded', 'QuotaExceededError'); - stored.set(key, String(value)); - }, - removeItem: (key) => stored.delete(key), - }; - globalThis.localStorage = storage; - const viewer = { - camera: { - flyTo: (options) => Promise.resolve().then(() => options.complete?.()), - cancelFlight() {}, - }, - }; - const style = { - runImmediateNavigation: (_name, run) => run(), - getCameraState: () => pose, - getVisualState: () => ({ style: 'normal' }), - setRecordingMode() {}, - applyVisualState: async () => true, - ...styleOverrides, - }; - const data = { - getAll: () => [], - getLayerParams: () => null, - setEnabled: async () => true, - setLayerParams: () => true, - ...dataOverrides, - }; - return { director: new SceneDirector(viewer, style, data), storage }; -} - -function observeStatus(director) { - let status = ''; - director.subscribe(({ state }) => { - status = state.status; - }); - return () => status; -} - -test('SceneDirector keeps recovery data and failed-save status accurate (#971)', async (t) => { - const priorDocument = globalThis.document; - const priorStorage = globalThis.localStorage; - const priorWarn = console.warn; - console.warn = () => {}; - installDocument(); - t.after(() => { - console.warn = priorWarn; - if (priorDocument === undefined) delete globalThis.document; - else globalThis.document = priorDocument; - if (priorStorage === undefined) delete globalThis.localStorage; - else globalThis.localStorage = priorStorage; - }); - - let made = make({ - [STORAGE_KEY]: '{"version":99,"scenes":[]}', - [CHECKPOINT_KEY]: 'EARLIER-CHECKPOINT', - }); - assert.equal( - made.storage.stored.get(STORAGE_KEY), - '{"version":99,"scenes":[]}', - ); - assert.equal(made.storage.stored.get(CHECKPOINT_KEY), 'EARLIER-CHECKPOINT'); - await made.director.destroy(); - - made = make({ [STORAGE_KEY]: project }); - const status = observeStatus(made.director); - made.storage.full = true; - - made.director.captureShot(); - assert.match(status(), /^Scene not saved/); - - made.director.updateSelectedShot(); - assert.match(status(), /^Scene not saved/); - - assert.equal( - await made.director.importProjectFile( - { name: 'fixture.json' }, - { prepared: { project: JSON.parse(project), assets: [] } }, - ), - true, - ); - assert.match(status(), /^Scene not saved/); - await made.director.destroy(); -}); - -test('SceneDirector returns results, resolves implicit ids, and counts scenes', async (t) => { - const priorDocument = globalThis.document; - const priorStorage = globalThis.localStorage; - installDocument(); - t.after(() => { - if (priorDocument === undefined) delete globalThis.document; - else globalThis.document = priorDocument; - if (priorStorage === undefined) delete globalThis.localStorage; - else globalThis.localStorage = priorStorage; - }); - - const made = make({ [STORAGE_KEY]: project }); - const options = { single: true, preview: false }; - assert.deepEqual(await made.director.startScene('s', options), { - started: true, - shots: 2, - }); - assert.equal(made.director._lastRun.scenesRun, 1); - assert.deepEqual( - await made.director.startScene(undefined, { - ...options, - afterShotId: 'a', - }), - { started: true, shots: 1 }, - ); - await made.director.destroy(); -}); - -test('SceneDirector does not report caught failures as success', async (t) => { - const priorDocument = globalThis.document; - const priorStorage = globalThis.localStorage; - const priorWarn = console.warn; - console.warn = () => {}; - installDocument(); - t.after(() => { - console.warn = priorWarn; - if (priorDocument === undefined) delete globalThis.document; - else globalThis.document = priorDocument; - if (priorStorage === undefined) delete globalThis.localStorage; - else globalThis.localStorage = priorStorage; - }); - - const made = make( - { [STORAGE_KEY]: project }, - { - styleOverrides: { - applyVisualState: async () => { - throw new Error('visual failed'); - }, - }, - }, - ); - assert.deepEqual( - await made.director.startScene('s', { - single: true, - preview: false, - }), - { started: false, reason: 'run-failed' }, - ); - await made.director.destroy(); -});