ohif-viewer/.agents/skills/ohif-test-agent/references/utilities.md

101 lines
7.9 KiB
Markdown

# OHIF Test Utility guide
> This file documents the **stable import rules and conventions** around `tests/utils/`. For the current list of exported helpers and their exact signatures, **read `tests/utils/index.ts` and the files it re-exports** — the barrel is always current; a static table here is not. Utilities get added, renamed, and refactored; the rules below change much more slowly.
## How to discover what's available
1. Open `tests/utils/index.ts`. Every symbol exported from the barrel is importable as `import { foo } from './utils'`.
2. If what you need isn't in the barrel, look in the rest of `tests/utils/` — there are a handful of specialized files (`keyboardUtils.ts`, `assertions.ts`, `download.ts`, …). These need the **deeper import path**; see the rule below.
3. For the actual signature, read the utility's `.ts` file. It's one short function per file in most cases.
4. To see a utility in context, grep `tests/` (`grep -rn visitStudy tests/`) — or open the seed spec for the relevant feature area ([patterns-by-feature.md](patterns-by-feature.md)). Existing specs are the most reliable signature reference because they co-evolve with the API.
Do not guess parameter shapes from memory, and do not treat this file as an API catalog — it intentionally isn't one.
---
## Stable rules
### Barrel vs. deep imports
Two import styles exist. They are not interchangeable.
```ts
// Barrel — anything re-exported from tests/utils/index.ts
import { test, expect, visitStudy, checkForScreenshot, screenShotPaths } from './utils';
// Deep — for files NOT re-exported by the barrel
import { press } from './utils/keyboardUtils';
import { assertNumberOfModalityLoadBadges } from './utils/assertions';
import { downloadAsString } from './utils/download';
```
If a symbol isn't in the barrel, `import { x } from './utils'` **compiles**, resolves `x` to `undefined`, and blows up at the first call site. Confirm by opening `tests/utils/index.ts` for your working revision. At the time of this writing, `press`, `downloadAsString`, and the `assert*` helpers live outside the barrel — but maintainers can move things in or out, so treat `index.ts` as the ground truth rather than this note.
### Never import `test` / `expect` from `@playwright/test`
```ts
// ✅ Correct — gets the fixture-extended runner
import { test, expect } from './utils';
// ❌ Wrong — compiles, but every page-object fixture is silently undefined
import { test, expect } from '@playwright/test';
```
If your test function's destructured arguments (like `viewportPageObject`) are `undefined`, this import is almost always why.
### `visitStudy` — 2000 ms is a convention, not a default
```ts
await visitStudy(page, studyInstanceUID, mode, 2000);
```
The function's own default delay is `0`. Nearly every spec passes `2000` to let the first render settle. The one consistent exception is `mode: 'tmtv'`, where the suite uniformly uses `10000` because PET fusion and SUV calculation add real wall-clock cost before the UI is interactive.
Notably, 3D layouts in `viewer` mode (3DOnly, 3DFourUp, 3DMain, 3DPrimary) and MPR also use `2000` — they're not "heavy" in the `visitStudy` sense. Their stabilization problem is solved at the interaction layer with `attemptAction(() => reduce3DViewportSize(page), 10, 100)`, not by a longer visit delay.
So: pick `10000` when the mode is `tmtv`, `2000` otherwise, and only ramp up if a specific test flakes on first-render assertions. The delay is a good first lever for "not visible" flakes, but it's not a universal upgrade.
### `checkForScreenshot` — use the object form, never screenshot the full app
**This is the direction going forward** The suite is being migrated off *full-app* screenshots — not off screenshots altogether. Screenshots are the correct and required tool whenever what you're verifying is canvas-only raster output with no DOM signal; don't avoid them there. Avoid them only where a faithful DOM/SVG signal exists (e.g. a vector overlay's color via `getSvgAttribute`) or where you'd be capturing the whole app. See SKILL.md → "Screenshot vs. DOM assertion — how to choose". Any new spec must follow the rules below, and any modification to an older spec should bring it in line when reasonable.
- **Object form** (required for all new specs): `checkForScreenshot({ page, screenshotPath, normalizedClip?, ... })`
- **Positional form**: legacy. It still appears in older specs because they haven't been migrated yet. **Do not treat existing positional-form usage as a pattern to copy** — those specs are the thing being moved away from. Do not introduce the positional form in new code.
**Hard rules for new screenshots:**
1. Use the object form.
2. Scope by passing a `locator``viewportPageObject.grid` for the whole grid, or a specific viewport pane locator. **Never screenshot the full app.** `normalizedClip` is computed *relative to the locator* (and defaults to the full page when no locator is given), so `{ x: 0, y: 0, width: 1, height: 1 }` alone does not scope anything — reserve `normalizedClip` for clipping to a sub-region of a locator. If you find yourself reaching for `fullPage: true`, stop and pass a locator instead.
Do not tune `maxDiffPixelRatio` or `threshold` to make a screenshot pass — those are intentionally rarely touched and not the right knob for flakes. If a baseline mismatches, regenerate it (`--update-snapshots`) after a human review of the diff, or fix the underlying instability. Check the current signature in `tests/utils/checkForScreenshot.ts` if something looks off.
### `screenShotPaths` — use keys, not raw strings
```ts
await checkForScreenshot({
page,
locator: viewportPageObject.grid,
screenshotPath: screenShotPaths.length.lengthDisplayedCorrectly,
});
```
The full tree of categories lives in `tests/utils/screenShotPaths.ts`. When you need a new baseline, **add the key there and reference it by name** rather than typing a raw path. A typo in a key becomes a compile error instead of a silent mismatch, and other tests become discoverable through the object.
### Object-param convention
Many OHIF test utilities take a single **object argument** rather than positional arguments — notably `press({ page, key, nTimes? })`, the `simulate*` helpers, and the `assert*` helpers. If a call looks like it should work but throws "cannot read properties of undefined," check whether you're passing positional args to something that expects `{ page, ... }`.
Read the one-line signature at the top of the utility's source file before calling it — it's faster than guessing, and it's always right.
---
## Utility shapes worth flagging
Most utilities are obvious once you read the source; these earn a mention:
- **`waitForViewportRenderCycle(page, options?)`** — preferred replacement for `page.waitForTimeout(...)` after any viewport-mutating action (hydration confirm, layout change, segmentation add, series load, etc.). Start it **before** the action, await it after — it captures the `needsRender → rendered` transition the action triggers. Use `waitForViewportsRendered(page)` (the second-half-only variant) when the render is already in flight before you can attach a watcher. Source: `tests/utils/waitForViewportsRendered.ts`. Seed: `tests/SEGHydrationFromMPR.spec.ts`. The "Wait for renders, don't sleep" section in [SKILL.md](../SKILL.md) covers the idiom in full.
- **`subscribeToMeasurementAdded(page)`** — returns `{ waitFired(timeout?), unsubscribe() }`. Use in freehand/livewire/spline specs to assert the event fired. Always wrap in `try { ... } finally { await sub.unsubscribe() }` so a failing assertion doesn't leak the listener across tests.
- **`attemptAction(action, attempts?, delay?)`** — retries a flaky async action without masking real failures. Mainly used to stabilize 3D scenes (`attemptAction(() => reduce3DViewportSize(page), 10, 100)`).
For everything else, the pattern is: find a spec that uses it (see [patterns-by-feature.md](patterns-by-feature.md)), copy the shape, adapt.