fix: Restore `responsive` e2e driver option by pedronfigueiredo · Pull Request #25932 · MetaMask/metamask-extension

pedronfigueiredo · 2024-07-18T08:43:57Z

Description

In #18347, we changed the driver option responsive to openDevToolsForTabs. This was because we had a new test suite that intended to test browser behaviour related to Service Worker restart on manifest v3 builds. Later changes to chrome's mv3 implementation meant that service worker no longer restarted, and those e2e tests no longer exist (test/e2e/mv3/service-worker-restart.spec.js).

However, this change had an unwanted side effect on another test suite (test/e2e/tests/metamask-responsive-ui.spec.js). As @Gudahtt puts it in a slack message:

It looks like in this PR, the responsive option in the Chrome webdriver was replaced with openDevToolsForTabs. This is functionally what that option did, but this doesn't match the semantic purpose for why the browser opened the dev tools.

Browser driver options are shared between the Firefox and Chrome drivers, so now they don't match. The test suite meant to test our UI in a small viewport is now passing in the openDevToolsForTabs option, but this doesn't work for the Firefox browser because it doesn't recognize that option. So the tests are "passing" but they aren't actually running the steps that are meant to be under test.

This PR reverts the option to its original name.

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.

github-actions · 2024-07-18T08:44:12Z

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.

DDDDDanica · 2024-07-18T11:24:40Z

LGTm !

test/e2e/webdriver/chrome.js

codecov · 2024-07-18T14:41:45Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.17%. Comparing base (7916145) to head (ef00684).
Report is 5 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #25932   +/-   ##
========================================
  Coverage    70.17%   70.17%           
========================================
  Files         1423     1423           
  Lines        49844    49844           
  Branches     13851    13851           
========================================
  Hits         34975    34975           
  Misses       14869    14869

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

Gudahtt · 2024-07-18T14:45:23Z

test/e2e/tests/bridge/bridge-test-utils.ts

@micaelae @BZahory I noticed that this option was added in #25044

Could you clarify what this option was used for? I am wondering whether responsive or openDevToolsForTabs is what you want here, or whether this is needed at all.

And the same question foes for the usage in test/e2e/tests/swap-send/swap-send-test-utils.ts (added here: #23703)

Gudahtt

LGTM! Just one non-blocking question (we can adjust this option in those test suites later if we need to)

greptile-apps

PR Summary

Reverted the responsive e2e driver option to address issues with Firefox compatibility and ensure correct test execution for responsive UI.

test/e2e/tests/account/metamask-responsive-ui.spec.js: Reverted openDevToolsForTabs to responsive to fix Firefox test execution.
test/e2e/tests/bridge/bridge-test-utils.ts: Updated driverOptions to revert to responsive, ensuring correct semantic purpose.
test/e2e/tests/swap-send/swap-send-test-utils.ts: Reverted driverOptions to responsive for proper responsive UI testing.
test/e2e/webdriver/chrome.js: Reintroduced responsive option to force smaller viewport for UI tests.
test/e2e/webdriver/index.js: Re-added responsive option to buildWebDriver function for correct test simulation.

_{5 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

metamaskbot · 2024-08-15T14:02:36Z

Builds ready [28e1ba1]

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

Page Load Metrics (251 ± 241 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	86	155	117	19	9
		domContentLoaded	12	58	37	13	6
		load	50	1778	251	503	241
		domInteractive	12	58	37	13	6

Bundle size diffs

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

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

sonarqubecloud · 2024-08-16T15:47:16Z

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

metamaskbot · 2024-08-16T16:17:44Z

Builds ready [ef00684]

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, 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 (162 ± 190 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	75	262	106	40	19
		domContentLoaded	12	75	27	17	8
		load	46	1878	162	395	190
		domInteractive	12	75	27	17	8

Bundle size diffs

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

metamaskbot · 2024-08-16T18:30:50Z

Builds ready [ef00684]

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, 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 (172 ± 232 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	65	203	97	31	15
		domContentLoaded	9	73	22	15	7
		load	39	2281	172	484	232
		domInteractive	9	73	22	15	7

Bundle size diffs

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

pedronfigueiredo added the team-confirmations Push issues to confirmations team label Jul 18, 2024

pedronfigueiredo self-assigned this Jul 18, 2024

pedronfigueiredo marked this pull request as ready for review July 18, 2024 08:44

pedronfigueiredo requested a review from a team as a code owner July 18, 2024 08:44

pedronfigueiredo changed the title ~~fix: Revert e2e driver option~~ fix: Restore responsive e2e driver option Jul 18, 2024

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from a5abc63 to ef39d0d Compare July 18, 2024 09:12

seaona mentioned this pull request Jul 18, 2024

e2e: Improve the responsive logic and its documentation for testing small window size #25935

Open

9 tasks

DDDDDanica previously approved these changes Jul 18, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from ef39d0d to f860a3f Compare July 18, 2024 11:55

Gudahtt reviewed Jul 18, 2024

View reviewed changes

test/e2e/webdriver/chrome.js Outdated Show resolved Hide resolved

pedronfigueiredo dismissed DDDDDanica’s stale review via fcd4b28 July 18, 2024 12:55

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch 4 times, most recently from 49c50e4 to 6907ab6 Compare July 18, 2024 14:33

Gudahtt reviewed Jul 18, 2024

View reviewed changes

Gudahtt previously approved these changes Jul 18, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from 6907ab6 to cd9a318 Compare July 19, 2024 08:50

DDDDDanica previously approved these changes Jul 19, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch 4 times, most recently from 1146e27 to f5506c5 Compare July 22, 2024 09:32

zone-live dismissed stale reviews from DDDDDanica and Gudahtt via 5715ce8 July 23, 2024 08:56

greptile-apps bot reviewed Jul 23, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from 5715ce8 to 4b72b3c Compare July 23, 2024 11:32

Gudahtt previously approved these changes Aug 12, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch 7 times, most recently from 1627f5d to d740d79 Compare August 14, 2024 10:38

pedronfigueiredo dismissed stale reviews from DDDDDanica and Gudahtt via 28e1ba1 August 15, 2024 13:29

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from d740d79 to 28e1ba1 Compare August 15, 2024 13:29

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from 28e1ba1 to d8d79fe Compare August 15, 2024 15:31

digiwand approved these changes Aug 15, 2024

View reviewed changes

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from d8d79fe to 253f6c3 Compare August 16, 2024 10:37

pedronfigueiredo and others added 4 commits August 16, 2024 16:45

fix: Revert e2e driver option

8d5bc12

Update test/e2e/webdriver/chrome.js

0a1f02d

Co-authored-by: Mark Stacey <markjstacey@gmail.com>

wip

4c44092

wip

ef00684

pedronfigueiredo force-pushed the pnf/revert-responsive-e2e-driver-option branch from 253f6c3 to ef00684 Compare August 16, 2024 15:45

Gudahtt approved these changes Aug 16, 2024

View reviewed changes

HowardBraham merged commit 0562f18 into develop Aug 16, 2024

HowardBraham deleted the pnf/revert-responsive-e2e-driver-option branch August 16, 2024 20:36

github-actions bot locked and limited conversation to collaborators Aug 16, 2024

metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 16, 2024

gauthierpetetin added release-12.4.0 Issue or pull request that will be included in release 12.4.0 and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Restore `responsive` e2e driver option#25932

fix: Restore `responsive` e2e driver option#25932
HowardBraham merged 4 commits intodevelopfrom
pnf/revert-responsive-e2e-driver-option

pedronfigueiredo commented Jul 18, 2024 •

edited

Loading

Uh oh!

github-actions bot commented Jul 18, 2024

Uh oh!

DDDDDanica commented Jul 18, 2024

Uh oh!

Uh oh!

codecov bot commented Jul 18, 2024 •

edited

Loading

Uh oh!

Gudahtt Jul 18, 2024

Uh oh!

Gudahtt left a comment

Uh oh!

greptile-apps bot left a comment

Uh oh!

metamaskbot commented Aug 15, 2024

Uh oh!

sonarqubecloud bot commented Aug 16, 2024

Uh oh!

metamaskbot commented Aug 16, 2024

Uh oh!

metamaskbot commented Aug 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

pedronfigueiredo commented Jul 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

Uh oh!

github-actions bot commented Jul 18, 2024

Uh oh!

DDDDDanica commented Jul 18, 2024

Uh oh!

Uh oh!

codecov bot commented Jul 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Gudahtt Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

Gudahtt left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

Uh oh!

metamaskbot commented Aug 15, 2024

Uh oh!

sonarqubecloud bot commented Aug 16, 2024

Quality Gate passed

Uh oh!

metamaskbot commented Aug 16, 2024

Uh oh!

metamaskbot commented Aug 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

pedronfigueiredo commented Jul 18, 2024 •

edited

Loading

codecov bot commented Jul 18, 2024 •

edited

Loading