From 8f1bb0e8020b6916741b19dcadb3cfd4de2ef350 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 18 Aug 2026 11:57:48 -0400 Subject: [PATCH 1/2] test: replace unit-test module mocks with injected dependencies --- eslint.config.js | 23 ++ .../__tests__/loadDataSourcesNotices.spec.ts | 68 ++-- src/actions/loadUserFiles.ts | 14 +- .../__tests__/MeasurementDetails.spec.ts | 25 +- src/components/__tests__/PlayControls.spec.ts | 30 +- src/components/__tests__/latentGating.spec.ts | 32 +- .../cine/__tests__/DicomCineImage.spec.ts | 48 +-- src/core/cine/__tests__/cineFixtures.ts | 74 ++++ .../views/__tests__/effectiveView.spec.ts | 12 +- .../import/__tests__/degradedRestore.spec.ts | 70 ++-- .../__tests__/restoreCoveredErrors.spec.ts | 185 +++++---- .../__tests__/restoreProcessorFixtures.ts | 35 ++ .../__tests__/restoreStateIdCollision.spec.ts | 1 + src/io/import/importDataSources.ts | 29 +- .../__tests__/serializeResilience.spec.ts | 63 ++- src/io/state-file/serialize.ts | 46 ++- .../applyResults.annotations.spec.ts | 136 +++---- src/processing/__tests__/applyResults.spec.ts | 365 ++++++++---------- src/processing/__tests__/store.spec.ts | 151 +++----- src/processing/applyResults.ts | 129 +++++-- .../components/__tests__/JobsModule.spec.ts | 14 +- src/processing/store.ts | 17 +- src/referenceLines/__tests__/store.spec.ts | 7 +- .../__tests__/useReferenceLines.spec.ts | 7 +- src/store/__tests__/datasetFixtures.ts | 31 ++ .../__tests__/datasets-dicom-cine.spec.ts | 3 + src/store/__tests__/datasets-layers.spec.ts | 89 +++-- src/store/__tests__/fillHoles.spec.ts | 5 +- .../legacyManifestSegmentGroups.spec.ts | 1 + src/store/__tests__/remote-save-state.spec.ts | 37 +- src/store/__tests__/saveFixtures.ts | 17 + .../segmentGroupDescriptorlessParity.spec.ts | 1 + .../segmentGroupRestoreResilience.spec.ts | 1 + src/store/__tests__/views.spec.ts | 9 +- src/store/remote-save-state.ts | 19 +- src/store/tools/__tests__/crosshairs.spec.ts | 7 +- src/utils/__tests__/token.spec.ts | 42 +- src/utils/token.ts | 6 +- 38 files changed, 994 insertions(+), 855 deletions(-) create mode 100644 src/core/cine/__tests__/cineFixtures.ts create mode 100644 src/io/import/__tests__/restoreProcessorFixtures.ts create mode 100644 src/store/__tests__/datasetFixtures.ts create mode 100644 src/store/__tests__/saveFixtures.ts diff --git a/eslint.config.js b/eslint.config.js index 41057a805..287844321 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -152,6 +152,29 @@ export default tseslint.config( '@typescript-eslint/no-unused-expressions': 'off', }, }, + // ------------------------------------------------------------------------- + // Unit tests wire collaborators in, they do not replace modules. + // + // Expressed with the built-in `no-restricted-syntax` for the same reason the + // feature boundaries use `no-restricted-imports`: no new plugin dependency. + // `vi.hoisted` is deliberately not listed: it exists to hoist values used by + // `vi.mock`, so flagging it as well would only double the report on a site + // the `vi.mock` selector already catches. + // ------------------------------------------------------------------------- + { + files: ['**/__tests__/**/*.{js,ts}', '**/tests/unit/**/*.spec.{js,ts}'], + rules: { + 'no-restricted-syntax': [ + 'error', + { + selector: + "CallExpression[callee.object.name='vi'][callee.property.name=/^(mock|doMock)$/]", + message: + 'Do not replace a module with vi.mock. Give the unit under test a parameter for the collaborator with the real one as its default (`useThing(dep = realDep)`), then hand a real object in from the spec. vi.fn and vi.spyOn on a real object are still fine.', + }, + ], + }, + }, { files: ['src/vtk/**/*.{js,ts}'], rules: { diff --git a/src/actions/__tests__/loadDataSourcesNotices.spec.ts b/src/actions/__tests__/loadDataSourcesNotices.spec.ts index 764da6633..fc992f845 100644 --- a/src/actions/__tests__/loadDataSourcesNotices.spec.ts +++ b/src/actions/__tests__/loadDataSourcesNotices.spec.ts @@ -4,10 +4,15 @@ import { setActivePinia, createPinia } from 'pinia'; import { loadDataSources, loadUrlsWithOutcome, + type DataSourceImporter, } from '@/src/actions/loadUserFiles'; import useLoadDataStore from '@/src/store/load-data'; import type { DataSource } from '@/src/io/import/dataSource'; -import { asErrorResult, asOkayResult } from '@/src/io/import/common'; +import { + asErrorResult, + asOkayResult, + type ImportDataSourcesResult, +} from '@/src/io/import/common'; // --------------------------------------------------------------------------- // ONE consolidated notice for degraded composed opens: importDataSources owns @@ -18,15 +23,13 @@ import { asErrorResult, asOkayResult } from '@/src/io/import/common'; // the error results loadDataSources receives, no more and no less. // --------------------------------------------------------------------------- -const mocks = vi.hoisted(() => ({ - importDataSources: vi.fn(), -})); - -vi.mock('@/src/io/import/importDataSources', async (importOriginal) => { - const actual = - await importOriginal(); - return { ...actual, importDataSources: mocks.importDataSources }; -}); +// Returns whatever the test lined up, one queued batch per call. +const importerServing = ( + ...batches: Array +): DataSourceImporter => { + const queue = [...batches]; + return async () => queue.shift() ?? []; +}; // What importDataSources returns for a failure it already surfaced itself. const coveredFailure = (source: DataSource) => asOkayResult(source); @@ -47,27 +50,25 @@ const standaloneSource = (): DataSource => ({ describe('loadDataSources — notice exclusivity for restore-covered failures', () => { beforeEach(() => { setActivePinia(createPinia()); - mocks.importDataSources.mockReset(); }); it('a restore-covered failure does NOT raise the generic load error', async () => { const leaf = composedLeaf('ds-a'); - mocks.importDataSources.mockResolvedValue([coveredFailure(leaf)]); const spy = vi.spyOn(useLoadDataStore(), 'setError'); - await loadDataSources([leaf]); + await loadDataSources([leaf], importerServing([coveredFailure(leaf)])); expect(spy).not.toHaveBeenCalled(); }); it('a returned error result still raises the generic load error', async () => { const source = standaloneSource(); - mocks.importDataSources.mockResolvedValue([ - asErrorResult(new Error('boom'), source), - ]); const spy = vi.spyOn(useLoadDataStore(), 'setError'); - await loadDataSources([source]); + await loadDataSources( + [source], + importerServing([asErrorResult(new Error('boom'), source)]) + ); expect(spy).toHaveBeenCalledTimes(1); expect(String(spy.mock.calls[0][0])).toContain('plain.nrrd'); @@ -76,13 +77,15 @@ describe('loadDataSources — notice exclusivity for restore-covered failures', it('a mixed result reports ONLY the error entries', async () => { const leaf = composedLeaf('ds-a'); const source = standaloneSource(); - mocks.importDataSources.mockResolvedValue([ - coveredFailure(leaf), - asErrorResult(new Error('boom'), source), - ]); const spy = vi.spyOn(useLoadDataStore(), 'setError'); - await loadDataSources([leaf, source]); + await loadDataSources( + [leaf, source], + importerServing([ + coveredFailure(leaf), + asErrorResult(new Error('boom'), source), + ]) + ); expect(spy).toHaveBeenCalledTimes(1); const message = String(spy.mock.calls[0][0]); @@ -92,21 +95,20 @@ describe('loadDataSources — notice exclusivity for restore-covered failures', it('distinguishes a successful zero-dataset restore from an uncovered error', async () => { const leaf = composedLeaf('ds-a'); - mocks.importDataSources.mockResolvedValueOnce([coveredFailure(leaf)]); - await expect( - loadUrlsWithOutcome({ - urls: ['https://example.com/session.volview.json'], - }) + loadUrlsWithOutcome( + { urls: ['https://example.com/session.volview.json'] }, + importerServing([coveredFailure(leaf)]) + ) ).resolves.toEqual({ datasetIds: [], hadErrors: false }); - mocks.importDataSources.mockResolvedValueOnce([ - asErrorResult(new Error('not found'), standaloneSource()), - ]); await expect( - loadUrlsWithOutcome({ - urls: ['https://example.com/missing.volview.json'], - }) + loadUrlsWithOutcome( + { urls: ['https://example.com/missing.volview.json'] }, + importerServing([ + asErrorResult(new Error('not found'), standaloneSource()), + ]) + ) ).resolves.toEqual({ datasetIds: [], hadErrors: true }); }); }); diff --git a/src/actions/loadUserFiles.ts b/src/actions/loadUserFiles.ts index a798d3ebd..e1ef6ad7a 100644 --- a/src/actions/loadUserFiles.ts +++ b/src/actions/loadUserFiles.ts @@ -266,7 +266,7 @@ function loadSegmentations( }); } -type DataSourceImporter = ( +export type DataSourceImporter = ( sources: DataSource[] ) => Promise; @@ -371,8 +371,11 @@ function loadDataSourcesWithOutcome( return wrapWithLoading(load)(); } -export function loadDataSources(sources: DataSource[]) { - return loadDataSourcesWithOutcome(sources, importDataSources).then( +export function loadDataSources( + sources: DataSource[], + importer: DataSourceImporter = importDataSources +) { + return loadDataSourcesWithOutcome(sources, importer).then( ({ datasetIds, completed }) => (completed ? datasetIds : undefined) ); } @@ -425,7 +428,8 @@ export async function loadUrls(params: UrlParams | LoadUrlsParams) { } export async function loadUrlsWithOutcome( - params: UrlParams | LoadUrlsParams + params: UrlParams | LoadUrlsParams, + importer: DataSourceImporter = importDataSources ): Promise> { if (!params.urls) { return { datasetIds: [], hadErrors: false }; @@ -434,7 +438,7 @@ export async function loadUrlsWithOutcome( const urls = wrapInArray(params.urls); const names = wrapInArray(params.names ?? []); const sources = urlsToDataSources(urls, names); - const outcome = await loadDataSourcesWithOutcome(sources, importDataSources); + const outcome = await loadDataSourcesWithOutcome(sources, importer); return { datasetIds: outcome.datasetIds, hadErrors: outcome.hadErrors, diff --git a/src/components/__tests__/MeasurementDetails.spec.ts b/src/components/__tests__/MeasurementDetails.spec.ts index e1a58ccae..6c7ba0736 100644 --- a/src/components/__tests__/MeasurementDetails.spec.ts +++ b/src/components/__tests__/MeasurementDetails.spec.ts @@ -1,14 +1,14 @@ import { mount } from '@vue/test-utils'; -import { describe, expect, it, vi } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; +import { beforeEach, describe, expect, it } from 'vitest'; import MeasurementToolDetails from '@/src/components/MeasurementToolDetails.vue'; import MeasurementRulerDetails from '@/src/components/MeasurementRulerDetails.vue'; +import { useRulerStore } from '@/src/store/tools/rulers'; import { ToolID } from '@/src/types/annotation-tool'; -vi.mock('@/src/store/tools/rulers', () => ({ - useRulerStore: () => ({ - lengthByID: { 'tool-1': 12.345 }, - }), -})); +beforeEach(() => { + setActivePinia(createPinia()); +}); const stubs = { 'v-row': { template: '
' }, @@ -48,18 +48,27 @@ describe('MeasurementToolDetails', () => { }); describe('MeasurementRulerDetails', () => { + // The component reads the length off the ruler store, so the ruler has to + // exist there: a 3-4-5 triangle gives a length of 5.00mm. + const seatRuler = () => + useRulerStore().addRuler({ + firstPoint: [0, 0, 0], + secondPoint: [3, 4, 0], + }); + it('shows slice number for a volume ruler', () => { const wrapper = mount(MeasurementRulerDetails, { - props: { tool: { ...baseTool, slice: 9 } }, + props: { tool: { ...baseTool, id: seatRuler(), slice: 9 } }, global: { stubs }, }); expect(wrapper.text()).toContain('Slice: 10'); + expect(wrapper.text()).toContain('5.00mm'); expect(wrapper.text()).not.toContain('Frame:'); }); it('shows frame number for a cine ruler', () => { const wrapper = mount(MeasurementRulerDetails, { - props: { tool: { ...baseTool, slice: 0, frame: 2 } }, + props: { tool: { ...baseTool, id: seatRuler(), slice: 0, frame: 2 } }, global: { stubs }, }); expect(wrapper.text()).toContain('Frame: 3'); diff --git a/src/components/__tests__/PlayControls.spec.ts b/src/components/__tests__/PlayControls.spec.ts index 8a6a930e4..51afe8c76 100644 --- a/src/components/__tests__/PlayControls.spec.ts +++ b/src/components/__tests__/PlayControls.spec.ts @@ -1,42 +1,22 @@ import { mount, VueWrapper } from '@vue/test-utils'; import { createPinia, setActivePinia } from 'pinia'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { ComponentPublicInstance, nextTick, ref } from 'vue'; +import { ComponentPublicInstance, nextTick } from 'vue'; import PlayControls from '@/src/components/PlayControls.vue'; +import { seatCineImage } from '@/src/core/cine/__tests__/cineFixtures'; type PlayControlsProps = { viewId: string; imageId: string | null }; type PlayControlsWrapper = VueWrapper< ComponentPublicInstance >; -vi.mock('@/src/core/cine/isCineImage', () => ({ - getCineImage: (imageId: string | null) => { - const frameTimesByImageId: Record = { - 'image-1': 50, - 'image-2': 25, - }; - - return imageId - ? { - header: { - frameTimeMs: frameTimesByImageId[imageId] ?? 40, - }, - } - : null; - }, -})); - -vi.mock('@/src/composables/useSliceConfig', () => ({ - useSliceConfig: () => ({ - slice: ref(0), - range: ref([0, 2]), - }), -})); - describe('PlayControls', () => { beforeEach(() => { setActivePinia(createPinia()); vi.useFakeTimers(); + // Frame time drives the default FPS: 50ms is 20fps, 25ms is 40fps. + seatCineImage('image-1', { frameTimeMs: 50 }); + seatCineImage('image-2', { frameTimeMs: 25 }); }); afterEach(() => { diff --git a/src/components/__tests__/latentGating.spec.ts b/src/components/__tests__/latentGating.spec.ts index 74cf5845d..810e0239c 100644 --- a/src/components/__tests__/latentGating.spec.ts +++ b/src/components/__tests__/latentGating.spec.ts @@ -3,31 +3,17 @@ // - Jobs tab ⇒ ModulePanel reveals it only when `configs.size > 0`. // - Remote save ⇒ the surface/egress engage only when `saveUrl !== ''`. -import { describe, it, beforeEach, afterEach, expect, vi } from 'vitest'; +import { describe, it, beforeEach, afterEach, expect } from 'vitest'; import { shallowMount, flushPromises, VueWrapper } from '@vue/test-utils'; import { createPinia, setActivePinia } from 'pinia'; // Keep the Jobs async-component cheap and DOM-safe if it ever renders. -vi.mock('@/src/processing/components/JobsModule.vue', () => ({ - default: { name: 'JobsModule', template: '
' }, -})); - -// Observe remote-save egress without a network round-trip or the heavy -// serialize path. -vi.mock('@/src/utils/fetch', () => ({ - $fetch: vi.fn().mockResolvedValue({ ok: true }), -})); -vi.mock('@/src/io/state-file/serialize', () => ({ - serialize: vi - .fn() - .mockResolvedValue(new Blob(['x'], { type: 'application/zip' })), -})); import ModulePanel from '@/src/components/ModulePanel.vue'; import { useProcessingJobsStore } from '@/src/processing'; import useRemoteSaveStateStore from '@/src/store/remote-save-state'; import { ConnectionState, useServerStore } from '@/src/store/server'; -import { $fetch } from '@/src/utils/fetch'; +import { savePost, saveDependencies } from '@/src/store/__tests__/saveFixtures'; import type { ProcessingProviderConfig } from '@/src/processing'; // Stub the Vuetify shell so mounting ModulePanel exercises only its own gating @@ -110,9 +96,11 @@ describe('Jobs tab is latent — gated on provider presence', () => { }); describe('Remote save is latent — gated on a save target', () => { + let post = savePost(); + beforeEach(() => { setActivePinia(createPinia()); - vi.mocked($fetch).mockClear(); + post = savePost(); }); it('exposes no save target and performs no egress when unconfigured', async () => { @@ -122,9 +110,9 @@ describe('Remote save is latent — gated on a save target', () => { // hidden (ControlsStrip/WelcomePage gate on `saveUrl !== ''`). expect(store.saveUrl).toBe(''); - await store.saveState(); + await store.saveState(saveDependencies(post)); - expect($fetch).not.toHaveBeenCalled(); + expect(post).not.toHaveBeenCalled(); }); it('performs egress only after a save target is set', async () => { @@ -132,9 +120,9 @@ describe('Remote save is latent — gated on a save target', () => { const saveUrl = `${window.location.origin}/save`; store.setSaveUrl(saveUrl); - await store.saveState(); + await store.saveState(saveDependencies(post)); - expect($fetch).toHaveBeenCalledTimes(1); - expect(vi.mocked($fetch).mock.calls[0][0]).toBe(saveUrl); + expect(post).toHaveBeenCalledTimes(1); + expect(post.mock.calls[0][0]).toBe(saveUrl); }); }); diff --git a/src/core/cine/__tests__/DicomCineImage.spec.ts b/src/core/cine/__tests__/DicomCineImage.spec.ts index 3c8e3093c..1e89175e6 100644 --- a/src/core/cine/__tests__/DicomCineImage.spec.ts +++ b/src/core/cine/__tests__/DicomCineImage.spec.ts @@ -1,58 +1,12 @@ import { describe, expect, it } from 'vitest'; import DicomCineImage from '../DicomCineImage'; -import type { CineHeader, CineParseResult } from '../parseCineDicom'; +import { cineHeader, cineParseResult as parseResult } from './cineFixtures'; -const TS_EXPLICIT_VR_LE = '1.2.840.10008.1.2.1'; const TS_JPEG_BASELINE = '1.2.840.10008.1.2.4.50'; const TS_JPEG_EXTENDED = '1.2.840.10008.1.2.4.51'; const TS_UNSUPPORTED = '1.2.840.10008.1.2.5'; -function cineHeader(overrides: Partial = {}): CineHeader { - return { - transferSyntaxUID: TS_EXPLICIT_VR_LE, - rows: 2, - cols: 2, - numberOfFrames: 2, - samplesPerPixel: 1, - bitsAllocated: 8, - planarConfiguration: 0, - photometricInterpretation: 'MONOCHROME2', - pixelSpacing: null, - frameTimeMs: null, - patient: { - PatientID: 'patient-1', - PatientName: 'Test Patient', - PatientBirthDate: '', - PatientSex: '', - }, - study: { - StudyID: 'study-1', - StudyInstanceUID: 'study-uid', - StudyDate: '', - StudyTime: '', - AccessionNumber: '', - StudyDescription: '', - }, - series: { - SeriesInstanceUID: 'series-uid', - SeriesNumber: '1', - SeriesDescription: 'Cine', - Modality: 'US', - }, - regions: [], - ...overrides, - }; -} - -function parseResult(header: CineHeader): CineParseResult { - return { - header, - frames: [new Uint8Array(4), new Uint8Array(4)], - encapsulated: false, - }; -} - describe('DicomCineImage.isSupported', () => { it('accepts only MONOCHROME2 for native one-sample 8-bit pixels', () => { expect(DicomCineImage.isSupported(cineHeader())).toBe(true); diff --git a/src/core/cine/__tests__/cineFixtures.ts b/src/core/cine/__tests__/cineFixtures.ts new file mode 100644 index 000000000..b278bf831 --- /dev/null +++ b/src/core/cine/__tests__/cineFixtures.ts @@ -0,0 +1,74 @@ +import { useImageCacheStore } from '@/src/store/image-cache'; +import { seatVolume } from '@/src/store/__tests__/datasetFixtures'; +import DicomCineImage from '@/src/core/cine/DicomCineImage'; +import type { + CineHeader, + CineParseResult, +} from '@/src/core/cine/parseCineDicom'; + +const TS_EXPLICIT_VR_LE = '1.2.840.10008.1.2.1'; + +/** + * Seats a volume the real `isCineImage` reports as cine. Requires an active + * pinia. + */ +export const markCine = (imageID: string) => + seatVolume(imageID, { kind: 'cine' }); + +export const cineHeader = ( + overrides: Partial = {} +): CineHeader => ({ + transferSyntaxUID: TS_EXPLICIT_VR_LE, + rows: 2, + cols: 2, + numberOfFrames: 2, + samplesPerPixel: 1, + bitsAllocated: 8, + planarConfiguration: 0, + photometricInterpretation: 'MONOCHROME2', + pixelSpacing: null, + frameTimeMs: null, + patient: { + PatientID: 'patient-1', + PatientName: 'Test Patient', + PatientBirthDate: '', + PatientSex: '', + }, + study: { + StudyID: 'study-1', + StudyInstanceUID: 'study-uid', + StudyDate: '', + StudyTime: '', + AccessionNumber: '', + StudyDescription: '', + }, + series: { + SeriesInstanceUID: 'series-uid', + SeriesNumber: '1', + SeriesDescription: 'Cine', + Modality: 'US', + }, + regions: [], + ...overrides, +}); + +export const cineParseResult = (header: CineHeader): CineParseResult => ({ + header, + frames: [new Uint8Array(4), new Uint8Array(4)], + encapsulated: false, +}); + +/** + * Seats a real two-frame clip under `imageID`, both in the DICOM store (so + * `isCineImage` says yes) and in the image cache (so `getCineImage` hands the + * clip back). Requires an active pinia. + */ +export const seatCineImage = ( + imageID: string, + header: Partial = {} +) => { + markCine(imageID); + const image = new DicomCineImage(cineParseResult(cineHeader(header))); + useImageCacheStore().addProgressiveImage(image, { id: imageID }); + return image; +}; diff --git a/src/core/views/__tests__/effectiveView.spec.ts b/src/core/views/__tests__/effectiveView.spec.ts index 48ca990df..86f4787c1 100644 --- a/src/core/views/__tests__/effectiveView.spec.ts +++ b/src/core/views/__tests__/effectiveView.spec.ts @@ -1,8 +1,10 @@ -import { describe, it, expect, vi } from 'vitest'; +import { describe, it, beforeEach, expect } from 'vitest'; +import { setActivePinia, createPinia } from 'pinia'; import { computeEffectiveView, volume2DViewsOfImage, } from '@/src/core/views/effectiveView'; +import { markCine } from '@/src/core/cine/__tests__/cineFixtures'; import type { ViewInfo, ViewInfo2D, @@ -10,10 +12,10 @@ import type { ViewInfoOblique, } from '@/src/types/views'; -vi.mock('@/src/core/cine/isCineImage', () => ({ - isCineImage: (id: string | null | undefined) => id === 'cine-image', - getCineImage: () => null, -})); +beforeEach(() => { + setActivePinia(createPinia()); + markCine('cine-image'); +}); const view2D: ViewInfo2D = { id: 'v-2d', diff --git a/src/io/import/__tests__/degradedRestore.spec.ts b/src/io/import/__tests__/degradedRestore.spec.ts index 744f95764..b3172ba95 100644 --- a/src/io/import/__tests__/degradedRestore.spec.ts +++ b/src/io/import/__tests__/degradedRestore.spec.ts @@ -1,7 +1,11 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; import { importDataSources } from '@/src/io/import/importDataSources'; import { useMessageStore, MessageType } from '@/src/store/messages'; +import { + recordingRestoreProcessors, + yields, +} from '@/src/io/import/__tests__/restoreProcessorFixtures'; // --------------------------------------------------------------------------- // Auto-degrade-to-ephemeral: a scene @@ -10,41 +14,34 @@ import { useMessageStore, MessageType } from '@/src/store/messages'; // import NEVER becomes an error loop or a rejected promise. // --------------------------------------------------------------------------- -const processorMocks = vi.hoisted(() => ({ - restoreStateFile: vi.fn(), - completeStateFileRestore: vi.fn(), -})); +const aSetup = yields({ + type: 'stateFileSetup', + dataSources: [], + manifest: { version: '6.4.0', dataSources: [] }, + stateFiles: [], + missingFiles: [], +}); -vi.mock('@/src/io/import/processors/restoreStateFile', () => ({ - restoreStateFile: processorMocks.restoreStateFile, - completeStateFileRestore: processorMocks.completeStateFileRestore, -})); +const sessionFile = () => + new File(['{}'], 'session.volview.json', { type: 'application/json' }); describe('importDataSources — degraded restore', () => { beforeEach(() => { setActivePinia(createPinia()); - processorMocks.restoreStateFile.mockReset(); - processorMocks.completeStateFileRestore.mockReset(); }); it('a mid-restore throw degrades to an ephemeral open with ONE notice', async () => { - processorMocks.restoreStateFile.mockResolvedValue({ - type: 'stateFileSetup', - dataSources: [], - manifest: { version: '6.4.0', dataSources: [] }, - stateFiles: [], - missingFiles: [], + const restore = recordingRestoreProcessors({ + setup: aSetup, + completion: async () => { + throw new Error('segment group deserialize exploded'); + }, }); - processorMocks.completeStateFileRestore.mockRejectedValue( - new Error('segment group deserialize exploded') - ); - const file = new File(['{}'], 'session.volview.json', { - type: 'application/json', - }); - const results = await importDataSources([ - { type: 'file', file, fileType: 'application/json' }, - ]); + const results = await importDataSources( + [{ type: 'file', file: sessionFile(), fileType: 'application/json' }], + restore.processors + ); expect(results.filter((result) => result.type === 'error')).toEqual([]); @@ -57,23 +54,14 @@ describe('importDataSources — degraded restore', () => { }); it('a clean restore fires no degrade notice', async () => { - processorMocks.restoreStateFile.mockResolvedValue({ - type: 'stateFileSetup', - dataSources: [], - manifest: { version: '6.4.0', dataSources: [] }, - stateFiles: [], - missingFiles: [], - }); - processorMocks.completeStateFileRestore.mockResolvedValue(undefined); + const restore = recordingRestoreProcessors({ setup: aSetup }); - const file = new File(['{}'], 'session.volview.json', { - type: 'application/json', - }); - await importDataSources([ - { type: 'file', file, fileType: 'application/json' }, - ]); + await importDataSources( + [{ type: 'file', file: sessionFile(), fileType: 'application/json' }], + restore.processors + ); - expect(processorMocks.completeStateFileRestore).toHaveBeenCalledTimes(1); + expect(restore.completions).toHaveLength(1); expect(useMessageStore().messages).toEqual([]); }); }); diff --git a/src/io/import/__tests__/restoreCoveredErrors.spec.ts b/src/io/import/__tests__/restoreCoveredErrors.spec.ts index 535a4a894..a2cc43d30 100644 --- a/src/io/import/__tests__/restoreCoveredErrors.spec.ts +++ b/src/io/import/__tests__/restoreCoveredErrors.spec.ts @@ -1,9 +1,13 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; import { importDataSources } from '@/src/io/import/importDataSources'; import type { DataSource } from '@/src/io/import/dataSource'; import type { ImportDataSourcesResult } from '@/src/io/import/common'; import { Skip } from '@/src/utils/evaluateChain'; +import { + recordingRestoreProcessors, + yieldsFor, +} from '@/src/io/import/__tests__/restoreProcessorFixtures'; // --------------------------------------------------------------------------- // importDataSources owns reporting for failures it has already surfaced: a @@ -16,16 +20,6 @@ import { Skip } from '@/src/utils/evaluateChain'; // it must surface through the generic load-error path. // --------------------------------------------------------------------------- -const processorMocks = vi.hoisted(() => ({ - restoreStateFile: vi.fn(), - completeStateFileRestore: vi.fn(), -})); - -vi.mock('@/src/io/import/processors/restoreStateFile', () => ({ - restoreStateFile: processorMocks.restoreStateFile, - completeStateFileRestore: processorMocks.completeStateFileRestore, -})); - // Garbage bytes with no recognizable magic: updateFileMimeType throws // "Unrecognized file type", producing an error result deterministically // (no network, no readers). @@ -37,20 +31,26 @@ const sessionFile = () => // Emits the setup for the session file only; every re-queued leaf source // skips through to the ordinary processors (and fails there). -const mockSetupWith = (leafSources: DataSource[]) => { - processorMocks.restoreStateFile.mockImplementation((ds: DataSource) => { - if (ds.type === 'file' && ds.file.name === 'session.volview.json') { - return { - type: 'stateFileSetup', - dataSources: leafSources, - manifest: { version: '6.4.0', dataSources: [] }, - stateFiles: [], - missingFiles: [], - }; - } - return Skip; - }); -}; +const setupWith = (leafSources: DataSource[]) => + yieldsFor((ds) => + ds.type === 'file' && ds.file.name === 'session.volview.json' + ? { + type: 'stateFileSetup', + dataSources: leafSources, + manifest: { version: '6.4.0', dataSources: [] }, + stateFiles: [], + missingFiles: [], + } + : Skip + ); + +const skipEverything = yieldsFor(() => Skip); + +const openSession = (restore: ReturnType) => + importDataSources( + [{ type: 'file', file: sessionFile(), fileType: 'application/json' }], + restore.processors + ); const resultsByType = (results: ImportDataSourcesResult[]) => ({ errors: results.filter((r) => r.type === 'error'), @@ -60,27 +60,21 @@ const resultsByType = (results: ImportDataSourcesResult[]) => ({ describe('importDataSources — restore-covered failures return as ok, not error', () => { beforeEach(() => { setActivePinia(createPinia()); - processorMocks.restoreStateFile.mockReset(); - processorMocks.completeStateFileRestore.mockReset(); - processorMocks.restoreStateFile.mockReturnValue(Skip); }); it('demotes a failed leaf covered by a completed restore notice to ok', async () => { - mockSetupWith([ - { - type: 'file', - file: unrecognizedFile('ds-a.bin'), - fileType: '', - stateFileLeaf: { stateID: 'ds-a' }, - }, - ]); - processorMocks.completeStateFileRestore.mockResolvedValue(undefined); + const restore = recordingRestoreProcessors({ + setup: setupWith([ + { + type: 'file', + file: unrecognizedFile('ds-a.bin'), + fileType: '', + stateFileLeaf: { stateID: 'ds-a' }, + }, + ]), + }); - const { errors, okays } = resultsByType( - await importDataSources([ - { type: 'file', file: sessionFile(), fileType: 'application/json' }, - ]) - ); + const { errors, okays } = resultsByType(await openSession(restore)); expect(errors).toHaveLength(0); expect(okays).toHaveLength(1); @@ -93,88 +87,87 @@ describe('importDataSources — restore-covered failures return as ok, not error name: 'ds-a.nrrd', stateFileLeaf: { stateID: 'ds-a' }, }; - mockSetupWith([ - { - type: 'file', - file: unrecognizedFile('ds-a.bin'), - fileType: '', - parent: leafParent, - }, - ]); - processorMocks.completeStateFileRestore.mockResolvedValue(undefined); + const restore = recordingRestoreProcessors({ + setup: setupWith([ + { + type: 'file', + file: unrecognizedFile('ds-a.bin'), + fileType: '', + parent: leafParent, + }, + ]), + }); - const { errors, okays } = resultsByType( - await importDataSources([ - { type: 'file', file: sessionFile(), fileType: 'application/json' }, - ]) - ); + const { errors, okays } = resultsByType(await openSession(restore)); expect(errors).toHaveLength(0); expect(okays).toHaveLength(1); }); it('hands the restore the failed leaves for its consolidated notice', async () => { - mockSetupWith([ - { - type: 'file', - file: unrecognizedFile('ds-a.bin'), - fileType: '', - stateFileLeaf: { stateID: 'ds-a' }, - }, - ]); - processorMocks.completeStateFileRestore.mockResolvedValue(undefined); + const restore = recordingRestoreProcessors({ + setup: setupWith([ + { + type: 'file', + file: unrecognizedFile('ds-a.bin'), + fileType: '', + stateFileLeaf: { stateID: 'ds-a' }, + }, + ]), + }); - await importDataSources([ - { type: 'file', file: sessionFile(), fileType: 'application/json' }, - ]); + await openSession(restore); - const failedLeaves = processorMocks.completeStateFileRestore.mock - .calls[0][4] as Array<{ stateID: string; name: string }>; + const [, , , , failedLeaves] = restore.completions[0]; expect(failedLeaves).toEqual([{ stateID: 'ds-a', name: 'ds-a.bin' }]); }); it('keeps a leaf failure as an error when the restore never completed', async () => { - mockSetupWith([ - { - type: 'file', - file: unrecognizedFile('ds-a.bin'), - fileType: '', - stateFileLeaf: { stateID: 'ds-a' }, + const restore = recordingRestoreProcessors({ + setup: setupWith([ + { + type: 'file', + file: unrecognizedFile('ds-a.bin'), + fileType: '', + stateFileLeaf: { stateID: 'ds-a' }, + }, + ]), + completion: async () => { + throw new Error('deserialize exploded'); }, - ]); - processorMocks.completeStateFileRestore.mockRejectedValue( - new Error('deserialize exploded') - ); + }); - const { errors } = resultsByType( - await importDataSources([ - { type: 'file', file: sessionFile(), fileType: 'application/json' }, - ]) - ); + const { errors } = resultsByType(await openSession(restore)); expect(errors).toHaveLength(1); }); it('keeps a leaf-carrying failure with no restore behind it as an error', async () => { + const restore = recordingRestoreProcessors({ setup: skipEverything }); const { errors } = resultsByType( - await importDataSources([ - { - type: 'file', - file: unrecognizedFile('ds-a.bin'), - fileType: '', - stateFileLeaf: { stateID: 'ds-a' }, - }, - ]) + await importDataSources( + [ + { + type: 'file', + file: unrecognizedFile('ds-a.bin'), + fileType: '', + stateFileLeaf: { stateID: 'ds-a' }, + }, + ], + restore.processors + ) ); expect(errors).toHaveLength(1); }); it('keeps a standalone failure as an error', async () => { + const restore = recordingRestoreProcessors({ setup: skipEverything }); const { errors } = resultsByType( - await importDataSources([ - { type: 'file', file: unrecognizedFile('plain.bin'), fileType: '' }, - ]) + await importDataSources( + [{ type: 'file', file: unrecognizedFile('plain.bin'), fileType: '' }], + restore.processors + ) ); expect(errors).toHaveLength(1); diff --git a/src/io/import/__tests__/restoreProcessorFixtures.ts b/src/io/import/__tests__/restoreProcessorFixtures.ts new file mode 100644 index 000000000..a9ffb2e7f --- /dev/null +++ b/src/io/import/__tests__/restoreProcessorFixtures.ts @@ -0,0 +1,35 @@ +import type { DataSource } from '@/src/io/import/dataSource'; +import type { ImportHandler } from '@/src/io/import/common'; +import type { RestoreProcessors } from '@/src/io/import/importDataSources'; + +type CompletionCall = Parameters; + +/** + * Stands in for the two restore processors so a spec can drive the pipeline + * without a real state file: `setup` decides what the handler yields, and + * `completion` decides whether applying it succeeds. + */ +export const recordingRestoreProcessors = (options: { + setup: ImportHandler; + completion?: () => Promise; +}) => { + const completions: CompletionCall[] = []; + const processors: RestoreProcessors = { + restoreStateFile: options.setup, + completeStateFileRestore: async (...call: CompletionCall) => { + completions.push(call); + await options.completion?.(); + }, + }; + return { completions, processors }; +}; + +export const yields = + (setup: unknown): ImportHandler => + () => + setup as ReturnType; + +export const yieldsFor = + (decide: (dataSource: DataSource) => unknown): ImportHandler => + (dataSource) => + decide(dataSource) as ReturnType; diff --git a/src/io/import/__tests__/restoreStateIdCollision.spec.ts b/src/io/import/__tests__/restoreStateIdCollision.spec.ts index 5d4ea2937..b321d3a6b 100644 --- a/src/io/import/__tests__/restoreStateIdCollision.spec.ts +++ b/src/io/import/__tests__/restoreStateIdCollision.spec.ts @@ -32,6 +32,7 @@ const ioMocks = vi.hoisted(() => ({ writeSegmentation: vi.fn(async () => new Uint8Array([1, 2, 3])), })); +// eslint-disable-next-line no-restricted-syntax -- ITK-wasm image IO has no counterpart in the node test environment vi.mock('@/src/io/readWriteImage', () => ({ readImage: ioMocks.readImage, writeSegmentation: ioMocks.writeSegmentation, diff --git a/src/io/import/importDataSources.ts b/src/io/import/importDataSources.ts index b88d0d003..44acb1ba6 100644 --- a/src/io/import/importDataSources.ts +++ b/src/io/import/importDataSources.ts @@ -122,9 +122,25 @@ async function importDicomChunkSources(sources: ChunkSource[]) { type ImportPolicy = 'application' | 'volume-data'; +/** + * The two halves of a state-file restore: the handler that reads the manifest + * out of the pipeline, and the step that applies it once the bases have + * loaded. + */ +export type RestoreProcessors = { + restoreStateFile: typeof restoreStateFile; + completeStateFileRestore: typeof completeStateFileRestore; +}; + +export const appRestoreProcessors = (): RestoreProcessors => ({ + restoreStateFile, + completeStateFileRestore, +}); + async function importDataSourcesWithPolicy( dataSources: DataSource[], - policy: ImportPolicy + policy: ImportPolicy, + restore: RestoreProcessors = appRestoreProcessors() ): Promise { const cleanupHandlers: Array<() => void> = []; const onCleanup = (fn: () => void) => { @@ -138,12 +154,12 @@ async function importDataSourcesWithPolicy( fetchFileCache: new Map(), onCleanup, importDataSources: (sources: DataSource[]) => - importDataSourcesWithPolicy(sources, policy), + importDataSourcesWithPolicy(sources, policy, restore), }; const applicationHandlers = policy === 'application' - ? [handleConfig, restoreStateFile, handleRemoteManifest] + ? [handleConfig, restore.restoreStateFile, handleRemoteManifest] : []; const handlers = [ @@ -269,7 +285,7 @@ async function importDataSourcesWithPolicy( const reportedStateIDs = new Set(); for (const setup of stateFileSetups) { try { - await completeStateFileRestore( + await restore.completeStateFileRestore( setup.manifest, setup.stateFiles, stateIDToStoreID, @@ -315,9 +331,10 @@ async function importDataSourcesWithPolicy( } export function importDataSources( - dataSources: DataSource[] + dataSources: DataSource[], + restore: RestoreProcessors = appRestoreProcessors() ): Promise { - return importDataSourcesWithPolicy(dataSources, 'application'); + return importDataSourcesWithPolicy(dataSources, 'application', restore); } export function importVolumeDataSources( diff --git a/src/io/state-file/__tests__/serializeResilience.spec.ts b/src/io/state-file/__tests__/serializeResilience.spec.ts index 03d879999..11f3e7bae 100644 --- a/src/io/state-file/__tests__/serializeResilience.spec.ts +++ b/src/io/state-file/__tests__/serializeResilience.spec.ts @@ -2,12 +2,13 @@ import { describe, expect, it, vi } from 'vitest'; import JSZip from 'jszip'; import { reactive } from 'vue'; import type { Manifest, StateFile } from '@/src/io/state-file/schema'; +import type { MessageOptions } from '@/src/store/messages'; import { debug } from '@/src/utils/loggers'; -const mocks = vi.hoisted(() => ({ - addWarning: vi.fn(), - writeSegmentGroups: vi.fn(), -})); +const writeDatasets = (stateFile: StateFile) => { + stateFile.manifest.datasets = [{ id: 'dataset-1', dataSourceId: 1 }]; + stateFile.manifest.dataSources = [{ id: 1, type: 'uri', uri: '/dataset-1' }]; +}; const writeOneInvalidGroup = async (stateFile: StateFile) => { stateFile.manifest.segmentGroups = reactive([ @@ -31,34 +32,15 @@ const writeOneInvalidGroup = async (stateFile: StateFile) => { ]) as never; }; -vi.mock('@/src/store/datasets', () => ({ - useDatasetStore: () => ({ - serialize: vi.fn((stateFile: StateFile) => { - stateFile.manifest.datasets = [{ id: 'dataset-1', dataSourceId: 1 }]; - stateFile.manifest.dataSources = [ - { id: 1, type: 'uri', uri: '/dataset-1' }, - ]; - }), - }), -})); -vi.mock('@/src/store/views', () => ({ - useViewStore: () => ({ serialize: vi.fn() }), -})); -vi.mock('@/src/store/view-configs', () => ({ - useViewConfigStore: () => ({ serialize: vi.fn() }), -})); -vi.mock('@/src/store/segmentGroups', () => ({ - useSegmentGroupStore: () => ({ serialize: mocks.writeSegmentGroups }), -})); -vi.mock('@/src/store/tools', () => ({ - useToolStore: () => ({ serialize: vi.fn() }), -})); -vi.mock('@/src/store/datasets-layers', () => ({ - useLayersStore: () => ({ serialize: vi.fn() }), -})); -vi.mock('@/src/store/messages', () => ({ - useMessageStore: () => ({ addWarning: mocks.addWarning }), -})); +const recordWarnings = () => { + const warnings: Array<{ title: string; options: MessageOptions }> = []; + return { + warnings, + addWarning: (title: string, options: MessageOptions) => { + warnings.push({ title, options }); + }, + }; +}; import { MANIFEST, @@ -76,17 +58,22 @@ const manifestWithSelection = (primarySelection: string): Manifest => ({ describe('state-file serialization resilience', () => { it('writes a restorable zip when one manifest entry is malformed', async () => { - mocks.writeSegmentGroups.mockImplementation(writeOneInvalidGroup); - const blob = await serialize(); + const sink = recordWarnings(); + const blob = await serialize({ + writers: [writeDatasets, writeOneInvalidGroup], + addWarning: sink.addWarning, + }); const zip = await JSZip.loadAsync(blob); const manifest = JSON.parse(await zip.file(MANIFEST)!.async('string')); expect(manifest.segmentGroups).toHaveLength(1); expect(manifest.segmentGroups[0].id).toBe('valid-group'); - expect(mocks.addWarning).toHaveBeenCalledWith( - 'Some session content could not be saved', - expect.objectContaining({ persist: true }) - ); + expect(sink.warnings).toEqual([ + { + title: 'Some session content could not be saved', + options: expect.objectContaining({ persist: true }), + }, + ]); }); it('aborts when the core dataset graph is incoherent', () => { diff --git a/src/io/state-file/serialize.ts b/src/io/state-file/serialize.ts index 6f33cfa90..2d74fdda4 100644 --- a/src/io/state-file/serialize.ts +++ b/src/io/state-file/serialize.ts @@ -10,13 +10,14 @@ import { ManifestSchema, ParentToLayers, SegmentGroup, + StateFile, } from '@/src/io/state-file/schema'; import { retypeFile } from '@/src/io'; import { ARCHIVE_FILE_TYPES } from '@/src/io/mimeTypes'; import { migrateManifest } from '@/src/io/state-file/migrations'; import { useViewConfigStore } from '@/src/store/view-configs'; -import { useMessageStore } from '@/src/store/messages'; +import { useMessageStore, type MessageOptions } from '@/src/store/messages'; import { collectManifestRefs, declareManifestRefs, @@ -275,13 +276,31 @@ export function normalizeManifest(manifest: Manifest, zip: JSZip) { return { manifest: normalized, omitted }; } -export async function serialize() { - const datasetStore = useDatasetStore(); - const viewStore = useViewStore(); - const labelStore = useSegmentGroupStore(); - const toolStore = useToolStore(); - const layersStore = useLayersStore(); +/** + * Everything `serialize` reaches outside itself: the writers that each + * contribute their slice of the manifest, in order, and the sink that reports + * content the normalizer had to drop. + */ +export type SerializeDependencies = { + writers: Array<(stateFile: StateFile) => void | Promise>; + addWarning: (title: string, options: MessageOptions) => void; +}; +export const appSerializeDependencies = (): SerializeDependencies => ({ + writers: [ + (stateFile) => useDatasetStore().serialize(stateFile), + (stateFile) => useViewStore().serialize(stateFile), + (stateFile) => useViewConfigStore().serialize(stateFile), + (stateFile) => useSegmentGroupStore().serialize(stateFile), + (stateFile) => useToolStore().serialize(stateFile), + (stateFile) => useLayersStore().serialize(stateFile), + ], + addWarning: (title, options) => useMessageStore().addWarning(title, options), +}); + +export async function serialize( + dependencies: SerializeDependencies = appSerializeDependencies() +) { const zip = new JSZip(); const manifest: Manifest = { version: MANIFEST_VERSION, @@ -314,15 +333,14 @@ export async function serialize() { manifest, }; - await datasetStore.serialize(stateFile); - viewStore.serialize(stateFile); - await useViewConfigStore().serialize(stateFile); - await labelStore.serialize(stateFile); - toolStore.serialize(stateFile); - await layersStore.serialize(stateFile); + // Writers run in order: later ones read manifest entries the earlier ones + // wrote. + for (const write of dependencies.writers) { + await write(stateFile); + } const repaired = normalizeManifest(manifest, zip); if (repaired.omitted.length > 0) { - useMessageStore().addWarning('Some session content could not be saved', { + dependencies.addWarning('Some session content could not be saved', { details: `Invalid entries were omitted: ${repaired.omitted.join(', ')}`, persist: true, }); diff --git a/src/processing/__tests__/applyResults.annotations.spec.ts b/src/processing/__tests__/applyResults.annotations.spec.ts index 468510f91..52133f9e1 100644 --- a/src/processing/__tests__/applyResults.annotations.spec.ts +++ b/src/processing/__tests__/applyResults.annotations.spec.ts @@ -1,11 +1,17 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it } from 'vitest'; import { createPinia, setActivePinia } from 'pinia'; import { nextTick } from 'vue'; import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; import vtkDataArray from '@kitware/vtk.js/Common/Core/DataArray'; -import { applyIntent } from '@/src/processing/applyResults'; -import type { SubmittedJobContext } from '@/src/processing/types'; +import { + appApplyDependencies, + applyIntent, +} from '@/src/processing/applyResults'; +import type { + ProcessingResult, + SubmittedJobContext, +} from '@/src/processing/types'; import { useImageCacheStore } from '@/src/store/image-cache'; import { useDICOMStore } from '@/src/store/datasets-dicom'; import { useRulerStore } from '@/src/store/tools/rulers'; @@ -21,20 +27,26 @@ import { usePolygonStore } from '@/src/store/tools/polygons'; // tell the truth about. Only the heavy import/download edges are mocked. // --------------------------------------------------------------------------- -const mocks = vi.hoisted(() => ({ - fetchProcessingResult: vi.fn(), -})); - -vi.mock('@/src/processing/engine/resultDownload', () => ({ - fetchProcessingResult: mocks.fetchProcessingResult, -})); -vi.mock('@/src/io/import/dataSource', () => ({ uriToDataSource: vi.fn() })); -vi.mock('@/src/io/import/importDataSources', () => ({ - importVolumeDataSources: vi.fn(), - toDataSelection: vi.fn(), -})); -vi.mock('@/src/io/import/common', () => ({ isVolumeResult: vi.fn() })); -vi.mock('@/src/actions/loadUserFiles', () => ({ loadVolumeUrls: vi.fn() })); +// Stands in for the download edge: records what was asked for and hands back +// whatever the test last served. +const resultServer = () => { + const downloads: ProcessingResult[] = []; + let body = ''; + return { + downloads, + serve: (next: unknown) => { + body = typeof next === 'string' ? next : JSON.stringify(next); + }, + fetchResult: async (result: ProcessingResult) => { + downloads.push(result); + return new File([body], 'out.annotations.json', { + type: 'application/json', + }); + }, + }; +}; + +let results = resultServer(); const IMAGE_ID = 'img-1'; @@ -142,12 +154,16 @@ const annotationsFile = (): WireFile => ({ }, }); -const serveFile = (body: unknown) => { - const text = typeof body === 'string' ? body : JSON.stringify(body); - mocks.fetchProcessingResult.mockResolvedValue( - new File([text], 'out.annotations.json', { type: 'application/json' }) - ); -}; +const serveFile = (body: unknown) => results.serve(body); + +const apply = ( + resultIntent: Parameters[0], + jobContext: Parameters[1] +) => + applyIntent(resultIntent, jobContext, { + ...appApplyDependencies(), + fetchResult: results.fetchResult, + }); const toolCounts = () => ({ rulers: useRulerStore().toolIDs.length, @@ -161,7 +177,7 @@ const onlyTool = (store: { }) => store.toolByID[store.toolIDs[0]]; beforeEach(() => { - vi.clearAllMocks(); + results = resultServer(); setActivePinia(createPinia()); seatImage(); serveFile(annotationsFile()); @@ -169,7 +185,7 @@ beforeEach(() => { describe('applyIntent — add-annotations', () => { it('adds every tool kind to the job image, deriving the slice from the frame', async () => { - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(toolCounts()).toEqual({ rulers: 1, rectangles: 1, polygons: 1 }); @@ -226,7 +242,7 @@ describe('applyIntent — add-annotations', () => { ]; serveFile(file); - const outcome = await applyIntent(intent(), context(imageID)); + const outcome = await apply(intent(), context(imageID)); expect( outcome.status, @@ -263,7 +279,7 @@ describe('applyIntent — add-annotations', () => { }; serveFile(file); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(onlyTool(useRulerStore()).frameOfReference.planeNormal).toEqual( @@ -280,7 +296,7 @@ describe('applyIntent — add-annotations', () => { }; serveFile(file); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('failed'); expect(String((outcome as { error: Error }).error)).toContain( @@ -290,7 +306,7 @@ describe('applyIntent — add-annotations', () => { }); it('keeps a label name that repeats across kinds independent per store', async () => { - await applyIntent(intent(), context(IMAGE_ID)); + await apply(intent(), context(IMAGE_ID)); const ruler = onlyTool(useRulerStore()); const rectangle = onlyTool(useRectangleStore()); @@ -320,9 +336,7 @@ describe('applyIntent — add-annotations', () => { file.tools.polygons = []; serveFile(file); - expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( - 'applied' - ); + expect((await apply(intent(), context(IMAGE_ID))).status).toBe('applied'); expect(Object.keys(rulerStore.labels)).toHaveLength(before); expect(rulerStore.labels[existingId].color).toBe('#123456'); expect(onlyTool(rulerStore).label).toBe(existingId); @@ -340,9 +354,7 @@ describe('applyIntent — add-annotations', () => { file.tools.rulers[0].labelName = 'fresh'; serveFile(file); - expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( - 'applied' - ); + expect((await apply(intent(), context(IMAGE_ID))).status).toBe('applied'); expect(rulerStore.activeLabel).toBe(activeBefore); // The label still landed; only the picker was left alone. expect(onlyTool(rulerStore).labelName).toBe('fresh'); @@ -362,60 +374,56 @@ describe('applyIntent — add-annotations', () => { file.tools.polygons = []; serveFile(file); - expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( - 'applied' - ); + expect((await apply(intent(), context(IMAGE_ID))).status).toBe('applied'); const ruler = onlyTool(useRulerStore()); expect(ruler.label).toBe(''); expect(ruler.labelName).toBe(''); }); it('is a no-op when a tool already carries the same source', async () => { - expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( - 'applied' - ); - mocks.fetchProcessingResult.mockClear(); + expect((await apply(intent(), context(IMAGE_ID))).status).toBe('applied'); + results.downloads.length = 0; - const second = await applyIntent(intent(), context(IMAGE_ID)); + const second = await apply(intent(), context(IMAGE_ID)); expect(second.status).toBe('applied'); expect(toolCounts()).toEqual({ rulers: 1, rectangles: 1, polygons: 1 }); // The receipt short-circuits before the download. - expect(mocks.fetchProcessingResult).not.toHaveBeenCalled(); + expect(results.downloads).toEqual([]); }); it('re-applies a result from a different job even at the same output id', async () => { - await applyIntent(intent(), context(IMAGE_ID)); + await apply(intent(), context(IMAGE_ID)); const other = { ...source, jobId: 'job-2' }; - await applyIntent(intent({ source: other }), context(IMAGE_ID)); + await apply(intent({ source: other }), context(IMAGE_ID)); expect(toolCounts()).toEqual({ rulers: 2, rectangles: 2, polygons: 2 }); }); it('applies an empty result as a no-op', async () => { serveFile({ schemaVersion: 1, space: 'LPS', tools: {} }); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); }); it('fails without ever downloading when no image is bound', async () => { - const outcome = await applyIntent(intent(), context(undefined)); + const outcome = await apply(intent(), context(undefined)); expect(outcome.status).toBe('failed'); expect(String((outcome as { error: Error }).error)).toContain( "Load the job's input image" ); - expect(mocks.fetchProcessingResult).not.toHaveBeenCalled(); + expect(results.downloads).toEqual([]); expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); }); it('fails when the bound image is no longer in the cache', async () => { - const outcome = await applyIntent(intent(), context('img-gone')); + const outcome = await apply(intent(), context('img-gone')); expect(outcome.status).toBe('failed'); - expect(mocks.fetchProcessingResult).not.toHaveBeenCalled(); + expect(results.downloads).toEqual([]); }); it('fails on a malformed result body without touching the stores', async () => { serveFile('not json at all'); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('failed'); expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); }); @@ -432,7 +440,7 @@ describe('applyIntent — add-annotations', () => { }; serveFile(file); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('failed'); expect(String((outcome as { error: Error }).error)).toContain( 'not aligned' @@ -450,7 +458,7 @@ describe('applyIntent — add-annotations', () => { file.tools.rulers[0].frameOfReference = axialAt(500); serveFile(file); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(onlyTool(useRulerStore()).slice).toBe(500); @@ -461,7 +469,7 @@ describe('applyIntent — add-annotations', () => { file.tools.rulers[0].frameOfReference = axialAt(5.5); serveFile(file); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('failed'); expect(String((outcome as { error: Error }).error)).toContain( @@ -474,9 +482,7 @@ describe('applyIntent — add-annotations', () => { const file = annotationsFile(); file.tools.rulers[0].labelName = 'undeclared'; serveFile(file); - expect((await applyIntent(intent(), context(IMAGE_ID))).status).toBe( - 'failed' - ); + expect((await apply(intent(), context(IMAGE_ID))).status).toBe('failed'); expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); }); @@ -491,13 +497,13 @@ describe('applyIntent — add-annotations', () => { }); serveFile(file); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('failed'); expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); }); it('applies without a source when the producer omitted one', async () => { - const outcome = await applyIntent( + const outcome = await apply( intent({ source: undefined }), context(IMAGE_ID) ); @@ -540,7 +546,7 @@ describe('applyIntent — add-annotations', () => { it('drops a stray frame when the target is a static volume', async () => { rulerOnlyFile(3); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(onlyTool(useRulerStore()).frame).toBeUndefined(); }); @@ -548,7 +554,7 @@ describe('applyIntent — add-annotations', () => { it('keeps an in-range integral frame on a cine target', async () => { markCine(8); rulerOnlyFile(7); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(onlyTool(useRulerStore()).frame).toBe(7); }); @@ -556,7 +562,7 @@ describe('applyIntent — add-annotations', () => { it('applies a frameless tool to a cine target (every frame)', async () => { markCine(8); rulerOnlyFile(); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(onlyTool(useRulerStore()).frame).toBeUndefined(); }); @@ -571,7 +577,7 @@ describe('applyIntent — add-annotations', () => { async (_label, frame) => { markCine(8); rulerOnlyFile(frame); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('failed'); // All-or-nothing: nothing may land. expect(toolCounts()).toEqual({ rulers: 0, rectangles: 0, polygons: 0 }); @@ -583,7 +589,7 @@ describe('applyIntent — add-annotations', () => { it('drops an out-of-range frame on a cine target', async () => { markCine(8); rulerOnlyFile(8); - const outcome = await applyIntent(intent(), context(IMAGE_ID)); + const outcome = await apply(intent(), context(IMAGE_ID)); expect(outcome.status).toBe('applied'); expect(onlyTool(useRulerStore()).frame).toBeUndefined(); }); diff --git a/src/processing/__tests__/applyResults.spec.ts b/src/processing/__tests__/applyResults.spec.ts index f77c0fb4d..d0582e390 100644 --- a/src/processing/__tests__/applyResults.spec.ts +++ b/src/processing/__tests__/applyResults.spec.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { createPinia, setActivePinia } from 'pinia'; import { applyIntent, @@ -8,51 +9,42 @@ import type { ProcessingResult, SubmittedJobContext, } from '@/src/processing/types'; -import type { ProcessingResultSource } from '@/src/types'; - -const mocks = vi.hoisted(() => ({ - uriToDataSource: vi.fn(), - importVolumeDataSources: vi.fn(), - toDataSelection: vi.fn(), - isVolumeResult: vi.fn(), - loadVolumeUrls: vi.fn(), - addLayer: vi.fn(), +import type { ResultSource } from '@/backend-contract'; +import { useMessageStore } from '@/src/store/messages'; + +// --------------------------------------------------------------------------- +// Intent routing: which scene edge each result intent reaches for, and what it +// reports back. The download, import and scene-mutation edges are handed in as +// recorders, so the decisions are exercised without a loaded scene; the message +// store is the real one. +// --------------------------------------------------------------------------- + +const recordingDependencies = () => ({ + fetchResult: vi.fn(), + openVolumeUrls: vi.fn(async () => ['dataset-live']), + importVolume: vi.fn(async (): Promise => 'child-selection'), removeDataset: vi.fn(), - convertImageToLabelmap: vi.fn(), - updateSegment: vi.fn(), - metadataByID: {} as Record, - addError: vi.fn(), -})); - -vi.mock('@/src/io/import/dataSource', () => ({ - uriToDataSource: mocks.uriToDataSource, -})); -vi.mock('@/src/io/import/importDataSources', () => ({ - importVolumeDataSources: mocks.importVolumeDataSources, - toDataSelection: mocks.toDataSelection, -})); -vi.mock('@/src/io/import/common', () => ({ - isVolumeResult: mocks.isVolumeResult, -})); -vi.mock('@/src/actions/loadUserFiles', () => ({ - loadVolumeUrls: mocks.loadVolumeUrls, -})); -vi.mock('@/src/store/datasets', () => ({ - useDatasetStore: () => ({ remove: mocks.removeDataset }), -})); -vi.mock('@/src/store/datasets-layers', () => ({ - useLayersStore: () => ({ addLayer: mocks.addLayer }), -})); -vi.mock('@/src/store/segmentGroups', () => ({ - useSegmentGroupStore: () => ({ - convertImageToLabelmap: mocks.convertImageToLabelmap, - updateSegment: mocks.updateSegment, - metadataByID: mocks.metadataByID, - }), -})); -vi.mock('@/src/store/messages', () => ({ - useMessageStore: () => ({ addError: mocks.addError }), -})); + addLayer: vi.fn(async (): Promise => 'layer-1'), + segmentGroups: { + resultSourcesInScene: vi.fn((): Array => []), + convertImageToLabelmap: vi.fn(async () => ['seg-group']), + updateSegment: vi.fn(), + }, +}); + +let deps = recordingDependencies(); + +const apply = ( + resultIntent: Parameters[0], + jobContext: Parameters[1] +) => applyIntent(resultIntent, jobContext, deps); + +const autoLoad = ( + results: ProcessingResult[], + jobContext: SubmittedJobContext | undefined +) => autoLoadProcessingResults(results, jobContext, deps); + +const errorMessages = () => useMessageStore().messages; const file = { id: 'r1', url: 'https://example/out.nrrd', name: 'out.nrrd' }; const rgba = (r: number, g: number, b: number, a: number) => @@ -76,17 +68,8 @@ const result = ( }); beforeEach(() => { - vi.clearAllMocks(); - mocks.metadataByID = {}; - mocks.uriToDataSource.mockReturnValue({ type: 'uri' }); - mocks.importVolumeDataSources.mockResolvedValue([ - { type: 'data', dataID: 'child-1' }, - ]); - mocks.isVolumeResult.mockReturnValue(true); - mocks.toDataSelection.mockReturnValue('child-selection'); - mocks.loadVolumeUrls.mockResolvedValue(['dataset-live']); - mocks.convertImageToLabelmap.mockResolvedValue(['seg-group']); - mocks.addLayer.mockResolvedValue('layer-1'); + setActivePinia(createPinia()); + deps = recordingDependencies(); }); afterEach(() => { @@ -95,113 +78,118 @@ afterEach(() => { describe('applyIntent', () => { it('add-base-image opens the file as a new dataset', async () => { - const applied = await applyIntent( + const applied = await apply( { intent: 'add-base-image', ...file }, context('parent') ); expect(applied.status).toBe('applied'); - expect(mocks.loadVolumeUrls).toHaveBeenCalledWith({ + expect(deps.openVolumeUrls).toHaveBeenCalledWith({ urls: [file.url], names: [file.name], }); - expect(mocks.addLayer).not.toHaveBeenCalled(); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(deps.addLayer).not.toHaveBeenCalled(); + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); }); it('add-layer attaches a layer onto the originating dataset', async () => { - const applied = await applyIntent( + const applied = await apply( { intent: 'add-layer', ...file }, context('parent') ); expect(applied.status).toBe('applied'); - expect(mocks.addLayer).toHaveBeenCalledWith('parent', 'child-selection'); - expect(mocks.importVolumeDataSources).toHaveBeenCalledWith([ - { type: 'uri' }, - ]); - expect(mocks.loadVolumeUrls).not.toHaveBeenCalled(); + expect(deps.addLayer).toHaveBeenCalledWith('parent', 'child-selection'); + expect(deps.importVolume).toHaveBeenCalledWith( + expect.objectContaining({ url: file.url, name: file.name }) + ); + expect(deps.openVolumeUrls).not.toHaveBeenCalled(); }); it('add-layer with no originating dataset falls back to opening', async () => { - await applyIntent({ intent: 'add-layer', ...file }, context(undefined)); - expect(mocks.addLayer).not.toHaveBeenCalled(); - expect(mocks.loadVolumeUrls).toHaveBeenCalledWith({ + await apply({ intent: 'add-layer', ...file }, context(undefined)); + expect(deps.addLayer).not.toHaveBeenCalled(); + expect(deps.openVolumeUrls).toHaveBeenCalledWith({ urls: [file.url], names: [file.name], }); }); it('add-segment-group converts the labelmap and applies descriptors to the created group', async () => { - mocks.convertImageToLabelmap.mockResolvedValue(['group-1']); + deps.segmentGroups.convertImageToLabelmap.mockResolvedValue(['group-1']); const segments = [ { value: 1, name: 'liver', color: rgba(255, 0, 0, 255) }, { value: 2, name: 'tumor', color: rgba(0, 255, 0, 255), visible: false }, ]; - await applyIntent( + await apply( { intent: 'add-segment-group', ...file, segments }, context('parent') ); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', undefined ); - expect(mocks.updateSegment).toHaveBeenCalledTimes(2); - expect(mocks.updateSegment).toHaveBeenCalledWith('group-1', 1, { - name: 'liver', - color: [255, 0, 0, 255], - }); - expect(mocks.updateSegment).toHaveBeenCalledWith('group-1', 2, { - name: 'tumor', - color: [0, 255, 0, 255], - visible: false, - }); - expect(mocks.loadVolumeUrls).not.toHaveBeenCalled(); + expect(deps.segmentGroups.updateSegment).toHaveBeenCalledTimes(2); + expect(deps.segmentGroups.updateSegment).toHaveBeenCalledWith( + 'group-1', + 1, + { + name: 'liver', + color: [255, 0, 0, 255], + } + ); + expect(deps.segmentGroups.updateSegment).toHaveBeenCalledWith( + 'group-1', + 2, + { + name: 'tumor', + color: [0, 255, 0, 255], + visible: false, + } + ); + expect(deps.openVolumeUrls).not.toHaveBeenCalled(); }); it('add-segment-group removes the temporarily imported child dataset', async () => { - const outcome = await applyIntent( + const outcome = await apply( { intent: 'add-segment-group', ...file }, context('parent') ); expect(outcome.status).toBe('applied'); - expect(mocks.removeDataset).toHaveBeenCalledWith('child-selection'); - expect(mocks.removeDataset.mock.invocationCallOrder[0]).toBeGreaterThan( - mocks.convertImageToLabelmap.mock.invocationCallOrder[0] + expect(deps.removeDataset).toHaveBeenCalledWith('child-selection'); + expect(deps.removeDataset.mock.invocationCallOrder[0]).toBeGreaterThan( + deps.segmentGroups.convertImageToLabelmap.mock.invocationCallOrder[0] ); }); it('add-segment-group removes the imported child even when conversion fails', async () => { - mocks.convertImageToLabelmap.mockRejectedValue( + deps.segmentGroups.convertImageToLabelmap.mockRejectedValue( new Error('bounds do not intersect') ); - const outcome = await applyIntent( + const outcome = await apply( { intent: 'add-segment-group', ...file }, context('parent') ); expect(outcome.status).toBe('failed'); - expect(mocks.removeDataset).toHaveBeenCalledWith('child-selection'); + expect(deps.removeDataset).toHaveBeenCalledWith('child-selection'); }); it('add-layer keeps its imported child dataset (the layer references it)', async () => { - const outcome = await applyIntent( + const outcome = await apply( { intent: 'add-layer', ...file }, context('parent') ); expect(outcome.status).toBe('applied'); - expect(mocks.removeDataset).not.toHaveBeenCalled(); + expect(deps.removeDataset).not.toHaveBeenCalled(); }); it('add-segment-group with no segments still converts (embedded metadata)', async () => { - await applyIntent( - { intent: 'add-segment-group', ...file }, - context('parent') - ); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + await apply({ intent: 'add-segment-group', ...file }, context('parent')); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', undefined ); - expect(mocks.updateSegment).not.toHaveBeenCalled(); + expect(deps.segmentGroups.updateSegment).not.toHaveBeenCalled(); }); it('stamps structured provider-qualified provenance on the created group', async () => { @@ -210,11 +198,11 @@ describe('applyIntent', () => { jobId: 'job-abc123', outputId: 'outputLabelmap', }; - await applyIntent( + await apply( { intent: 'add-segment-group', ...file, source }, context('parent') ); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', source @@ -227,44 +215,36 @@ describe('applyIntent', () => { jobId: 'job-abc123', outputId: 'outputLabelmap', }; - mocks.metadataByID = { - restored: { source }, - }; + deps.segmentGroups.resultSourcesInScene.mockReturnValue([source]); - const outcome = await applyIntent( + const outcome = await apply( { intent: 'add-segment-group', ...file, source }, context('parent') ); expect(outcome.status).toBe('applied'); - expect(mocks.importVolumeDataSources).not.toHaveBeenCalled(); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); - expect(mocks.loadVolumeUrls).not.toHaveBeenCalled(); + expect(deps.importVolume).not.toHaveBeenCalled(); + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(deps.openVolumeUrls).not.toHaveBeenCalled(); }); it('applies a different output from the same restored job', async () => { - mocks.metadataByID = { - restored: { - source: { - providerId: 'p1', - jobId: 'job-abc123', - outputId: 'existing-output', - }, - }, - }; + deps.segmentGroups.resultSourcesInScene.mockReturnValue([ + { providerId: 'p1', jobId: 'job-abc123', outputId: 'existing-output' }, + ]); const source = { providerId: 'p1', jobId: 'job-abc123', outputId: 'new-output', }; - const outcome = await applyIntent( + const outcome = await apply( { intent: 'add-segment-group', ...file, source }, context('parent') ); expect(outcome.status).toBe('applied'); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', source @@ -272,24 +252,22 @@ describe('applyIntent', () => { }); it('applies matching raw job and output ids from a different provider', async () => { - mocks.metadataByID = { - restored: { - source: { providerId: 'provider-a', jobId: '1', outputId: 'seg' }, - }, - }; + deps.segmentGroups.resultSourcesInScene.mockReturnValue([ + { providerId: 'provider-a', jobId: '1', outputId: 'seg' }, + ]); const source = { providerId: 'provider-b', jobId: '1', outputId: 'seg', }; - const outcome = await applyIntent( + const outcome = await apply( { intent: 'add-segment-group', ...file, source }, context('parent') ); expect(outcome.status).toBe('applied'); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', source @@ -297,54 +275,51 @@ describe('applyIntent', () => { }); it('does not infer an application receipt when provenance is absent', async () => { - mocks.metadataByID = { restored: {} }; + deps.segmentGroups.resultSourcesInScene.mockReturnValue([undefined]); - const outcome = await applyIntent( + const outcome = await apply( { intent: 'add-segment-group', ...file }, context('parent') ); expect(outcome.status).toBe('applied'); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledTimes(1); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledTimes(1); }); it('add-segment-group with no originating dataset falls back to opening', async () => { - await applyIntent( - { intent: 'add-segment-group', ...file }, - context(undefined) - ); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); - expect(mocks.loadVolumeUrls).toHaveBeenCalledWith({ + await apply({ intent: 'add-segment-group', ...file }, context(undefined)); + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(deps.openVolumeUrls).toHaveBeenCalledWith({ urls: [file.url], names: [file.name], }); }); it('add-segment-group reports an explicit failure when the result fails to load (#7)', async () => { - mocks.importVolumeDataSources.mockResolvedValue([]); - const applied = await applyIntent( + deps.importVolume.mockResolvedValue(null); + const applied = await apply( { intent: 'add-segment-group', ...file }, context('parent') ); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); expect(applied.status).toBe('failed'); - expect(mocks.addError).not.toHaveBeenCalled(); + expect(errorMessages()).toEqual([]); }); it('add-layer reports an explicit failure when the result fails to load (#7)', async () => { - mocks.importVolumeDataSources.mockResolvedValue([]); - const applied = await applyIntent( + deps.importVolume.mockResolvedValue(null); + const applied = await apply( { intent: 'add-layer', ...file }, context('parent') ); - expect(mocks.addLayer).not.toHaveBeenCalled(); + expect(deps.addLayer).not.toHaveBeenCalled(); expect(applied.status).toBe('failed'); - expect(mocks.addError).not.toHaveBeenCalled(); + expect(errorMessages()).toEqual([]); }); it('resolves to failed (never rejects) when the fallback open throws', async () => { - mocks.loadVolumeUrls.mockRejectedValue(new Error('bad result url')); - const applied = await applyIntent( + deps.openVolumeUrls.mockRejectedValue(new Error('bad result url')); + const applied = await apply( { intent: 'add-base-image', ...file }, context('parent') ); @@ -352,21 +327,21 @@ describe('applyIntent', () => { }); it('add-layer reports failure when the layer fails to build (addLayer swallows the throw)', async () => { - mocks.addLayer.mockResolvedValue(undefined); - const applied = await applyIntent( + deps.addLayer.mockResolvedValue(undefined); + const applied = await apply( { intent: 'add-layer', ...file }, context('parent') ); - expect(mocks.addLayer).toHaveBeenCalledWith('parent', 'child-selection'); + expect(deps.addLayer).toHaveBeenCalledWith('parent', 'child-selection'); expect(applied.status).toBe('failed'); - expect(mocks.removeDataset).toHaveBeenCalledWith('child-selection'); - expect(mocks.addError).not.toHaveBeenCalled(); + expect(deps.removeDataset).toHaveBeenCalledWith('child-selection'); + expect(errorMessages()).toEqual([]); }); it('is additive-only: writes into the NEW group, never a pre-existing one', async () => { - mocks.metadataByID = { 'existing-group': {} }; - mocks.convertImageToLabelmap.mockResolvedValue(['new-group']); - await applyIntent( + deps.segmentGroups.resultSourcesInScene.mockReturnValue([undefined]); + deps.segmentGroups.convertImageToLabelmap.mockResolvedValue(['new-group']); + await apply( { intent: 'add-segment-group', ...file, @@ -374,13 +349,13 @@ describe('applyIntent', () => { }, context('parent') ); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledTimes(1); - expect(mocks.updateSegment).toHaveBeenCalledWith( + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledTimes(1); + expect(deps.segmentGroups.updateSegment).toHaveBeenCalledWith( 'new-group', 1, expect.anything() ); - expect(mocks.updateSegment).not.toHaveBeenCalledWith( + expect(deps.segmentGroups.updateSegment).not.toHaveBeenCalledWith( 'existing-group', expect.anything(), expect.anything() @@ -390,8 +365,8 @@ describe('applyIntent', () => { describe('autoLoadProcessingResults', () => { it('routes every supported intent through the shared applier', async () => { - mocks.convertImageToLabelmap.mockResolvedValue(['seg-group']); - await autoLoadProcessingResults( + deps.segmentGroups.convertImageToLabelmap.mockResolvedValue(['seg-group']); + await autoLoad( [ result({ id: 'a', intent: 'add-base-image' }), result({ id: 'b', intent: 'add-layer' }), @@ -404,49 +379,43 @@ describe('autoLoadProcessingResults', () => { ], context('parent') ); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledTimes(1); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledTimes(1); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', { providerId: 'p1', jobId: 'j1', outputId: 'seg' } ); - expect(mocks.updateSegment).toHaveBeenCalledTimes(1); - expect(mocks.loadVolumeUrls).toHaveBeenCalledTimes(1); - expect(mocks.loadVolumeUrls).toHaveBeenCalledWith({ + expect(deps.segmentGroups.updateSegment).toHaveBeenCalledTimes(1); + expect(deps.openVolumeUrls).toHaveBeenCalledTimes(1); + expect(deps.openVolumeUrls).toHaveBeenCalledWith({ urls: [file.url], names: [file.name], }); - expect(mocks.addLayer).toHaveBeenCalledWith('parent', 'child-selection'); + expect(deps.addLayer).toHaveBeenCalledWith('parent', 'child-selection'); }); it('does not auto-apply an unknown intent', async () => { - await autoLoadProcessingResults( - [result({ intent: 'add-polygon' })], - context('parent') - ); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); - expect(mocks.loadVolumeUrls).not.toHaveBeenCalled(); + await autoLoad([result({ intent: 'add-polygon' })], context('parent')); + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(deps.openVolumeUrls).not.toHaveBeenCalled(); }); it('opens base images even when there is no originating dataset', async () => { - await autoLoadProcessingResults( - [result({ intent: 'add-base-image' })], - context(undefined) - ); - expect(mocks.loadVolumeUrls).toHaveBeenCalledWith({ + await autoLoad([result({ intent: 'add-base-image' })], context(undefined)); + expect(deps.openVolumeUrls).toHaveBeenCalledWith({ urls: [file.url], names: [file.name], }); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); }); it('opens a parentless segment-group result as an ordinary dataset', async () => { - await autoLoadProcessingResults( + await autoLoad( [result({ intent: 'add-segment-group' })], context(undefined) ); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); - expect(mocks.loadVolumeUrls).toHaveBeenCalledWith({ + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(deps.openVolumeUrls).toHaveBeenCalledWith({ urls: [file.url], names: [file.name], }); @@ -454,23 +423,23 @@ describe('autoLoadProcessingResults', () => { it('keeps applying after one segment-group result throws', async () => { const err = vi.spyOn(console, 'error').mockImplementation(() => {}); - mocks.convertImageToLabelmap + deps.segmentGroups.convertImageToLabelmap .mockRejectedValueOnce(new Error('boom')) .mockResolvedValueOnce(['g2']); - const application = await autoLoadProcessingResults( + const application = await autoLoad( [ result({ id: 'a', intent: 'add-segment-group' }), result({ id: 'b', intent: 'add-segment-group' }), ], context('parent') ); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledTimes(2); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledTimes(2); expect(err).toHaveBeenCalled(); expect(application.failedResultIds).toEqual(['a']); }); it('reports success when every known intent applies', async () => { - const application = await autoLoadProcessingResults( + const application = await autoLoad( [result({ intent: 'add-base-image' })], context('parent') ); @@ -489,9 +458,9 @@ describe('autoLoadProcessingResults', () => { jobId: 'j1', outputId: 'new', }; - mocks.metadataByID = { restored: { source: restoredSource } }; + deps.segmentGroups.resultSourcesInScene.mockReturnValue([restoredSource]); - const application = await autoLoadProcessingResults( + const application = await autoLoad( [ result({ id: 'restored', @@ -508,9 +477,9 @@ describe('autoLoadProcessingResults', () => { ); expect(application.failedResultIds).toEqual([]); - expect(mocks.importVolumeDataSources).toHaveBeenCalledTimes(1); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledTimes(1); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + expect(deps.importVolume).toHaveBeenCalledTimes(1); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledTimes(1); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', newSource @@ -523,14 +492,14 @@ describe('autoLoadProcessingResults — labelmap auto-apply', () => { result({ id: 'seg', intent: 'add-segment-group', ...overrides }); it('auto-applies an importable labelmap', async () => { - mocks.convertImageToLabelmap.mockResolvedValue(['seg-group']); - await autoLoadProcessingResults([segResult()], context('parent')); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledTimes(1); + deps.segmentGroups.convertImageToLabelmap.mockResolvedValue(['seg-group']); + await autoLoad([segResult()], context('parent')); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledTimes(1); }); it('lets the conversion path decide whether an imported labelmap can attach', async () => { - await autoLoadProcessingResults([segResult()], context('parent')); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + await autoLoad([segResult()], context('parent')); + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', undefined @@ -538,22 +507,22 @@ describe('autoLoadProcessingResults — labelmap auto-apply', () => { }); it('does not auto-apply a result that fails to decode, and surfaces the failure', async () => { - mocks.importVolumeDataSources.mockResolvedValue([]); - await autoLoadProcessingResults([segResult()], context('parent')); - expect(mocks.convertImageToLabelmap).not.toHaveBeenCalled(); - expect(mocks.addError).toHaveBeenCalled(); + deps.importVolume.mockResolvedValue(null); + await autoLoad([segResult()], context('parent')); + expect(deps.segmentGroups.convertImageToLabelmap).not.toHaveBeenCalled(); + expect(errorMessages()).toHaveLength(1); }); }); describe('autoLoadProcessingResults — born-persistent (no confirm gate)', () => { it('applies the group immediately with no confirm gate', async () => { const source = { providerId: 'p1', jobId: 'j1', outputId: 'seg' }; - mocks.convertImageToLabelmap.mockResolvedValue(['seg-group']); - await autoLoadProcessingResults( + deps.segmentGroups.convertImageToLabelmap.mockResolvedValue(['seg-group']); + await autoLoad( [result({ id: 'seg', intent: 'add-segment-group', source })], context('parent') ); - expect(mocks.convertImageToLabelmap).toHaveBeenCalledWith( + expect(deps.segmentGroups.convertImageToLabelmap).toHaveBeenCalledWith( 'child-selection', 'parent', source diff --git a/src/processing/__tests__/store.spec.ts b/src/processing/__tests__/store.spec.ts index c2fd327de..849bde9c4 100644 --- a/src/processing/__tests__/store.spec.ts +++ b/src/processing/__tests__/store.spec.ts @@ -19,31 +19,21 @@ import type { import { jobKey } from '@/src/processing/types'; import { defer } from '@/src/utils'; import { makeFakeProvider } from './fakeProvider'; +import { + seatDataSource, + seatVolume, +} from '@/src/store/__tests__/datasetFixtures'; + +// Records what the store hands the result applier, standing in for the apply +// pass the store itself never reaches into. +const recordingAutoLoad = (failedResultIds: string[] = []) => + vi.fn(async () => ({ failedResultIds })); -const { datasetState } = vi.hoisted(() => ({ - datasetState: { - ids: [] as string[], - sources: {} as Record, - }, -})); -vi.mock('@/src/store/datasets', () => ({ - useDatasetStore: () => ({ - idsAsSelections: datasetState.ids, - getDataSource: (id: string) => datasetState.sources[id], - }), -})); - -const { autoLoadMock } = vi.hoisted(() => ({ autoLoadMock: vi.fn() })); -vi.mock('@/src/processing/applyResults', () => ({ - autoLoadProcessingResults: autoLoadMock, -})); - -const { createProviderMock } = vi.hoisted(() => ({ - createProviderMock: vi.fn(), -})); -vi.mock('@/src/processing/engine/transport', () => ({ - createEngineTransport: createProviderMock, -})); +let autoLoad = recordingAutoLoad(); + +beforeEach(() => { + autoLoad = recordingAutoLoad(); +}); const makeProvider = ( overrides: Partial @@ -121,7 +111,6 @@ describe('Providers store — job lifecycle (async with sync fast-path)', () => beforeEach(() => { setActivePinia(createPinia()); vi.useFakeTimers(); - datasetState.ids = []; }); afterEach(() => { @@ -419,7 +408,6 @@ describe('Providers store — live-only durability + failure UX', () => { beforeEach(() => { setActivePinia(createPinia()); vi.useFakeTimers(); - datasetState.ids = []; }); afterEach(() => { @@ -644,9 +632,8 @@ describe('Providers store — live-only durability + failure UX', () => { }); it('shares one in-flight result application for the same provider and job', async () => { - autoLoadMock.mockReset(); const gate = defer<{ failedResultIds: string[] }>(); - autoLoadMock.mockReturnValue(gate.promise); + autoLoad.mockReturnValue(gate.promise); const store = useProcessingJobsStore(); const jobRef = ref('job-apply-once'); store.recordSubmittedContext({ @@ -657,13 +644,13 @@ describe('Providers store — live-only durability + failure UX', () => { }); store.jobResults.set(keyFor(jobRef.jobId), sampleResults); - const automatic = store.applyJobResults(jobRef); + const automatic = store.applyJobResults(jobRef, autoLoad); const shared = store.jobResultApplications.get(keyFor(jobRef.jobId)); - const manual = store.applyJobResults(jobRef); + const manual = store.applyJobResults(jobRef, autoLoad); expect(store.jobResultApplications.get(keyFor(jobRef.jobId))).toBe(shared); await Promise.resolve(); - expect(autoLoadMock).toHaveBeenCalledTimes(1); + expect(autoLoad).toHaveBeenCalledTimes(1); gate.resolve({ failedResultIds: [] }); await Promise.all([automatic, manual]); @@ -673,8 +660,7 @@ describe('Providers store — live-only durability + failure UX', () => { }); it('keys concurrent result applications by provider and raw job id', async () => { - autoLoadMock.mockReset(); - autoLoadMock.mockResolvedValue({ failedResultIds: [] }); + autoLoad.mockResolvedValue({ failedResultIds: [] }); const store = useProcessingJobsStore(); const jobA = ref('shared-id', 'A'); const jobB = ref('shared-id', 'B'); @@ -689,20 +675,19 @@ describe('Providers store — live-only durability + failure UX', () => { }); await Promise.all([ - store.applyJobResults(jobA), - store.applyJobResults(jobB), + store.applyJobResults(jobA, autoLoad), + store.applyJobResults(jobB, autoLoad), ]); - expect(autoLoadMock).toHaveBeenCalledTimes(2); + expect(autoLoad).toHaveBeenCalledTimes(2); expect(store.jobResultsApplied.has(jobKey(jobA))).toBe(true); expect(store.jobResultsApplied.has(jobKey(jobB))).toBe(true); }); it('waits for result application before deleting the job', async () => { - autoLoadMock.mockReset(); const gate = defer<{ failedResultIds: string[] }>(); const events: string[] = []; - autoLoadMock.mockImplementation(async () => { + autoLoad.mockImplementation(async () => { await gate.promise; events.push('applied'); return { failedResultIds: [] }; @@ -721,7 +706,7 @@ describe('Providers store — live-only durability + failure UX', () => { }); store.jobResults.set(keyFor(jobRef.jobId), sampleResults); - const applying = store.applyJobResults(jobRef); + const applying = store.applyJobResults(jobRef, autoLoad); const deleting = store.deleteJob(jobRef); await Promise.resolve(); @@ -733,8 +718,7 @@ describe('Providers store — live-only durability + failure UX', () => { }); it('clears a rejected application so the job can be retried', async () => { - autoLoadMock.mockReset(); - autoLoadMock + autoLoad .mockRejectedValueOnce(new Error('application failed')) .mockResolvedValueOnce({ failedResultIds: [] }); const store = useProcessingJobsStore(); @@ -747,13 +731,13 @@ describe('Providers store — live-only durability + failure UX', () => { }); store.jobResults.set(keyFor(jobRef.jobId), sampleResults); - await expect(store.applyJobResults(jobRef)).rejects.toThrow( + await expect(store.applyJobResults(jobRef, autoLoad)).rejects.toThrow( 'application failed' ); expect(store.jobResultApplications.has(keyFor(jobRef.jobId))).toBe(false); - await store.applyJobResults(jobRef); - expect(autoLoadMock).toHaveBeenCalledTimes(2); + await store.applyJobResults(jobRef, autoLoad); + expect(autoLoad).toHaveBeenCalledTimes(2); expect(store.jobResultsApplied.has(keyFor(jobRef.jobId))).toBe(true); }); @@ -879,7 +863,6 @@ describe('Providers store — live-only durability + failure UX', () => { }); it('detects a base image deleted mid-job and messages without dropping the result', async () => { - datasetState.ids = []; const store = useProcessingJobsStore(); const status = jobStatus('job-orphan', 'success'); @@ -911,7 +894,7 @@ describe('Providers store — live-only durability + failure UX', () => { }); it('does not flag a missing base image when the originating dataset is still loaded', async () => { - datasetState.ids = ['ds-present']; + seatVolume('ds-present'); const store = useProcessingJobsStore(); const status = jobStatus('job-ok', 'success'); @@ -941,7 +924,7 @@ describe('Providers store — live-only durability + failure UX', () => { }); it('surfaces a partial-loss warning on a non-zero missing count, still applying the results', async () => { - datasetState.ids = ['ds-present']; + seatVolume('ds-present'); const store = useProcessingJobsStore(); const status = jobStatus('job-miss', 'success', { @@ -977,7 +960,7 @@ describe('Providers store — live-only durability + failure UX', () => { }); it('surfaces no partial-loss warning when nothing is missing', async () => { - datasetState.ids = ['ds-present']; + seatVolume('ds-present'); const store = useProcessingJobsStore(); const status = jobStatus('job-clean', 'success'); @@ -1033,17 +1016,14 @@ describe('Providers store — re-discovered job history: slim observability adop const store = useProcessingJobsStore(); store.registerProviderConfig(config); store.instances.set('p1', provider); - datasetState.ids = ['ds1']; - datasetState.sources = { ds1: { type: 'uri', uri: '/f/a' } }; + seatVolume('ds1'); + seatDataSource('ds1', { type: 'uri', uri: '/f/a', name: 'a.nrrd' }); return store; }; beforeEach(() => { setActivePinia(createPinia()); vi.useFakeTimers(); - datasetState.ids = []; - datasetState.sources = {}; - autoLoadMock.mockReset(); }); afterEach(() => { @@ -1081,7 +1061,7 @@ describe('Providers store — re-discovered job history: slim observability adop await store.adoptJobHistory(); expect(provider.getJob).not.toHaveBeenCalled(); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); expect(store.submittedContexts.size).toBe(0); }); @@ -1201,7 +1181,7 @@ describe('Providers store — re-discovered job history: slim observability adop expect(store.submittedContexts.get(keyFor('jr'))?.taskId).toBe('t1'); expect(provider.getJob).not.toHaveBeenCalled(); expect(provider.getResults).not.toHaveBeenCalled(); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); }); it.each(['error', 'cancelled'] as const)( @@ -1220,7 +1200,7 @@ describe('Providers store — re-discovered job history: slim observability adop expect(provider.getJob).not.toHaveBeenCalled(); expect(provider.getResults).not.toHaveBeenCalled(); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); expect(store.jobs.get(keyFor('jr'))?.state).toBe(state); expect(store.submittedContexts.get(keyFor('jr'))?.taskId).toBe('t1'); } @@ -1241,7 +1221,7 @@ describe('Providers store — re-discovered job history: slim observability adop expect(provider.getJob).not.toHaveBeenCalled(); expect(store.jobs.get(keyFor('jr'))?.state).toBe('success'); expect(provider.getResults).not.toHaveBeenCalled(); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); }); it('a still-running re-discovered job is tracked for polling, not applied', async () => { @@ -1264,7 +1244,7 @@ describe('Providers store — re-discovered job history: slim observability adop store.submittedContexts.get(keyFor('jr'))?.activeDatasetId ).toBeUndefined(); expect(provider.getResults).not.toHaveBeenCalled(); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); }); it("retries a transient failure on a re-discovered job's first status read", async () => { @@ -1411,13 +1391,13 @@ describe('Providers store — re-discovered job history: slim observability adop getResults: vi.fn().mockResolvedValue(resultsBundle(sampleResults)), }); const store = arrange(provider); - datasetState.sources.ds1 = { + seatDataSource('ds1', { type: 'collection', sources: [ - { type: 'uri', uri: '/f/a' }, - { type: 'uri', uri: '/f/a' }, + { type: 'uri', uri: '/f/a', name: 'a.nrrd' }, + { type: 'uri', uri: '/f/a', name: 'a.nrrd' }, ], - }; + }); await store.adoptJobHistory(); await store.loadJobResults(ref('jr')); @@ -1441,17 +1421,15 @@ describe('Providers store — re-discovered job history: slim observability adop getResults: vi.fn().mockResolvedValue(resultsBundle(sampleResults)), }); const store = arrange(provider); - datasetState.ids = ['ds1', 'ds2']; - datasetState.sources = { - ds1: { type: 'uri', uri: '/f/a' }, - ds2: { - type: 'collection', - sources: [ - { type: 'uri', uri: '/f/a' }, - { type: 'uri', uri: '/f/a' }, - ], - }, - }; + seatVolume('ds2'); + seatDataSource('ds1', { type: 'uri', uri: '/f/a', name: 'a.nrrd' }); + seatDataSource('ds2', { + type: 'collection', + sources: [ + { type: 'uri', uri: '/f/a', name: 'a.nrrd' }, + { type: 'uri', uri: '/f/a', name: 'a.nrrd' }, + ], + }); await store.adoptJobHistory(); await store.loadJobResults(ref('jr')); @@ -1551,7 +1529,7 @@ describe('Providers store — re-discovered job history: slim observability adop expect(store.submittedContexts.get(keyFor('jr'))?.activeDatasetId).toBe( 'ds1' ); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); }); it('a job that settles between listing and the first poll completes once', async () => { @@ -1580,7 +1558,7 @@ describe('Providers store — re-discovered job history: slim observability adop const store = arrange(provider); await expect(store.adoptJobHistory()).resolves.toBeUndefined(); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); expect(err).toHaveBeenCalled(); }); @@ -1603,7 +1581,7 @@ describe('Providers store — re-discovered job history: slim observability adop await store.adoptJobHistory(); expect(provider.getJob).not.toHaveBeenCalled(); - expect(autoLoadMock).not.toHaveBeenCalled(); + expect(store.jobResultsApplied.size).toBe(0); }); // Boot adoption can race an in-flight submit of the same job, so both paths start a poll loop. @@ -1693,9 +1671,6 @@ describe('Providers store — immutable registration + provider-qualified job ke beforeEach(() => { setActivePinia(createPinia()); vi.useFakeTimers(); - datasetState.ids = []; - datasetState.sources = {}; - createProviderMock.mockReset(); }); afterEach(() => { @@ -1825,15 +1800,16 @@ describe('Providers store — immutable registration + provider-qualified job ke store.registerProviderConfig(cfg({ id: 'retry' })); const provider = makeProvider({ config: cfg({ id: 'retry' }) }); - createProviderMock - .mockImplementationOnce(() => { - throw new Error('load failed'); - }) - .mockImplementationOnce(() => provider); + const load = vi + .fn() + .mockRejectedValueOnce(new Error('load failed')) + .mockResolvedValueOnce(provider); - await expect(store.getProvider('retry')).rejects.toThrow('load failed'); - await expect(store.getProvider('retry')).resolves.toEqual(provider); - expect(createProviderMock).toHaveBeenCalledTimes(2); + await expect(store.getProvider('retry', load)).rejects.toThrow( + 'load failed' + ); + await expect(store.getProvider('retry', load)).resolves.toEqual(provider); + expect(load).toHaveBeenCalledTimes(2); }); }); @@ -1841,7 +1817,6 @@ describe('Providers store — generation-guarded continuations', () => { beforeEach(() => { setActivePinia(createPinia()); vi.useFakeTimers(); - datasetState.ids = []; }); afterEach(() => { diff --git a/src/processing/applyResults.ts b/src/processing/applyResults.ts index 1b7f48c8e..0c07392d9 100644 --- a/src/processing/applyResults.ts +++ b/src/processing/applyResults.ts @@ -28,6 +28,7 @@ import { } from '@/src/io/import/importDataSources'; import { isVolumeResult } from '@/src/io/import/common'; import type { ImageMetadata } from '@/src/types/image'; +import type { SegmentMask } from '@/src/types/segment'; import { useDatasetStore } from '@/src/store/datasets'; import { useDICOMStore } from '@/src/store/datasets-dicom'; import { useLayersStore } from '@/src/store/datasets-layers'; @@ -58,12 +59,15 @@ const sameResultSource = ( source.jobId === target.jobId && source.outputId === target.outputId; -function segmentGroupResultInScene(intent: SegmentGroupIntent): boolean { +function segmentGroupResultInScene( + intent: SegmentGroupIntent, + segmentGroups: SegmentGroupWriter +): boolean { const target = intent.source; if (!target) return false; - return Object.values(useSegmentGroupStore().metadataByID).some(({ source }) => - sameResultSource(source, target) - ); + return segmentGroups + .resultSourcesInScene() + .some((source) => sameResultSource(source, target)); } async function loadAsImport(file: ResultFile) { @@ -77,12 +81,12 @@ async function loadAsImport(file: ResultFile) { function applySegmentDescriptors( segmentGroupID: string, - segments: SegmentDescriptor[] + segments: SegmentDescriptor[], + segmentGroups: SegmentGroupWriter ) { - const segmentGroupStore = useSegmentGroupStore(); segments.forEach((seg) => { try { - segmentGroupStore.updateSegment(segmentGroupID, seg.value, { + segmentGroups.updateSegment(segmentGroupID, seg.value, { name: seg.name, color: seg.color, ...(seg.visible == null ? {} : { visible: seg.visible }), @@ -98,17 +102,19 @@ function applySegmentDescriptors( async function convertAndDescribe( childSelection: string, parentSelection: string, - intent: SegmentGroupIntent + intent: SegmentGroupIntent, + segmentGroups: SegmentGroupWriter ): Promise { - const segmentGroupStore = useSegmentGroupStore(); - const ids = await segmentGroupStore.convertImageToLabelmap( + const ids = await segmentGroups.convertImageToLabelmap( childSelection, parentSelection, intent.source ); // A seg.nrrd with embedded metadata carries no descriptors. if (intent.segments?.length) { - ids.forEach((id) => applySegmentDescriptors(id, intent.segments!)); + ids.forEach((id) => + applySegmentDescriptors(id, intent.segments!, segmentGroups) + ); } return ids; } @@ -292,7 +298,8 @@ const toolPayload = ( async function applyAnnotations( intent: AnnotationsIntent, - parentSelection: string | undefined + parentSelection: string | undefined, + fetchResult: FetchProcessingResult ): Promise { if (annotationResultInScene(intent)) return { status: 'applied' }; @@ -311,7 +318,7 @@ async function applyAnnotations( }; } - const file = await fetchProcessingResult({ + const file = await fetchResult({ id: intent.id, name: intent.name, url: intent.url, @@ -354,15 +361,76 @@ async function applyAnnotations( return { status: 'applied' }; } +type FetchProcessingResult = typeof fetchProcessingResult; + +type SegmentGroupWriter = { + /** Result provenance of every segment group in the scene, in scene order. */ + resultSourcesInScene: () => Array; + convertImageToLabelmap: ( + childSelection: string, + parentSelection: string, + source: ResultSource | undefined + ) => Promise; + updateSegment: ( + segmentGroupID: string, + segmentValue: number, + segmentUpdate: Partial> + ) => void; +}; + +/** + * The download, import and scene-mutation edges, so a caller can drive the + * intent routing without a loaded scene behind it. + */ +export type ApplyDependencies = { + fetchResult: FetchProcessingResult; + openVolumeUrls: typeof loadVolumeUrls; + importVolume: (file: ResultFile) => Promise; + removeDataset: (selection: string) => void; + addLayer: ( + parentSelection: string, + childSelection: string + ) => Promise; + segmentGroups: SegmentGroupWriter; +}; + +export const appApplyDependencies = (): ApplyDependencies => ({ + fetchResult: fetchProcessingResult, + openVolumeUrls: loadVolumeUrls, + importVolume: loadAsImport, + removeDataset: (selection) => useDatasetStore().remove(selection), + addLayer: (parentSelection, childSelection) => + useLayersStore().addLayer(parentSelection, childSelection), + segmentGroups: { + resultSourcesInScene: () => + Object.values(useSegmentGroupStore().metadataByID).map( + ({ source }) => source + ), + convertImageToLabelmap: (childSelection, parentSelection, source) => + useSegmentGroupStore().convertImageToLabelmap( + childSelection, + parentSelection, + source + ), + updateSegment: (segmentGroupID, segmentValue, segmentUpdate) => + useSegmentGroupStore().updateSegment( + segmentGroupID, + segmentValue, + segmentUpdate + ), + }, +}); + export async function applyIntent( intent: KnownResultIntent, - context: SubmittedJobContext | undefined + context: SubmittedJobContext | undefined, + dependencies: ApplyDependencies = appApplyDependencies() ): Promise { const parentSelection = context?.activeDatasetId; const openVolumeAsDatasetOutcome = async ( file: ResultFile ): Promise => { - const datasetIds = await loadVolumeUrls({ + const datasetIds = await dependencies.openVolumeUrls({ urls: [file.url], names: [file.name], }); @@ -380,16 +448,16 @@ export async function applyIntent( if (!parentSelection) { return await openVolumeAsDatasetOutcome(intent); } - const childSelection = await loadAsImport(intent); + const childSelection = await dependencies.importVolume(intent); if (!childSelection) return { status: 'failed', error: new Error('Result did not load') }; // addLayer swallows build failures and resolves undefined, so the id is the only failure signal. - const layerId = await useLayersStore().addLayer( + const layerId = await dependencies.addLayer( parentSelection, childSelection ); if (!layerId) { - useDatasetStore().remove(childSelection); + dependencies.removeDataset(childSelection); return { status: 'failed', error: new Error('Failed to attach layer'), @@ -401,23 +469,33 @@ export async function applyIntent( // Session-restored groups retain their result source. Treat that // durable provenance as an application receipt so retrying Load is // idempotent instead of creating a duplicate group. - if (segmentGroupResultInScene(intent)) return { status: 'applied' }; + if (segmentGroupResultInScene(intent, dependencies.segmentGroups)) + return { status: 'applied' }; if (!parentSelection) { return await openVolumeAsDatasetOutcome(intent); } - const childSelection = await loadAsImport(intent); + const childSelection = await dependencies.importVolume(intent); if (!childSelection) return { status: 'failed', error: new Error('Result did not load') }; try { - await convertAndDescribe(childSelection, parentSelection, intent); + await convertAndDescribe( + childSelection, + parentSelection, + intent, + dependencies.segmentGroups + ); return { status: 'applied' }; } finally { // The group owns its own labelmap image; the import was only a vehicle. - useDatasetStore().remove(childSelection); + dependencies.removeDataset(childSelection); } } case 'add-annotations': { - return await applyAnnotations(intent, parentSelection); + return await applyAnnotations( + intent, + parentSelection, + dependencies.fetchResult + ); } default: { const exhaustive: never = intent; @@ -435,13 +513,14 @@ export async function applyIntent( export async function autoLoadProcessingResults( results: ProcessingResult[], - context: SubmittedJobContext | undefined + context: SubmittedJobContext | undefined, + dependencies: ApplyDependencies = appApplyDependencies() ): Promise<{ failedResultIds: string[] }> { const failedResultIds: string[] = []; for (const result of results) { const intent = resultToIntent(result); if (!intent) continue; - const outcome = await applyIntent(intent, context); + const outcome = await applyIntent(intent, context, dependencies); if (outcome.status === 'failed') { failedResultIds.push(result.id); // The completion toast already promised results. diff --git a/src/processing/components/__tests__/JobsModule.spec.ts b/src/processing/components/__tests__/JobsModule.spec.ts index 6664abe13..d3b7d8cb0 100644 --- a/src/processing/components/__tests__/JobsModule.spec.ts +++ b/src/processing/components/__tests__/JobsModule.spec.ts @@ -16,16 +16,12 @@ import { type FakeProvider, } from '@/src/processing/__tests__/fakeProvider'; -const registry = new Map(); -vi.mock('@/src/processing/engine/transport', () => ({ - createEngineTransport: (config: { id: string }) => registry.get(config.id), -})); - // `writeSegmentation` spawns a real Worker; keep the IO module out of the test. const ioMocks = vi.hoisted(() => ({ readImage: vi.fn(), writeSegmentation: vi.fn(async () => new Uint8Array([1, 2, 3])), })); +// eslint-disable-next-line no-restricted-syntax -- ITK-wasm image IO has no counterpart in the node test environment vi.mock('@/src/io/readWriteImage', () => ({ readImage: ioMocks.readImage, writeSegmentation: ioMocks.writeSegmentation, @@ -73,8 +69,12 @@ const registerFake = ( store: ReturnType, provider: FakeProvider ) => { - registry.set(provider.config.id, provider as unknown as ProcessingProvider); store.registerProviderConfig(provider.config); + // Seating the instance is what a transport load would have produced. + store.instances.set( + provider.config.id, + provider as unknown as ProcessingProvider + ); }; // Auto-stubs drop slot content, hiding the panel children. @@ -109,7 +109,6 @@ describe('JobsModule — race-free provider/task selection', () => { let pinia: ReturnType; beforeEach(() => { - registry.clear(); pinia = createPinia().use(CorePiniaProviderPlugin()); // Core stores read injected tool singletons, which need an app to install onto. createApp({}).use(pinia); @@ -427,7 +426,6 @@ describe('JobsModule — segment group staging', () => { let pinia: ReturnType; beforeEach(() => { - registry.clear(); ioMocks.writeSegmentation.mockClear(); pinia = createPinia().use(CorePiniaProviderPlugin()); createApp({}).use(pinia); diff --git a/src/processing/store.ts b/src/processing/store.ts index c1bd7c487..a93d1601c 100644 --- a/src/processing/store.ts +++ b/src/processing/store.ts @@ -225,14 +225,17 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { clearJobs(); } - async function getProvider(id: string): Promise { + async function getProvider( + id: string, + load: typeof loadProvider = loadProvider + ): Promise { const existing = instances.get(id); if (existing) return existing; const inflight = loading.get(id); if (inflight) return inflight; const config = configs.get(id); if (!config) throw new Error(`Unknown provider id: ${id}`); - const promise = loadProvider(config).then((provider) => { + const promise = load(config).then((provider) => { instances.set(id, provider); if (loading.get(id) === promise) loading.delete(id); return provider; @@ -782,7 +785,10 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { : results; } - function applyJobResults(jobRef: TrackedJobRef): Promise { + function applyJobResults( + jobRef: TrackedJobRef, + autoLoad: typeof autoLoadProcessingResults = autoLoadProcessingResults + ): Promise { const key = jobKey(jobRef); const existing = jobResultApplications.get(key); if (existing) return existing; @@ -801,10 +807,7 @@ export const useProcessingJobsStore = defineStore('processingJobs', () => { if (!isCurrent(key, gen)) return; const context = contextForAutoLoad(submittedContexts.get(key)); const pending = resultsPendingApplication(jobRef); - const { failedResultIds } = await autoLoadProcessingResults( - pending, - context - ); + const { failedResultIds } = await autoLoad(pending, context); if (!isCurrent(key, gen)) return; recordJobResultApplication(jobRef, failedResultIds); }); diff --git a/src/referenceLines/__tests__/store.spec.ts b/src/referenceLines/__tests__/store.spec.ts index eac1cf2d8..bd2559de9 100644 --- a/src/referenceLines/__tests__/store.spec.ts +++ b/src/referenceLines/__tests__/store.spec.ts @@ -1,15 +1,10 @@ -import { describe, it, beforeEach, expect, vi } from 'vitest'; +import { describe, it, beforeEach, expect } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; import { nextTick } from 'vue'; import { useReferenceLinesStore } from '../store'; import { useToolStore } from '@/src/store/tools'; import { Tools } from '@/src/store/tools/types'; -vi.mock('@/src/core/cine/isCineImage', () => ({ - isCineImage: () => false, - getCineImage: () => null, -})); - describe('Reference lines store', () => { beforeEach(() => { localStorage.clear(); diff --git a/src/referenceLines/__tests__/useReferenceLines.spec.ts b/src/referenceLines/__tests__/useReferenceLines.spec.ts index bee260189..557af68b5 100644 --- a/src/referenceLines/__tests__/useReferenceLines.spec.ts +++ b/src/referenceLines/__tests__/useReferenceLines.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, beforeEach, expect, vi } from 'vitest'; +import { describe, it, beforeEach, expect } from 'vitest'; import { ref } from 'vue'; import { setActivePinia, createPinia } from 'pinia'; import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; @@ -10,11 +10,6 @@ import type { ViewInfo2D } from '@/src/types/views'; import type { LPSAxis } from '@/src/types/lps'; import { useReferenceLines } from '../useReferenceLines'; -vi.mock('@/src/core/cine/isCineImage', () => ({ - isCineImage: () => false, - getCineImage: () => null, -})); - const DIMS: [number, number, number] = [10, 20, 30]; const seatImage = (id: string) => { diff --git a/src/store/__tests__/datasetFixtures.ts b/src/store/__tests__/datasetFixtures.ts new file mode 100644 index 000000000..8d3f206c1 --- /dev/null +++ b/src/store/__tests__/datasetFixtures.ts @@ -0,0 +1,31 @@ +import { useDICOMStore, type VolumeInfo } from '@/src/store/datasets-dicom'; +import { useDatasetStore } from '@/src/store/datasets'; +import type { DataSource } from '@/src/io/import/dataSource'; + +/** + * Seats a volume in the DICOM store so it counts as a loaded dataset + * everywhere the stores derive selections from. Requires an active pinia. + */ +export const seatVolume = ( + imageID: string, + info: Partial = {} +): VolumeInfo => { + const volumeInfo: VolumeInfo = { + NumberOfSlices: 10, + VolumeID: imageID, + Modality: 'US', + SeriesInstanceUID: '1.2.3.4', + SeriesNumber: '1', + SeriesDescription: 'clip', + WindowLevel: '128', + WindowWidth: '256', + kind: 'volume', + ...info, + }; + useDICOMStore().volumeInfo[imageID] = volumeInfo; + return volumeInfo; +}; + +/** Records where a seated dataset's bytes came from, as an import would. */ +export const seatDataSource = (dataID: string, dataSource: DataSource) => + useDatasetStore().addDataSources([{ dataID, dataSource }]); diff --git a/src/store/__tests__/datasets-dicom-cine.spec.ts b/src/store/__tests__/datasets-dicom-cine.spec.ts index f43536af8..2d9d424b2 100644 --- a/src/store/__tests__/datasets-dicom-cine.spec.ts +++ b/src/store/__tests__/datasets-dicom-cine.spec.ts @@ -66,10 +66,12 @@ const mocks = vi.hoisted(() => { }; }); +// eslint-disable-next-line no-restricted-syntax -- DICOM splitting runs in wasm; unavailable in the node test environment vi.mock('@/src/io/dicom', () => ({ splitAndSort: mocks.splitAndSort, })); +// eslint-disable-next-line no-restricted-syntax -- reads real DICOM bytes through wasm vi.mock('@/src/core/cine/parseCineDicom', async (importOriginal) => { const actual = await importOriginal(); @@ -79,6 +81,7 @@ vi.mock('@/src/core/cine/parseCineDicom', async (importOriginal) => { }; }); +// eslint-disable-next-line no-restricted-syntax -- needs a streaming chunk source the node environment cannot provide vi.mock('@/src/core/streaming/dicomChunkImage', () => ({ default: mocks.MockDicomChunkImage, })); diff --git a/src/store/__tests__/datasets-layers.spec.ts b/src/store/__tests__/datasets-layers.spec.ts index b0373abe7..1e73059ef 100644 --- a/src/store/__tests__/datasets-layers.spec.ts +++ b/src/store/__tests__/datasets-layers.spec.ts @@ -1,42 +1,46 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; - -// Mock only the vtk/image leaf dependencies `_addLayer` reaches, so the REAL -// layers store + REAL `useErrorMessage` wrapper run. The former suite mocked -// `addLayer` itself, which hid the bug this file guards: `addLayer` must return -// the built layer id on success (and `undefined` only when a build throws and -// `useErrorMessage` swallows it). vtkBoundingBox stays real — its `intersects` -// is deterministic over the numeric bounds below. -const { getImage, ensureSameSpace, untilLoaded, imageCache } = vi.hoisted( - () => ({ - getImage: vi.fn(), - ensureSameSpace: vi.fn(), - untilLoaded: vi.fn(), - imageCache: { - getImageMetadata: vi.fn(), - addVTKImageData: vi.fn(), - removeImage: vi.fn(), - onImageDeleted: vi.fn(() => () => {}), - }, - }) -); - -vi.mock('@/src/utils/dataSelection', () => ({ getImage })); +import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; +import vtkDataArray from '@kitware/vtk.js/Common/Core/DataArray'; + +// The REAL layers store, image cache and `useErrorMessage` wrapper run here. +// An earlier suite mocked `addLayer` itself, which hid the bug this file +// guards: `addLayer` must return the built layer id on success (and +// `undefined` only when a build throws and `useErrorMessage` swallows it). +const { ensureSameSpace } = vi.hoisted(() => ({ ensureSameSpace: vi.fn() })); +// eslint-disable-next-line no-restricted-syntax -- resampling runs in wasm; unavailable in the node test environment vi.mock('@/src/io/resample/resample', () => ({ ensureSameSpace })); -vi.mock('@/src/composables/untilLoaded', () => ({ untilLoaded })); -vi.mock('@/src/store/image-cache', () => ({ - useImageCacheStore: () => imageCache, -})); import { useLayersStore } from '@/src/store/datasets-layers'; +import { useImageCacheStore } from '@/src/store/image-cache'; +import { useMessageStore } from '@/src/store/messages'; + +// A unit-spacing cube at `origin`, so its bounds are the numbers the overlap +// check reads: an n-wide cube at o spans [o, o + n - 1] on every axis. +const seatImage = (id: string, origin: number, size = 3) => { + const image = vtkImageData.newInstance(); + image.setOrigin([origin, origin, origin]); + image.setDimensions(size, size, size); + image.getPointData().setScalars( + vtkDataArray.newInstance({ + name: 'scalars', + numberOfComponents: 1, + values: new Uint8Array(size ** 3), + }) + ); + return useImageCacheStore().addVTKImageData(image, id, { id }); +}; -const imageWithBounds = (bounds: number[]) => ({ getBounds: () => bounds }); +const cached = (id: string) => id in useImageCacheStore().imageById; + +const seatOverlappingPair = () => { + seatImage('parent', 0); + seatImage('source', 0); +}; beforeEach(() => { setActivePinia(createPinia()); vi.clearAllMocks(); - untilLoaded.mockResolvedValue(undefined); - imageCache.getImageMetadata.mockReturnValue({ name: 'layer' }); // ensureSameSpace echoes the source image; identity is enough here. ensureSameSpace.mockImplementation( async (_parent: unknown, source: unknown) => source @@ -45,42 +49,37 @@ beforeEach(() => { describe('useLayersStore.addLayer return contract', () => { it('returns the built layer id when a valid pair overlaps', async () => { - // Both images share physical space, so the build succeeds end to end. - getImage.mockImplementation(async () => - imageWithBounds([0, 2, 0, 2, 0, 2]) - ); + seatOverlappingPair(); const store = useLayersStore(); const id = await store.addLayer('parent', 'source'); expect(id).toBe('parent::source'); expect(store.getLayers('parent')).toHaveLength(1); - expect(imageCache.addVTKImageData).toHaveBeenCalledTimes(1); + expect(cached('parent::source')).toBe(true); }); it('returns undefined and removes the provisional layer when the build fails', async () => { // Non-intersecting bounds: `_addLayer` deletes its provisional layer and // throws; `useErrorMessage` swallows the throw and resolves to `undefined`. - getImage.mockImplementation(async (selection: unknown) => - selection === 'parent' - ? imageWithBounds([0, 1, 0, 1, 0, 1]) - : imageWithBounds([5, 6, 5, 6, 5, 6]) - ); + seatImage('parent', 0, 2); + seatImage('source', 5, 2); const store = useLayersStore(); const id = await store.addLayer('parent', 'source'); expect(id).toBeUndefined(); expect(store.getLayers('parent')).toHaveLength(0); - expect(imageCache.addVTKImageData).not.toHaveBeenCalled(); + expect(cached('parent::source')).toBe(false); + expect(useMessageStore().messages[0].options.details).toContain( + 'no overlap in physical space' + ); }); }); describe('useLayersStore.remove', () => { beforeEach(() => { - getImage.mockImplementation(async () => - imageWithBounds([0, 2, 0, 2, 0, 2]) - ); + seatOverlappingPair(); }); it('removing a base image prunes and disposes the layers it owns', async () => { @@ -92,7 +91,7 @@ describe('useLayersStore.remove', () => { store.remove('parent'); expect(store.getLayers('parent')).toHaveLength(0); - expect(imageCache.removeImage).toHaveBeenCalledWith('parent::source'); + expect(cached('parent::source')).toBe(false); }); it('removing a layer source prunes it from every parent layer list', async () => { @@ -102,6 +101,6 @@ describe('useLayersStore.remove', () => { store.remove('source'); expect(store.getLayers('parent')).toHaveLength(0); - expect(imageCache.removeImage).toHaveBeenCalledWith('parent::source'); + expect(cached('parent::source')).toBe(false); }); }); diff --git a/src/store/__tests__/fillHoles.spec.ts b/src/store/__tests__/fillHoles.spec.ts index 4368c5422..66ddb2195 100644 --- a/src/store/__tests__/fillHoles.spec.ts +++ b/src/store/__tests__/fillHoles.spec.ts @@ -13,16 +13,13 @@ import { useViewStore } from '@/src/store/views'; const fillHolesWorkerMock = vi.hoisted(() => vi.fn(async (input) => input)); +// eslint-disable-next-line no-restricted-syntax -- the fill-holes worker has no counterpart in the node test environment vi.mock('comlink', () => ({ wrap: () => ({ fillHolesWorker: fillHolesWorkerMock, }), })); -vi.mock('@/src/store/image-stats', () => ({ - useImageStatsStore: () => ({ stats: {} }), -})); - function addScalars(image: vtkImageData, values: Uint8Array) { image.getPointData().setScalars( vtkDataArray.newInstance({ diff --git a/src/store/__tests__/legacyManifestSegmentGroups.spec.ts b/src/store/__tests__/legacyManifestSegmentGroups.spec.ts index c527aa6ae..c66fd86bf 100644 --- a/src/store/__tests__/legacyManifestSegmentGroups.spec.ts +++ b/src/store/__tests__/legacyManifestSegmentGroups.spec.ts @@ -23,6 +23,7 @@ const ioMocks = vi.hoisted(() => ({ writeSegmentation: vi.fn(async () => new Uint8Array([1, 2, 3])), })); +// eslint-disable-next-line no-restricted-syntax -- ITK-wasm image IO has no counterpart in the node test environment vi.mock('@/src/io/readWriteImage', () => ({ readImage: ioMocks.readImage, writeSegmentation: ioMocks.writeSegmentation, diff --git a/src/store/__tests__/remote-save-state.spec.ts b/src/store/__tests__/remote-save-state.spec.ts index ce2635ec4..d518ad3d2 100644 --- a/src/store/__tests__/remote-save-state.spec.ts +++ b/src/store/__tests__/remote-save-state.spec.ts @@ -1,23 +1,16 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { createPinia, setActivePinia } from 'pinia'; -vi.mock('@/src/utils/fetch', () => ({ - $fetch: vi.fn().mockResolvedValue({ ok: true }), -})); -vi.mock('@/src/io/state-file/serialize', () => ({ - serialize: vi - .fn() - .mockResolvedValue(new Blob(['x'], { type: 'application/zip' })), -})); - import useRemoteSaveStateStore from '@/src/store/remote-save-state'; -import { $fetch } from '@/src/utils/fetch'; import { useMessageStore } from '@/src/store/messages'; +import { savePost, saveDependencies } from '@/src/store/__tests__/saveFixtures'; + +let post = savePost(); describe('remote save target', () => { beforeEach(() => { setActivePinia(createPinia()); - vi.mocked($fetch).mockClear(); + post = savePost(); }); afterEach(() => { @@ -50,8 +43,8 @@ describe('remote save target', () => { // Empty target inerts the save UI, which is gated on a non-empty saveUrl. expect(store.saveUrl).toBe(''); - await store.saveState(); - expect($fetch).not.toHaveBeenCalled(); + await store.saveState(saveDependencies(post)); + expect(post).not.toHaveBeenCalled(); expect( useMessageStore().messages.some((m) => m.title === 'Save Disabled') ).toBe(true); @@ -81,7 +74,7 @@ describe('remote save target', () => { describe('resume repoint on save', () => { beforeEach(() => { setActivePinia(createPinia()); - vi.mocked($fetch).mockClear(); + post = savePost(); }); afterEach(() => { @@ -91,7 +84,7 @@ describe('resume repoint on save', () => { it('repoints urls= to the resumeUrl and leaves the save target alone', async () => { const resumeUrl = '/api/v1/item/session-123/volview'; - vi.mocked($fetch).mockResolvedValue( + post.mockResolvedValue( new Response(JSON.stringify({ resumeUrl }), { status: 200 }) ); const replace = vi @@ -101,13 +94,13 @@ describe('resume repoint on save', () => { const launchSaveUrl = `${window.location.origin}/api/session/save`; store.setSaveUrl(launchSaveUrl); - await store.saveState(); + await store.saveState(saveDependencies(post)); - expect(vi.mocked($fetch)).toHaveBeenCalledWith( + expect(post).toHaveBeenCalledWith( launchSaveUrl, expect.objectContaining({ method: 'POST' }) ); - expect(vi.mocked($fetch).mock.calls[0][1]?.redirect).toBeUndefined(); + expect(post.mock.calls[0][1]?.redirect).toBeUndefined(); expect(replace).toHaveBeenCalledTimes(1); const nextUrl = new URL(replace.mock.calls[0][2] as string); expect(nextUrl.searchParams.get('urls')).toBe(resumeUrl); @@ -130,7 +123,7 @@ describe('resume repoint on save', () => { '?urls=%2Fchest.nrrd&names=chest.nrrd&save=%2Fapi%2Fsave' ); const resumeUrl = '/api/v1/item/session-123/volview'; - vi.mocked($fetch).mockResolvedValue( + post.mockResolvedValue( new Response(JSON.stringify({ resumeUrl }), { status: 200 }) ); const replace = vi @@ -139,7 +132,7 @@ describe('resume repoint on save', () => { const store = useRemoteSaveStateStore(); store.setSaveUrl('/api/save'); - await store.saveState(); + await store.saveState(saveDependencies(post)); expect(replace).toHaveBeenCalledTimes(1); const nextUrl = new URL(replace.mock.calls[0][2] as string); @@ -149,7 +142,7 @@ describe('resume repoint on save', () => { }); it('leaves the tab as-is when the response carries no resumeUrl', async () => { - vi.mocked($fetch).mockResolvedValue( + post.mockResolvedValue( new Response(JSON.stringify({ ok: true }), { status: 200 }) ); const replace = vi @@ -158,7 +151,7 @@ describe('resume repoint on save', () => { const store = useRemoteSaveStateStore(); store.setSaveUrl(`${window.location.origin}/api/session/save`); - await store.saveState(); + await store.saveState(saveDependencies(post)); expect(replace).not.toHaveBeenCalled(); expect( diff --git a/src/store/__tests__/saveFixtures.ts b/src/store/__tests__/saveFixtures.ts new file mode 100644 index 000000000..09867614f --- /dev/null +++ b/src/store/__tests__/saveFixtures.ts @@ -0,0 +1,17 @@ +import { vi } from 'vitest'; +import type { SaveDependencies } from '@/src/store/remote-save-state'; + +/** Records the save egress in place of a network round-trip. */ +export const savePost = () => + vi.fn(async () => new Response(null, { status: 200 })); + +/** + * A save that skips the real zip: the archive's content is the serializer's + * contract, not this one's. + */ +export const saveDependencies = ( + post: ReturnType +): SaveDependencies => ({ + serializeSession: async () => new Blob(['x'], { type: 'application/zip' }), + post, +}); diff --git a/src/store/__tests__/segmentGroupDescriptorlessParity.spec.ts b/src/store/__tests__/segmentGroupDescriptorlessParity.spec.ts index 6f74aedb8..427b2c6e9 100644 --- a/src/store/__tests__/segmentGroupDescriptorlessParity.spec.ts +++ b/src/store/__tests__/segmentGroupDescriptorlessParity.spec.ts @@ -24,6 +24,7 @@ const ioMocks = vi.hoisted(() => ({ writeSegmentation: vi.fn(async () => new Uint8Array([1, 2, 3])), })); +// eslint-disable-next-line no-restricted-syntax -- ITK-wasm image IO has no counterpart in the node test environment vi.mock('@/src/io/readWriteImage', () => ({ readImage: ioMocks.readImage, writeSegmentation: ioMocks.writeSegmentation, diff --git a/src/store/__tests__/segmentGroupRestoreResilience.spec.ts b/src/store/__tests__/segmentGroupRestoreResilience.spec.ts index 8194967b9..1020001ad 100644 --- a/src/store/__tests__/segmentGroupRestoreResilience.spec.ts +++ b/src/store/__tests__/segmentGroupRestoreResilience.spec.ts @@ -27,6 +27,7 @@ const ioMocks = vi.hoisted(() => ({ writeSegmentation: vi.fn(async () => new Uint8Array([1, 2, 3])), })); +// eslint-disable-next-line no-restricted-syntax -- ITK-wasm image IO has no counterpart in the node test environment vi.mock('@/src/io/readWriteImage', () => ({ readImage: ioMocks.readImage, writeSegmentation: ioMocks.writeSegmentation, diff --git a/src/store/__tests__/views.spec.ts b/src/store/__tests__/views.spec.ts index e167b41d4..41d6057fb 100644 --- a/src/store/__tests__/views.spec.ts +++ b/src/store/__tests__/views.spec.ts @@ -1,17 +1,14 @@ -import { describe, it, beforeEach, expect, vi } from 'vitest'; +import { describe, it, beforeEach, expect } from 'vitest'; import { setActivePinia, createPinia } from 'pinia'; import { useViewStore } from '@/src/store/views'; import { computeEffectiveView } from '@/src/core/views/effectiveView'; +import { markCine } from '@/src/core/cine/__tests__/cineFixtures'; import type { Manifest } from '@/src/io/state-file/schema'; -vi.mock('@/src/core/cine/isCineImage', () => ({ - isCineImage: (imageID: string | null) => imageID === 'cine-image', - getCineImage: () => null, -})); - describe('View store', () => { beforeEach(() => { setActivePinia(createPinia()); + markCine('cine-image'); }); it('selects the first initial visible view', () => { diff --git a/src/store/remote-save-state.ts b/src/store/remote-save-state.ts index 486fd7ba3..745bc6900 100644 --- a/src/store/remote-save-state.ts +++ b/src/store/remote-save-state.ts @@ -6,6 +6,17 @@ import { repointLaunchUrls } from '@/src/utils/urlParams'; import { defineStore } from 'pinia'; import { ref } from 'vue'; +/** The serialize and network edges, so a save can be driven without either. */ +export type SaveDependencies = { + serializeSession: typeof serialize; + post: typeof $fetch; +}; + +export const appSaveDependencies = (): SaveDependencies => ({ + serializeSession: serialize, + post: $fetch, +}); + const useRemoteSaveStateStore = defineStore('remoteSaveState', () => { const saveUrl = ref(''); const isSaving = ref(false); @@ -29,13 +40,15 @@ const useRemoteSaveStateStore = defineStore('remoteSaveState', () => { saveUrl.value = url; }; - const saveState = async () => { + const saveState = async ( + dependencies: SaveDependencies = appSaveDependencies() + ) => { if (!saveUrl.value || isSaving.value) return; try { isSaving.value = true; - const blob = await serialize(); - const saveResult = await $fetch(saveUrl.value, { + const blob = await dependencies.serializeSession(); + const saveResult = await dependencies.post(saveUrl.value, { method: 'POST', headers: { 'Content-Type': 'application/zip', diff --git a/src/store/tools/__tests__/crosshairs.spec.ts b/src/store/tools/__tests__/crosshairs.spec.ts index 66c76fc2c..405408cec 100644 --- a/src/store/tools/__tests__/crosshairs.spec.ts +++ b/src/store/tools/__tests__/crosshairs.spec.ts @@ -1,4 +1,4 @@ -import { describe, it, beforeEach, expect, vi } from 'vitest'; +import { describe, it, beforeEach, expect } from 'vitest'; import { nextTick } from 'vue'; import { setActivePinia, createPinia } from 'pinia'; import vtkImageData from '@kitware/vtk.js/Common/DataModel/ImageData'; @@ -10,11 +10,6 @@ import type { ViewInfo2D } from '@/src/types/views'; import type { LPSAxis } from '@/src/types/lps'; import { useCrosshairsToolStore } from '@/src/store/tools/crosshairs'; -vi.mock('@/src/core/cine/isCineImage', () => ({ - isCineImage: () => false, - getCineImage: () => null, -})); - const SMALL: [number, number, number] = [10, 20, 30]; const LARGE: [number, number, number] = [40, 50, 60]; diff --git a/src/utils/__tests__/token.spec.ts b/src/utils/__tests__/token.spec.ts index 69631fc41..e7e9ac4ce 100644 --- a/src/utils/__tests__/token.spec.ts +++ b/src/utils/__tests__/token.spec.ts @@ -1,10 +1,4 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; - -const extractURLParameters = vi.fn(); -vi.mock('@kitware/vtk.js/Common/Core/URLExtract', () => ({ - default: { extractURLParameters: () => extractURLParameters() }, -})); - import { populateAuthorizationToken } from '@/src/utils/token'; import { globalHeaders, deleteGlobalHeader } from '@/src/utils/fetch'; @@ -26,9 +20,9 @@ describe('populateAuthorizationToken', () => { const bearer = () => globalHeaders.get('Authorization'); it('sets the bearer synchronously from token=', async () => { - extractURLParameters.mockReturnValue({ token: 'abc' }); + const urlParams = { token: 'abc' }; - await populateAuthorizationToken(); + await populateAuthorizationToken(urlParams); expect(bearer()).toBe('Bearer abc'); expect(fetchStub).not.toHaveBeenCalled(); @@ -37,23 +31,23 @@ describe('populateAuthorizationToken', () => { // Regression: the 2xx check was `status % 100 !== 2`, which takes the last // two digits — it rejected every ordinary 200 and accepted 202/302/502. it.each([200, 201, 204])('accepts a %i token response', async (status) => { - extractURLParameters.mockReturnValue({ + const urlParams = { tokenUrl: 'https://example.com/userToken', - }); + }; fetchStub.mockResolvedValue(new Response('tok', { status })); - await populateAuthorizationToken(); + await populateAuthorizationToken(urlParams); expect(bearer()).toBe('Bearer tok'); }); it.each([302, 404, 502])('rejects a %i token response', async (status) => { - extractURLParameters.mockReturnValue({ + const urlParams = { tokenUrl: 'https://example.com/userToken', - }); + }; fetchStub.mockResolvedValue(new Response('nope', { status })); - await populateAuthorizationToken(); + await populateAuthorizationToken(urlParams); expect(bearer()).toBeNull(); }); @@ -61,9 +55,9 @@ describe('populateAuthorizationToken', () => { it('resolves only after the tokenUrl bearer is set', async () => { // The caller awaits this before loading, so the first data request carries // the header rather than racing an un-awaited fetch. - extractURLParameters.mockReturnValue({ + const urlParams = { tokenUrl: 'https://example.com/userToken', - }); + }; let release: (r: Response) => void = () => {}; fetchStub.mockReturnValue( new Promise((resolve) => { @@ -71,7 +65,7 @@ describe('populateAuthorizationToken', () => { }) ); - const pending = populateAuthorizationToken(); + const pending = populateAuthorizationToken(urlParams); expect(bearer()).toBeNull(); release(new Response('late', { status: 200 })); @@ -81,13 +75,13 @@ describe('populateAuthorizationToken', () => { }); it('uses tokenUrlMethod when given', async () => { - extractURLParameters.mockReturnValue({ + const urlParams = { tokenUrl: 'https://example.com/userToken', tokenUrlMethod: 'POST', - }); + }; fetchStub.mockResolvedValue(new Response('tok', { status: 200 })); - await populateAuthorizationToken(); + await populateAuthorizationToken(urlParams); expect(fetchStub).toHaveBeenCalledWith('https://example.com/userToken', { method: 'POST', @@ -95,12 +89,14 @@ describe('populateAuthorizationToken', () => { }); it('continues without a bearer when the token fetch fails', async () => { - extractURLParameters.mockReturnValue({ + const urlParams = { tokenUrl: 'https://example.com/userToken', - }); + }; fetchStub.mockRejectedValue(new Error('network down')); - await expect(populateAuthorizationToken()).resolves.toBeUndefined(); + await expect( + populateAuthorizationToken(urlParams) + ).resolves.toBeUndefined(); expect(bearer()).toBeNull(); }); diff --git a/src/utils/token.ts b/src/utils/token.ts index 15be438f9..7ea6ced6e 100644 --- a/src/utils/token.ts +++ b/src/utils/token.ts @@ -14,9 +14,9 @@ export function stripTokenFromUrl() { // that lands after loading has started would leave the first requests // unauthenticated. A failure is non-fatal — the app continues without a bearer // and the data requests fail on their own terms. -export async function populateAuthorizationToken() { - const urlParams = vtkURLExtract.extractURLParameters() as UrlParams; - +export async function populateAuthorizationToken( + urlParams: UrlParams = vtkURLExtract.extractURLParameters() as UrlParams +) { if (urlParams.token) { setGlobalHeader('Authorization', `Bearer ${urlParams.token}`); } From 4564829951b393e6396846035b63daf1718d8113 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 18 Aug 2026 14:31:49 -0400 Subject: [PATCH 2/2] test(e2e): select an annotation once its widget pick resolves vtk.js only picks the widget under the cursor on its MouseMove event, and the interactor reports the first pointer move after ~200ms of stillness as StartMouseMove instead. A lone move therefore picks nothing, and any move clears the pick until an async render pass refills it, so a press dispatched right after one reads an empty pick and clears the selection. Hover by arriving from a nudge so the move that lands on the annotation is a MouseMove, wait for the view to show the hover cursor as proof the pick resolved, then press without moving so that pick still stands. Holding the pointer between chains needs a fixed action id and no release, as releasing resets it to the viewport origin. Retrying the click could not recover from this: on a runner where each iteration takes over 200ms every move is a StartMouseMove, which is why the macOS job failed while the same spec passed locally. --- tests/specs/annotationTestUtils.ts | 54 +++++++++++++++++++ tests/specs/delete-selected-annotation.e2e.ts | 17 +++--- 2 files changed, 61 insertions(+), 10 deletions(-) diff --git a/tests/specs/annotationTestUtils.ts b/tests/specs/annotationTestUtils.ts index 457f7a100..9aa2fb10c 100644 --- a/tests/specs/annotationTestUtils.ts +++ b/tests/specs/annotationTestUtils.ts @@ -11,6 +11,60 @@ export const moveTo = (x: number, y: number) => pointerAt(x, y).perform(); export const clickAt = (x: number, y: number) => pointerAt(x, y).down().up().perform(); +// One input source held across action chains, so a press can land where the last +// hover left the pointer. Chains that keep it perform without releasing actions, +// as releasing resets the pointer to the viewport origin. +const HOVERING_MOUSE = 'hovering-mouse'; +const hoveringMouse = () => browser.action('pointer', { id: HOVERING_MOUSE }); + +// vtk.js only picks the widget under the cursor on its MouseMove event, and its +// interactor reports the first pointer move after ~200ms of stillness as +// StartMouseMove, which picks nothing. Arriving from a nudge makes the move that +// lands on the target a MouseMove, so the pick runs. +const NUDGE_PX = 2; + +const nudgeTo = (x: number, y: number) => + hoveringMouse() + .move({ x: Math.round(x) + NUDGE_PX, y: Math.round(y) + NUDGE_PX }) + .move({ x: Math.round(x), y: Math.round(y) }) + .perform(true); + +// vtk.js sets the view's cursor from the pick it just resolved: the hover cursor +// when it found a widget, the default one when it found nothing. +const HOVER_CURSOR = 'pointer'; + +const viewCursor = async (view: ChainablePromiseElement) => { + const container = await view.$('div.view'); + const { value } = await container.getCSSProperty('cursor'); + return value; +}; + +/** + * Moves onto (x, y) and waits for the view to show the hover cursor, which says + * vtk.js resolved a pick and found a widget there. Leaves the pointer on the + * annotation so pressAtPointer can act on that pick. + */ +export const hoverUntilPicked = ( + view: ChainablePromiseElement, + x: number, + y: number +) => + browser.waitUntil( + async () => { + await nudgeTo(x, y); + return (await viewCursor(view)) === HOVER_CURSOR; + }, + { + timeout: 10000, + interval: 200, + timeoutMsg: `Hovering ${x},${y} should put the view in its hover cursor`, + } + ); + +// Pressing without moving keeps the pick hoverUntilPicked waited for: vtk.js +// drops it on every mouse move and only refills it a render pass later. +export const pressAtPointer = () => hoveringMouse().down().up().perform(true); + export const rightClickAt = (x: number, y: number) => pointerAt(x, y).down({ button: 2 }).up({ button: 2 }).perform(); diff --git a/tests/specs/delete-selected-annotation.e2e.ts b/tests/specs/delete-selected-annotation.e2e.ts index 8049dff73..6fe3bcb01 100644 --- a/tests/specs/delete-selected-annotation.e2e.ts +++ b/tests/specs/delete-selected-annotation.e2e.ts @@ -2,7 +2,8 @@ import { type ChainablePromiseElement } from 'webdriverio'; import AppPage from '../pageobjects/volview.page'; import { clickAt, - moveTo, + hoverUntilPicked, + pressAtPointer, setupTest, waitForCircleCount, } from './annotationTestUtils'; @@ -19,17 +20,13 @@ const clickToSelect = async ( y: number ) => { await AppPage.selectTool('mdi-cursor-default'); - // The widget manager resolves what is under the cursor from a render pass - // driven by mouse move, so hover first and retry until the click picks it up. - await moveTo(x, y); + await hoverUntilPicked(axialView, x, y); + await pressAtPointer(); + await browser.waitUntil( - async () => { - await clickAt(x, y); - return (await getSelectionRectCount(axialView)) === 1; - }, + async () => (await getSelectionRectCount(axialView)) === 1, { - timeout: 10000, - interval: 500, + timeout: 5000, timeoutMsg: 'Clicking the annotation should draw a selection rectangle', } );