Skip to content

Commit d00a7db

Browse files
authored
test(segmentation): cover navigation, stats and bidirectional on drawn segmentations (#6351)
1 parent 6d841e9 commit d00a7db

14 files changed

Lines changed: 684 additions & 28 deletions

‎.agents/skills/ohif-test-agent/SKILL.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,6 @@ When to use which:
142142
|-----------|--------|
143143
| Click that triggers a re-render and you want to assert after | `waitForViewportRenderCycle(page)` started before the click |
144144
| Layout switch / series load — render already in flight | `await waitForViewportsRendered(page)` after the call |
145-
| Slice navigation (`sliceNavigation.toSlice(...)`, `scrollBy`, …) | `await waitForViewportsRendered(page)` after the call (per the `getSliceNavigation` docstring) |
146145
| Compose with another await (e.g. screenshot the same time as load) | Save the promise, `await` it later |
147146

148147
Replace patterns like this:
@@ -188,6 +187,8 @@ Two page objects are **not** fixture-injected:
188187

189188
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.
190189

190+
Page objects locate elements and perform actions; keep `expect` assertions in the spec.
191+
191192
## When the control you need has no page object yet
192193

193194
A spec must not reach for `page.getByTestId(...)` / `getByRole(...)` directly for

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,9 @@ Read the one-line signature at the top of the utility's source file before calli
9696

9797
Most utilities are obvious once you read the source; these earn a mention:
9898

99-
- **`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, and after `sliceNavigation.to*()` / `scrollBy` (their docstring says so). 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.
99+
- **`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.
100100
- **`expectAnnotationStatsText({ page, activeViewport, rightPanelPageObject, toolName, expectedPanelPrimaryLines, expectedSvgLines, assertStats? })`** — asserts a measurement's panel row, its SVG text, and its `cachedStats` in one call. Build the expected lines with `measurementTextFormatters`. Seeds: `tests/Length.spec.ts`, `tests/Probe.spec.ts`. Source: `tests/utils/expectAnnotationText.ts`.
101+
- **`getSegmentStats({ page, segmentIndex })`** — call it before clicking a painted labelmap segment's row: the jump needs the segment's center, which Cornerstone computes in the background. Seed: `tests/ManuallyDrawnSegmentation.spec.ts`.
101102
- **`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.
102103
- **`attemptAction(action, attempts?, delay?)`** — retries a flaky async action without masking real failures. Mainly used to stabilize 3D scenes (`attemptAction(() => reduce3DViewportSize(page), 10, 100)`).
103104

‎extensions/cornerstone/src/customizations/CustomSegmentStatisticsHeader.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export const CustomSegmentStatisticsHeader = ({
4242
variant="ghost"
4343
size="sm"
4444
className="text-primary flex items-center gap-2"
45+
dataCY="segment-bidirectional-compute"
4546
onClick={() => {
4647
commandsManager.run('runSegmentBidirectional', {
4748
segmentationId,
@@ -76,10 +77,10 @@ export const CustomSegmentStatisticsHeader = ({
7677
<div className="-mt-2 space-y-2">
7778
<div className="flex items-center justify-between">
7879
<div className="text-foreground">
79-
<div>
80+
<div data-cy="segment-bidirectional-length">
8081
L: {roundNumber(max)} {unit}
8182
</div>
82-
<div>
83+
<div data-cy="segment-bidirectional-width">
8384
W: {roundNumber(min)} {unit}
8485
</div>
8586
</div>
@@ -117,6 +118,7 @@ export const CustomSegmentStatisticsHeader = ({
117118
<Button
118119
variant="ghost"
119120
size="icon"
121+
dataCY="segment-bidirectional-jump"
120122
onClick={() => {
121123
if (bidirectional.annotationUID) {
122124
commandsManager.run('jumpToMeasurement', {

‎tests/FreehandContourSegmentation.spec.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ test('should keep a freehand contour drawn on a slice after navigating away and
6565

6666
//Force navigate to first slice
6767
await activeViewport.sliceNavigation.toFirstSlice();
68-
await waitForViewportsRendered(page);
6968

7069
const sliceIndicator = activeViewport.overlayText.bottomRight.instanceNumber;
7170
const drawingSliceInfo = await sliceIndicator.innerText();
@@ -86,13 +85,11 @@ test('should keep a freehand contour drawn on a slice after navigating away and
8685

8786
//Navigate to last slice
8887
await activeViewport.sliceNavigation.toLastSlice();
89-
await waitForViewportsRendered(page);
9088
await expect(sliceIndicator, 'Expected to leave drawing slice').not.toHaveText(drawingSliceInfo);
9189
await expect(paths, 'Expected no contour on the other slice').toHaveCount(0);
9290

9391
// Come back to first slice
9492
await activeViewport.sliceNavigation.toFirstSlice();
95-
await waitForViewportsRendered(page);
9693
await expect(sliceIndicator, 'Expected to return to drawing slice').toHaveText(drawingSliceInfo);
9794
await expect(paths, 'Expected the contour to re-render on the drawing slice').toHaveCount(1);
9895
await expect(paths.nth(0), 'Expected the persisted contour to be visible').toBeVisible();

‎tests/LivewireContourSegmentation.spec.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {
66
screenShotPaths,
77
test,
88
visitStudy,
9-
waitForViewportsRendered,
109
withKeyHeld,
1110
} from './utils';
1211

@@ -108,7 +107,6 @@ test('should keep a completed Livewire contour segmentation after navigating awa
108107

109108
// Navigate away: the contour only renders on the slice it was drawn on.
110109
await activeViewport.sliceNavigation.toLastSlice();
111-
await waitForViewportsRendered(page);
112110
await expect(paths, 'Expected no contour paths on the last slice').toHaveCount(0);
113111

114112
// Clicking Segment 1 activates it and jumps the viewport back to its contour.

0 commit comments

Comments
 (0)