fix: ohif tests to run with cornerstone 3d 5.0 (#6043)

* chore(tests): Update multiple screenshot test images for various specs

* feat(screenshot-reviewer): Add screenshot review tool and update package.json scripts

* fix(DICOMSRDisplayTool): Improve actor presence check in viewport

* chore(tests): Update multiple screenshot assets for various specs

* chore(tests): Integrate waitForPaintToSettle and waitForViewportsRendered in multiple specs for improved rendering stability

* chore(tests): Update screenshot assets for SEGHydration and SEGNoHydration specs

* test: update progressive loading screenshots

* jest 30 test fixes for compatibility with pnpm cs3d

* Use correct setDisplaySets instead of setDataId

* fix: Naming change for LegacyVolumeViewport3D

* Update to allow tolerance for contour tests

* update

* fix

* refactor: Replace instanceof checks with utility functions for viewport type validation

* fix: Update createSegmentationForViewport to handle undefined displaySetInstanceUID gracefully

* bun lock

* fix: Install cs3d with pnpm instead of bun

* Update node version for playwright

* Update to v5.0.0 of cs3d

* fix: Build dependency

* audit

* Change to a web await retry assert

* Fix timing related test failures

* fix: Freehand close

---------

Co-authored-by: Bill Wallace <wayfarer3130@gmail.com>

platform/core/package.json

@@ -37,8 +37,9 @@
     "@cornerstonejs/codec-libjpeg-turbo-8bit": "1.2.2",
     "@cornerstonejs/codec-openjpeg": "1.3.0",
     "@cornerstonejs/codec-openjph": "2.4.7",
-    "@cornerstonejs/core": "4.22.10",
-    "@cornerstonejs/dicom-image-loader": "4.22.10",
+    "@cornerstonejs/core": "5.0.0",
+    "@cornerstonejs/dicom-image-loader": "5.0.0",
+    "@cornerstonejs/metadata": "5.0.0",
     "@ohif/ui": "3.13.0-beta.88",
     "cornerstone-math": "0.1.10",
     "dicom-parser": "1.8.21"

platform/core/src/types/AppTypes.ts

@@ -95,6 +95,14 @@ declare global {
       useNorm16Texture?: boolean;
       useCPURendering?: boolean;
       strictZSpacingForVolumeViewport?: boolean;
+      /**
+       * Routes legacy cornerstone viewport types (STACK / ORTHOGRAPHIC / VOLUME_3D / etc.)
+       * through the new GenericViewport-backed render paths while preserving the legacy
+       * public API via compatibility adapters. Maps to cornerstone
+       * config.rendering.useGenericViewport. Defaults to false (legacy classes).
+       * Requires @cornerstonejs/core >= 5.0.0-beta (GenericViewport architecture).
+       */
+      useGenericViewport?: boolean;
       useCursors?: boolean;
       maxCacheSize?: number;
       max3DTextureSize?: number;

platform/core/src/utils/Queue.test.js

@@ -30,7 +30,7 @@ describe('Queue', () => {
   //   });
   //   const end = await timer(threshold);
   //   expect(end - start >= 2 * threshold).toBe(true);
-  //   expect(mockedTimeout).toBeCalledTimes(2);
+  //   expect(mockedTimeout).toHaveBeenCalledTimes(2);
   // });
   it('should prevent task execution when queue limit is reached', async () => {
     const queue = new Queue(1);
@@ -46,7 +46,7 @@ describe('Queue', () => {
     }
     const elapsed = await promise;
     expect(elapsed >= threshold && elapsed < 2 * threshold).toBe(true);
-    expect(mockedTimeout).toBeCalledTimes(1);
+    expect(mockedTimeout).toHaveBeenCalledTimes(1);
   });
   it('should safely bind tasks to the queue', async () => {
     const queue = new Queue(1);
@@ -57,13 +57,13 @@ describe('Queue', () => {
     const promise = timer(threshold).then(time => time - start);
     await timer(threshold);
     expect(Date.now() - start < threshold).toBe(true);
-    expect(mockedErrorHandler).toBeCalledTimes(1);
-    expect(mockedErrorHandler).nthCalledWith(
+    expect(mockedErrorHandler).toHaveBeenCalledTimes(1);
+    expect(mockedErrorHandler).toHaveBeenNthCalledWith(
       1,
       expect.objectContaining({ message: 'Queue limit reached' })
     );
     const elapsed = await promise;
     expect(elapsed >= threshold && elapsed < 2 * threshold).toBe(true);
-    expect(mockedTimeout).toBeCalledTimes(1);
+    expect(mockedTimeout).toHaveBeenCalledTimes(1);
   });
 });

platform/core/src/utils/absoluteUrl.test.js

@@ -1,17 +1,24 @@
+/**
+ * @jest-environment node
+ *
+ * Runs in node (not jsdom): jsdom (jest 30) exposes `window.location` as an
+ * unforgeable, non-configurable property that can't be replaced or stubbed.
+ * absoluteUrl only reads `window.location.origin` and this suite imports nothing
+ * that needs the DOM, so we run without jsdom and provide a plain `window`.
+ */
 import absoluteUrl from './absoluteUrl';
 
 describe('absoluteUrl', () => {
+  const setOrigin = url => {
+    global.window = { location: { origin: url } };
+  };
+
+  afterEach(() => {
+    delete global.window;
+  });
+
   test('should return /path_1/path_2/path_3/path_to_destination when the window.location.origin is http://dummy.com/path_1/path_2 and the path is /path_3/path_to_destination', () => {
-    let global = {
-      window: Object.create(window),
-    };
-    const url = 'http://dummy.com/path_1/path_2';
-    Object.defineProperty(window, 'location', {
-      value: {
-        origin: url,
-      },
-      writable: true,
-    });
+    setOrigin('http://dummy.com/path_1/path_2');
     const absoluteUrlOutput = absoluteUrl('/path_3/path_to_destination');
     expect(absoluteUrlOutput).toEqual('/path_1/path_2/path_3/path_to_destination');
   });
@@ -22,22 +29,13 @@ describe('absoluteUrl', () => {
   });
 
   test('should return the original path when there path in the window.origin after the domain and port', () => {
-    delete global.window.location;
-    const url = 'http://dummy.com';
-    global.window.location = {
-      origin: url,
-    };
+    setOrigin('http://dummy.com');
     const absoluteUrlOutput = absoluteUrl('path_1/path_2/path_3');
     expect(absoluteUrlOutput).toEqual('/path_1/path_2/path_3');
   });
 
   test('should be able to return the absolute path even when the path contains duplicates', () => {
-    global.window ||= Object.create(window);
-    const url = 'http://dummy.com';
-    delete global.window.location;
-    global.window.location = {
-      origin: url,
-    };
+    setOrigin('http://dummy.com');
     const absoluteUrlOutput = absoluteUrl('path_1/path_1/path_1');
     expect(absoluteUrlOutput).toEqual('/path_1/path_1/path_1');
   });

platform/core/src/utils/addServer.test.js

@@ -31,7 +31,7 @@ describe('addServers', () => {
 
   test('should be able to add a server and dispatch to the store successfuly', () => {
     addServers(servers, store);
-    expect(store.dispatch).toBeCalledWith({
+    expect(store.dispatch).toHaveBeenCalledWith({
       server: {
         authority: 'http://127.0.0.1/auth/realms/ohif',
         client_id: 'ohif-viewer',
@@ -43,7 +43,7 @@ describe('addServers', () => {
       },
       type: 'ADD_SERVER',
     });
-    expect(store.dispatch).toBeCalledWith({
+    expect(store.dispatch).toHaveBeenCalledWith({
       server: {
         imageRendering: 'wadors',
         name: 'DCM4CHEE',
@@ -59,19 +59,19 @@ describe('addServers', () => {
   });
 
   test('should throw an error if servers list is not defined', () => {
-    expect(() => addServers(undefined, store)).toThrowError(
+    expect(() => addServers(undefined, store)).toThrow(
       new Error('The servers and store must be defined')
     );
   });
 
   test('should throw an error if store is not defined', () => {
-    expect(() => addServers(servers, undefined)).toThrowError(
+    expect(() => addServers(servers, undefined)).toThrow(
       new Error('The servers and store must be defined')
     );
   });
 
   test('should throw an error when both server and store are not defined', () => {
-    expect(() => addServers(undefined, undefined)).toThrowError(
+    expect(() => addServers(undefined, undefined)).toThrow(
       new Error('The servers and store must be defined')
     );
   });

platform/core/src/utils/hierarchicalListUtils.test.js

@@ -69,12 +69,12 @@ describe('hierarchicalListUtils', function () {
       const fn = jest.fn();
       forEach(sharedList, fn);
       expect(fn).toHaveBeenCalledTimes(6);
-      expect(fn).nthCalledWith(1, '1.2.3.1', '1.2.3.1.1');
-      expect(fn).nthCalledWith(2, '1.2.3.1', '1.2.3.1.2');
-      expect(fn).nthCalledWith(3, '1.2.3.2');
-      expect(fn).nthCalledWith(4, '1.2.3.3', '1.2.3.3.1');
-      expect(fn).nthCalledWith(5, '1.2.3.3', '1.2.3.3.2', '1.2.3.3.2.1');
-      expect(fn).nthCalledWith(6, '1.2.3.3', '1.2.3.3.2', '1.2.3.3.2.2');
+      expect(fn).toHaveBeenNthCalledWith(1, '1.2.3.1', '1.2.3.1.1');
+      expect(fn).toHaveBeenNthCalledWith(2, '1.2.3.1', '1.2.3.1.2');
+      expect(fn).toHaveBeenNthCalledWith(3, '1.2.3.2');
+      expect(fn).toHaveBeenNthCalledWith(4, '1.2.3.3', '1.2.3.3.1');
+      expect(fn).toHaveBeenNthCalledWith(5, '1.2.3.3', '1.2.3.3.2', '1.2.3.3.2.1');
+      expect(fn).toHaveBeenNthCalledWith(6, '1.2.3.3', '1.2.3.3.2', '1.2.3.3.2.2');
     });
   });
 

platform/core/src/utils/progressTrackingUtils.test.js

@@ -43,7 +43,7 @@ describe('progressTrackingUtils', () => {
       const promises = [Promise.resolve('A'), Promise.resolve('B'), Promise.resolve('C')];
       promises.forEach(promise => void utils.waitOn(list, promise));
       return Promise.all(promises).then(() => {
-        expect(observer).toBeCalledTimes(6);
+        expect(observer).toHaveBeenCalledTimes(6);
         [
           {
             failures: 0,
@@ -83,7 +83,7 @@ describe('progressTrackingUtils', () => {
           },
         ].forEach((item, i) => {
           const result = expect.objectContaining(item);
-          expect(observer).nthCalledWith(i + 1, result, list);
+          expect(observer).toHaveBeenNthCalledWith(i + 1, result, list);
         });
         expect(utils.getOverallProgress(list)).toStrictEqual({
           failures: 0,
@@ -102,11 +102,11 @@ describe('progressTrackingUtils', () => {
       download.deferred.resolve(fakeRequest(update));
       utils.waitOn(list, processing);
       return processing.then(() => {
-        expect(update).toBeCalledTimes(4);
+        expect(update).toHaveBeenCalledTimes(4);
         [0.25, 0.5, 0.75, 1.0].forEach(
-          (value, i) => void expect(update).nthCalledWith(i + 1, value)
+          (value, i) => void expect(update).toHaveBeenNthCalledWith(i + 1, value)
         );
-        expect(observer).toBeCalledTimes(7);
+        expect(observer).toHaveBeenCalledTimes(7);
         [
           {
             failures: 0,
@@ -152,7 +152,7 @@ describe('progressTrackingUtils', () => {
           },
         ].forEach((item, i) => {
           const result = expect.objectContaining(item);
-          expect(observer).nthCalledWith(i + 1, result, list);
+          expect(observer).toHaveBeenNthCalledWith(i + 1, result, list);
         });
       });
     });