Skip to content

Commit

Permalink
fix: flaky test `Request-queue UI changes should gracefully handle de…
Browse files Browse the repository at this point in the history
…leted network @no-mmi.` (#27393)

<!--
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**

The problem is that we are trying to get state:

`window.stateHooks?.getCleanAppState?.(),`

before it is fully loaded, making the `metamask` property being
undefined

```
TypeError: Cannot destructure property 'metamask' of 'notificationWindowState' as it is null.
  (Ran on CircleCI Node 0 of 20, Job test-e2e-chrome)
    at switchToDialogPopoverValidateDetails (test/e2e/tests/request-queuing/ui.spec.js:111:15)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async /home/circleci/project/test/e2e/tests/request-queuing/ui.spec.js:434:9
    at async withFixtures (test/e2e/helpers.js:217:5)
    at async Context.<anonymous> (test/e2e/tests/request-queuing/ui.spec.js:369:5)
```

We could add a retry logic, but I found that if we make sure controllers
are loaded, then test passes 100%, so I think there's no need for retry
logic.

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

## **Related issues**

Fixes: [Request-queue UI changes should gracefully handle
de...](#27304)

## **Manual testing steps**

1. Check ci
2. Run test locally `yarn test:e2e:single
test/e2e/tests/request-queuing/ui.spec.js --browser=chrome
--leave-running=true`

## **Screenshots/Recordings**

![Screenshot from 2024-09-25
15-59-06](https://github.com/user-attachments/assets/525964af-9ec6-4982-a4a3-5c0c7c3e8185)

See how you some UI is loaded before the controller css class is added /
controllers are ready and state is ready.


https://github.com/user-attachments/assets/5f8a9777-d09a-439c-b6c6-4afd68094189




## **Pre-merge author checklist**

- [x] 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).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] 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 Sep 25, 2024
1 parent b46a642 commit ed1e0bf
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
1 change: 1 addition & 0 deletions test/e2e/tests/request-queuing/ui.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ async function switchToDialogPopoverValidateDetails(driver, expectedDetails) {
});

// Get state details
await driver.waitForControllersLoaded();
const notificationWindowState = await driver.executeScript(() =>
window.stateHooks?.getCleanAppState?.(),
);
Expand Down
21 changes: 17 additions & 4 deletions test/e2e/webdriver/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -777,14 +777,27 @@ class Driver {
const response = await this.driver.get(`${this.extensionUrl}/${page}.html`);
// Wait for asynchronous JavaScript to load
if (waitForControllers) {
await this.driver.wait(
until.elementLocated(this.buildLocator('.controller-loaded')),
10 * 1000,
);
await this.waitForControllersLoaded();
}
return response;
}

/**
* Waits for the controllers to be loaded on the page.
*
* This function waits until an element with the class 'controller-loaded' is located,
* indicating that the controllers have finished loading.
*
* @returns {Promise<void>} A promise that resolves when the controllers are loaded.
* @throws {Error} Will throw an error if the element is not located within the timeout period.
*/
async waitForControllersLoaded() {
await this.driver.wait(
until.elementLocated(this.buildLocator('.controller-loaded')),
10 * 1000,
);
}

/**
* Retrieves the current URL of the browser session.
*
Expand Down

0 comments on commit ed1e0bf

Please sign in to comment.