From 8141ff2b29637ce5817c6595f6ec7b1bbaa41302 Mon Sep 17 00:00:00 2001 From: seaona <54408225+seaona@users.noreply.github.com> Date: Thu, 13 Jun 2024 20:53:11 +0200 Subject: [PATCH] fix: 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` (#25295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **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 :raised_hands: https://github.com/MetaMask/metamask-extension/pull/25150 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/25295?quickstart=1) ## **Related issues** Fixes: https://github.com/MetaMask/metamask-extension/issues/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. --- test/e2e/webdriver/driver.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/test/e2e/webdriver/driver.js b/test/e2e/webdriver/driver.js index 6815460f3091..98e55ee28111 100644 --- a/test/e2e/webdriver/driver.js +++ b/test/e2e/webdriver/driver.js @@ -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} 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} 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'); } /**