fix: Update and fix e2e tests to handle the presence of the offscreen document under MV3 by danjm · Pull Request #24675 · MetaMask/metamask-extension

danjm · 2024-05-21T09:46:58Z

Description

Our MV3 builds add a "Offscreen Document" to our extension. This means that when our e2e tests call driver.getAllWindowHandles(), a new window is included in the list (the window for the offscreen document). This PR updates the e2e tests to handle this change. Comments are left on each file to explain why the change was needed.

Related issues

Fixes: #21496

Manual testing steps

Any of the test files modified in this PR should pass when run locally with MV3 builds

Pre-merge author checklist

I’ve followed MetaMask 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-05-21T09:47:10Z

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.

danjm · 2024-05-21T09:50:32Z

test/e2e/accounts/common.ts

switchToOrOpenDapp attempts "an unusually fast switchToWindowWithTitle, just 1 second" and if that fails, then calls openDapp. The problem the switchToWindowWithTitle assumes that there will be a window other than the offscreen document present. It seems that openDapp fails if the drivers current window is the offscreen document. The change in this file ensures that, for this e2e test, the extension window is selected first and then the dapp can be opened from there.

danjm · 2024-05-21T09:50:58Z

test/e2e/helpers.js

This is needed for the update to test/e2e/json-rpc/switchEthereumChain.spec.js, which can be seen below

…ment under MV3

danjm · 2024-05-21T09:53:59Z

test/e2e/json-rpc/switchEthereumChain.spec.js

@@ -53,13 +53,8 @@ describe('Switch Ethereum Chain for two dapps', function () {
        await driver.clickElement('.request-queue-toggle');



This file assumed that window handles at certain indices would always map to the dapps that are opened. The problem is that the offscreen document can actually be one of the window handles at those indices, The solution is to return the new handle with the openDapp call, and then use that handle to select the right windows for the opened dapps

danjm · 2024-05-21T09:56:30Z

test/e2e/tests/settings/full-size-view-settings.spec.js

@@ -33,28 +33,25 @@ describe('Full-size View Setting @no-mmi', function () {
        await unlockWallet(driver);
        await toggleFullSizeViewSetting(driver);
        await openDapp(driver);


This file assumed that window handles at certain indices would always map to the dapp and popup window, but again the offscreen document can be at one of these indices.

The solution, which maintains what this test was actually testing, is to update it to compare window handles before and after the user action being tested, and to make an assertion against the new window handle

danjm · 2024-05-21T09:57:33Z

test/e2e/tests/transaction/edit-gas-fee.spec.js

@@ -176,8 +176,7 @@ describe('Editing Confirm Transaction', function () {
        });


Instead of selecting window handles by assumed indices, which could end up switching to the offscreen window, this file switches to windows based on title

danjm · 2024-05-21T09:57:51Z

test/e2e/tests/transaction/multiple-transactions.spec.js

+  WINDOW_TITLES,
 } = require('../../helpers');
 const FixtureBuilder = require('../../fixture-builder');



Instead of selecting window handles by assumed indices, which could end up switching to the offscreen window, this file switches to windows based on title

danjm · 2024-05-21T09:58:23Z

test/e2e/webdriver/driver.js

@@ -509,8 +509,9 @@ class Driver {
  }

  async openNewPage(url) {


This change is needed to support the changes in switchEthereumChain.spec.js (which can be seen above)

danjm · 2024-05-21T09:59:27Z

test/e2e/webdriver/driver.js

    return await this.driver.getAllWindowHandles();
  }

-  async waitUntilXWindowHandles(x, delayStep = 1000, timeout = this.timeout) {


Under MV3, there will always be 1 more window handle than previously assumed, so this is a global fix for waitUntilXWindowHandles, to accomodate the MV3 offscreen document

metamaskbot · 2024-05-21T12:30:40Z

Builds ready [584a572]

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, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1365 ± 698 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	68	191	114	40	19
		domContentLoaded	10	87	25	22	10
		load	54	3796	1365	1453	698
		domInteractive	10	87	25	22	10

Bundle size diffs

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

metamaskbot · 2024-05-21T17:44:47Z

Builds ready [f896bed]

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, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1357 ± 679 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	73	206	128	39	19
		domContentLoaded	11	73	24	16	8
		load	61	3618	1357	1415	679
		domInteractive	11	73	24	16	8

Bundle size diffs

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

… openDappAndSwitchChain

…n MV2 tests

danjm · 2024-05-22T12:01:35Z

The test-e2e-chrome failure here is related to changes in this PR in the request-queue test. I thought that change would be safe for mv2... I will investigate

codecov · 2024-05-23T10:21:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.42%. Comparing base (0ef3c22) to head (4e31e06).
Report is 23 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #24675   +/-   ##
========================================
  Coverage    67.42%   67.42%           
========================================
  Files         1289     1289           
  Lines        50241    50241           
  Branches     13014    13014           
========================================
  Hits         33871    33871           
  Misses       16370    16370

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metamaskbot · 2024-05-23T10:34:34Z

Builds ready [4e31e06]

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, 2, 3, 4, 5, 6, 7, 8
- content-script: 0
- ui: 0, 1, 10, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (650 ± 481 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	65	151	94	23	11
		domContentLoaded	9	49	15	10	5
		load	54	2443	650	1001	481
		domInteractive	9	49	15	10	5

Bundle size diffs

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

danjm · 2024-05-23T10:35:18Z

The test-e2e-chrome failure here is related to changes in this PR in the request-queue test. I thought that change would be safe for mv2... I will investigate

I was unable to repro this locally, and it didn't occur again after multiple re-runs on CI (two on this branch and two on another branch I pushed up).

There probably is some flakiness here, but it is hard to recreate, even on CI. I am going to merge and then we can observe if this flake happens again, as we will have this test running more frequently once merge.

metamaskbot · 2024-06-04T20:07:28Z

Missing release label release-11.16.6 on PR. Adding release label release-11.16.6 on PR and removing other release labels(release-11.18.0), as PR was cherry-picked in branch 11.16.6.

danjm requested a review from a team as a code owner May 21, 2024 09:46

metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label May 21, 2024

danjm commented May 21, 2024

View reviewed changes

Update and fix e2e tests to handle the presence of the offscreen docu…

584a572

…ment under MV3

danjm force-pushed the handle-offscreen-document-e2e branch from 1021d45 to 584a572 Compare May 21, 2024 09:51

danjm commented May 21, 2024

View reviewed changes

Merge branch 'develop' into handle-offscreen-document-e2e

f896bed

danjm changed the title ~~Update and fix e2e tests to handle the presence of the offscreen document under MV3~~ fix: Update and fix e2e tests to handle the presence of the offscreen document under MV3 May 21, 2024

danjm added the team-extension-platform Extension Platform team label May 21, 2024

danjm added 2 commits May 22, 2024 06:50

Ensure request-queuing.spec.js accounts for offscreen document within…

21d57de

… openDappAndSwitchChain

Make sure test/e2e/json-rpc/switchEthereumChain.spec.js still works o…

4e31e06

…n MV2 tests

DDDDDanica approved these changes May 22, 2024

View reviewed changes

vthomas13 approved these changes May 22, 2024

View reviewed changes

danjm merged commit 9db82cf into develop May 23, 2024

danjm deleted the handle-offscreen-document-e2e branch May 23, 2024 10:35

github-actions bot locked and limited conversation to collaborators May 23, 2024

metamaskbot added the release-11.18.0 label May 23, 2024

metamaskbot added release-11.16.6 Issue or pull request that will be included in release 11.16.6 and removed release-11.18.0 labels Jun 4, 2024

		@@ -53,13 +53,8 @@ describe('Switch Ethereum Chain for two dapps', function () {
		await driver.clickElement('.request-queue-toggle');

		@@ -176,8 +176,7 @@ describe('Editing Confirm Transaction', function () {
		});

Conversation

danjm commented May 21, 2024 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented May 21, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

metamaskbot commented May 21, 2024

Uh oh!

metamaskbot commented May 21, 2024

Uh oh!

danjm commented May 22, 2024

Uh oh!

codecov bot commented May 23, 2024

Codecov Report

Uh oh!

metamaskbot commented May 23, 2024

Uh oh!

danjm commented May 23, 2024

Uh oh!

metamaskbot commented Jun 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danjm commented May 21, 2024 •

edited by github-actions bot

Loading