Multichain E2E Test: Multi dapp #29751

ffmcgee725 · 2025-01-16T15:31:12Z

-- test: multi dapp e2e test setup;

Description

Related issues

Fixes:

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

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.

… on sequence of requests

ffmcgee725 · 2025-01-16T15:34:20Z

test/e2e/flask/multichain-api/testHelpers.ts

+  if (!multipleDapps) {
+    await unlockWallet(driver);
+  }
+  await openDapp(driver, undefined, url);


I'm not a big fan of this extra 4th (optional) param, but I needed this util function to only unlock the wallet once for a multi dapp setup. So in the actual test we call unlockWallet once, and then loop through dapps to actually open the dapps themselves.

Should we make another util func that's used for multi dapp setup, or other suggested approaches, or ya'll fine with this ?

I would just move the unlockWallet step out of this method entirely. It's not uncommon in the e2e specs to see a call to unlockWallet() as the first step. I agree with your thinking that adding this multipleDapps optional param is weird

this would require going through every other current multichain dapp e2e existing test and adding in await unlockWallet(driver);
are you okay with doing that ?

ffmcgee725 · 2025-01-16T15:35:08Z

test/e2e/flask/multichain-api/multi-dapp-wallet_invokeMethod.spec.ts

+
+            // Assert
+            for (const [i, dapp] of DAPP_URLS.entries()) {
+              const accountWebElement = await driver.findElement(


Multiple loops because we want to make sure we queue up all the requests first, and then make assertions that the sequence of requests, the origin and account are correct for each confirmation in the queue (as per ticket description)

…p-e2e

test/e2e/fixture-builder.js

metamaskbot · 2025-01-18T00:25:51Z

Builds ready [ee1029b]

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

Page Load Metrics (1655 ± 64 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1455	2136	1661	144	69
		domContentLoaded	1438	2062	1640	132	64
		load	1446	2073	1655	134	64
		domInteractive	24	72	40	17	8
		backgroundConnect	8	57	18	14	7
		firstReactRender	15	104	39	29	14
		getState	4	54	12	15	7
		initialActions	0	0	0	0	0
		loadScripts	1036	1519	1182	105	50
		setupStore	5	70	17	20	10
		uiStartup	1637	2393	1984	197	95

jiexi · 2025-01-28T16:34:43Z

test/e2e/flask/multichain-api/multi-dapp-wallet_invokeMethod.spec.ts

+    dappPath: path.join(
+      '..',
+      '..',
+      'node_modules',
+      '@metamask',
+      'test-dapp-multichain',
+      'build',
+    ),


did we have a constant for this somewhere already?

We had a constant for this for single dapp setup, it's slightly different for multi dapp setup

jiexi · 2025-01-28T16:36:10Z

test/e2e/flask/multichain-api/multi-dapp-wallet_invokeMethod.spec.ts

+              await initCreateSessionScopes(driver, [SCOPE], ACCOUNTS);
+              await addAccountInWalletAndAuthorize(driver);
+              await driver.clickElement({ text: 'Connect', tag: 'button' });
+              await driver.delay(largeDelayMs);
+              await driver.switchToWindowWithUrl(dapp);


is this create session necessary given the fixtures?

removed in 9638656, does not seem necessary for read operations test

jiexi · 2025-01-28T16:38:19Z

test/e2e/flask/multichain-api/multi-dapp-wallet_invokeMethod.spec.ts

+              await initCreateSessionScopes(driver, [SCOPE], ACCOUNTS);
+              await addAccountInWalletAndAuthorize(driver);
+              await driver.clickElement({ text: 'Connect', tag: 'button' });


same question about if this manual connection is needed since the fixture includes accounts

If I don't manually create sessions in this second one, when calling eth_sendTransaction downstream the wallet extension UI is not popping up for some reason, causing the test to fail 🤔

This one seems quite flaky, it's sometimes failing on CI (but passing locally) with a StaleElementReferenceError: stale element reference: stale element not found in the current frame which is weird. I may need to re-visit this implementation for a more consistent reliable approach

test/e2e/flask/multichain-api/multi-dapp-wallet_invokeMethod.spec.ts

metamaskbot · 2025-01-30T12:12:44Z

Builds ready [ea96578]

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

Page Load Metrics (1605 ± 69 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	1379	1908	1603	140	67
		domContentLoaded	1374	1858	1585	133	64
		load	1379	1910	1605	145	69
		domInteractive	24	103	43	23	11
		backgroundConnect	7	54	22	16	8
		firstReactRender	15	100	45	29	14
		getState	4	71	14	18	9
		initialActions	0	1	0	0	0
		loadScripts	980	1395	1154	120	57
		setupStore	6	67	12	16	8
		uiStartup	1577	2254	1855	204	98

ffmcgee725 added 3 commits January 16, 2025 00:18

test: multi dapp e2e test setup (wip)

e13c029

fix: fix failing test case

a7ca50e

refactor: make sure to queue up all requests before making assertions…

c71c37c

… on sequence of requests

ffmcgee725 requested a review from a team January 16, 2025 15:31

metamaskbot added the team-wallet-api-platform label Jan 16, 2025

ffmcgee725 commented Jan 16, 2025

View reviewed changes

ffmcgee725 added 2 commits January 16, 2025 16:45

refactor: make sure to use static constant variables for accounts in

f067dc7

Merge branch 'jl/caip-multichain-migrate-core' into jc/multi-test-dap…

ad1ebd4

…p-e2e

adonesky1 reviewed Jan 17, 2025

View reviewed changes

test/e2e/fixture-builder.js Outdated Show resolved Hide resolved

refactor: revert changes to fixture-builder.js from commit f067dc7

688509c

ffmcgee725 requested a review from adonesky1 January 17, 2025 16:22

ffmcgee725 added 3 commits January 17, 2025 17:27

fix: address fixture-builder.js change

5e44957

minor change

d462d3c

fix: fixture-builder.js

4bff421

jiexi reviewed Jan 17, 2025

View reviewed changes

test/e2e/fixture-builder.js Outdated Show resolved Hide resolved

fix: properly setup permissions per dapp

13cf223

ffmcgee725 requested a review from jiexi January 17, 2025 23:44

merge jl/caip-multichain-migrate-core and fix conflicts

ee1029b

jiexi reviewed Jan 28, 2025

View reviewed changes

test/e2e/flask/multichain-api/multi-dapp-wallet_invokeMethod.spec.ts Outdated Show resolved Hide resolved

ffmcgee725 added 2 commits January 30, 2025 12:18

test: minor adjustments

9638656

lint: remove unused variable

ea96578

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multichain E2E Test: Multi dapp #29751

Multichain E2E Test: Multi dapp #29751

ffmcgee725 commented Jan 16, 2025 •

edited by github-actions bot

Loading

ffmcgee725 Jan 16, 2025

jiexi Jan 28, 2025

ffmcgee725 Jan 30, 2025

ffmcgee725 Jan 16, 2025

metamaskbot commented Jan 18, 2025

jiexi Jan 28, 2025

ffmcgee725 Jan 30, 2025

jiexi Jan 28, 2025

ffmcgee725 Jan 30, 2025

jiexi Jan 28, 2025

ffmcgee725 Jan 30, 2025

ffmcgee725 Jan 30, 2025 •

edited

Loading

metamaskbot commented Jan 30, 2025

Multichain E2E Test: Multi dapp #29751

Are you sure you want to change the base?

Multichain E2E Test: Multi dapp #29751

Conversation

ffmcgee725 commented Jan 16, 2025 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metamaskbot commented Jan 18, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ffmcgee725 Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

metamaskbot commented Jan 30, 2025

ffmcgee725 commented Jan 16, 2025 •

edited by github-actions bot

Loading

ffmcgee725 Jan 30, 2025 •

edited

Loading