Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
279 changes: 279 additions & 0 deletions .agents/skills/ohif-test-agent/SKILL.md

Large diffs are not rendered by default.

48 changes: 48 additions & 0 deletions .agents/skills/ohif-test-agent/assets/spec-template.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import {
checkForScreenshot,
expect,
screenShotPaths,
test,
visitStudy,
waitForViewportRenderCycle,
} from './utils';

test.beforeEach(async ({ page }) => {
// Pick the right UID + mode for your feature (see references/patterns-by-feature.md)
const studyInstanceUID = '1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5';
const mode = 'viewer';
await visitStudy(page, studyInstanceUID, mode, 2000);
});

test.describe('FEATURE NAME', () => {
test('describes the behaviour in one sentence', async ({
page,
viewportPageObject,
mainToolbarPageObject,
rightPanelPageObject,
DOMOverlayPageObject, // capital D on purpose
}) => {
// 1. Arrange — select a tool, open a panel, etc.

// 2. Act — interact with the viewport.
// Prefer normalized (0–1) coordinates:
// const activeViewport = await viewportPageObject.active;
// await activeViewport.normalizedClickAt([{ x: 0.3, y: 0.3 }, { x: 0.7, y: 0.7 }]);

// For actions that re-render the viewport, gate on the render cycle
// instead of sleeping — start the watcher BEFORE the action:
// const cycle = waitForViewportRenderCycle(page);
// await action();
// await cycle;

// 3. Handle prompts (first measurement prompts for tracking; SEG/RT/SR prompts for hydration)
// await DOMOverlayPageObject.viewport.measurementTracking.confirm.click();

// 4. Assert canvas output via visual regression
// await checkForScreenshot(page, page, screenShotPaths.YOUR_CATEGORY.YOUR_KEY);

// 5. Assert DOM state directly
// const count = await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount();
// expect(count).toBe(1);
});
});
70 changes: 70 additions & 0 deletions .agents/skills/ohif-test-agent/references/failure-triage.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# Failure triage

Before debugging, classify. Most OHIF test failures are timing or hydration — not real regressions.

| Category | Symptom | Fix |
|----------|---------|-----|
| Timing | Element not visible, action timeout | Add / increase the `delay` param of `visitStudy`; for actions that re-render viewports, use `waitForViewportRenderCycle(page)` (started before the action) instead of `waitForTimeout`; wrap the assertion in `expect.toPass({ timeout })` |
| Selector | Element not found | Verify `data-cy` on the target; confirm the panel is open (`toggle()` / `select()` before interacting); check for capital `D` in `DOMOverlayPageObject` when destructuring |
| Hydration | Segmentation/RT not interactive | Ensure the `segmentationHydration.yes.click()` fired; add `waitForTimeout(3000)` after `loadSeriesByModality('SEG'\|'RTSTRUCT'\|'SR')` |
| Data | Study not found, empty viewport | Confirm the UID is in the canonical list (see [patterns-by-feature.md](patterns-by-feature.md)); confirm the mode supports the feature (segmentation tools aren't in `viewer` mode) |
| Visual drift | Screenshot mismatch but feature works | Have a human review the diff, then regenerate the baseline with `yarn playwright test --update-snapshots`. Do not adjust `maxDiffPixelRatio` or `threshold` to make a failing screenshot pass. |
| Real regression | Feature is actually broken | Report as a bug — this is the test doing its job |

## Prefer render-cycle waits over sleeps

If you're tempted to add `await page.waitForTimeout(2000)` after an action, ask whether the action re-rendered the viewport. If it did, use:

```ts
const cycle = waitForViewportRenderCycle(page);
await action();
await cycle;
await check();
```

The watcher must be created **before** the action — it waits for `needsRender` first, and that transition is gone by the time the action returns. See the "Wait for renders, don't sleep" section in [SKILL.md](../SKILL.md) and `tests/SEGHydrationFromMPR.spec.ts`.

### When the cycle helper times out at `waitForAnyViewportNeedsRender`

Symptom: the test fails inside `waitForAnyViewportNeedsRender` after 5s, with the action having actually completed in the UI. The action just doesn't transition the viewport through `needsRender` synchronously. Known cases:

- Hanging-protocol changes.
- **RTSTRUCT / contour segmentation hydration confirm.** SEG (labelmap) hydration does fire `needsRender`; contour does not. The fix is to gate on the actual end-state — for hydration in `beforeEach`, `await expect(page.getByTestId('data-row')).toHaveCount(N)` is the right wait.

Don't react by raising the cycle's timeout — the transition isn't coming. Replace the cycle wrapper with an auto-retrying DOM/SVG assertion, or `expect.toPass({ timeout })` around the assertion block.

## The `toPass` pattern

When an assertion needs to wait for async render / propagation:

```ts
await expect(async () => {
await rightPanelPageObject.labelMapSegmentationPanel.panel.segmentByText('Spleen').click();
await expect(activeViewport.overlayText.bottomRight.instanceNumber).toContainText('17/');
}).toPass({ timeout: 10_000 });
```

`toPass` reruns the assertion block until it succeeds or the timeout expires — cleaner than a hand-rolled retry loop and surfaces the last failure reason if it times out.

## Common `DOMOverlayPageObject` mistake

The fixture key is capital-D `DOMOverlayPageObject`, not lowercase. If your destructure is silently `undefined`, check the casing.

## `press` import mistake

`press` is NOT re-exported from `./utils`. `import { press } from './utils'` resolves to `undefined` and fails at runtime. Use:

```ts
import { press } from './utils/keyboardUtils';
await press({ page, key: 'ArrowDown', nTimes: 50 }); // object param, not (page, key)
```

## Visual regression iteration

Screenshots live under `tests/screenshots/chromium/<testFilePath>/`. To accept new output as the baseline:

```sh
yarn playwright test tests/YourSpec.spec.ts --update-snapshots
```

Review the resulting PNGs carefully — an agent-accepted baseline that's subtly wrong is worse than a failing test.
90 changes: 90 additions & 0 deletions .agents/skills/ohif-test-agent/references/page-objects.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# OHIF Page Object guide

> This file documents the **stable structural rules** of the page object system. For the current list of methods and properties on any class, **read the source under `tests/pages/`** — it is always authoritative, and it evolves as the product does. A static method table in a reference file goes stale the moment someone refactors; the source does not.

## How to discover the API of a page object

1. Find the relevant class in `tests/pages/`. File names match class names.
2. Read it end-to-end once — most are under a few hundred lines.
3. Some classes compose sub-objects (e.g. `RightPanelPageObject` holds a measurementsPanel, contourSegmentationPanel, labelMapSegmentationPanel, tmtvPanel, etc.). Those sub-objects usually live in the same file or a sibling under `tests/pages/`.
4. To see how a method is actually used, grep `tests/` or open the seed spec listed in [patterns-by-feature.md](patterns-by-feature.md). Real usage beats a synthesized signature every time.

Do not try to memorize a method surface from this file — it intentionally does not list one. It lists only the rules you cannot derive from the source by reading a single file.

---

## Stable rules

### Fixture keys (case-sensitive)

These are injected via `tests/utils/fixture.ts`. Destructure them from the test function's first argument — do not `new` them, because the fixture wires sub-objects to the correct `page` and hand-constructed instances skip that wiring.

- `viewportPageObject`
- `mainToolbarPageObject`
- `leftPanelPageObject`
- `rightPanelPageObject`
- `DOMOverlayPageObject` — **capital D**. A silent `undefined` destructure is almost always a casing typo here.
- `notFoundStudyPageObject`

If the fixture file is updated and new keys are added, they will show up there first — check it if something feels missing.

### Non-fixture page objects

Some page object classes are not fixture-injected. They are reached through an injected fixture:

- `DicomTagBrowserPageObject` → via `DOMOverlayPageObject.dialog.dicomTagBrowser`
- `DataOverlayPageObject` → via `viewportPageObject.getById(viewportId).overlayMenu.dataOverlay`

Both can be constructed manually (`new DataOverlayPageObject(page)`) if a test really needs a fresh instance, but the accessor path is the idiomatic one.

### Viewport wrapper vs. viewport instance

`viewportPageObject` is a **wrapper**. You almost always want a specific viewport out of it first:

- `await viewportPageObject.active` — the currently focused viewport
- `viewportPageObject.getAll()` — every viewport in the grid
- `viewportPageObject.getNth(i)` — zero-indexed
- `viewportPageObject.getById(cornerstoneViewportId)` — e.g. `'default'`, `'ctAXIAL'`

The object these return is the one with `normalizedClickAt`, `normalizedDragAt`, `overlayText`, `nthAnnotation`, etc. Reach for the viewport instance first, then call methods on it.

### Panel access order

Every `rightPanelPageObject` sub-panel follows the same three-step idiom: **open the side panel, `.select()` the sub-panel tab, then interact with `.panel.*`**. Skipping either of the first two is the most common cause of "element not found".

```ts
await rightPanelPageObject.toggle();
await rightPanelPageObject.measurementsPanel.select();
const count = await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount();
```

The exact row/action methods vary by panel — check the source file for the one you need.

### Layout identifiers are camelCase JS properties

`mainToolbarPageObject.layoutSelection.<layout>.click()` — access layouts by camelCase property name (e.g. `threeDFourUp`, `axialPrimary`), not with bracket-escaped DICOM-ish strings like `['3DFourUp']`. This is a convention enforced by how the class exposes its tools.

### Sub-tools auto-open their dropdown

Tools nested inside a toolbar dropdown (measurement tools, more tools, layouts) each expose a `.click()` that opens the parent menu for you. You almost never need to open the menu first. `await mainToolbarPageObject.measurementTools.length.click()` does both the expand and the select.

---

## Page object map — what each class is *for*

This table exists to help you pick the right file to open, not to enumerate methods. Source of truth for any specific method remains the `.ts` file.

| Class | File | Covers |
|-------|------|--------|
| ViewportPageObject | `tests/pages/ViewportPageObject.ts` | Cornerstone viewports — clicks, drags, overlays, annotations, crosshairs |
| MainToolbarPageObject | `tests/pages/MainToolbarPageObject.ts` | Top toolbar — measurement tools, more tools, layouts, crosshairs, pan |
| LeftPanelPageObject | `tests/pages/LeftPanelPageObject.ts` | Study browser — thumbnails, load by modality or description |
| RightPanelPageObject | `tests/pages/RightPanelPageObject.ts` | Side panels — measurements, contour seg, labelmap seg, TMTV, microscopy |
| DOMOverlayPageObject | `tests/pages/DOMOverlayPageObject.ts` | DOM overlays — dialogs, hydration/tracking prompts, context menus, tag-browser accessor |
| NotFoundStudyPageObject | `tests/pages/NotFoundStudyPageObject.ts` | Study-not-found error page |
| DicomTagBrowserPageObject | `tests/pages/DicomTagBrowserPageObject.ts` | Tag-browser dialog (non-fixture; reach via `DOMOverlayPageObject.dialog`) |
| DataOverlayPageObject | `tests/pages/DataOverlayPageObject.ts` | Data-overlay menu (non-fixture; reach via `viewport.overlayMenu`) |

If the directory adds or renames a file, that diff is your first clue and this table is your second — trust the directory.

For live usage, the seed spec in [patterns-by-feature.md](patterns-by-feature.md) shows how a class is actually called; the source file tells you everything else that's on it.
125 changes: 125 additions & 0 deletions .agents/skills/ohif-test-agent/references/patterns-by-feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
# Seed specs by feature area

> When writing a new OHIF test, find the closest feature area below and read the listed spec in full before writing. Playwright's own guidance says the seed test "serves as an example of all the generated tests" — that applies here.
>
> **If a spec listed below has moved or been renamed**, grep `tests/` for a remaining example (e.g. `grep -rn "loadSeriesByModality('RTSTRUCT')" tests/`). The pattern matters more than the exact filename — specs get renamed, the feature area persists.

## Canonical study UIDs

| UID | Mode(s) | What it has |
|-----|---------|-------------|
| `1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5` | `viewer` | CT — default for measurement/annotation tests |
| `1.3.6.1.4.1.14519.5.2.1.1706.8374.643249677828306008300337414785` | `viewer` | CT volume for 3D/MPR/crosshairs |
| `1.3.6.1.4.1.14519.5.2.1.256467663913010332776401703474716742458` | `viewer` or `segmentation` | CT + SEG for labelmap tests |
| `1.2.840.113619.2.290.3.3767434740.226.1600859119.501` | `viewer` / `segmentation` / `tmtv` | CT + RTSTRUCT + PET, used for contour and TMTV |
| `1.3.6.1.4.1.14519.5.2.1.7695.4007.324475281161490036195179843543` | `viewer` | SR structured report |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should consider adding more here.


Do not invent UIDs — they must exist on the e2e data server.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call.


---

## 1. Simple measurement tools (length, angle, bidirectional, rectangle, ellipse, circle)

**Seed:** `tests/Length.spec.ts`, `tests/Angle.spec.ts`

Pattern: select tool via `mainToolbarPageObject.measurementTools.<tool>.click()`, place N points via `activeViewport.normalizedClickAt([...])`, confirm the tracking prompt, screenshot via `screenShotPaths.<name>.<name>DisplayedCorrectly`.

## 2. Freehand / spline / livewire ROIs

**Seed:** `tests/FreehandROI.spec.ts`, `tests/Livewire.spec.ts`, `tests/Spline.spec.ts`

Pattern: `normalizedDragAt({ start, end, config: { steps: 20, delay: 30 } })` for smooth strokes; `subscribeToMeasurementAdded` to assert the event fires; `activeViewport.nthAnnotation(0)` to reference what was drawn.

## 3. Annotations (arrow, probe)

**Seed:** `tests/ArrowAnnotate.spec.ts`, `tests/Probe.spec.ts`

Arrow annotate opens `DOMOverlayPageObject.dialog.input` for the label. Use `fillAndSave(label)`.

## 4. Measurement panel interactions

**Seed:** `tests/MeasurementPanel.spec.ts`

Panel access: `rightPanelPageObject.toggle()` → `.measurementsPanel.select()` → `.panel.nthMeasurement(i)` → `.actions.rename|delete|toggleLock|duplicate|...`. Also demonstrates `addLengthMeasurement(page)` and panel-row `click()` for jump-to.

## 5. Context menu (right-click on annotation)

**Seed:** `tests/ContextMenu.spec.ts`

Two ways to open: `activeViewport.normalizedClickAt([{...}], 'right')` on an empty area, or `activeViewport.nthAnnotation(0).contextMenu.open()` on a drawn annotation. Then `DOMOverlayPageObject.viewport.annotationContextMenu.addLabel|delete.click()`.

## 6. Labelmap segmentation (SEG) hydration

**Seed:** `tests/SEGHydration.spec.ts`, `tests/SEGHydrationThenMPR.spec.ts`

Flow: `leftPanelPageObject.loadSeriesByModality('SEG')` → `waitForTimeout(3000)` → `DOMOverlayPageObject.viewport.segmentationHydration.yes.click()`. Often pokes Cornerstone state directly via `page.evaluate(() => window.cornerstone...)` for zoom/render.

## 7. Contour segmentation (RTSTRUCT) + interactions

**Seeds:**
- Hydration: `tests/RTHydration.spec.ts`
- Rename: `tests/ContourSegmentRename.spec.ts`
- Navigation between segments: `tests/ContourSegNavigation.spec.ts`
- Duplicate: `tests/ContourSegmentDuplicate.spec.ts`
- Visibility: `tests/ContourSegmentToggleVisibility.spec.ts`
- Locking: `tests/ContourSegLocking.spec.ts`

Pattern: load RTSTRUCT series → hydrate → use `rightPanelPageObject.contourSegmentationPanel.panel.nthSegment(i)` / `.segmentByText('Small Sphere')` and their `.actions.rename|delete`, or the segment's `.click()` to jump.

## 8. Labelmap segmentation editing (brush / eraser / threshold)

**Seed:** `tests/LabelMapSegLocking.spec.ts`, `tests/SEGDrawingToolsResizing.spec.ts`

Pattern: `rightPanelPageObject.labelMapSegmentationPanel.tools.brush.setRadius(n)` / `.click()`, then `activeViewport.normalizedDragAt(...)` to paint.

## 9. TMTV / PET

**Seeds:** `tests/TMTVCSVReport.spec.ts`, `tests/TMTVSUV.spec.ts`, `tests/TMTVAlignment.spec.ts`, `tests/TMTVRendering.spec.ts`

Visit `mode: 'tmtv'` with a longer delay (`10000`). `rightPanelPageObject.tmtvPanel` for the side panel. Exporting a report uses `page.waitForEvent('download')` + `downloadAsString(download)`.

## 10. MPR and 3D layouts

**Seeds:** `tests/MPR.spec.ts`, `tests/3DOnly.spec.ts`, `tests/3DFourUp.spec.ts`, `tests/3DMain.spec.ts`, `tests/AxialPrimary.spec.ts`

Pattern: `mainToolbarPageObject.layoutSelection.<layoutName>.click()`. For 3D, wrap stabilization with `attemptAction(() => reduce3DViewportSize(page), 10, 100)` to settle the render before asserting.

## 11. Crosshairs

**Seed:** `tests/Crosshairs.spec.ts`

Pattern: `initializeMousePositionTracker(page)` in `beforeEach`, `mainToolbarPageObject.crosshairs.click()`, then `viewportPageObject.crosshairs.axial.rotate()` / `.increase()`.

## 12. Overlays — data overlay menu, window/level, orientation

**Seeds:** `tests/DataOverlayMenu.spec.ts`, `tests/WindowLevelOverlayText.spec.ts`, `tests/MultipleSegmentationDataOverlays.spec.ts`

Pattern: `viewportPageObject.getById('default').overlayMenu.dataOverlay.toggle()`, then `.addSegmentation(name)` / `.changeSegmentation(from, to)` / `.remove(name)`. For keyboard navigation, remember `press({ page, key, nTimes })` imports from `./utils/keyboardUtils`.

## 13. DICOM Tag Browser

**Seed:** `tests/DicomTagBrowser.spec.ts`

Open with `mainToolbarPageObject.moreTools.tagBrowser.click()`, then interact via `DOMOverlayPageObject.dialog.dicomTagBrowser.waitVisible()` / `.seriesSelect.selectOption(i)` / `.seriesSelect.getOptionText(i)`.

## 14. Study validation / not-found / worklist

**Seeds:** `tests/StudyValidation.spec.ts`, `tests/Worklist.spec.ts`

These are the rare specs that use `page.goto(...)` directly instead of `visitStudy()` — because they test error states or non-study pages. `notFoundStudyPageObject` gives you `errorMessage`, `returnMessage`, `studyListLink`.

## 15. SR (Structured Report) hydration

**Seeds:** `tests/SRHydration.spec.ts`, `tests/SRHydrationThenReload.spec.ts`

Same shape as SEG hydration but load via `loadSeriesByModality('SR')`.

---

## Advanced patterns worth knowing

- **`subscribeToMeasurementAdded`** (`tests/FreehandROI.spec.ts`) — for async "was a measurement actually added?" assertions. Always `try { ... } finally { await measurementAdded.unsubscribe() }`.
- **`attemptAction`** (`tests/3DOnly.spec.ts`) — retry flaky setup (3D render, heavy layout change) without silencing real failures.
- **`addOHIFConfiguration`** (`tests/RTHydrationDisableConfirmation.spec.ts`) — pre-load config overrides before `visitStudy`.
- **`page.evaluate(() => window.services...)`** — used in several SEG/SR specs to set customizations or poke viewport state. Treat as an escape hatch, not a default.
- **`expect.toPass({ timeout })`** — wrap flaky assertions (common for jump-to-measurement tests where rendering settles asynchronously).
Loading
Loading