diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index fcf2ebb5b..f7de41b8b 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -23,7 +23,7 @@ concurrency: jobs: playwright-tests: timeout-minutes: 120 - runs-on: self-hosted + runs-on: [self-hosted, nashua] strategy: fail-fast: false matrix: @@ -32,14 +32,6 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 with: persist-credentials: false - # Self-hosted runner hygiene: wipe any stale/corrupted pnpm self-install - # before action-setup reinstalls it. A prior `cache: pnpm` restore left a - # pnpm install missing dist/worker.js, breaking `pnpm install`. Plain shell - # so it runs even when pnpm itself is broken. Cheap: action-setup reinstalls. - - name: Reset stale pnpm self-install - shell: bash - run: rm -rf "$HOME/setup-pnpm" - - uses: pnpm/action-setup@0e279bb959325dab635dd2c09392533439d90093 # v6.0.8 - uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 with: node-version: ${{ matrix.node-version }} @@ -47,6 +39,19 @@ jobs: # pnpm store on local disk. The Actions cache doesn't preserve the # pnpm self-install's links faithfully, dropping pnpm/dist/worker.js # and breaking `pnpm install` with MODULE_NOT_FOUND. + # Install pnpm via Corepack instead of pnpm/action-setup. The action's + # self-installer is a JS action run under the runner's bundled + # externals/node{version} and derives the `npm` path from that runtime — which + # on this self-hosted runner is corrupted (Cannot find module + # '../lib/cli.js'), so it dies regardless of `standalone:true`. Corepack + # ships inside the Node that setup-node just installed, reads the pinned + # `packageManager` (pnpm@11.5.2) from package.json, and fetches pnpm via + # Node's own https — it never invokes the npm CLI. + - name: Enable Corepack (pnpm) + shell: bash + run: | + corepack enable + corepack prepare --activate # ── CS3D integration: detect label and ref type ────────────────────── - name: Check for CS3D integration label @@ -119,7 +124,12 @@ jobs: # ── Common: run tests ─────────────────────────────────────────────── - name: Install Playwright browsers - run: npx playwright install + # Only chromium is used (see playwright.config.ts and tests/globalSetup.ts); + # firefox/webkit projects are commented out. Scoping the install to chromium + # avoids downloading + dep-validating browsers we never launch (the WebKit + # validation is what surfaces the "missing libwoff1/libflite1/..." error on + # hosts without those libs). + run: npx playwright install chromium - name: Run Playwright tests run: | export NODE_OPTIONS="--max_old_space_size=10192" @@ -161,7 +171,8 @@ jobs: fi node .scripts/log-build-context.mjs env: - BUILD_TYPE: ${{ steps.cs3d-ref.outputs.type == 'branch' && 'ohif-downstream' || 'ohif-upstream' }} + BUILD_TYPE: + ${{ steps.cs3d-ref.outputs.type == 'branch' && 'ohif-downstream' || 'ohif-upstream' }} CS3D_REF_TYPE: ${{ steps.cs3d-ref.outputs.type }} - name: Build OHIF viewer (CS3D preview) @@ -203,7 +214,7 @@ jobs: # ── Separate job: block merge when using a CS3D branch ───────────── cs3d-branch-merge-guard: - name: "CS3D Branch Merge Guard" + name: 'CS3D Branch Merge Guard' runs-on: ubuntu-latest timeout-minutes: 5 steps: diff --git a/playwright.config.ts b/playwright.config.ts index 9b367fd7c..f0822fd8f 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -7,23 +7,22 @@ export default defineConfig({ forbidOnly: !!process.env.CI, retries: process.env.CI ? 1 : 0, maxFailures: process.env.CI ? 10 : undefined, - workers: process.env.CI ? 6 : undefined, + workers: process.env.CI ? 8 : undefined, snapshotPathTemplate: './tests/screenshots{/projectName}/{testFilePath}/{arg}{ext}', outputDir: './tests/test-results', - reporter: [ - ['html', { outputFolder: './tests/playwright-report' }], - ], + reporter: [['html', { outputFolder: './tests/playwright-report' }]], globalTimeout: 800_000, timeout: 800_000, use: { baseURL: 'http://localhost:3335', trace: 'on-first-retry', - video: 'on-first-retry', + video: 'retain-on-failure', testIdAttribute: 'data-cy', actionTimeout: 10_000, launchOptions: { // do not hide the scrollbars so that we can assert their look-and-feel ignoreDefaultArgs: ['--hide-scrollbars'], + args: ['--use-gl=egl'], }, }, diff --git a/tests/3DPrimary.spec.ts b/tests/3DPrimary.spec.ts index 5bd2e3f67..3687d7f94 100644 --- a/tests/3DPrimary.spec.ts +++ b/tests/3DPrimary.spec.ts @@ -24,10 +24,13 @@ test.describe('3D primary Test', async () => { await attemptAction(() => reduce3DViewportSize(page), 10, 100); await waitForViewportsRendered(page); - await checkForScreenshot( + await checkForScreenshot({ page, - viewportPageObject.grid, - screenShotPaths.threeDPrimary.threeDPrimaryDisplayedCorrectly - ); + locator: viewportPageObject.grid, + // Volume-3D ray-cast output is GPU/driver-noisy run-to-run; match the + // tolerance already used by the sibling 3DOnly test. + maxDiffPixelRatio: 0.03, + screenshotPath: screenShotPaths.threeDPrimary.threeDPrimaryDisplayedCorrectly, + }); }); }); diff --git a/tests/ArrowAnnotate.spec.ts b/tests/ArrowAnnotate.spec.ts index c18d70221..24c28fc8a 100644 --- a/tests/ArrowAnnotate.spec.ts +++ b/tests/ArrowAnnotate.spec.ts @@ -1,11 +1,52 @@ import { checkForScreenshot, + expect, + getAnnotationStats, screenShotPaths, test, visitStudy, waitForViewportRenderCycle, } from './utils'; +/** + * Asserts the arrow annotation text across every surface that should reflect it: + * - the tracked measurements side panel row title, + * - the DOM SVG linked text box rendered in the viewport, and + * - the source-of-truth cornerstone annotation state (`data.label`). + * + * This avoids relying on the screenshot alone (whose font rendering differs + * between systems) to assert the text is correct, mirroring the DOM/state + * assertions in the SCOORD rectangle test. + */ +async function expectArrowText({ + page, + activeViewport, + rightPanelPageObject, + annotationUID, + expectedText, +}) { + // Side panel: the measurement row title reflects the arrow text. + await expect(rightPanelPageObject.measurementsPanel.panel.nthMeasurement(0).title).toHaveText( + expectedText + ); + + // DOM SVG: the arrow's linked text box renders the arrow text, independent of + // any system font differences. + const svgTextLines = activeViewport.getSvgAnnotationStatTextLines(annotationUID); + await expect(svgTextLines).toHaveCount(1); + await expect(svgTextLines.nth(0)).toHaveText(expectedText); + + // Source-of-truth annotation state. ArrowAnnotate stores its text on + // `data.label` rather than computed `cachedStats`, so read with requireStats: false. + const arrows = await getAnnotationStats(page, { + toolName: 'ArrowAnnotate', + requireStats: false, + }); + const arrow = arrows.find(a => a.annotationUID === annotationUID); + expect(arrow).toBeDefined(); + expect(arrow.label).toBe(expectedText); +} + test.beforeEach(async ({ page }) => { const studyInstanceUID = '1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5'; const mode = 'viewer'; @@ -41,11 +82,28 @@ test('should display the arrow tool and allow free-form text to be entered', asy await checkForScreenshot({ page, - maxDiffPixelRatio: 0.0075, screenshotPath: screenShotPaths.arrowAnnotate.arrowAnnotateDisplayedCorrectly0, }); - // Now edit the arrow text and the label should not change. + // Resolve the arrow annotation UID once; it remains stable across subsequent edits. + const arrows = await getAnnotationStats(page, { + toolName: 'ArrowAnnotate', + requireStats: false, + }); + expect(arrows.length).toBeGreaterThan(0); + const annotationUID = arrows[0].annotationUID; + + await expectArrowText({ + page, + activeViewport, + rightPanelPageObject, + annotationUID, + expectedText: 'Ringo Starr was the drummer for The Beatles', + }); + + // Double-clicking the arrow re-opens the text dialog. ArrowAnnotate stores its + // text on `data.label`, so the new text replaces it across the viewport SVG, + // the side panel and the annotation state. await activeViewport.doubleClickAt({ x: 164, y: 234 }); @@ -53,11 +111,19 @@ test('should display the arrow tool and allow free-form text to be entered', asy await checkForScreenshot({ page, - maxDiffPixelRatio: 0.0075, screenshotPath: screenShotPaths.arrowAnnotate.arrowAnnotateDisplayedCorrectly1, }); - // Now edit the label and the text should not change. + await expectArrowText({ + page, + activeViewport, + rightPanelPageObject, + annotationUID, + expectedText: 'Neil Peart was the drummer for Rush', + }); + + // Renaming from the side panel updates the same `data.label`, so the new text + // is reflected everywhere as well. await rightPanelPageObject.measurementsPanel.panel .nthMeasurement(0) @@ -65,7 +131,14 @@ test('should display the arrow tool and allow free-form text to be entered', asy await checkForScreenshot({ page, - maxDiffPixelRatio: 0.0075, screenshotPath: screenShotPaths.arrowAnnotate.arrowAnnotateDisplayedCorrectly2, }); + + await expectArrowText({ + page, + activeViewport, + rightPanelPageObject, + annotationUID, + expectedText: 'Drummer annotation arrow', + }); }); diff --git a/tests/RTDataOverlayNoHydrationThenMPR.spec.ts b/tests/RTDataOverlayNoHydrationThenMPR.spec.ts index 04e3a4010..dce0be8eb 100644 --- a/tests/RTDataOverlayNoHydrationThenMPR.spec.ts +++ b/tests/RTDataOverlayNoHydrationThenMPR.spec.ts @@ -24,7 +24,6 @@ test('should launch MPR with unhydrated RTSTRUCT chosen from the data overlay me const dataOverlayPageObject = (await viewportPageObject.getById('default')).overlayMenu .dataOverlay; await dataOverlayPageObject.toggle(); - const viewportRenderCycle = waitForViewportRenderCycle(page); await dataOverlayPageObject.addSegmentation('ARIA RadOnc Structure Sets'); // Adding an overlay should not show the LOAD button. @@ -32,8 +31,7 @@ test('should launch MPR with unhydrated RTSTRUCT chosen from the data overlay me // Hide the overlay menu. await dataOverlayPageObject.toggle(); - - await viewportRenderCycle; + await waitForViewportsRendered(page); await checkForScreenshot( page, @@ -43,8 +41,8 @@ test('should launch MPR with unhydrated RTSTRUCT chosen from the data overlay me await mainToolbarPageObject.layoutSelection.MPR.click(); - await waitForViewportsRendered(page, { timeout: 40000 }); - + // Waiting for the render via waitForViewportRenderCycle appears to wait 'forever', so instead + // we use the baked in wait for screenshot comparisons. await checkForScreenshot( page, viewportPageObject.grid, diff --git a/tests/SEGHydrationFrom3DFourUp.spec.ts b/tests/SEGHydrationFrom3DFourUp.spec.ts index 3456e09cc..ce232397a 100644 --- a/tests/SEGHydrationFrom3DFourUp.spec.ts +++ b/tests/SEGHydrationFrom3DFourUp.spec.ts @@ -5,7 +5,6 @@ import { screenShotPaths, test, visitStudy, - waitForPaintToSettle, waitForViewportsRendered, waitForViewportRenderCycle, expect, @@ -44,9 +43,6 @@ test.describe('3D four up SEG hydration', async () => { await expect(DOMOverlayPageObject.viewport.segmentationHydration.locator).toBeVisible({ timeout: 60000, }); - await waitForViewportsRendered(page, { timeout: 60000 }); - await page.waitForTimeout(3000); - await waitForPaintToSettle(page); await checkForScreenshot( page, @@ -58,18 +54,12 @@ test.describe('3D four up SEG hydration', async () => { // High rendered timeout needed: layout has 4 viewports (3D volume + MPR planes + SEG overlays), // which can take significantly longer time to fully render - viewportRenderCycle = waitForViewportRenderCycle(page, { renderedTimeout: 180000 }); + viewportRenderCycle = waitForViewportRenderCycle(page, { renderedTimeout: 240000 }); await DOMOverlayPageObject.viewport.segmentationHydration.yes.click(); // Wait until all viewports have finished rendering await viewportRenderCycle; - // 3D volume rendering keeps streaming refined geometry after the load - // status flips; give the GPU a window to present the final frame before - // screenshotting. - await waitForViewportsRendered(page, { timeout: 180000 }); - await page.waitForTimeout(3000); - await waitForPaintToSettle(page); await checkForScreenshot({ page, @@ -92,12 +82,15 @@ test.describe('3D four up to 3x2 layout SEG hydration', () => { mainToolbarPageObject, viewportPageObject, }) => { + let viewportRenderCycle = waitForViewportRenderCycle(page, { renderedTimeout: 240000 }); await mainToolbarPageObject.layoutSelection.threeDFourUp.click(); + await viewportRenderCycle; - let viewportRenderCycle = waitForViewportRenderCycle(page); // Switch to a manual 3x2 grid layout await mainToolbarPageObject.layoutSelection.grid(3, 2).click(); - await viewportRenderCycle; + + // Wait for the 3x2 grid to be rendered. + await expect(viewportPageObject.getNthLocator(5)).toBeVisible(); // Activate the 3rd viewport (index 2) then load the SEG into it await viewportPageObject.getNthLocator(2).click(); diff --git a/tests/screenshots/chromium/3DPrimary.spec.ts/threeDPrimaryDisplayedCorrectly.png b/tests/screenshots/chromium/3DPrimary.spec.ts/threeDPrimaryDisplayedCorrectly.png index 71407494f..2191ed61f 100644 Binary files a/tests/screenshots/chromium/3DPrimary.spec.ts/threeDPrimaryDisplayedCorrectly.png and b/tests/screenshots/chromium/3DPrimary.spec.ts/threeDPrimaryDisplayedCorrectly.png differ diff --git a/tests/utils/getAnnotationStats.ts b/tests/utils/getAnnotationStats.ts index ad948989b..b97632991 100644 --- a/tests/utils/getAnnotationStats.ts +++ b/tests/utils/getAnnotationStats.ts @@ -5,6 +5,8 @@ type AnnotationStats = { toolName: string; referencedImageId?: string; FrameOfReferenceUID?: string; + label?: string; + text?: string; cachedStats: Record; targetIds: string[]; firstTargetStats: TargetStats | undefined; @@ -14,10 +16,17 @@ type Options = { toolName?: string | string[]; attempts?: number; intervalMs?: number; + /** + * When true (default), only resolves once at least one matching annotation has + * populated `cachedStats`. Set to false for tools whose source of truth is + * `data.label`/`data.text` rather than computed stats (e.g. `ArrowAnnotate`), + * in which case any matching annotation is returned. + */ + requireStats?: boolean; }; const getAnnotationStats = async (page, options: Options = {}): Promise => { - const { toolName, attempts = 20, intervalMs = 500 } = options; + const { toolName, attempts = 20, intervalMs = 500, requireStats = true } = options; const toolNames = toolName === undefined ? undefined : Array.isArray(toolName) ? toolName : [toolName]; @@ -38,6 +47,8 @@ const getAnnotationStats = async (page, options: Options = {}): Promise r.targetIds.length > 0); - if (populated.length > 0) { - return populated; + const matching = requireStats ? results.filter(r => r.targetIds.length > 0) : results; + if (matching.length > 0) { + return matching; } await new Promise(resolve => setTimeout(resolve, intervalMs)); } throw new Error( - `No annotations${toolNames ? ` for tool(s) ${toolNames.join(',')}` : ''} with populated cachedStats found` + `No annotations${toolNames ? ` for tool(s) ${toolNames.join(',')}` : ''}${ + requireStats ? ' with populated cachedStats' : '' + } found` ); };