fix: flaky test `Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx should queue send tx after switch network confirmation and transaction should target the correct network after switch is confirmed` #27352

seaona · 2024-09-24T07:11:05Z

Description

There is a race condition with window management that makes this test fail quite often. The test performs these actions and there are different behaviors happening, depending on the build:

Action: trigger a network switch -> this opens a new dialog window (we have 4)
Action: trigger a send -> we are still with the network switch window open
Action: we click switch network -> this closes the dialog (now we have 3 windows for a brief moment - if this is fast, like in webpack, it fails with the error Error: waitUntilXWindowHandles timed out polling window handles. Expected: 3, Actual: 4)
A new dialog is open automatically -> now we have 4 windows again, but the dialog handle ids are different
Sometimes this dialog is closed and then a new one is open
We switch to the dialog, but often the context is invalidated as we are switching to either 1 or 4 or 5

Extra Notes:
This PR fixes the flakiness of waiting for 3 windows by adding a delay. This won't fix the root cause of the 2 new identified issues, but after re-runs has proven to stabilize the test

In Webpack, sometimes the dialog for the transaction never appears, and the wallet get's in a frozen state. The issue is on the wallet level, see [Bug]: Wallet API - MM unresponsive after Send tx request queued after a network switch request #27414
see issue [Bug]: Wallet API - In Firefox and Webpack builds, when we trigger a transaction with a pending Switch network, once the Switch network is resolved, an extra popup is open and closed before the transaction appears #27360
Since there is only a brief moment for having 3 windows, waiting for 3 windows and then proceeding also makes the test flaky, so it's not effective.

Related issues

Fixes: #27387

Manual testing steps

Check ci
Run test locally yarn test:e2e:single test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js --browser=chrome

Screenshots/Recordings

Problem in FF and Webpack, see how popup is open and closed, and then a new popup is open with the transaction.

ff-dialog-open-close.mp4

Problem in Webpack, where tx does not open after the change networks

webpack-send-tx-not-appear.mp4

Problem Webpack, see how popup is open and closed, and then a new popup is open with the transaction.

webpack-2-txs.mp4

Pre-merge author checklist

I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards.
I've completed the PR template to the best of my ability
I’ve included tests if applicable
I’ve documented my code using JSDoc format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-09-24T07:11:15Z

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 · 2024-09-24T09:53:16Z

test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js

@@ -51,17 +49,15 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () {
        await driver.findClickableElement({ text: 'Connect', tag: 'button' });
        await driver.clickElement('#connectButton');

-        await driver.delay(regularDelayMs);
-
-        await switchToNotificationWindow(driver);


removed deprecated method and using the correct one (no need to add the delay)

seaona · 2024-09-24T09:54:08Z

test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js

@@ -80,9 +76,6 @@ describe('Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx', function () {
          css: 'p',
        });

-        // Wait for the first dapp's connect confirmation to disappear
-        await driver.waitUntilXWindowHandles(2);
-


no need to add this, as we handle the close window into the `clickElementAndWaitForWindowToClose

sonarcloud · 2024-09-26T09:18:39Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

seaona · 2024-09-26T09:30:23Z

test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js

        // Wait for switch confirmation to close then tx confirmation to show.
-        await driver.waitUntilXWindowHandles(3);


this point is where test breaks in webpack: if the next dialog window is opened really fast, this condition is never met

seaona · 2024-09-26T09:30:35Z

test/e2e/tests/request-queuing/dapp1-switch-dapp2-send.spec.js


        // Wait for switch confirmation to close then tx confirmation to show.
-        await driver.waitUntilXWindowHandles(3);


this point is where test breaks in webpack: if the next dialog window is opened really fast, this condition is never met

metamaskbot · 2024-09-26T09:42:33Z

Builds ready [c1e9a64]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5, 6
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1739 ± 85 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	325	2507	1680	368	177
		domContentLoaded	1535	2353	1714	171	82
		load	1550	2406	1739	177	85
		domInteractive	14	238	50	53	25

Bundle size diffs

background: 0 Bytes (0.00%)
ui: 0 Bytes (0.00%)
common: 0 Bytes (0.00%)

chloeYue

Looks good for me the fix ! Thanks !

window management

8c85d5d

seaona requested a review from a team as a code owner September 24, 2024 07:11

seaona self-assigned this Sep 24, 2024

seaona added team-extension-platform flaky tests area-qa Relating to QA work (Quality Assurance) labels Sep 24, 2024

seaona added 3 commits September 24, 2024 10:08

fix windows

4ce7e5b

Fix lint

70453e1

Add driver function for popup mgmt

20bb373

seaona commented Sep 24, 2024

View reviewed changes

seaona and others added 11 commits September 24, 2024 12:15

add missing check

fb6ae91

Merge branch 'develop' into flaky-dapp-rqs-windows

f865b2d

debug

969eab0

fix ff case

9c96bac

fix condition

fec77aa

wait to see element before proceeding for webpack race cond

c7fd526

tweak

ed3fbf6

fix lint

3298b59

delay

dffd70c

this

c116223

increase delay

c1e9a64

seaona commented Sep 26, 2024

View reviewed changes

chloeYue approved these changes Sep 26, 2024

View reviewed changes

DDDDDanica approved these changes Sep 26, 2024

View reviewed changes

seaona merged commit 1ee0cbd into develop Sep 26, 2024
107 checks passed

seaona deleted the flaky-dapp-rqs-windows branch September 26, 2024 17:14

github-actions bot locked and limited conversation to collaborators Sep 26, 2024

metamaskbot added the release-12.6.0 Issue or pull request that will be included in release 12.6.0 label Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: flaky test `Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx should queue send tx after switch network confirmation and transaction should target the correct network after switch is confirmed` #27352

fix: flaky test `Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx should queue send tx after switch network confirmation and transaction should target the correct network after switch is confirmed` #27352

seaona commented Sep 24, 2024 •

edited

Loading

github-actions bot commented Sep 24, 2024

seaona Sep 24, 2024 •

edited

Loading

seaona Sep 24, 2024

sonarcloud bot commented Sep 26, 2024

seaona Sep 26, 2024

seaona Sep 26, 2024

metamaskbot commented Sep 26, 2024

chloeYue left a comment

		// Wait for switch confirmation to close then tx confirmation to show.
		await driver.waitUntilXWindowHandles(3);

fix: flaky test Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx should queue send tx after switch network confirmation and transaction should target the correct network after switch is confirmed #27352

fix: flaky test Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx should queue send tx after switch network confirmation and transaction should target the correct network after switch is confirmed #27352

Conversation

seaona commented Sep 24, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Sep 24, 2024

seaona Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

seaona Sep 24, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Sep 26, 2024

Quality Gate passed

seaona Sep 26, 2024

Choose a reason for hiding this comment

seaona Sep 26, 2024

Choose a reason for hiding this comment

metamaskbot commented Sep 26, 2024

chloeYue left a comment

Choose a reason for hiding this comment

fix: flaky test `Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx should queue send tx after switch network confirmation and transaction should target the correct network after switch is confirmed` #27352

fix: flaky test `Request Queuing Dapp 1, Switch Tx -> Dapp 2 Send Tx should queue send tx after switch network confirmation and transaction should target the correct network after switch is confirmed` #27352

seaona commented Sep 24, 2024 •

edited

Loading

seaona Sep 24, 2024 •

edited

Loading