Skip to content

Commit b7a79d0

Browse files
seaonaseaona
authored
fix: flaky anti-pattern getText + assert 2 (#28043)
<!-- 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** 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 ](#27591 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) #19870 ## **Manual testing steps** 1. Check ci ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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.
1 parent fa97321 commit b7a79d0

5 files changed

+40
-69
lines changed

‎test/e2e/tests/request-queuing/batch-txs-per-dapp-same-network.spec.js

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { strict: assert } = require('assert');
1+
const { By } = require('selenium-webdriver');
22
const FixtureBuilder = require('../../fixture-builder');
33
const {
44
withFixtures,
@@ -10,7 +10,6 @@ const {
1010
WINDOW_TITLES,
1111
defaultGanacheOptions,
1212
largeDelayMs,
13-
switchToNotificationWindow,
1413
} = require('../../helpers');
1514
const { PAGES } = require('../../webdriver/driver');
1615

@@ -59,7 +58,7 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function
5958

6059
await driver.delay(regularDelayMs);
6160

62-
await switchToNotificationWindow(driver);
61+
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
6362

6463
await driver.clickElement({
6564
text: 'Connect',
@@ -98,7 +97,7 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function
9897

9998
await driver.delay(regularDelayMs);
10099

101-
await switchToNotificationWindow(driver, 4);
100+
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
102101

103102
await driver.clickElement({
104103
text: 'Connect',
@@ -134,16 +133,12 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function
134133
await driver.clickElement('#sendButton');
135134
await driver.clickElement('#sendButton');
136135

137-
await switchToNotificationWindow(driver, 4);
136+
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
138137

139-
let navigationElement = await driver.findElement(
140-
'.confirm-page-container-navigation',
138+
await driver.waitForSelector(
139+
By.xpath("//div[normalize-space(.)='1 of 2']"),
141140
);
142141

143-
let navigationText = await navigationElement.getText();
144-
145-
assert.equal(navigationText.includes('1 of 2'), true);
146-
147142
// Check correct network on confirm tx.
148143
await driver.findElement({
149144
css: '[data-testid="network-display"]',
@@ -162,14 +157,10 @@ describe('Request Queuing for Multiple Dapps and Txs on same networks', function
162157
await driver.delay(largeDelayMs);
163158
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
164159

165-
navigationElement = await driver.findElement(
166-
'.confirm-page-container-navigation',
160+
await driver.waitForSelector(
161+
By.xpath("//div[normalize-space(.)='1 of 2']"),
167162
);
168163

169-
navigationText = await navigationElement.getText();
170-
171-
assert.equal(navigationText.includes('1 of 2'), true);
172-
173164
// Check correct network on confirm tx.
174165
await driver.findElement({
175166
css: '[data-testid="network-display"]',

‎test/e2e/tests/request-queuing/watchAsset-switchChain-watchAsset.spec.js

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ const {
77
DAPP_URL,
88
regularDelayMs,
99
WINDOW_TITLES,
10-
switchToNotificationWindow,
1110
defaultGanacheOptions,
1211
} = require('../../helpers');
1312

@@ -45,31 +44,25 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () {
4544

4645
// Create Token
4746
await driver.clickElement({ text: 'Create Token', tag: 'button' });
48-
await switchToNotificationWindow(driver);
49-
await driver.findClickableElement({ text: 'Confirm', tag: 'button' });
47+
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
5048
await driver.clickElement({ text: 'Confirm', tag: 'button' });
5149

5250
// Wait for token address to populate in dapp
5351
await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
54-
await driver.wait(async () => {
55-
const tokenAddressesElement = await driver.findElement(
56-
'#tokenAddresses',
57-
);
58-
const tokenAddresses = await tokenAddressesElement.getText();
59-
return tokenAddresses !== '';
60-
}, 10000);
52+
await driver.waitForSelector({
53+
css: '#tokenAddresses',
54+
text: '0x581c3C1A2A4EBDE2A0Df29B5cf4c116E42945947',
55+
});
6156

6257
// Watch Asset 1st call
6358
await driver.clickElement({
6459
text: 'Add Token(s) to Wallet',
6560
tag: 'button',
6661
});
6762

68-
await driver.waitUntilXWindowHandles(3);
6963
await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
7064

7165
// Switch Ethereum Chain
72-
await driver.findClickableElement('#switchEthereumChain');
7366
await driver.clickElement('#switchEthereumChain');
7467

7568
await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
@@ -83,7 +76,7 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () {
8376
// Wait for token to show in list of tokens to watch
8477
await driver.delay(regularDelayMs);
8578

86-
await switchToNotificationWindow(driver);
79+
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
8780

8881
const multipleSuggestedtokens = await driver.findElements(
8982
'.confirm-add-suggested-token__token-list-item',
@@ -92,7 +85,7 @@ describe('Request Queue WatchAsset -> SwitchChain -> WatchAsset', function () {
9285
// Confirm only 1 token is present in suggested token list
9386
assert.equal(multipleSuggestedtokens.length, 1);
9487

95-
await switchToNotificationWindow(driver);
88+
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
9689

9790
await driver.waitUntilXWindowHandles(2);
9891

‎test/e2e/tests/responsive-ui/metamask-responsive-ui.spec.js

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ describe('MetaMask Responsive UI', function () {
7272
await driver.clickElement('[data-testid="pin-extension-done"]');
7373
await driver.assertElementNotPresent('.loading-overlay__spinner');
7474
// assert balance
75-
const balance = await driver.findElement(
76-
'[data-testid="eth-overview__primary-currency"]',
77-
);
78-
assert.ok(/^0\sETH$/u.test(await balance.getText()));
75+
await driver.waitForSelector({
76+
css: '[data-testid="eth-overview__primary-currency"]',
77+
text: '0',
78+
});
7979
},
8080
);
8181
});
@@ -93,11 +93,14 @@ describe('MetaMask Responsive UI', function () {
9393
await driver.navigate();
9494

9595
// Import Secret Recovery Phrase
96-
const restoreSeedLink = await driver.findClickableElement(
97-
'.unlock-page__link',
98-
);
99-
assert.equal(await restoreSeedLink.getText(), 'Forgot password?');
100-
await restoreSeedLink.click();
96+
await driver.waitForSelector({
97+
tag: 'span',
98+
text: 'Localhost 8545',
99+
});
100+
await driver.clickElement({
101+
css: '.unlock-page__link',
102+
text: 'Forgot password?',
103+
});
101104

102105
await driver.pasteIntoField(
103106
'[data-testid="import-srp__srp-word-0"]',

‎test/e2e/tests/settings/about-metamask-ui-validation.spec.ts

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,16 +68,11 @@ describe('Setting - About MetaMask : @no-mmi', function (this: Suite) {
6868
);
6969

7070
// verify the version number of the MetaMask
71-
const metaMaskVersion = await driver.findElement(
72-
selectors.metaMaskVersion,
73-
);
74-
const getVersionNumber = await metaMaskVersion.getText();
7571
const { version } = packageJson;
76-
assert.equal(
77-
getVersionNumber,
78-
version,
79-
'Meta Mask version is incorrect in the about view section',
80-
);
72+
await driver.waitForSelector({
73+
css: selectors.metaMaskVersion,
74+
text: version,
75+
});
8176

8277
// Validating the header text
8378
const isHeaderTextPresent = await driver.isElementPresent(

‎test/e2e/tests/settings/address-book.spec.js

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,11 @@ describe('Address Book', function () {
3939

4040
await driver.clickElement({ css: 'button', text: 'Contacts' });
4141

42-
const recipientTitle = await driver.findElement(
43-
'.address-list-item__label',
44-
);
42+
await driver.waitForSelector({
43+
css: '.address-list-item__label',
44+
text: 'Test Name 1',
45+
});
4546

46-
const recipientRowTitleString = await recipientTitle.getText();
47-
assert.equal(recipientRowTitleString, 'Test Name 1');
4847
await driver.clickElement('.address-list-item__label');
4948

5049
await driver.fill('input[placeholder="0"]', '2');
@@ -111,25 +110,15 @@ describe('Address Book', function () {
111110

112111
await driver.clickElement('[data-testid="page-container-footer-next"]');
113112

114-
const recipientUsername = await driver.findElement({
113+
await driver.waitForSelector({
115114
text: 'Test Name Edit',
116115
css: '.address-list-item__label',
117116
});
118117

119-
assert.equal(
120-
await recipientUsername.getText(),
121-
'Test Name Edit',
122-
'Username is not edited correctly',
123-
);
124-
125-
const recipientAddress = await driver.findElement(
126-
'[data-testid="address-list-item-address"]',
127-
);
128-
assert.equal(
129-
await recipientAddress.getText(),
130-
shortenAddress('0x74cE91B75935D6Bedc27eE002DeFa566c5946f74'),
131-
'Recipient address is not edited correctly',
132-
);
118+
await driver.waitForSelector({
119+
css: '[data-testid="address-list-item-address"]',
120+
text: shortenAddress('0x74cE91B75935D6Bedc27eE002DeFa566c5946f74'),
121+
});
133122
},
134123
);
135124
});

0 commit comments

Comments
 (0)