# Ohif Test Agent

> Generate runnable Playwright E2E tests for the OHIF Viewer using its custom fixture system, page objects, and normalized WebGL viewport coordinates. Use this skill whenever the user asks to write, add, modify, or debug tests in an OHIF/Viewers context — including vague asks like "write a test for X" when working in the OHIF repo, tests touching platform/app/tests/, or anything involving Cornerstone viewports, DICOM studies, measurements, segmentations, or OHIF modes/extensions. Prefer this skill over generic test-writing even if the user doesn't say "Playwright" or "E2E" explicitly.

- Skill: `ohif/ohif-test-agent` (Agent Skill, multi-file: 6 files)
- Install (CLI): `npx skillmds@latest add ohif/ohif-test-agent`
- Raw SKILL.md: https://api.skillmd.com/api/skills/ohif/ohif-test-agent/raw
- Safety review: pending (external: skill-scanner PASS, skillspector CAUTION)
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: AI & ML
- Author: ohif (https://skillmd.com/u/ohif)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/ohif/ohif-test-agent

---


# OHIF Test Agent

This skill teaches you to generate correct, runnable Playwright end-to-end tests for the OHIF Viewer. Follow the workflow below.

## Environment model

This package follows the agentskills.io SKILL.md convention. `SKILL.md` is the entire behavior contract — there is no separate runtime entrypoint.

## Workflow: how to write a new OHIF test

1. **Classify the feature.** What area does the test belong to — a measurement tool, segmentation hydration, contour panel interaction, MPR layout, crosshairs, tag browser, etc.? The area determines the mode, the StudyInstanceUID, and the seed spec you'll read.
2. **Read the seed spec.** Consult [references/patterns-by-feature.md](references/patterns-by-feature.md) to find the canonical existing spec for that area. Read it end-to-end before writing. This is the single most important step — OHIF specs follow consistent idioms that are easier to mimic than to reconstruct from first principles. (This mirrors Playwright's own agent guidance: use seed tests as the example for generated tests.)
3. **Scaffold from the template.** Start from [assets/spec-template.ts](assets/spec-template.ts) — or copy the seed spec and adapt.
4. **Look up specifics in the source, not from memory.** The reference files [page-objects.md](references/page-objects.md) and [utilities.md](references/utilities.md) capture the **stable rules** — fixture keys, import conventions, access idioms, the reasons certain things trip people up. They deliberately do not enumerate methods. For the current method surface or a utility's exact signature, open the relevant file under `tests/pages/` or `tests/utils/` — the source evolves, and the source is always right. The seed spec you picked in step 2 is usually the fastest second source, because it co-evolves with the API.
5. **Run the test when execution is available.** `pnpm run test:e2e:ci` runs the whole suite, but for iteration use `TEST_ENV=true pnpm exec playwright test tests/YourNew.spec.ts` (or the Playwright VS Code extension). Invoke Playwright directly for targeted flags; `pnpm run test:e2e -- ...` inserts a `--` separator that can prevent Playwright from parsing options such as `--update-snapshots` and `--reporter`.
6. **If runtime execution is unavailable, do static validation.** Validate import source, fixture keys, normalized viewport usage, UID/mode pairing, and hydration/tracking prompt handling. Then report clearly that execution was not performed.
7. **If it fails, triage before debugging.** Use [references/failure-triage.md](references/failure-triage.md) — most OHIF test failures are timing / hydration, not real regressions.

## Architecture

OHIF uses Playwright with a custom fixture system. Tests are **not** vanilla Playwright — they import `test`, `expect`, and utilities from `./utils`, which re-exports an extended test runner that injects page objects.

```text
playwright.config.ts            → Chromium-only, port 3335, data-cy as testId
  └─ tests/utils/fixture.ts     → Extends playwright-test-coverage, injects page objects
       └─ tests/*.spec.ts       → Each imports { test, expect, ... } from './utils'
            ├─ tests/pages/     → Page objects (ViewportPageObject, MainToolbarPageObject, …)
            └─ tests/utils/     → Utilities (visitStudy, checkForScreenshot, screenShotPaths, …)
```

Why the custom fixture matters: the page objects are created for each test and bound to the right Playwright `page`. If you `new ViewportPageObject(page)` manually, you skip the fixture wiring and some sub-objects won't resolve correctly.

### Import rule

```ts
// Correct
import { test, expect, visitStudy, checkForScreenshot, screenShotPaths } from './utils';

// Wrong — will compile but fixtures won't be injected
import { test, expect } from '@playwright/test';
```

A few utilities (`press`, `downloadAsString`, the `assert*` helpers) are NOT re-exported from `./utils`. See [references/utilities.md](references/utilities.md) for the correct import path per utility.

## The viewport is WebGL

OHIF renders medical images onto a WebGL canvas. You cannot query *canvas* pixels by CSS selector. Use **normalized coordinates** (0–1 range, top-left is `{x:0, y:0}`) for clicks and drags, and **visual regression** (screenshot comparison) for canvas assertions. (Not everything in the viewport is canvas — some overlays render as SVG you *can* query via DOM, e.g. a vector overlay's color through `getSvgAttribute`. The canvas rule is about raster output painted onto the WebGL surface.)

```ts
const activeViewport = await viewportPageObject.active;

await activeViewport.normalizedClickAt([{ x: 0.5, y: 0.5 }]);                 // click center
await activeViewport.normalizedClickAt([{ x: 0.3, y: 0.3 }, { x: 0.7, y: 0.7 }]); // draw two points
await activeViewport.normalizedClickAt([{ x: 0.5, y: 0.5 }], 'right');         // right-click
await activeViewport.normalizedDragAt({
  start: { x: 0.3, y: 0.3 },
  end:   { x: 0.7, y: 0.7 },
});
```

Pixel coordinates (`clickAt`, `doubleClickAt`) exist but prefer normalized for portability across viewport sizes.

For DOM-rendered state (panel counts, dialog text, overlay text values, button enabled states), assert directly:

```ts
await expect(activeViewport.overlayText.bottomRight.instanceNumber).toContainText('17/');
const count = await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount();
expect(count).toBe(1);
```

## Study loading lifecycle

Every test follows this sequence. Skipping steps causes flakiness:

```ts
test.beforeEach(async ({ page }) => {
  const studyInstanceUID = '1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5';
  const mode = 'viewer';
  await visitStudy(page, studyInstanceUID, mode, 2000); // 2s delay is the community norm
});
```

`visitStudy` navigates to `/{mode}/ohif?StudyInstanceUIDs={uid}`, waits for `domcontentloaded`, then `networkidle`, then the explicit delay. Default delay is `0`, but most specs pass `2000` to let the first render settle.

Delay by scene type (observed across the current suite, not a rule to apply blindly):

| Scene | Delay |
|-------|-------|
| Default (viewer mode, 2D/MPR/3D layouts, crosshairs) | `2000` |
| `mode: 'tmtv'` | `10000` — PET fusion + SUV calculation takes noticeably longer |

Start at the convention for your scene; ramp only if the test flakes on initial render. 3D layouts in `viewer` mode already stay at `2000` — the stabilization problem there is solved with `attemptAction(() => reduce3DViewportSize(page), 10, 100)`, not with a longer `visitStudy` delay.

If the study has DICOM SEG, RT, or SR data, OHIF asks whether to hydrate. Handle it:

```ts
await leftPanelPageObject.loadSeriesByModality('SEG'); // or 'RTSTRUCT', 'SR'
await page.waitForTimeout(3000);                        // allow the prompt to appear
await expect(DOMOverlayPageObject.viewport.segmentationHydration.locator).toBeVisible();
await DOMOverlayPageObject.viewport.segmentationHydration.yes.click();
```

The first measurement you create triggers a "start tracking?" prompt:

```ts
await DOMOverlayPageObject.viewport.measurementTracking.confirm.click();
```

For 3D / MPR scenes, wrap stabilization in `attemptAction(() => reduce3DViewportSize(page), 10, 100)` or insert a `page.waitForTimeout(...)` after the layout change before asserting.

## Wait for renders, don't sleep

`page.waitForTimeout(...)` after an action that re-renders the viewport is a smell. The viewports tell us when they're done — use that signal. `tests/utils/waitForViewportsRendered.ts` exposes three helpers, all barrel-exported from `./utils`:

- `waitForViewportRenderCycle(page)` — wait for the next full cycle: a viewport enters `needsRender`, then **all** viewports report `rendered` (and volumes are loaded, by default).
- `waitForViewportsRendered(page)` — only the second half: wait until all viewports are `rendered`. Use this when the action has already requested a render before you started waiting (e.g. a layout change or `loadSeriesByDescription`).
- `waitForAnyViewportNeedsRender(page)` — only the first half. Rarely needed directly.

The canonical idiom — **start the watcher before the action, await it after**:

```ts
// start watching for the next render cycle
const viewportRenderCycle = waitForViewportRenderCycle(page);

await action(); // e.g. segmentationHydration.yes.click(), layoutSelection.MPR.click(), addSegmentation, etc.

// wait for the render to finish
await viewportRenderCycle;

await check(); // e.g. checkForScreenshot, count assertion, overlay text
```

Why "start before"? `waitForViewportRenderCycle` first waits for a viewport to enter `needsRender`. If you start it **after** the action, that transition may already be over and you'll hang until the timeout. Starting it first captures the cycle the action is about to trigger.

When to use which:

| Situation | Helper |
|-----------|--------|
| Click that triggers a re-render and you want to assert after | `waitForViewportRenderCycle(page)` started before the click |
| Layout switch / series load — render already in flight | `await waitForViewportsRendered(page)` after the call |
| Compose with another await (e.g. screenshot the same time as load) | Save the promise, `await` it later |

Replace patterns like this:

```ts
// ❌ Sleep-and-pray
await action();
await page.waitForTimeout(5000);
await checkForScreenshot(...);

// ✅ Wait on the actual signal
const cycle = waitForViewportRenderCycle(page);
await action();
await cycle;
await checkForScreenshot(...);
```

This shaves real wall-clock time off the suite and removes a class of flake (sleep too short → flake; sleep too long → slow). `tests/SEGHydrationFromMPR.spec.ts` is the canonical seed for this pattern.

Caveats:
- These helpers wait on Cornerstone viewport state. They won't help for purely DOM-side state (panel rows appearing, dialogs opening) — for those, prefer `expect(locator).toHaveCount(n)` / `toBeVisible()` which auto-retry, or `expect.toPass({ timeout })`.
- For some actions (hanging-protocol changes are the documented example) the viewport doesn't transition through `needsRender` synchronously — those still need a short `waitForTimeout`. The source comment in `waitForViewportsRendered.ts` calls this out.

## Fixture-injected page objects

Destructure these from the test function argument. **Never `new` them manually.**

```ts
test('my test', async ({
  page,
  viewportPageObject,
  mainToolbarPageObject,
  leftPanelPageObject,
  rightPanelPageObject,
  DOMOverlayPageObject,          // note the capital D — this matches the fixture key
  notFoundStudyPageObject,
}) => { ... });
```

Two page objects are **not** fixture-injected:
- `DataOverlayPageObject` — reach via `viewportPageObject.getById(id).overlayMenu.dataOverlay`.
- `DicomTagBrowserPageObject` — reach via `DOMOverlayPageObject.dialog.dicomTagBrowser`.

See [references/page-objects.md](references/page-objects.md) for fixture rules and a map of which file covers which concern; read the `.ts` file under `tests/pages/` for the current method surface.

## When the control you need has no page object yet

A spec must not reach for `page.getByTestId(...)` / `getByRole(...)` directly for
application controls. If the button, menu, dialog, or field you need isn't already
exposed by a page object, **add it to one — or create a new page object — instead of
inlining a raw selector.** Raw selectors in a spec are the clearest sign a test was
written without reading the existing suite: they duplicate locators, bypass the
fixture wiring, and rot silently when the DOM changes.

Where new coverage goes:

| What you need | Where it belongs |
|---|---|
| A toolbar button or tool (Zoom, Pan) | a getter on `MainToolbarPageObject`, next to `crosshairs` / `measurementTools` |
| A menu, prompt, context menu, or small dialog | `DOMOverlayPageObject` |
| A substantial dialog with its own fields (User Preferences) | its **own** page object class, reached through `DOMOverlayPageObject` — follow `DicomTagBrowserPageObject` (`DOMOverlayPageObject.dialog.dicomTagBrowser`) |
| Each field/row inside that dialog | a method or sub-object on the dialog's page object — not a raw selector in the spec |
| A side-panel control | `LeftPanelPageObject` / `RightPanelPageObject` |

If the control has no `data-cy`, **add `data-cy` to the source component** and target
it — don't fall back to `getByRole`/text selectors, which are brittle and
locale-sensitive (`testIdAttribute` is `data-cy`, so `getByTestId('Zoom')` resolves
`[data-cy="Zoom"]`). Call out any `data-cy` you add so it ships in the same PR.

**Worked example — "set the Zoom hotkey in User Preferences":** the options menu, the
preferences dialog, each preference field, and the Zoom toolbar button should all be
page-object surface — e.g. `mainToolbarPageObject.zoom`,
`DOMOverlayPageObject.optionsMenu.settings.click()`,
`DOMOverlayPageObject.dialog.userPreferences.hotkey('Zoom').set('q')`. The spec then
reads as intent, not as a pile of `getByTestId` calls.

## Assert the effect, not just the attribute

Prefer asserting the actual rendered result over a proxy attribute. Activating Zoom and
checking `data-active="true"` confirms the *button* toggled — not that zoom works. Drag
on the viewport and assert the image actually zoomed (a viewport-scoped screenshot, or a
measurable state change). Attribute checks are fine as a secondary signal, not the whole
test.

## Visual regression

**Direction:** the suite is moving off *full-app* screenshots — not off screenshots
altogether. "Avoid screenshots" means: don't screenshot the whole page, and don't
screenshot something that has a faithful DOM/state signal. It does **not** mean avoid
screenshots for output that is genuinely canvas-only — for that output a screenshot is
the correct and required tool, and you should use it without apology. Older specs that
screenshot the whole page are the legacy pattern being phased out; viewport-scoped
screenshots are not.

### Screenshot vs. DOM assertion — how to choose

Reach for the cheapest *faithful* signal, in this order:

1. **A faithful DOM/SVG/state signal exists → assert on it.** Panel counts, dialog and
   overlay text, enabled/disabled state, and any overlay that renders as SVG (a vector
   overlay's color is readable via `getSvgAttribute`) are all readable from the DOM —
   assert on them directly, no screenshot.
2. **The thing under test exists only as pixels on the WebGL canvas → a screenshot is
   correct and required.** A painted pixel exposes no element or attribute to read. Capture
   a viewport pane with `checkForViewportScreenshot`, or scope a `checkForScreenshot` to the
   grid — this is the right tool, not a last resort, whenever what you're verifying is the
   rendered canvas itself.
3. **Never substitute a service/state read for a render assertion.** Reading a service's
   state (any `window.services...`) asserts the *data model*, not the pixels the user sees —
   it passes even when rendering is broken. `page.evaluate(() => window.services...)` is an
   escape hatch for *setup*, not for *appearance* assertions.

For a screenshot comparison scoped to a specific viewport, use `checkForViewportScreenshot` — it hides the viewport's overlay text for the capture:

```ts
await checkForViewportScreenshot({
  page,
  viewport: activeViewport, // captures the viewport pane with its text hidden
  screenshotPath: screenShotPaths.length.lengthDisplayedCorrectly,
});
```

Both helpers retry up to 10 times at 1250 ms intervals by default (`attempts` and `delay` are configurable) (`checkForViewportScreenshot` delegates to `checkForScreenshot`; use the latter directly only for non-viewport locators such as the grid or a panel). Use `screenShotPaths.<category>.<name>` rather than a hand-typed string — the tree of valid keys lives in `tests/utils/screenShotPaths.ts`.

Rules (apply to all new screenshot assertions):

- **Use the object form.** The positional form is legacy; don't introduce it in new code, and don't treat existing positional-form usage as a pattern to copy.
- **No text in baselines.** Overlay text (date, series description, W/L, slice index) drifts with data, locale, and font rendering, so a baseline that contains it is fragile — new viewport baselines must be text-free. Capture viewports through `checkForViewportScreenshot`, which hides all viewport text for the shot; a raw `checkForScreenshot` on a viewport pane bakes the text in.
- **Never screenshot the full app.** Full-page screenshots include panels, toolbars, and dialogs that drift independently of what's under test and make baselines fragile. Scope by passing a `locator` — `viewportPageObject.grid` for the grid, or a specific viewport pane. A bare `normalizedClip: { x: 0, y: 0, width: 1, height: 1 }` with no `locator` is **not** scoping — it clips to the full page. Use `normalizedClip` only to target a sub-region *of a locator* (e.g. a scrollbar strip). `fullPage: true` only takes effect when no `locator` is passed — that *is* the full-app capture this rule forbids, so pass a `locator` instead. (Through `checkForViewportScreenshot` the flag is inert: the capture is always scoped to the viewport pane.)
- **Do not tune `maxDiffPixelRatio` or `threshold`** to make a screenshot pass. If a baseline mismatches, regenerate it after a human review of the diff, or fix the underlying flake.

## Playwright config facts worth remembering

| Setting | Value | Why |
|---------|-------|-----|
| `baseURL` | `http://localhost:3335` | OHIF e2e uses 3335, not 3000 |
| `testIdAttribute` | `data-cy` | `getByTestId(...)` maps to `[data-cy="..."]` |
| `browser` | Chromium only | Firefox/WebKit disabled (SharedArrayBuffer + stability) |
| `retries` | 3 in CI, 0 locally | Flaky rendering needs CI retries |
| `workers` | 6 in CI, undefined locally | Parallel execution |
| `globalTimeout` / `timeout` | 800_000 ms | Medical image loads are slow |
| `actionTimeout` | 10_000 ms | Per-action cap |
| `webServer.command` | `cross-env APP_CONFIG=config/e2e.js COVERAGE=true OHIF_PORT=3335 nyc yarn start` | e2e config + coverage |

## Test data: which study for which test

| StudyInstanceUID | Mode | Used for |
|------------------|------|----------|
| `1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5` | `viewer` | General measurements, annotations, context menu |
| `1.3.6.1.4.1.14519.5.2.1.1706.8374.643249677828306008300337414785` | `viewer` | 3D, MPR, crosshairs |
| `1.3.6.1.4.1.14519.5.2.1.256467663913010332776401703474716742458` | `viewer` or `segmentation` | Labelmap SEG |
| `1.2.840.113619.2.290.3.3767434740.226.1600859119.501` | `viewer` / `segmentation` / `tmtv` | RTSTRUCT/contour and TMTV |
| `1.3.6.1.4.1.14519.5.2.1.7695.4007.324475281161490036195179843543` | `viewer` | SR hydration |

Full mapping in [references/patterns-by-feature.md](references/patterns-by-feature.md). **Do not invent UIDs** — they must exist on the e2e data server.

## Rules (short, so they're actually read)

1. Import `test`, `expect`, and utilities from `./utils`, not `@playwright/test`.
2. Destructure fixture-injected page objects; don't `new` them.
3. Use normalized coordinates (0–1) for viewport interactions.
4. Use `visitStudy` with a real UID, correct mode, and a non-zero delay (2000 is conventional).
5. Handle hydration and measurement-tracking prompts where applicable.
6. Choose the faithful signal: DOM/SVG assertions where the rendered result has one (panels, dialogs, overlay text, SVG/vector overlays), a viewport-scoped screenshot when what you're verifying is canvas-only raster output, and never a `window.services` state read standing in for a render check. Screenshots use the object form, scoped via a `locator` — never the full app.
7. Use `data-cy` selectors (already wired via `testIdAttribute`).
8. When an assertion needs retry tolerance, wrap it in `expect.toPass({ timeout })`.
9. Test in the correct mode — segmentation tools aren't available in `viewer` mode.
10. If a utility isn't exported from `./utils`, import from the deeper path (see [references/utilities.md](references/utilities.md)).
11. After an action that re-renders the viewport, prefer `waitForViewportRenderCycle(page)` (started before the action) over `page.waitForTimeout(...)`. See the "Wait for renders, don't sleep" section.
12. Don't inline raw `page.getByTestId(...)` / `getByRole(...)` for app controls. If a control has no page object, create or augment one (see "When the control you need has no page object yet"), adding a `data-cy` to the source if needed.
13. Assert the actual effect (e.g. the image zoomed), not just a proxy attribute like `data-active`.

## Pre-output self-check (mandatory)

Before returning a generated OHIF test, confirm all items:

1. Imports `test`/`expect` from `./utils` (not `@playwright/test`).
2. Uses fixture-injected keys and exact casing (especially `DOMOverlayPageObject`).
3. Uses normalized viewport interactions (`normalizedClickAt` / `normalizedDragAt`) unless there is a strong reason otherwise.
4. Uses a valid canonical StudyInstanceUID and compatible mode.
5. Handles hydration or measurement tracking prompts when the workflow requires them.
6. Uses the faithful signal for each assertion — DOM/SVG where the result is readable from the DOM, a viewport-scoped screenshot when what's verified exists only as pixels on the canvas, and never a `window.services` state read in place of a render check. Any `checkForScreenshot` call uses the object form, scoped via a `locator` (viewport pane or grid) — no full-app screenshots.
7. Replaces `page.waitForTimeout(...)` after viewport-rendering actions with `waitForViewportRenderCycle(page)` (started before the action) — keeps `waitForTimeout` only for non-render waits like the hydration prompt in `beforeEach`.
8. If execution was skipped, states that explicitly and provides concrete run commands.
9. Every application control is reached through a page object — no raw `getByTestId`/`getByRole` in the spec for buttons, menus, dialogs, or fields. Any control not already covered was added to the right page object (or a new one), with a source `data-cy` if it lacked one.
10. Assertions verify the real effect where feasible (e.g. the image visibly zoomed), not only an attribute toggle.

## Output contract (for non-executing agents)

When execution cannot be performed in the current environment, the response should include:

1. The test code.
2. Assumptions made (if any).
3. Static checks that were verified.
4. What still must be run locally and exact commands to run.

## When to consult each reference

- **Before writing** → [references/patterns-by-feature.md](references/patterns-by-feature.md). Pick the seed spec for the feature area and read it. The seed spec is the closest thing to a live API example because it co-evolves with the code.
- **For a stable rule or idiom** (fixture keys, import paths, panel-access order, capital-D quirk, object-param convention) → [references/page-objects.md](references/page-objects.md), [references/utilities.md](references/utilities.md).
- **For a method name, property, or signature** → read the source under `tests/pages/` or `tests/utils/`. Do not rely on a static table for these; they drift as the code is refactored.
- **When a test fails** → [references/failure-triage.md](references/failure-triage.md).