Skip to content

Commit

Permalink
fix: snap flakiness on installSnapSimpleKeyring function (#26039)
Browse files Browse the repository at this point in the history
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

There is a race condition where we click `Create Account` on the snap
dapp, before the popup for Connecting hasn't been closed (see video
below). This causes that the click for Create Account has no effect and
the dialog never opens, with the error:
`Error: No window with title: MetaMask Dialog` - example of failure
[here](https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/92347/workflows/a12a664d-4eb8-4c49-a57c-1e58cb49361f/jobs/3437511/tests)
in the Remove Account Snap

The fix is to wait until the popup is closed before continue.
This source of flakiness can affect several snap tests (all the tests
that use the `installSnapSimpleKeyring` function).

Note: this does not mitigate the current Snap Account issues that we see
on ci, but does fix a source of flakiness introduced in this function
(I've been able to reproduce the flakiness locally and saw mitigated
after the fix, for the Remove Account test).

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/26039?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Check ci

## **Screenshots/Recordings**
Notice how, we start interacting with the dapp again, but the popup
hasn't been closed yet (second ~22)


https://github.com/user-attachments/assets/fbfe4281-9b1a-4311-bca3-6e746e965109




<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.
  • Loading branch information
seaona authored Jul 24, 2024
1 parent 335c497 commit f861486
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 0 deletions.
3 changes: 3 additions & 0 deletions test/e2e/accounts/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ export async function installSnapSimpleKeyring(
tag: 'button',
});

// Wait until popup is closed before proceeding
await driver.waitUntilXWindowHandles(2);

await driver.switchToWindowWithTitle(WINDOW_TITLES.SnapSimpleKeyringDapp);

await driver.waitForSelector({
Expand Down
9 changes: 9 additions & 0 deletions test/e2e/accounts/create-snap-account.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ describe('Create Snap Account', function (this: Suite) {
tag: 'button',
});

// Wait until popup is closed before proceeding
await driver.waitUntilXWindowHandles(2);

// move back to the Snap window to test the create account flow
await driver.switchToWindowWithTitle(
WINDOW_TITLES.SnapSimpleKeyringDapp,
Expand Down Expand Up @@ -128,6 +131,9 @@ describe('Create Snap Account', function (this: Suite) {
tag: 'button',
});

// Wait until popup is closed before proceeding
await driver.waitUntilXWindowHandles(2);

// move back to the Snap window to test the create account flow
await driver.switchToWindowWithTitle(
WINDOW_TITLES.SnapSimpleKeyringDapp,
Expand Down Expand Up @@ -220,6 +226,9 @@ describe('Create Snap Account', function (this: Suite) {
tag: 'button',
});

// Wait until popup is closed before proceeding
await driver.waitUntilXWindowHandles(2);

// move back to the Snap window to test the create account flow
await driver.switchToWindowWithTitle(
WINDOW_TITLES.SnapSimpleKeyringDapp,
Expand Down

0 comments on commit f861486

Please sign in to comment.