From bfaf2c9ed5db3f0976a3bff42ed188bb5e593c82 Mon Sep 17 00:00:00 2001 From: seaona <54408225+seaona@users.noreply.github.com> Date: Thu, 24 Oct 2024 10:42:45 +0200 Subject: [PATCH] fix: flaky anti-pattern getText + assert 2 (#28043) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## **Description** Continuing the work of removing the e2e anti-pattern of finding the element and then asserting its text. There are more occurrences, but this work is split in several PRs, for an easy review and a faster ci. Once all occurrences have been fixed, we'll be able to merge @HowardBraham 's [PR ](https://github.com/MetaMask/metamask-extension/pull/27591)for adding a lint rule which prevents introducing it again. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28043?quickstart=1) ## **Related issues** Fixes: (but doesn't yet closes) https://github.com/MetaMask/metamask-extension/issues/19870 ## **Manual testing steps** 1. Check ci ## **Screenshots/Recordings** ### **Before** ### **After** ## **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. --- .../batch-txs-per-dapp-same-network.spec.js | 25 +++++----------- .../watchAsset-switchChain-watchAsset.spec.js | 21 +++++--------- .../metamask-responsive-ui.spec.js | 21 ++++++++------ .../about-metamask-ui-validation.spec.ts | 13 +++------ test/e2e/tests/settings/address-book.spec.js | 29 ++++++------------- 5 files changed, 40 insertions(+), 69 deletions(-) diff --git a/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js b/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js index bd52558ec67f..c30d6a73c063 100644 --- a/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js +++ b/test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js @@ -1,4 +1,4 @@ -const { strict: assert } = require('assert'); +const { By } = require('selenium-webdriver'); const FixtureBuilder = require('../../fixture-builder'); const { withFixtures, @@ -10,7 +10,6 @@ const { WINDOW_TITLES, defaultGanacheOptions, largeDelayMs, - switchToNotificationWindow, } = require('../../helpers'); const { PAGES } = require('../../webdriver/driver'); @@ -59,7 +58,7 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.delay(regularDelayMs); - await switchToNotificationWindow(driver); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.clickElement({ text: 'Connect', @@ -98,7 +97,7 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.delay(regularDelayMs); - await switchToNotificationWindow(driver, 4); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.clickElement({ text: 'Connect', @@ -134,16 +133,12 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.clickElement('#sendButton'); await driver.clickElement('#sendButton'); - await switchToNotificationWindow(driver, 4); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); - let navigationElement = await driver.findElement( - '.confirm-page-container-navigation', + await driver.waitForSelector( + By.xpath("//div[normalize-space(.)='1 of 2']"), ); - let navigationText = await navigationElement.getText(); - - assert.equal(navigationText.includes('1 of 2'), true); - // Check correct network on confirm tx. await driver.findElement({ css: '[data-testid="network-display"]', @@ -162,14 +157,10 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function await driver.delay(largeDelayMs); await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); - navigationElement = await driver.findElement( - '.confirm-page-container-navigation', + await driver.waitForSelector( + By.xpath("//div[normalize-space(.)='1 of 2']"), ); - navigationText = await navigationElement.getText(); - - assert.equal(navigationText.includes('1 of 2'), true); - // Check correct network on confirm tx. await driver.findElement({ css: '[data-testid="network-display"]', diff --git a/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js b/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js index 1c1baa17fb5a..64ac781a20e0 100644 --- a/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js +++ b/test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js @@ -7,7 +7,6 @@ const { DAPP_URL, regularDelayMs, WINDOW_TITLES, - switchToNotificationWindow, defaultGanacheOptions, } = require('../../helpers'); @@ -45,19 +44,15 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { // Create Token await driver.clickElement({ text: 'Create Token', tag: 'button' }); - await switchToNotificationWindow(driver); - await driver.findClickableElement({ text: 'Confirm', tag: 'button' }); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.clickElement({ text: 'Confirm', tag: 'button' }); // Wait for token address to populate in dapp await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); - await driver.wait(async () => { - const tokenAddressesElement = await driver.findElement( - '#tokenAddresses', - ); - const tokenAddresses = await tokenAddressesElement.getText(); - return tokenAddresses !== ''; - }, 10000); + await driver.waitForSelector({ + css: '#tokenAddresses', + text: '0x581c3C1A2A4EBDE2A0Df29B5cf4c116E42945947', + }); // Watch Asset 1st call await driver.clickElement({ @@ -65,11 +60,9 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { tag: 'button', }); - await driver.waitUntilXWindowHandles(3); await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); // Switch Ethereum Chain - await driver.findClickableElement('#switchEthereumChain'); await driver.clickElement('#switchEthereumChain'); await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp); @@ -83,7 +76,7 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { // Wait for token to show in list of tokens to watch await driver.delay(regularDelayMs); - await switchToNotificationWindow(driver); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); const multipleSuggestedtokens = await driver.findElements( '.confirm-add-suggested-token__token-list-item', @@ -92,7 +85,7 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () { // Confirm only 1 token is present in suggested token list assert.equal(multipleSuggestedtokens.length, 1); - await switchToNotificationWindow(driver); + await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog); await driver.waitUntilXWindowHandles(2); diff --git a/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js b/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js index 446d579630bf..958854a5252c 100644 --- a/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js +++ b/test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js @@ -72,10 +72,10 @@ describe('MetaMask Responsive UI', function () { await driver.clickElement('[data-testid="pin-extension-done"]'); await driver.assertElementNotPresent('.loading-overlay__spinner'); // assert balance - const balance = await driver.findElement( - '[data-testid="eth-overview__primary-currency"]', - ); - assert.ok(/^0\sETH$/u.test(await balance.getText())); + await driver.waitForSelector({ + css: '[data-testid="eth-overview__primary-currency"]', + text: '0', + }); }, ); }); @@ -93,11 +93,14 @@ describe('MetaMask Responsive UI', function () { await driver.navigate(); // Import Secret Recovery Phrase - const restoreSeedLink = await driver.findClickableElement( - '.unlock-page__link', - ); - assert.equal(await restoreSeedLink.getText(), 'Forgot password?'); - await restoreSeedLink.click(); + await driver.waitForSelector({ + tag: 'span', + text: 'Localhost 8545', + }); + await driver.clickElement({ + css: '.unlock-page__link', + text: 'Forgot password?', + }); await driver.pasteIntoField( '[data-testid="import-srp__srp-word-0"]', diff --git a/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts b/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts index abc3c4857957..ed2702fef413 100644 --- a/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts +++ b/test/e2e/tests/settings/about-metamask-ui-validation.spec.ts @@ -68,16 +68,11 @@ describe('Setting - About MetaMask : @no-mmi', function (this: Suite) { ); // verify the version number of the MetaMask - const metaMaskVersion = await driver.findElement( - selectors.metaMaskVersion, - ); - const getVersionNumber = await metaMaskVersion.getText(); const { version } = packageJson; - assert.equal( - getVersionNumber, - version, - 'Meta Mask version is incorrect in the about view section', - ); + await driver.waitForSelector({ + css: selectors.metaMaskVersion, + text: version, + }); // Validating the header text const isHeaderTextPresent = await driver.isElementPresent( diff --git a/test/e2e/tests/settings/address-book.spec.js b/test/e2e/tests/settings/address-book.spec.js index e81bd7c544aa..886dc98c174f 100644 --- a/test/e2e/tests/settings/address-book.spec.js +++ b/test/e2e/tests/settings/address-book.spec.js @@ -39,12 +39,11 @@ describe('Address Book', function () { await driver.clickElement({ css: 'button', text: 'Contacts' }); - const recipientTitle = await driver.findElement( - '.address-list-item__label', - ); + await driver.waitForSelector({ + css: '.address-list-item__label', + text: 'Test Name 1', + }); - const recipientRowTitleString = await recipientTitle.getText(); - assert.equal(recipientRowTitleString, 'Test Name 1'); await driver.clickElement('.address-list-item__label'); await driver.fill('input[placeholder="0"]', '2'); @@ -111,25 +110,15 @@ describe('Address Book', function () { await driver.clickElement('[data-testid="page-container-footer-next"]'); - const recipientUsername = await driver.findElement({ + await driver.waitForSelector({ text: 'Test Name Edit', css: '.address-list-item__label', }); - assert.equal( - await recipientUsername.getText(), - 'Test Name Edit', - 'Username is not edited correctly', - ); - - const recipientAddress = await driver.findElement( - '[data-testid="address-list-item-address"]', - ); - assert.equal( - await recipientAddress.getText(), - shortenAddress('0x74cE91B75935D6Bedc27eE002DeFa566c5946f74'), - 'Recipient address is not edited correctly', - ); + await driver.waitForSelector({ + css: '[data-testid="address-list-item-address"]', + text: shortenAddress('0x74cE91B75935D6Bedc27eE002DeFa566c5946f74'), + }); }, ); });