test: removing race condition for asserting inner values (PR-#2) (#27664

)  ## **Description** This PR fixes an anti-pattern in our e2e tests, where we assert that an element value is equal to a desired value. This opens the door to a race condition where the element is already present, but it does not have the value we want yet, making the assertion to fail. We should find the element by its value, instead of asserting its inner value. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27664?quickstart=1) ## **Related issues** Fixes: #19870 Note: this is the second PR for this work. The first PR was merged [here](#27606) ## **Manual testing steps** 1. Check ci ## **Screenshots/Recordings** n/a ## **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.
MetaMask · Oct 8, 2024 · 83455b8 · 83455b8
1 parent 261e6bf
commit 83455b8
Show file tree

Hide file tree

Showing 11 changed files with 59 additions and 91 deletions.
diff --git a/test/e2e/flask/btc/create-btc-account.spec.ts b/test/e2e/flask/btc/create-btc-account.spec.ts
@@ -135,11 +135,10 @@ describe('Create BTC Account', function (this: Suite) {
         await driver.clickElement(
           '[data-testid="account-options-menu-button"]',
         );
-        const lockButton = await driver.findClickableElement(
-          '[data-testid="global-menu-lock"]',
-        );
-        assert.equal(await lockButton.getText(), 'Lock MetaMask');
-        await lockButton.click();
+        await driver.clickElement({
+          css: '[data-testid="global-menu-lock"]',
+          text: 'Lock MetaMask',
+        });
 
         await driver.clickElement({
           text: 'Forgot password?',

diff --git a/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts b/test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts
@@ -50,11 +50,10 @@ describe('Malicious Confirmation Signature - Bad Domain @no-mmi', function (this
       }: TestSuiteArguments) => {
         await openDappAndTriggerSignature(driver, SignatureType.SIWE_BadDomain);
 
-        await driver.clickElement(
+        await driver.clickElementAndWaitForWindowToClose(
           '[data-testid="confirm-footer-cancel-button"]',
         );
 
-        await driver.waitUntilXWindowHandles(2);
         await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
 
         const rejectionResult = await driver.waitForSelector({

diff --git a/test/e2e/tests/confirmations/signatures/personal-sign.spec.ts b/test/e2e/tests/confirmations/signatures/personal-sign.spec.ts
@@ -74,11 +74,10 @@ describe('Confirmation Signature - Personal Sign @no-mmi', function (this: Suite
       }: TestSuiteArguments) => {
         await openDappAndTriggerSignature(driver, SignatureType.PersonalSign);
 
-        await driver.clickElement(
+        await driver.clickElementAndWaitForWindowToClose(
           '[data-testid="confirm-footer-cancel-button"]',
         );
 
-        await driver.waitUntilXWindowHandles(2);
         await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
 
         const rejectionResult = await driver.waitForSelector({

diff --git a/test/e2e/tests/confirmations/signatures/sign-typed-data-v3.spec.ts b/test/e2e/tests/confirmations/signatures/sign-typed-data-v3.spec.ts
@@ -56,7 +56,6 @@ describe('Confirmation Signature - Sign Typed Data V3 @no-mmi', function (this:
 
         await assertInfoValues(driver);
         await scrollAndConfirmAndAssertConfirm(driver);
-        await driver.delay(1000);
         await assertSignatureConfirmedMetrics({
           driver,
           mockedEndpoints: mockedEndpoints as MockedEndpoint[],
@@ -81,10 +80,9 @@ describe('Confirmation Signature - Sign Typed Data V3 @no-mmi', function (this:
           SignatureType.SignTypedDataV3,
         );
 
-        await driver.clickElement(
+        await driver.clickElementAndWaitForWindowToClose(
           '[data-testid="confirm-footer-cancel-button"]',
         );
-        await driver.delay(1000);
 
         await assertSignatureRejectedMetrics({
           driver,
@@ -141,16 +139,13 @@ async function assertVerifiedResults(driver: Driver, publicAddress: string) {
   await driver.waitUntilXWindowHandles(2);
   await driver.switchToWindowWithTitle('E2E Test Dapp');
   await driver.clickElement('#signTypedDataV3Verify');
-  await driver.delay(500);
-
-  const verifyResult = await driver.findElement('#signTypedDataV3Result');
-  const verifyRecoverAddress = await driver.findElement(
-    '#signTypedDataV3VerifyResult',
-  );
+  await driver.waitForSelector({
+    css: '#signTypedDataV3Result',
+    text: '0x0a22f7796a2a70c8dc918e7e6eb8452c8f2999d1a1eb5ad714473d36270a40d6724472e5609948c778a07216bd082b60b6f6853d6354c731fd8ccdd3a2f4af261b',
+  });
 
-  assert.equal(
-    await verifyResult.getText(),
-    '0x0a22f7796a2a70c8dc918e7e6eb8452c8f2999d1a1eb5ad714473d36270a40d6724472e5609948c778a07216bd082b60b6f6853d6354c731fd8ccdd3a2f4af261b',
-  );
-  assert.equal(await verifyRecoverAddress.getText(), publicAddress);
+  await driver.waitForSelector({
+    css: '#signTypedDataV3VerifyResult',
+    text: publicAddress,
+  });
 }
diff --git a/test/e2e/tests/confirmations/signatures/sign-typed-data-v4.spec.ts b/test/e2e/tests/confirmations/signatures/sign-typed-data-v4.spec.ts
@@ -50,7 +50,6 @@ describe('Confirmation Signature - Sign Typed Data V4 @no-mmi', function (this:
 
         await assertInfoValues(driver);
         await scrollAndConfirmAndAssertConfirm(driver);
-        await driver.delay(1000);
 
         await assertAccountDetailsMetrics(
           driver,
@@ -87,10 +86,9 @@ describe('Confirmation Signature - Sign Typed Data V4 @no-mmi', function (this:
           SignatureType.SignTypedDataV4,
         );
 
-        await driver.clickElement(
+        await driver.clickElementAndWaitForWindowToClose(
           '[data-testid="confirm-footer-cancel-button"]',
         );
-        await driver.delay(1000);
 
         await assertSignatureRejectedMetrics({
           driver,

diff --git a/test/e2e/tests/confirmations/signatures/sign-typed-data.spec.ts b/test/e2e/tests/confirmations/signatures/sign-typed-data.spec.ts
@@ -76,10 +76,9 @@ describe('Confirmation Signature - Sign Typed Data @no-mmi', function (this: Sui
       }: TestSuiteArguments) => {
         await openDappAndTriggerSignature(driver, SignatureType.SignTypedData);
 
-        await driver.clickElement(
+        await driver.clickElementAndWaitForWindowToClose(
           '[data-testid="confirm-footer-cancel-button"]',
         );
-        await driver.delay(1000);
 
         await assertSignatureRejectedMetrics({
           driver,

diff --git a/test/e2e/tests/confirmations/signatures/signature-helpers.ts b/test/e2e/tests/confirmations/signatures/signature-helpers.ts
@@ -218,11 +218,10 @@ export async function clickHeaderInfoBtn(driver: Driver) {
 }
 
 export async function assertHeaderInfoBalance(driver: Driver) {
-  const headerBalanceEl = await driver.findElement(
-    '[data-testid="confirmation-account-details-modal__account-balance"]',
-  );
-  await driver.waitForNonEmptyElement(headerBalanceEl);
-  assert.equal(await headerBalanceEl.getText(), `${WALLET_ETH_BALANCE}\nETH`);
+  await driver.waitForSelector({
+    css: '[data-testid="confirmation-account-details-modal__account-balance"]',
+    text: `${WALLET_ETH_BALANCE} ETH`,
+  });
 }
 
 export async function copyAddressAndPasteWalletAddress(driver: Driver) {

diff --git a/test/e2e/tests/confirmations/signatures/siwe.spec.ts b/test/e2e/tests/confirmations/signatures/siwe.spec.ts
@@ -47,7 +47,6 @@ describe('Confirmation Signature - SIWE @no-mmi', function (this: Suite) {
         await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
         await assertInfoValues(driver);
         await scrollAndConfirmAndAssertConfirm(driver);
-        await driver.delay(1000);
 
         await assertVerifiedSiweMessage(
           driver,
@@ -77,18 +76,16 @@ describe('Confirmation Signature - SIWE @no-mmi', function (this: Suite) {
       }: TestSuiteArguments) => {
         await openDappAndTriggerSignature(driver, SignatureType.SIWE);
 
-        await driver.clickElement(
+        await driver.clickElementAndWaitForWindowToClose(
           '[data-testid="confirm-footer-cancel-button"]',
         );
 
-        await driver.waitUntilXWindowHandles(2);
         await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
 
-        const rejectionResult = await driver.findElement('#siweResult');
-        assert.equal(
-          await rejectionResult.getText(),
-          'Error: User rejected the request.',
-        );
+        await driver.waitForSelector({
+          css: '#siweResult',
+          text: 'Error: User rejected the request.',
+        });
         await assertSignatureRejectedMetrics({
           driver,
           mockedEndpoints: mockedEndpoints as MockedEndpoint[],
@@ -119,6 +116,8 @@ async function assertVerifiedSiweMessage(driver: Driver, message: string) {
   await driver.waitUntilXWindowHandles(2);
   await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
 
-  const verifySigUtil = await driver.findElement('#siweResult');
-  assert.equal(await verifySigUtil.getText(), message);
+  await driver.waitForSelector({
+    css: '#siweResult',
+    text: message,
+  });
 }
diff --git a/test/e2e/tests/dapp-interactions/dapp-tx-edit.spec.js b/test/e2e/tests/dapp-interactions/dapp-tx-edit.spec.js
@@ -1,10 +1,9 @@
-const { strict: assert } = require('assert');
 const {
   defaultGanacheOptions,
-  withFixtures,
+  logInWithBalanceValidation,
   openDapp,
-  unlockWallet,
   WINDOW_TITLES,
+  withFixtures,
 } = require('../../helpers');
 const { SMART_CONTRACTS } = require('../../seeder/smart-contracts');
 const FixtureBuilder = require('../../fixture-builder');
@@ -26,32 +25,22 @@ describe('Editing confirmations of dapp initiated contract interactions', functi
         const contractAddress = await contractRegistry.getContractAddress(
           smartContract,
         );
-        await unlockWallet(driver);
+        await logInWithBalanceValidation(driver);
 
         // deploy contract
         await openDapp(driver, contractAddress);
         // wait for deployed contract, calls and confirms a contract method where ETH is sent
         await driver.findClickableElement('#deployButton');
         await driver.clickElement('#depositButton');
-        await driver.waitUntilXWindowHandles(3);
-        const windowHandles = await driver.getAllWindowHandles();
 
-        await driver.switchToWindowWithTitle(
-          WINDOW_TITLES.Dialog,
-          windowHandles,
-        );
+        await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
         await driver.waitForSelector({
           css: '.confirm-page-container-summary__action__name',
           text: 'Deposit',
         });
-        const editTransactionButton = await driver.isElementPresentAndVisible(
+        await driver.assertElementNotPresent(
           '[data-testid="confirm-page-back-edit-button"]',
         );
-        assert.equal(
-          editTransactionButton,
-          false,
-          `Edit transaction button should not be visible on a contract interaction created by a dapp`,
-        );
       },
     );
   });
@@ -68,29 +57,19 @@ describe('Editing confirmations of dapp initiated contract interactions', functi
         title: this.test.fullTitle(),
       },
       async ({ driver }) => {
-        await unlockWallet(driver);
+        await logInWithBalanceValidation(driver);
 
         await openDapp(driver);
         await driver.clickElement('#sendButton');
-        await driver.waitUntilXWindowHandles(3);
-        const windowHandles = await driver.getAllWindowHandles();
 
-        await driver.switchToWindowWithTitle(
-          WINDOW_TITLES.Dialog,
-          windowHandles,
-        );
+        await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
         await driver.waitForSelector({
           css: '.confirm-page-container-summary__action__name',
           text: 'Sending ETH',
         });
-        const editTransactionButton = await driver.isElementPresentAndVisible(
+        await driver.assertElementNotPresent(
           '[data-testid="confirm-page-back-edit-button"]',
         );
-        assert.equal(
-          editTransactionButton,
-          false,
-          `Edit transaction button should not be visible on a simple send transaction created by a dapp`,
-        );
       },
     );
   });

diff --git a/test/e2e/tests/dapp-interactions/encrypt-decrypt.spec.js b/test/e2e/tests/dapp-interactions/encrypt-decrypt.spec.js
@@ -1,4 +1,3 @@
-const { strict: assert } = require('assert');
 const {
   defaultGanacheOptions,
   withFixtures,
@@ -46,11 +45,10 @@ async function decryptMessage(driver) {
 
 async function verifyDecryptedMessageMM(driver, message) {
   await driver.clickElement({ text: 'Decrypt message', tag: 'div' });
-  const notificationMessage = await driver.isElementPresent({
+  await driver.waitForSelector({
     text: message,
     tag: 'div',
   });
-  assert.equal(notificationMessage, true);
   await driver.clickElement({ text: 'Decrypt', tag: 'button' });
 }
 
@@ -91,10 +89,10 @@ describe('Encrypt Decrypt', function () {
         await decryptMessage(driver);
 
         // Account balance is converted properly
-        const decryptAccountBalanceLabel = await driver.findElement(
-          '.request-decrypt-message__balance-value',
-        );
-        assert.equal(await decryptAccountBalanceLabel.getText(), '25 ETH');
+        await driver.waitForSelector({
+          css: '.request-decrypt-message__balance-value',
+          text: '25 ETH',
+        });
         // Verify message in MetaMask Notification
         await verifyDecryptedMessageMM(driver, message);
 
@@ -187,10 +185,10 @@ describe('Encrypt Decrypt', function () {
           text: 'Request encryption public key',
         });
         // Account balance is converted properly
-        const accountBalanceLabel = await driver.findElement(
-          '.request-encryption-public-key__balance-value',
-        );
-        assert.equal(await accountBalanceLabel.getText(), '25 ETH');
+        await driver.waitForSelector({
+          css: '.request-encryption-public-key__balance-value',
+          text: '25 ETH',
+        });
       },
     );
   });
@@ -230,10 +228,10 @@ describe('Encrypt Decrypt', function () {
         });
 
         // Account balance is converted properly
-        const accountBalanceLabel = await driver.findElement(
-          '.request-encryption-public-key__balance-value',
-        );
-        assert.equal(await accountBalanceLabel.getText(), '25 ETH');
+        await driver.waitForSelector({
+          css: '.request-encryption-public-key__balance-value',
+          text: '25 ETH',
+        });
       },
     );
   });

diff --git a/test/e2e/tests/dapp-interactions/failing-contract.spec.js b/test/e2e/tests/dapp-interactions/failing-contract.spec.js
@@ -46,11 +46,13 @@ describe('Failing contract interaction ', function () {
         // display warning when transaction is expected to fail
         const warningText =
           'We were not able to estimate gas. There might be an error in the contract and this transaction may fail.';
-        const warning = await driver.findElement('.mm-banner-alert .mm-text');
+        await driver.waitForSelector({
+          css: '.mm-banner-alert .mm-text',
+          text: warningText,
+        });
         const confirmButton = await driver.findElement(
           '[data-testid="page-container-footer-next"]',
         );
-        assert.equal(await warning.getText(), warningText);
         assert.equal(await confirmButton.isEnabled(), false);
 
         // dismiss warning and confirm the transaction
@@ -113,11 +115,13 @@ describe('Failing contract interaction on non-EIP1559 network', function () {
         // display warning when transaction is expected to fail
         const warningText =
           'We were not able to estimate gas. There might be an error in the contract and this transaction may fail.';
-        const warning = await driver.findElement('.mm-banner-alert .mm-text');
+        await driver.waitForSelector({
+          css: '.mm-banner-alert .mm-text',
+          text: warningText,
+        });
         const confirmButton = await driver.findElement(
           '[data-testid="page-container-footer-next"]',
         );
-        assert.equal(await warning.getText(), warningText);
         assert.equal(await confirmButton.isEnabled(), false);
 
         // dismiss warning and confirm the transaction