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

test: chainid, switch custom net, batch tx diff, snaps #27725

Merged

Conversation

seaona
Copy link
Contributor

@seaona seaona commented Oct 9, 2024

Description

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

@seaona seaona requested a review from a team as a code owner October 9, 2024 14:34
Copy link
Contributor

github-actions bot commented Oct 9, 2024

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@seaona seaona requested a review from a team as a code owner October 9, 2024 14:43
await driver.clickElement({
text: 'Connect',
tag: 'button',
});

await driver.waitForSelector({ text: 'Connect' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed as clicking the element already implies the button is there and clickable

await driver.waitForSelector({ text: 'OK' });

await driver.clickElement({
await driver.clickElementAndWaitForWindowToClose({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this mitigates race conditions: we should not proceed to the next step until the dialog is closed

await switchToNotificationWindow(driver);
await driver.clickElement({
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.clickElementAndWaitForWindowToClose({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same, wait for the dialog to close before proceeding

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

await driver.waitForSelector({ text: 'Confirm' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed

await driver.waitForSelector({ text: 'OK' });

await driver.clickElement({
await driver.clickElementAndWaitForWindowToClose({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait for the dialog to close before proceeding

await switchToNotificationWindow(driver, 2);
await driver.clickElement({
await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
await driver.clickElementAndWaitForWindowToClose({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait for the dialog to close before proceeding


await driver.delay(regularDelayMs);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed

await switchToNotificationWindow(driver);

await driver.clickElement({
await driver.clickElementAndWaitForWindowToClose({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait for the dialog to close before proceeding

@@ -82,14 +77,11 @@ describe('Request Queuing for Multiple Dapps and Txs on different networks', fun
await openDapp(driver, undefined, DAPP_ONE_URL);

// Connect to dapp 2
await driver.findClickableElement({ text: 'Connect', tag: 'button' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed

await driver.findClickableElement({ text: 'Connect', tag: 'button' });
await driver.clickElement('#connectButton');

await driver.delay(regularDelayMs);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed


await driver.findElement(
await driver.waitForSelector(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a nit

By.xpath("//div[normalize-space(.)='1 of 2']"),
);

// Reject All Transactions
await driver.clickElement('.page-container__footer-secondary a');

await driver.clickElement({ text: 'Reject all', tag: 'button' }); // TODO: Do we want to confirm here?
// TODO: Do we want to confirm here?
await driver.clickElementAndWaitForWindowToClose({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wait for the dialog to close before proceeding


await driver.findElement(
await driver.waitForSelector(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nit


await driver.delay(regularDelayMs);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not needed

await driver.waitForSelector({
css: '[id="chainId"]',
text: '0x539',
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensure the test dapp has the new chain id too

await driver.waitForSelector({
css: '[id="chainId"]',
text: '0x1',
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ensure the test dapp has the new chain id too

Copy link

sonarcloud bot commented Oct 9, 2024

Copy link
Member

@NidhiKJha NidhiKJha left a comment

Choose a reason for hiding this comment

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

LGTM

@NidhiKJha NidhiKJha merged commit c3201f2 into multichain-e2e-tests-v2 Oct 9, 2024
71 of 72 checks passed
@NidhiKJha NidhiKJha deleted the multichain-e2e-chainid-custom-net-snaps branch October 9, 2024 15:27
@metamaskbot
Copy link
Collaborator

Builds ready [d4d1ffe]
Page Load Metrics (1929 ± 103 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint32024491838404194
domContentLoaded156023951896208100
load159424731929214103
domInteractive26194503818
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -297 Bytes (-0.01%)
  • ui: -3.13 KiB (-0.04%)
  • common: -200 Bytes (-0.00%)

@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants