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.

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

feat: added test network as selected network if globally selected for connection Request #27980

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 0 additions & 14 deletions test/e2e/api-specs/ConfirmationRejectionRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,6 @@ export class ConfirmationsRejectRule implements Rule {
tag: 'button',
});

const editButtons = await this.driver.findElements(
'[data-testid="edit"]',
);
await editButtons[1].click();

await this.driver.clickElement({
text: 'Localhost 8545',
tag: 'p',
});

await this.driver.clickElement(
'[data-testid="connect-more-chains-button"]',
);

Comment on lines -76 to -89
Copy link
Member Author

Choose a reason for hiding this comment

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

Not needed anymore since testnet is now included in the connections request

const screenshotTwo = await this.driver.driver.takeScreenshot();
call.attachments.push({
type: 'image',
Expand Down
10 changes: 0 additions & 10 deletions test/e2e/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -783,16 +783,6 @@ const connectToDapp = async (driver) => {

await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

const editButtons = await driver.findElements('[data-testid="edit"]');
await editButtons[1].click();

await driver.clickElement({
text: 'Localhost 8545',
tag: 'p',
});

await driver.clickElement('[data-testid="connect-more-chains-button"]');

await driver.clickElementAndWaitForWindowToClose({
text: 'Connect',
tag: 'button',
Expand Down
119 changes: 111 additions & 8 deletions test/e2e/json-rpc/switchEthereumChain.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,52 @@ describe('Switch Ethereum Chain for two dapps', function () {
});

await driver.switchToWindowWithUrl(DAPP_ONE_URL);
// Switch to Dapp One and connect it
await driver.switchToWindowWithUrl(DAPP_URL);
await driver.clickElement({ text: 'Connect', tag: 'button' });

await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

await driver.clickElementAndWaitForWindowToClose({
text: 'Connect',
tag: 'button',
});

// Switch to MM and disconnect localhost 8545 for Dapp One

await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);
await driver.clickElement(
'[data-testid ="account-options-menu-button"]',
);
await driver.clickElement({
text: 'All Permissions',
tag: 'div',
});
await driver.clickElementAndWaitToDisappear({
text: 'Got it',
tag: 'button',
});
await driver.clickElement({
text: '127.0.0.1:8080',
tag: 'p',
});
const editButtons = await driver.findElements('[data-testid="edit"]');

assert.ok(editButtons.length > 0, 'Edit buttons are available');

await editButtons[1].click();

// Disconnect Mainnet
await driver.clickElement({
text: 'Localhost 8546',
tag: 'p',
});

await driver.clickElement('[data-testid="connect-more-chains-button"]');
// Switch to Dapp Two
await driver.switchToWindowWithUrl(DAPP_ONE_URL);
Comment on lines +161 to +206
Copy link
Contributor

Choose a reason for hiding this comment

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

thoughts on the connection happening right after line 147? You also had a nice pattern in the other test of deselecting a network during the connection flow which would be great to use here instead of opening the wallet and disconnecting the chain after connecting it

// Initiate send transaction on Dapp two
await driver.clickElement('#sendButton');
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
Expand All @@ -174,15 +219,13 @@ describe('Switch Ethereum Chain for two dapps', function () {
const switchEthereumChainRequest = JSON.stringify({
jsonrpc: '2.0',
method: 'wallet_switchEthereumChain',
params: [{ chainId: '0x539' }],
params: [{ chainId: '0x53a' }],
});

// Initiate switchEthereumChain on Dapp One
await driver.executeScript(
`window.ethereum.request(${switchEthereumChainRequest})`,
);

// Switch to tx and confirm send tx.
await switchToNotificationWindow(driver, 4);
await driver.findClickableElements({
text: 'Confirm',
Expand All @@ -192,7 +235,6 @@ describe('Switch Ethereum Chain for two dapps', function () {
text: 'Confirm',
tag: 'button',
});

// Delay here after notification for second notification popup for switchEthereumChain
await driver.delay(1000);

Expand All @@ -203,7 +245,13 @@ describe('Switch Ethereum Chain for two dapps', function () {
text: 'Confirm',
tag: 'button',
});
await driver.clickElement({ text: 'Confirm', tag: 'button' });
await driver.clickElementAndWaitForWindowToClose({
text: 'Confirm',
tag: 'button',
});
await driver.delay(1000);
await driver.switchToWindowWithUrl(DAPP_URL);
await driver.findElement({ css: '#chainId', text: '0x53a' });
},
);
});
Expand Down Expand Up @@ -278,7 +326,36 @@ describe('Switch Ethereum Chain for two dapps', function () {
text: 'Connect',
tag: 'button',
});
await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);
await driver.clickElement(
'[data-testid ="account-options-menu-button"]',
);
await driver.clickElement({ text: 'All Permissions', tag: 'div' });
await driver.clickElementAndWaitToDisappear({
text: 'Got it',
tag: 'button',
});
await driver.clickElement({
text: '127.0.0.1:8081',
tag: 'p',
});
const editButtons = await driver.findElements('[data-testid="edit"]');

// Ensure there are edit buttons
assert.ok(editButtons.length > 0, 'Edit buttons are available');

// Click the first (0th) edit button
await editButtons[1].click();

// Disconnect Mainnet
await driver.clickElement({
text: 'Localhost 8545',
tag: 'p',
});

await driver.clickElement('[data-testid="connect-more-chains-button"]');
Comment on lines +329 to +358
Copy link
Contributor

Choose a reason for hiding this comment

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

same here, would be nice to just not connect Dapp Two to 0x539 during the connection flow itself rather than disconnecting it afterwards

await driver.switchToWindow(dappTwo);
assert.equal(await driver.getCurrentUrl(), `${DAPP_ONE_URL}/`);

Expand All @@ -293,14 +370,11 @@ describe('Switch Ethereum Chain for two dapps', function () {
await driver.executeScript(
`window.ethereum.request(${switchEthereumChainRequest})`,
);

// Switch to notification of switchEthereumChain
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.findClickableElements({
text: 'Confirm',
tag: 'button',
});

// Switch back to dapp one
await driver.switchToWindow(dappOne);
assert.equal(await driver.getCurrentUrl(), `${DAPP_URL}/`);
Expand Down Expand Up @@ -401,7 +475,36 @@ describe('Switch Ethereum Chain for two dapps', function () {
text: 'Connect',
tag: 'button',
});
await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);
await driver.clickElement(
'[data-testid ="account-options-menu-button"]',
);
await driver.clickElement({ text: 'All Permissions', tag: 'div' });
await driver.clickElementAndWaitToDisappear({
text: 'Got it',
tag: 'button',
});
await driver.clickElement({
text: '127.0.0.1:8081',
tag: 'p',
});
const editButtons = await driver.findElements('[data-testid="edit"]');

// Ensure there are edit buttons
assert.ok(editButtons.length > 0, 'Edit buttons are available');

// Click the first (0th) edit button
await editButtons[1].click();

// Disconnect Mainnet
await driver.clickElement({
text: 'Localhost 8545',
tag: 'p',
});

await driver.clickElement('[data-testid="connect-more-chains-button"]');
Comment on lines +478 to +507
Copy link
Contributor

Choose a reason for hiding this comment

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

same

await driver.switchToWindow(dappTwo);
assert.equal(await driver.getCurrentUrl(), `${DAPP_ONE_URL}/`);

Expand Down
9 changes: 0 additions & 9 deletions test/e2e/page-objects/pages/test-dapp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,15 +209,6 @@ class TestDapp {
await this.driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await this.driver.waitForSelector(this.connectMetaMaskMessage);

// TODO: Extra steps needed to preserve the current network.
// Following steps can be removed once the issue is fixed (#27891)
const editNetworkButton = await this.driver.findClickableElements(
this.editConnectButton,
);
await editNetworkButton[1].click();
await this.driver.clickElement(this.localhostCheckbox);
await this.driver.clickElement(this.updateNetworkButton);

await this.driver.clickElementAndWaitForWindowToClose(
this.confirmDialogButton,
);
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/snaps/test-snap-txinsights-v2.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ describe('Test Snap TxInsights-v2', function () {
tag: 'button',
text: 'Activity',
});
// wait for transaction confirmation
await driver.waitForSelector({
css: '.transaction-status-label',
text: 'Confirmed',
});
},
);
});
Expand Down
8 changes: 0 additions & 8 deletions test/e2e/tests/connections/edit-networks-flow.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ const {
} = require('../../helpers');
const FixtureBuilder = require('../../fixture-builder');

async function switchToNetworkByName(driver, networkName) {
await driver.clickElement('.mm-picker-network');
await driver.clickElement(`[data-testid="${networkName}"]`);
}

describe('Edit Networks Flow', function () {
it('should be able to edit networks', async function () {
await withFixtures(
Expand Down Expand Up @@ -43,9 +38,6 @@ describe('Edit Networks Flow', function () {
await driver.clickElement(
'.mm-modal-content__dialog button[aria-label="Close"]',
);

// Switch to first network, whose send transaction was just confirmed
await switchToNetworkByName(driver, 'Localhost 8545');
await locateAccountBalanceDOM(driver);
await driver.clickElement(
'[data-testid ="account-options-menu-button"]',
Expand Down
39 changes: 1 addition & 38 deletions test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,12 +98,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () {
`window.ethereum.request(${switchEthereumChainRequest})`,
);

await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.findElement({
text: 'Use your enabled networks',
tag: 'p',
});

Comment on lines -101 to -106
Copy link
Contributor

Choose a reason for hiding this comment

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

this check should still be valid. The globally selected network is 0x53a when Dapp Two connects, meaning that 0x539 should not be connected for Dapp Two and a Chain Permission approval should appear when trying to switch to it

await driver.switchToWindowWithUrl(DAPP_ONE_URL);

await driver.clickElement('#sendButton');
Expand All @@ -119,19 +113,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () {
// so we leave this delay until the issue is fixed (#27360)
await driver.delay(5000);

await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

// Check correct network on the send confirmation.
await driver.findElement({
css: '[data-testid="network-display"]',
text: 'Localhost 8546',
});

await driver.clickElementAndWaitForWindowToClose({
text: 'Confirm',
tag: 'button',
});

Comment on lines -122 to -134
Copy link
Contributor

Choose a reason for hiding this comment

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

this should also still be valid, but I would assume the confirmation is on Localhost 8545

// Switch back to the extension
await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
Expand Down Expand Up @@ -240,37 +221,19 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () {
`window.ethereum.request(${switchEthereumChainRequest})`,
);

await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
Copy link
Member Author

Choose a reason for hiding this comment

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

Since Dapp is already connected to Test Network, we don't need to show review permission screem

Copy link
Contributor

@jiexi jiexi Oct 24, 2024

Choose a reason for hiding this comment

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

the goal of the switch call is to trigger an approval to permit a chain that is not already permitted. This check still needs to occur. Instead the switch call above to should target 0x53a It may be easier to unselect 0x539 (Localhost 8545) when Dapp One is connecting

await driver.findElement({
text: 'Use your enabled networks',
tag: 'p',
});

await driver.switchToWindowWithUrl(DAPP_ONE_URL);

await driver.clickElement('#sendButton');

await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

await driver.clickElement({ text: 'Cancel', tag: 'button' });
Copy link
Contributor

Choose a reason for hiding this comment

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

this needs to stay the same as well since we are testing the scenario when there is an approval for permitting a chain

await driver.clickElement({ text: 'Confirm', tag: 'button' });
await driver.switchToWindowWithUrl(DAPP_ONE_URL);

// Wait for switch confirmation to close then tx confirmation to show.
// There is an extra window appearing and disappearing
// so we leave this delay until the issue is fixed (#27360)
await driver.delay(5000);
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

// Check correct network on the send confirmation.
await driver.findElement({
css: '[data-testid="network-display"]',
text: 'Localhost 8546',
});

await driver.clickElementAndWaitForWindowToClose({
text: 'Confirm',
tag: 'button',
});
Comment on lines -262 to -273
Copy link
Contributor

Choose a reason for hiding this comment

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

this should stay as well. The goal here is to confirm that the network switch on Dapp One did not affect Dapp Two


// Switch back to the extension
await driver.switchToWindowWithTitle(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks revok
await driver.switchToWindowWithUrl(DAPP_URL);
await driver.findElement({
css: '[id="chainId"]',
text: '0x1',
text: '0x539',
});
await driver.clickElement('#sendButton');

Expand All @@ -108,7 +108,7 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks revok
await driver.switchToWindowWithUrl(DAPP_URL);
await driver.findElement({
css: '[id="chainId"]',
text: '0x1',
text: '0x539',
});
await driver.assertElementNotPresent({
css: '[id="chainId"]',
Expand Down
Loading