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

E2e request queuing #22818

Merged
merged 15 commits into from
Feb 23, 2024
Merged
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
20 changes: 20 additions & 0 deletions test/e2e/fixture-builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,10 @@ function defaultFixture(inputChainId = CHAIN_IDS.LOCALHOST) {
useMultiAccountBalanceChecker: true,
useRequestQueue: false,
},
SelectedNetworkController: {
domains: {},
perDomainNetwork: false,
},
SmartTransactionsController: {
smartTransactionsState: {
fees: {},
Expand Down Expand Up @@ -445,6 +449,7 @@ class FixtureBuilder {
rpcUrl: 'http://localhost:8545',
ticker: 'ETH',
networkConfigurationId: 'networkConfigurationId',
id: 'networkConfigurationId',
},
'76e9cd59-d8e2-47e7-b369-9c205ccb602c': {
id: '76e9cd59-d8e2-47e7-b369-9c205ccb602c',
Expand Down Expand Up @@ -846,6 +851,21 @@ class FixtureBuilder {
});
}

withSelectedNetworkController(data) {
merge(this.fixture.data.SelectedNetworkController, data);
return this;
}

withSelectedNetworkControllerPerDomain() {
return this.withSelectedNetworkController({
domains: {
[DAPP_URL]: 'networkConfigurationId',
[DAPP_ONE_URL]: '76e9cd59-d8e2-47e7-b369-9c205ccb602c',
},
perDomainNetwork: true,
});
}

withSmartTransactionsController(data) {
merge(this.fixture.data.SmartTransactionsController, data);
return this;
Expand Down
46 changes: 46 additions & 0 deletions test/e2e/tests/request-queuing/enable-queuing.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
const {
withFixtures,
defaultGanacheOptions,
unlockWallet,
} = require('../../helpers');
const FixtureBuilder = require('../../fixture-builder');

describe('Toggle Request Queuing Setting', function () {
it('should enable the request queuing setting ', async function () {
await withFixtures(
{
dapp: true,
fixtures: new FixtureBuilder().build(),
ganacheOptions: defaultGanacheOptions,
failOnConsoleError: false,
title: this.test.fullTitle(),
},
async ({ driver }) => {
await driver.navigate();
await unlockWallet(driver);

// Open account menu button
const accountOptionsMenuSelector =
'[data-testid="account-options-menu-button"]';
await driver.waitForSelector(accountOptionsMenuSelector);
await driver.clickElement(accountOptionsMenuSelector);

// Click settings from dropdown menu
const globalMenuSettingsSelector =
'[data-testid="global-menu-settings"]';
await driver.waitForSelector(globalMenuSettingsSelector);
await driver.clickElement(globalMenuSettingsSelector);

// Click Experimental tab
const securityAndPrivacyTabRawLocator = {
text: 'Experimental',
tag: 'div',
};
await driver.clickElement(securityAndPrivacyTabRawLocator);

// Toggle request queue setting
await driver.clickElement('.request-queue-toggle');
},
);
});
});
172 changes: 172 additions & 0 deletions test/e2e/tests/request-queuing/multiple-networks-dapps-txs.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
const FixtureBuilder = require('../../fixture-builder');
const {
withFixtures,
openDapp,
unlockWallet,
DAPP_URL,
DAPP_ONE_URL,
regularDelayMs,
WINDOW_TITLES,
defaultGanacheOptions,
} = require('../../helpers');
const { PAGES } = require('../../webdriver/driver');

// TODO: Have to turn on the setting every time we want to test the setting!?!
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to add this to the fixture no?

Copy link
Contributor Author

@tmashuang tmashuang Feb 21, 2024

Choose a reason for hiding this comment

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

Yes, we should be able to. The issuesis that any type of refresh of the state that happens reverts the request queue setting behavior to default, the setting is off even though it is supposed to be on. This behavior can be replicated in prod with the steps in the next line comment.

// TODO: Test this in prod, refresh the extension when the setting is on and you have to disable/enable it for the switchEthereumChain notification to work.

describe('Request Queuing for Multiple Dapps and Txs on different networks.', function () {
it('should show switch network confirmations for per dapp selected networks when calling send transactions', async function () {
const port = 8546;
const chainId = 1338;
await withFixtures(
{
dapp: true,
fixtures: new FixtureBuilder()
.withNetworkControllerDoubleGanache()
.withPermissionControllerConnectedToTwoTestDapps()
.withSelectedNetworkControllerPerDomain()
.build(),
dappOptions: { numberOfDapps: 2 },
ganacheOptions: {
...defaultGanacheOptions,
concurrent: {
port,
chainId,
ganacheOptions2: defaultGanacheOptions,
},
},
title: this.test.fullTitle(),
},
async ({ driver }) => {
await unlockWallet(driver);

// Open account menu button
const accountOptionsMenuSelector =
'[data-testid="account-options-menu-button"]';
await driver.waitForSelector(accountOptionsMenuSelector);
await driver.clickElement(accountOptionsMenuSelector);

// Click settings from dropdown menu
const globalMenuSettingsSelector =
'[data-testid="global-menu-settings"]';
await driver.waitForSelector(globalMenuSettingsSelector);
await driver.clickElement(globalMenuSettingsSelector);

// Click Experimental tab
const securityAndPrivacyTabRawLocator = {
text: 'Experimental',
tag: 'div',
};
await driver.clickElement(securityAndPrivacyTabRawLocator);

// Toggle request queue setting
await driver.clickElement('.request-queue-toggle');

// Navigate to extension home screen
await driver.navigate(PAGES.HOME);

// Open Dapp One
await openDapp(driver, undefined, DAPP_URL);

await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);

// Network Selector
await driver.clickElement('[data-testid="network-display"]');

// Switch to second network
await driver.clickElement({
text: 'Localhost 8546',
css: 'p',
});

// TODO: Request Queuing bug when opening both dapps at the same time will have them stuck on the same network, with will be incorrect for one of them.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// TODO: Request Queuing bug when opening both dapps at the same time will have them stuck on the same network, with will be incorrect for one of them.
// TODO: Request Queuing bug when opening both dapps at the same time will have them stuck on the same network, which will be incorrect for one of them.

also, thats an odd one! defs something to look into. Might be worth making a branch that has a failing test as described here.

// Open Dapp Two
await openDapp(driver, undefined, DAPP_ONE_URL);

// Window Handling
const windowHandles = await driver.getAllWindowHandles();
const dappOne = windowHandles[1];
const dappTwo = windowHandles[2];

// Dapp one send tx
await driver.switchToWindow(dappOne);
await driver.clickElement('#sendButton');

// Dapp two send tx
await driver.switchToWindow(dappTwo);
await driver.clickElement('#sendButton');

// First switch network confirmation
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.clickElement({ text: 'Switch network', tag: 'button' });

// Wait for confirm tx after switch network confirmation.
await driver.delay(regularDelayMs);

await driver.waitUntilXWindowHandles(4);
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);

// Find correct network on confirm tx
await driver.findElement({
text: 'Localhost 8545',
tag: 'span',
});

// Reject Transaction
await driver.findClickableElement({ text: 'Confirm', tag: 'button' });
await driver.clickElement(
'[data-testid="page-container-footer-cancel"]',
);

// TODO: No second confirmation from dapp two will show, have to go back to the extension to see the switch chain & dapp two's tx.
Copy link
Contributor

Choose a reason for hiding this comment

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

nice find on getting this to work!

Similar to above, it might be good to have a failing test on a branch so we can track this as things are updated /merged

await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);

// TODO: Reload fix to have the confirmations show
await driver.executeScript(`window.location.reload()`);

// Second Switch Network Confirmation
await driver.switchToWindowWithTitle(
WINDOW_TITLES.ExtensionInFullScreenView,
);
await driver.findElement({
css: '[data-testid="network-switch-from-network"]',
text: 'Localhost 8545',
});

await driver.findElement({
css: '[data-testid="network-switch-to-network"]',
text: 'Localhost 8546',
});

// Switch Network
await driver.clickElement({ text: 'Switch network', tag: 'button' });

// Check for unconfirmed transaction in tx list
await driver.wait(async () => {
const unconfirmedTxes = await driver.findElements(
'.transaction-list-item--unconfirmed',
);
return unconfirmedTxes.length === 1;
}, 10000);

// Click Unconfirmed Tx
await driver.clickElement('.transaction-list-item--unconfirmed');

// Confirm Tx
await driver.clickElement('[data-testid="page-container-footer-next"]');

// Check for Confirmed Transaction
await driver.wait(async () => {
const confirmedTxes = await driver.findElements(
'.transaction-list__completed-transactions .activity-list-item',
);
return confirmedTxes.length === 1;
}, 10000);
},
);
});
});
Loading
Loading