Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: flaky anti-pattern getText + assert #28041

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+97 −137
Open

fix: flaky anti-pattern getText + assert #28041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions test/e2e/flask/btc/btc-account-overview.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { strict as assert } from 'assert';
import { Suite } from 'mocha';
import { DEFAULT_BTC_BALANCE } from '../../constants';
import { withBtcAccountSnap } from './common-btc';
Expand Down Expand Up @@ -47,16 +46,15 @@ describe('BTC Account - Overview', function (this: Suite) {
{ title: this.test?.fullTitle() },
async (driver) => {
// Wait for the balance to load up
await driver.delay(2000);

const balanceElement = await driver.findElement(
'.coin-overview__balance',
);
const balanceText = await balanceElement.getText();
await driver.waitForSelector({
css: '.currency-display-component__text',
text: `${DEFAULT_BTC_BALANCE}`,
});

const [balance, unit] = balanceText.split('\n');
assert(Number(balance) === DEFAULT_BTC_BALANCE);
assert(unit === 'BTC');
await driver.waitForSelector({
css: '.currency-display-component__suffix',
text: 'BTC',
});
},
);
});
Expand Down
9 changes: 2 additions & 7 deletions test/e2e/tests/account/incremental-security.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,10 @@ describe('Incremental Security', function () {
await driver.switchToWindow(extension);

// should have the correct amount of eth
let currencyDisplay = await driver.waitForSelector({
await driver.waitForSelector({
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to come up with a different approach because this does not fail if the text is changed for waitForSelector, and it does fail with the previous assertion

css: '.currency-display-component__text',
text: '1',
});
let balance = await currencyDisplay.getText();
assert.strictEqual(balance, '1');

// backs up the Secret Recovery Phrase
// should show a backup reminder
Expand Down Expand Up @@ -160,13 +158,10 @@ describe('Incremental Security', function () {
await driver.clickElement('[data-testid="recovery-phrase-confirm"]');

// should have the correct amount of eth
currencyDisplay = await driver.waitForSelector({
await driver.waitForSelector({
css: '.currency-display-component__text',
text: '1',
});
balance = await currencyDisplay.getText();

assert.strictEqual(balance, '1');

// The previous currencyDisplay wait already serves as the guard here for the assertElementNotPresent
await driver.assertElementNotPresent('.backup-notification');
Expand Down
84 changes: 47 additions & 37 deletions test/e2e/tests/dapp-interactions/signin-with-ethereum.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
const { strict: assert } = require('assert');
const {
defaultGanacheOptions,
withFixtures,
Expand All @@ -13,7 +12,7 @@ const FixtureBuilder = require('../../fixture-builder');
describe('Sign in with ethereum', function () {
it('user should be able to confirm sign in with ethereum', async function () {
const expectedSigninMessageTitle =
'This site is requesting to sign in with Account 1';
'This site is requesting to sign in with';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!!

const expectedSigninMessage =
'I accept the MetaMask Terms of Service: https://community.metamask.io/tos';
const expectedSignInResult =
Expand All @@ -36,48 +35,59 @@ describe('Sign in with ethereum', function () {
await driver.clickElement('#siwe');

// Wait for signature request popup and check the message title
await driver.waitUntilXWindowHandles(3);
let windowHandles = await driver.getAllWindowHandles();
await driver.switchToWindowWithTitle(
WINDOW_TITLES.Dialog,
windowHandles,
);
const title = await driver.findElement(
'.permissions-connect-header__title',
);
const origin = await driver.findElement('.site-origin');
assert.equal(await title.getText(), 'Sign-in request');
assert.equal(await origin.getText(), DAPP_URL);
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.waitForSelector({
css: '.permissions-connect-header__title',
text: 'Sign-in request',
});
await driver.waitForSelector({
css: '.site-origin',
text: DAPP_URL,
});

const displayedMessageTitle = await driver.findElement(
'.permissions-connect-header__subtitle',
);
const account = await driver.findElement(
'.account-list-item__account-name',
);
assert.equal(
`${await displayedMessageTitle.getText()} ${await account.getText()}`,
expectedSigninMessageTitle,
);
await driver.waitForSelector({
css: '.permissions-connect-header__subtitle',
text: expectedSigninMessageTitle,
});
await driver.findElement({
css: '.account-list-item__account-name',
text: 'Account 1',
});

// Check the displayed information in popup content
const [message, url, version, chainId] = await driver.findElements(
'.signature-request-siwe-message__sub-text',
);
assert.equal(await message.getText(), expectedSigninMessage);
assert.equal(await url.getText(), 'https://127.0.0.1:8080');
assert.equal(await version.getText(), '1');
assert.equal(await chainId.getText(), '1');
await driver.waitForSelector({
tag: 'p',
text: expectedSigninMessage,
});

await driver.waitForSelector({
tag: 'p',
text: 'https://127.0.0.1:8080',
});
await driver.waitForSelector({
tag: 'h4',
text: 'Version:',
});
await driver.findElements({
tag: 'p',
text: '1',
});
await driver.waitForSelector({
tag: 'h4',
text: 'Chain ID:',
});

// Click on extension popup to approve signin with ethereum
await driver.clickElement('[data-testid="page-container-footer-next"]');
await driver.waitUntilXWindowHandles(2);
await driver.clickElementAndWaitForWindowToClose(
'[data-testid="page-container-footer-next"]',
);

// Switch back to the dapp and verify the signed result
windowHandles = await driver.getAllWindowHandles();
await driver.switchToWindowWithTitle('E2E Test Dapp', windowHandles);
const result = await driver.findElement('#siweResult');
assert.equal(await result.getText(), expectedSignInResult);
await driver.switchToWindowWithTitle('E2E Test Dapp');
await driver.waitForSelector({
css: '#siweResult',
text: expectedSignInResult,
});
},
);
});
Expand Down
19 changes: 8 additions & 11 deletions test/e2e/tests/hardware-wallets/trezor-sign.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { strict as assert } from 'assert';
import { Suite } from 'mocha';
import { Driver } from '../../webdriver/driver';
import FixtureBuilder from '../../fixture-builder';
Expand Down Expand Up @@ -33,20 +32,18 @@ describe('Trezor Hardware Signatures', function (this: Suite) {
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

await driver.clickElement('.confirm-scroll-to-bottom__button');
await driver.clickElement({ text: 'Confirm', tag: 'button' });
await driver.clickElementAndWaitForWindowToClose({
text: 'Confirm',
tag: 'button',
});

await driver.waitUntilXWindowHandles(2);
await driver.switchToWindowWithTitle(WINDOW_TITLES.TestDApp);
await driver.clickElement('#signTypedDataV4Verify');

const verifyRecoverAddress = await driver.findElement(
'#signTypedDataV4VerifyResult',
);

assert.equal(
await verifyRecoverAddress.getText(),
KNOWN_PUBLIC_KEY_ADDRESSES[0].address.toLocaleLowerCase(),
);
await driver.waitForSelector({
css: '#signTypedDataV4VerifyResult',
text: KNOWN_PUBLIC_KEY_ADDRESSES[0].address.toLocaleLowerCase(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: could we have this in variable and then pass the variable. Its ok if you dont make the change too.
let address = KNOWN_PUBLIC_KEY_ADDRESSES[0].address.toLocaleLowerCase()

});
},
);
});
Expand Down
104 changes: 32 additions & 72 deletions test/e2e/tests/network/add-custom-network.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,30 +454,27 @@ describe('Custom network', function () {
text: 'Add',
});

const [currencySymbol, networkUrl] = await driver.findElements(
'.definition-list dd',
);
assert.equal(
await currencySymbol.getText(),
currencySYMBOL,
'Currency symbol is not correctly displayed',
);
assert.equal(
await networkUrl.getText(),
networkURL,
'Network Url is not correctly displayed',
);
await driver.waitForSelector({
tag: 'dd',
text: currencySYMBOL,
});
Comment on lines +457 to +460
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a more specific data-testid for this element or make be some other approach?
The reason I mention this is - I changed the text for currencySYMBOL from ETH to ET, the test would not fail leading to false negatives.
I also checked for any other unique identifiers but there aren't any so we need to add one in the code.

Copy link
Contributor

@hjetpoluru hjetpoluru Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update on my previous comment - I think changing the data-testid would work.


await driver.waitForSelector({
tag: 'dd',
text: networkURL,
});

await driver.clickElement({ tag: 'a', text: 'See details' });

const networkDetailsLabels = await driver.findElements('dd');
assert.equal(
await networkDetailsLabels[4].getText(),
blockExplorerURL,
'Block Explorer URL is not correct',
);
await driver.waitForSelector({
tag: 'dd',
text: blockExplorerURL,
});

await driver.clickElement({ tag: 'button', text: 'Approve' });
await driver.clickElementAndWaitToDisappear({
tag: 'button',
text: 'Approve',
});

// verify network switched
await driver.waitForSelector(
Expand Down Expand Up @@ -526,16 +523,13 @@ describe('Custom network', function () {
// ===========================================================>

// Go to Edit Menu
const networkMenu = await driver.findElement(
await driver.clickElement(
'[data-testid="network-list-item-options-button-0xa4b1"]',
);

await networkMenu.click();

const deleteButton = await driver.findElement(
await driver.clickElement(
'[data-testid="network-list-item-options-delete"]',
);
deleteButton.click();

await driver.clickElement({
tag: 'button',
Expand All @@ -545,12 +539,9 @@ describe('Custom network', function () {
await driver.clickElement('[data-testid="network-display"]');

// check if arbitrum is on the list of popular network
const popularNetworkArbitrum = await driver.findElement(
await driver.waitForSelector(
'[data-testid="popular-network-0xa4b1"]',
);

const existNetwork = popularNetworkArbitrum !== undefined;
assert.equal(existNetwork, true, 'Network is not deleted');
},
);
});
Expand Down Expand Up @@ -958,19 +949,10 @@ async function checkThatSafeChainsListValidationToggleIsOn(driver) {
await driver.waitForSelector(securityAndPrivacyTabRawLocator);
await driver.clickElement(securityAndPrivacyTabRawLocator);

const useSafeChainsListValidationToggleSelector =
'[data-testid="useSafeChainsListValidation"]';
const useSafeChainsListValidationToggleElement = await driver.waitForSelector(
useSafeChainsListValidationToggleSelector,
);
const useSafeChainsListValidationToggleState =
await useSafeChainsListValidationToggleElement.getText();

assert.equal(
useSafeChainsListValidationToggleState,
'ON',
'Safe chains list validation toggle is off',
);
await driver.findElement({
xpath:
"//div[@data-testid='useSafeChainsListValidation']//label[contains(@class, 'toggle-button') and contains(@class, 'toggle-button--on')]",
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to use an xpath here since there's a long list of toggles, without a way of differentiating to which feature belong to

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job!!


// return to the home screen
const appHeaderSelector = '[data-testid="app-header-logo"]';
Expand Down Expand Up @@ -1080,42 +1062,20 @@ async function toggleOffSafeChainsListValidation(driver) {
await driver.waitForSelector(securityAndPrivacyTabRawLocator);
await driver.clickElement(securityAndPrivacyTabRawLocator);

const useSafeChainsListValidationLabelSelector =
'[data-testid="useSafeChainsListValidation"]';
const useSafeChainsListValidationToggleSelector =
'[data-testid="useSafeChainsListValidation"] .toggle-button > div';

let useSafeChainsListValidationLabelElement = await driver.waitForSelector(
useSafeChainsListValidationLabelSelector,
);

let useSafeChainsListValidationToggleState =
await useSafeChainsListValidationLabelElement.getText();

assert.equal(
useSafeChainsListValidationToggleState,
'ON',
'Safe chains list validation toggle is OFF by default',
);
await driver.waitForSelector({
xpath:
"//div[@data-testid='useSafeChainsListValidation']//label[contains(@class, 'toggle-button') and contains(@class, 'toggle-button--on')]",
});

await driver.clickElement(useSafeChainsListValidationToggleSelector);

await driver.delay(regularDelayMs);

useSafeChainsListValidationLabelElement = await driver.waitForSelector(
useSafeChainsListValidationLabelSelector,
);

useSafeChainsListValidationToggleState =
await useSafeChainsListValidationLabelElement.getText();

assert.equal(
useSafeChainsListValidationToggleState,
'OFF',
'Safe chains list validation toggle is ON',
);

driver.delay(regularDelayMs);
await driver.waitForSelector({
xpath:
"//div[@data-testid='useSafeChainsListValidation']//label[contains(@class, 'toggle-button') and contains(@class, 'toggle-button--off')]",
});

// return to the home screen
const appHeaderSelector = '[data-testid="app-header-logo"]';
Expand Down