71 lines
4.0 KiB
Markdown
71 lines
4.0 KiB
Markdown
# 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.
|