Skip to content

Commit

Permalink
fix: flaky test `Full-size View Setting @no-mmi opens the extension i…
Browse files Browse the repository at this point in the history
…n popup view when opened from a dapp after enabling it in Advanced Settings` (#25295)

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

This PR fixes the flaky test `Full-size View Setting @no-mmi opens the
extension in popup view when opened from a dapp after enabling it in
Advanced Settings`.
The problem is that it can take some time to load the window title,
making the test fail.
Now, we've added some retry logic, to prevent such cases. See video
below

This is a follow-up on @hjetpoluru 's work and investigation, so all
credit to her 🙌

#25150

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

## **Related issues**

Fixes: #24642

## **Manual testing steps**

1.  Check ci
2. Run test locally multiple times

## **Screenshots/Recordings**
See how now we enter in the retry loop and the test passes


https://github.com/MetaMask/metamask-extension/assets/54408225/d65d3419-0c95-4601-a5f2-5e6ab3ba4037




## **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 Jun 13, 2024
1 parent 04642d2 commit 8141ff2
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions test/e2e/webdriver/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -861,11 +861,21 @@ class Driver {
* Retrieves the title of the window tab with the given handle ID.
*
* @param {int} handlerId - unique ID for the tab whose title is needed.
* @returns {Promise<string>} promise resolving to the tab title after command completion
* @param {number} retries - Number of times to retry fetching the title if not immediately available.
* @param {number} interval - Time in milliseconds to wait between retries.
* @returns {Promise<string>} Promise resolving to the tab title after command completion.
* @throws {Error} Throws an error if the window title does not load within the specified retries.
*/
async getWindowTitleByHandlerId(handlerId) {
async getWindowTitleByHandlerId(handlerId, retries = 5, interval = 1000) {
await this.driver.switchTo().window(handlerId);
return await this.driver.getTitle();
for (let attempt = 1; attempt <= retries; attempt++) {
const title = await this.driver.getTitle();
if (title) {
return title;
}
await new Promise((resolve) => setTimeout(resolve, interval));
}
throw new Error('Window title did not load within the specified retries');
}

/**
Expand Down

0 comments on commit 8141ff2

Please sign in to comment.