test(e2e): replace waitForVolumeLoad with viewport render waits

[GhadeerAlbattarni]

Context

• Removes the waitForVolumeLoad helper from all E2E tests and replaces it with precise, signal-based viewport render waits
• Fixes pre-existing bug where assertNumberOfModalityLoadBadges was called without await

Changes & Results

Testing

Checklist

PR

• [] My Pull Request title is descriptive, accurate and follows the
  semantic-release format and guidelines.

Code

• [] My code has been well-documented (function documentation, inline comments,
  etc.)

Public Documentation Updates

• [] The documentation page has been updated as necessary for any public API
  additions or removals.

Tested Environment

• OS: macOS 10.15.4
• Node version: v22.13.0
• Browser: Chrome 83.0.4103.116

Greptile Summary

This PR replaces the blunt waitForVolumeLoad helper (a fixed 2 s sleep + networkidle) with precise, signal-based viewport render waits (waitForViewportsRendered and waitForViewportRenderCycle) across 11 E2E test files, and fixes a pre-existing bug where assertNumberOfModalityLoadBadges was called without await.

• Most replacements are correct: the 3D layout tests, Invert, MPRThenRTOverlayNoHydration, RTDataOverlayNoHydrationThenMPR, and MultipleSegmentationDataOverlays all adopt the new pattern cleanly and fix the missing-await bug.
• Three files have incomplete migrations: mpr2.spec.ts removes the only render wait before its first screenshot; ArrowAnnotate.spec.ts leaves screenshots 2 and 3 unguarded after annotation edits; and the first two tests in DicomTagBrowser.spec.ts drop waitForVolumeLoad with no replacement before their pixel screenshots.
• MainToolbarPageObject.ts correctly removes the implicit waitForVolumeLoad from layout-selection helpers, delegating wait responsibility to each call site.

Confidence Score: 5/5

Safe to merge; all changes are in E2E test files only with no production code affected.

The change is entirely in test infrastructure. The new render-wait utilities are well-designed and the majority of migrations are correct. The three files with missing render waits introduce potential screenshot flakiness but do not break functionality.

tests/mpr2.spec.ts, tests/ArrowAnnotate.spec.ts, and the first two tests in tests/DicomTagBrowser.spec.ts are missing render waits before pixel screenshots that were previously guarded.

Important Files Changed

Filename	Overview
tests/mpr2.spec.ts	Removes waitForVolumeLoad before first screenshot; waitForViewportsRendered is added only before the second (zoomed) screenshot, leaving the first unguarded.
tests/ArrowAnnotate.spec.ts	Adds a correct waitForViewportRenderCycle before screenshot 1, but screenshots 2 and 3 (after fillAndSave and rename) still lack any render wait.
tests/DicomTagBrowser.spec.ts	waitForVolumeLoad removed from the first three tests without replacement; tests 4 and 5 correctly use waitForViewportsRendered after the layout change.
tests/MultipleSegmentationDataOverlays.spec.ts	Replaces all waitForVolumeLoad calls with waitForViewportRenderCycle/waitForViewportsRendered; also fixes the pre-existing missing-await bug on assertNumberOfModalityLoadBadges throughout.
tests/MPRThenRTOverlayNoHydration.spec.ts	Correctly replaces waitForVolumeLoad with waitForViewportsRendered and waitForViewportRenderCycle; promise is started before addSegmentation to avoid missing the needsRender signal.
tests/RTDataOverlayNoHydrationThenMPR.spec.ts	Replaces waitForVolumeLoad with signal-based waits and fixes missing await on assertNumberOfModalityLoadBadges; also increases timeout to 40000 ms for the MPR layout switch.
tests/pages/MainToolbarPageObject.ts	Removes implicit waitForVolumeLoad from all layout-selection click helpers, correctly delegating the wait responsibility to each call site.

Comments Outside Diff (3)

1. tests/mpr2.spec.ts, line 19-23 (link)

   First screenshot taken without a render wait

   The original test called waitForVolumeLoad() (networkidle + 2 s settle) before this screenshot. That guard is now gone; visitStudy with 10 000 ms provides only a navigation/page-ready wait, not a viewport-render guarantee. If the MPR volumes haven't finished rendering when this screenshot is taken the baseline comparison will be against a partially-loaded frame.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tests/mpr2.spec.ts
   Line: 19-23
   
   Comment:
   **First screenshot taken without a render wait**
   
   The original test called `waitForVolumeLoad()` (networkidle + 2 s settle) before this screenshot. That guard is now gone; `visitStudy` with 10 000 ms provides only a navigation/page-ready wait, not a viewport-render guarantee. If the MPR volumes haven't finished rendering when this screenshot is taken the baseline comparison will be against a partially-loaded frame.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. tests/ArrowAnnotate.spec.ts, line 50-58 (link)

   Missing render wait before screenshots 2 and 3

   The fillAndSave call (editing annotation text via double-click) and the rename call both mutate annotation state, which can trigger a canvas re-render. The original code had waitForVolumeLoad() after each of these before taking a screenshot. Those guards were removed without a replacement waitForViewportRenderCycle or waitForViewportsRendered, so the screenshot may be captured before the annotation overlay has fully re-painted — potentially causing flaky pixel-diff failures.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tests/ArrowAnnotate.spec.ts
   Line: 50-58
   
   Comment:
   **Missing render wait before screenshots 2 and 3**
   
   The `fillAndSave` call (editing annotation text via double-click) and the `rename` call both mutate annotation state, which can trigger a canvas re-render. The original code had `waitForVolumeLoad()` after each of these before taking a screenshot. Those guards were removed without a replacement `waitForViewportRenderCycle` or `waitForViewportsRendered`, so the screenshot may be captured before the annotation overlay has fully re-painted — potentially causing flaky pixel-diff failures.
   
   How can I resolve this? If you propose a fix, please make it concise.

3. tests/DicomTagBrowser.spec.ts, line 28-38 (link)

   waitForVolumeLoad removed before screenshot in first two tests with no replacement

   Tests 'should display the dicom tag browser' and 'should render the scroll bar...' previously called waitForVolumeLoad() (networkidle) between visitStudy and tagBrowser.click(). Both tests immediately take a full-page screenshot that includes the viewport area; if the initial series hasn't rendered yet the baseline will capture a loading/blank viewport. Tests 4 and 5 in the same file correctly add waitForViewportsRendered after their layout change — applying the same pattern here would make all tests consistent.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tests/DicomTagBrowser.spec.ts
   Line: 28-38
   
   Comment:
   **`waitForVolumeLoad` removed before screenshot in first two tests with no replacement**
   
   Tests `'should display the dicom tag browser'` and `'should render the scroll bar...'` previously called `waitForVolumeLoad()` (networkidle) between `visitStudy` and `tagBrowser.click()`. Both tests immediately take a full-page screenshot that includes the viewport area; if the initial series hasn't rendered yet the baseline will capture a loading/blank viewport. Tests 4 and 5 in the same file correctly add `waitForViewportsRendered` after their layout change — applying the same pattern here would make all tests consistent.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
tests/mpr2.spec.ts:16-23
**First screenshot taken without a render wait**

The original test called `waitForVolumeLoad()` (networkidle + 2 s settle) before this first `checkForScreenshot`. That guard was removed without a replacement; `visitStudy` with `10000` ms provides only a navigation/page-ready wait, not a viewport-render guarantee. If the MPR volumes are still streaming when the screenshot fires, the baseline will capture a partially-loaded frame. Consider adding `await waitForViewportsRendered(page)` before the first screenshot to mirror the protection that exists before the second.

### Issue 2 of 3
tests/ArrowAnnotate.spec.ts:50-70
**Missing render wait before screenshots 2 and 3**

The `fillAndSave` call on line 52 and the `rename` call on lines 62–64 both mutate annotation state and can trigger a canvas re-render. The original code wrapped each of these with `waitForVolumeLoad()` before taking a screenshot; those guards were removed without a replacement `waitForViewportRenderCycle` or `waitForViewportsRendered`. Screenshots taken immediately after annotation edits may capture an intermediate paint state, leading to flaky pixel-diff failures.

### Issue 3 of 3
tests/DicomTagBrowser.spec.ts:28-55
**`waitForVolumeLoad` removed before screenshot in first two tests with no replacement**

Tests `'should display the dicom tag browser'` (line 28) and `'should render the scroll bar...'` (line 41) previously called `waitForVolumeLoad()` between `visitStudy` and `tagBrowser.click()`. Both take full-page screenshots that include the viewport area; if the initial series hasn't finished rendering the baseline will capture a blank or loading viewport. Tests 4 and 5 in the same file correctly use `waitForViewportsRendered` after their layout change — applying the same pattern here would make all tests consistent.

_{Reviews (2): Last reviewed commit: "test: use let for viewportRenderCycle" | Re-trigger Greptile}

[netlify]

✅ Deploy Preview for ohif-dev canceled.

Name	Link
🔨 Latest commit	28a2781
🔍 Latest deploy log	https://app.netlify.com/projects/ohif-dev/deploys/69fcc65be4a8a60008b7ceeb

[jbocce]

You could use a let here instead of const and then just have a single variable used over and over again... up to you. No big deal either way.