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(); -});