tests(Playwright): Full implementation of Page Object Models (POMs) to Playwright tests #5608

ViniciusResende · 2025-12-03T17:46:50Z

Context

This PR finalizes the effort that was introduced at #5557, of the introduction of POMs to our tests, in a way that we'll benefit in terms of readability and reusability, allowing our test structure to grow without prejudicing maintainability.

This concludes the first step of the architecture enhancement of Playwright tests.

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: Windows 11 25H2 + WSL Ubuntu 22.04.4 LTS
Node version: 20.9.0
Browser: Microsoft Edge 141.0.3537.71

netlify · 2025-12-03T17:46:57Z

✅ Deploy Preview for ohif-dev canceled.

Name	Link
🔨 Latest commit	`c29e957`
🔍 Latest deploy log	https://app.netlify.com/projects/ohif-dev/deploys/69308f991ab0040009d97bcd

sedghi · 2025-12-03T19:59:12Z

@jbocce what is the status of this

jbocce · 2025-12-03T20:06:07Z

@jbocce what is the status of this

@sedghi it just got created. I will likely look at it next week.

jbocce

Great changes. A few comments to consider. Thanks.

jbocce · 2025-12-10T01:34:39Z

extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/untrackSeriesModal.tsx

        <FooterAction.Right>
-          <FooterAction.Secondary onClick={hide}>Cancel</FooterAction.Secondary>
+          <FooterAction.Secondary
+            dataCY="cancel-button"


Consider making this more specific, like 'untracked-series-modal-cancel-button'

jbocce · 2025-12-10T01:34:50Z

extensions/measurement-tracking/src/panels/PanelStudyBrowserTracking/untrackSeriesModal.tsx

+            Cancel
+          </FooterAction.Secondary>
          <FooterAction.Primary
+            dataCY="confirm-button"


This one too.

jbocce · 2025-12-10T01:35:06Z

platform/ui-next/src/components/DataRow/DataRow.tsx

                isSelected || !isVisible ? 'opacity-100' : 'opacity-0 group-hover:opacity-100'
              }`}
              aria-label={isVisible ? 'Hide' : 'Show'}
+              dataCY="visibilityToggle"


More specific.

jbocce · 2025-12-10T01:36:51Z

platform/ui-next/src/components/Thumbnail/Thumbnail.tsx

-              <div className="text-[11px] font-semibold text-white">{modality}</div>
+              <div
+                className="text-[11px] font-semibold text-white"
+                data-cy="series-modality-label"


If this is for a particular series consider adding something to distinguish it from the other series thumbnail, like series instance uid or something. If the locator though is straightforward, then maybe this is not so big a deal, but something to consider. This comment applies to the other data-cy values in this file too.

jbocce · 2025-12-10T01:44:10Z

tests/pages/OverlayPageObject.ts

+    };
+  }
+
+  get viewport() {


I am a little surprised to see viewport here. Do we need this level of indirection? And why are things like the overlay menu not here? It is a bit confusing. And why not the context menu that is above too? I think I would expect to see everything that appears in a viewport in the viewport PageObject regardless of where it is in the DOM. Something to consider anyway.

jbocce · 2025-12-10T01:46:04Z

tests/pages/RightPanelPageObject.ts

+        };
+      },
+      get contourMenuButton() {
+        const button = page.getByTestId('panelSegmentationWithToolsContour-btn');


This button and the labelMapMenuButton below are NOT part of the segmentation panel, but instead are the tabs/buttons to change what is displayed in the right panel. So please move these accessors directly into the RightPanelPageObject. Or maybe just rename them panelTabButton to make it clear these are the buttons to switch to that panel.

I think we should have two segmentation panel page objects: one for contour and one for label map. They have different tools and are actually two separate panels.

jbocce · 2025-12-10T01:54:07Z

tests/pages/DataOverlayPageObject.ts

+    this.page = page;
+  }
+
+  get button() {


This is weird to have button always get the first dataOverlayMenu. Is it being used? If so rename to make it clear that it is a specific (i.e. the first) instance. If not please remove.

jbocce · 2025-12-10T01:58:40Z

tests/pages/DataOverlayPageObject.ts

+    await this.page.getByTestId(`${to}-SEG`).click();
+  }
+
+  async remove() {


This currently only ever removes the segmentation named (literally) SEGMENTATION. The name of the segmentation overlay should be passed in and templated into the selectors/locators.

jbocce · 2025-12-10T02:04:32Z

tests/utils/assertions.ts

  expectedCount: number;
 }) {
-  await expect(page.locator('css=div[data-cy^="ModalityLoadBadge-"]')).toHaveCount(expectedCount);
+  const overlayPageObject = new OverlayPageObject(page);


Shouldn't this page object be one of those that get created in fixture.ts?

jbocce · 2025-12-10T02:06:51Z

tests/SegmentationPanel.spec.ts

-
-  await expect(page.getByTestId('data-row')).toContainText('Segment One');
-  await expect(page.getByTestId('data-row')).not.toContainText('Segment 1');
+  await expect(segment1.locator).toContainText('Segment One');


I like how this was all simplified. Good work!

ViniciusResende added 10 commits December 3, 2025 14:36

feat: 🎸 adding data-cy to improve automated testing capabilitie

844c80f

feat: 🎸 adding Left Panel Page Object to playwright

64bfe0e

feat: 🎸 adding Data Overlay Page Object to playwright

9291522

feat: 🎸 enhancing Main Toolbar Page Object capabilities

17a562d

feat: 🎸 introducing new Overlay Page Object to playwright

fea16f5

feat: 🎸 added new Right Panel Page Object to playwright

a4dbfdb

feat: 🎸 enhancing Viewport Page Object capabilities

10d390f

feat: 🎸 using new Page Object into utilities

edcb653

feat: 🎸 making new Page Object available on Playwright env

ae8858a

test: 💍 updating all playwright tests to use POMs now

68ace87

ViniciusResende added 4 commits December 3, 2025 15:27

fix: 🐛 Crosshairs increase functionality on Viewport POM

1fc9f7f

fix: 🐛 ambiguous segmentation panel menu button click

695949a

fix: 🐛 mistakenly added additional click to addLengthMeasuremen

a974076

fix: 🐛 Data Overlay POM for specific viewports

c29e957

ViniciusResende requested a review from jbocce December 3, 2025 19:47

ViniciusResende marked this pull request as ready for review December 3, 2025 19:47

jbocce requested changes Dec 10, 2025

View reviewed changes

Uh oh!

tests(Playwright): Full implementation of Page Object Models (POMs) to Playwright tests #5608

Are you sure you want to change the base?

tests(Playwright): Full implementation of Page Object Models (POMs) to Playwright tests #5608

Conversation

ViniciusResende commented Dec 3, 2025

Context

Changes & Results

Testing

Checklist

PR

Code

Public Documentation Updates

Tested Environment

Uh oh!

netlify bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for ohif-dev canceled.

Uh oh!

sedghi commented Dec 3, 2025

Uh oh!

jbocce commented Dec 3, 2025

Uh oh!

jbocce left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

netlify bot commented Dec 3, 2025 •

edited

Loading