test: Delete site E2E case supports native dialogs - #186
Conversation
Playwright lacks support for interacting with native dialogs, so we mock the dialog module to simulate the user clicking the "Delete site" confirmation button with "Delete site files from my computer" checked. See: microsoft/playwright#2143 See: microsoft/playwright#8278 (comment)
Combine `toBeVisible`'s interval with the ability to assert the precence of one of multiple elements to ensure the UI is ready before conditionally proceeding with the onboarding steps. This reduces the likelihood that the UI is not ready when the test interactions begin and also allows the Sites tests to succeed regardless of whether sites already exists when the tests begin to run.
| // This fails, not sure why... | ||
| // return this.page.getByTestId( 'onboarding' ); |
There was a problem hiding this comment.
My guess is that the unexpected failure may have related to attempting queries before the UI was visible. With the new approach proposed in these changes, the original query appears to work consistently.
|
|
||
| get continueButton() { | ||
| return this.locator.getByRole( 'button', { name: 'Continue' } ); | ||
| return this.locator.getByRole( 'button', { name: 'Add site' } ); |
| const sidebar = new MainSidebar( onboardingMainWindow ); | ||
|
|
||
| // Await UI visibility before proceeding. | ||
| await expect( onboarding.heading.or( sidebar.addSiteButton ) ).toBeVisible(); |
There was a problem hiding this comment.
By relying upon toBeVisible's interval-based query and the or query, this approach should guarantee the app UI is visible before proceeding with interactions.
derekblank
left a comment
derekblank
left a comment
There was a problem hiding this comment.
LGTM. I observed the e2e tests have passed (both locally and on CI), and the code changes make sense. 🚀
|
@dcalhoun changes look good and work fine. It caught me at first as I couldn't make e2e work locally today, either in this branch or trunk. I noticed that Studio was using x64 binary, and then I realized I needed to clean |
3 participants
Fixes https://github.com/Automattic/dotcom-forge/issues/7236.
Proposed Changes
support interactions with this OS UI.
Testing Instructions
N/A, no user-facing changes.
Pre-merge Checklist