fix: flaky test `Settings Redirects to ENS domains when user inputs ENS into address bar` #24898

seaona · 2024-05-30T10:09:48Z

Description

This PR fixes some flakiness of the test Settings Redirects to ENS domains when user inputs ENS into address bar. After looking into the code I've identified a source of flakiness, as well as a problem that prevents to upload the artifacts (screenshot) if the test fail.

The test was not using any mock: this is a source of flakiness, and it's now fixed by adding the necessary mocks to the ENS contracts and Infura calls (we need to set Mainnet, since this feature is only available on Mainnet)
The test was not using fixtures: this per se is not a problem, but the issue here is that we are not triggering the verboseReportOnFailure(title,error) on error, making it harder to debug, as no artifacts are uploaded for this test, if it fails. Now we are going to be able to see the screenshot if the test ever fails again

Circle ci job: https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/83569/workflows/0883aa51-cd94-4066-9f96-77d1375e5e9b/jobs/2994904/tests

Failure log: TimeoutError: Wait timed out after 213ms

Extra note: @davidmurdoch is also working on mitigating the rest of the flakiness by waiting for the metamask to be fully initialized before proceeding with the test. This work will be done in a separate PR.

Related issues

Fixes:

Manual testing steps

Go to this page...

Screenshots/Recordings

See no artifact file was uploaded:

https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/83569/workflows/0883aa51-cd94-4066-9f96-77d1375e5e9b/jobs/2994904/artifacts

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-30T10:10:00Z

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-05-30T12:10:01Z

test/e2e/tests/settings/ipfs-ens-resolution.spec.js

-      return currentUrl === ENS_DESTINATION_URL;
-    }, tinyDelayMs);
-
-    await driver.quit();


no need to start or stop the driver manually, as now it's done within the fixtures

seaona · 2024-05-30T12:10:46Z

test/e2e/tests/settings/ipfs-ens-resolution.spec.js

@@ -10,25 +155,36 @@ describe('Settings', function () {
  it('Redirects to ENS domains when user inputs ENS into address bar', async function () {
    // Using proxy port that doesn't resolve so that the browser can error out properly
    // on the ".eth" hostname.  The proxy does too much interference with 8000.
-    const { driver } = await buildWebDriver({ proxyUrl: '127.0.0.1:8001' });


to preserve the proxy number, we now pass this as an option in fixtures, and forward it to the chrome/firefox drivers as an option

codecov · 2024-05-30T20:16:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.92%. Comparing base (a16d1be) to head (0c255ca).
Report is 29 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #24898   +/-   ##
========================================
  Coverage    64.92%   64.92%           
========================================
  Files         1385     1385           
  Lines        54875    54875           
  Branches     14409    14409           
========================================
  Hits         35625    35625           
  Misses       19250    19250

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

hjetpoluru · 2024-05-30T20:38:23Z

@seaona, I am getting this error when I ran locally against firefox and its not consistent.

<testsuites name="Mocha Tests" time="9.584" tests="2" failures="2">
 <testsuite name="Root Suite" timestamp="2024-05-30T20:21:30" tests="0" time="0.000" failures="0">
 </testsuite>
 <testsuite name="Settings" timestamp="2024-05-30T20:21:30" tests="2" file="/Users/harikajetpoluru/repo/metamask-extension/test/e2e/tests/settings/ipfs-ens-resolution.spec.js" time="9.583" failures="2">
   <testcase name="Settings Redirects to ENS domains when user inputs ENS into address bar" time="9.179" classname="Redirects to ENS domains when user inputs ENS into address bar">
     <failure message="Wait timed out after 212ms" type="TimeoutError"><![CDATA[TimeoutError: Wait timed out after 212ms
   at /Users/harikajetpoluru/repo/metamask-extension/node_modules/selenium-webdriver/lib/webdriver.js:929:17
   at processTicksAndRejections (node:internal/process/task_queues:95:5)]]></failure>
   </testcase>
   <testcase name="Settings Does not fetch ENS data for ENS Domain when ENS and IPFS switched off" time="0.342" classname="Does not fetch ENS data for ENS Domain when ENS and IPFS switched off">
     <failure message="listen EADDRINUSE: address already in use 127.0.0.1:8545." type="Error"><![CDATA[Error: listen EADDRINUSE: address already in use 127.0.0.1:8545.
   at /Users/harikajetpoluru/repo/metamask-extension/node_modules/ganache/dist/node/webpack:/Ganache/core/lib/src/server.js:193:33
   at async Promise.allSettled (index 1)
   at Ganache.start (test/e2e/seeder/ganache.ts:19:9)
   at withFixtures (test/e2e/helpers.js:29:55)
   at Context.<anonymous> (test/e2e/tests/settings/ipfs-ens-resolution.spec.js:174:9)]]></failure>
   </testcase>
 </testsuite>
</testsuites>

metamaskbot · 2024-05-30T20:53:24Z

Builds ready [5da3ac2]

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
- 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 (837 ± 559 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	70	202	107	38	18
		domContentLoaded	9	62	19	15	7
		load	56	2851	837	1164	559
		domInteractive	9	62	19	15	7

Bundle size diffs

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

test/e2e/webdriver/chrome.js

test/e2e/webdriver/firefox.js

Co-authored-by: legobeat <[email protected]>

seaona · 2024-05-31T06:06:14Z

@seaona, I am getting this error when I ran locally against firefox and its not consistent.

hey @hjetpoluru 👋 thank you for reporting this 🙇‍♀️ I'll trigger a couple of more ci runs to double-check it does not fail and report back here 🙏

metamaskbot · 2024-05-31T13:15:32Z

Builds ready [852ccdf]

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

Page Load Metrics (214 ± 216 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	74	152	99	19	9
		domContentLoaded	9	78	17	15	7
		load	47	1583	214	449	216
		domInteractive	9	78	17	15	7

hjetpoluru · 2024-05-31T16:58:12Z

@seaona , Good part is that - the issue is partially fixed - we are now able to obtain the failure details in the artifacts. However, the downside is that it's still flaky.https://app.circleci.com/pipelines/github/MetaMask/metamask-extension/84316/workflows/2cea2ca2-c1d0-4e82-96dc-a51859d177dc/jobs/3033481/artifacts

seaona · 2024-06-03T08:28:44Z

yeah @hjetpoluru I move this to draft to investigate further the flakiness observed in ci, now that we have screenshots available with the current fix 👍 thank you!!

metamaskbot · 2024-06-20T05:09:42Z

Builds ready [2fe41ee]

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
- 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 (42 ± 2 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	62	134	75	16	7
		domContentLoaded	8	11	9	1	0
		load	38	53	42	4	2
		domInteractive	8	11	9	1	0

Bundle size diffs

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

Needs a little more work

seaona · 2024-06-21T09:39:01Z

Needs a little more work

The redirect flakiness has now been mitigated using @HowardBraham 's suggestion with the use of a greater max value.
The PR is now ready for review

metamaskbot · 2024-06-21T10:08:38Z

Builds ready [0c255ca]

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
- 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 (132 ± 164 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	66	122	84	14	7
		domContentLoaded	8	33	12	5	2
		load	40	1624	132	343	164
		domInteractive	8	33	12	5	2

Bundle size diffs

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

add fixtures and mocks

27cadfb

add http

7ef4782

seaona added team-extension-platform flaky tests labels May 30, 2024

seaona marked this pull request as ready for review May 30, 2024 11:51

seaona requested a review from a team as a code owner May 30, 2024 11:51

seaona commented May 30, 2024

View reviewed changes

hjetpoluru requested review from HowardBraham, hjetpoluru and legobeat May 30, 2024 14:11

DDDDDanica previously approved these changes May 30, 2024

View reviewed changes

danjm and others added 2 commits May 30, 2024 13:05

Merge branch 'develop' into flaky-ens-redirect

bb7fcb6

Merge branch 'develop' into flaky-ens-redirect

5da3ac2

legobeat reviewed May 31, 2024

View reviewed changes

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

legobeat reviewed May 31, 2024

View reviewed changes

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

Update test/e2e/webdriver/chrome.js

f9e112d

Co-authored-by: legobeat <[email protected]>

seaona dismissed DDDDDanica’s stale review via f9e112d May 31, 2024 05:56

move const inside func

be460de

seaona and others added 3 commits May 31, 2024 09:58

Merge branch 'develop' into flaky-ens-redirect

2348da1

Merge branch 'develop' into flaky-ens-redirect

34aac81

Merge branch 'develop' into flaky-ens-redirect

852ccdf

legobeat previously approved these changes May 31, 2024

View reviewed changes

seaona marked this pull request as draft June 3, 2024 08:27

HowardBraham added a commit that referenced this pull request Jun 14, 2024

from #24898

5ffee01

HowardBraham added a commit that referenced this pull request Jun 14, 2024

from #24898

25aed01

HowardBraham added a commit that referenced this pull request Jun 15, 2024

from #24898

5cf3e5f

HowardBraham added a commit that referenced this pull request Jun 15, 2024

from #24898

22b63dd

HowardBraham added a commit that referenced this pull request Jun 15, 2024

from #24898

5a7de7f

HowardBraham added a commit that referenced this pull request Jun 15, 2024

from #24898

12f3dfa

HowardBraham previously approved these changes Jun 17, 2024

View reviewed changes

seaona marked this pull request as ready for review June 18, 2024 08:56

Merge branch 'develop' into flaky-ens-redirect

2fe41ee

mitigate redirect flakiness action with max value

0c255ca

seaona dismissed legobeat’s stale review via 0c255ca June 21, 2024 09:37

seaona requested review from HowardBraham, legobeat and DDDDDanica June 21, 2024 09:37

chloeYue approved these changes Jun 21, 2024

View reviewed changes

HowardBraham approved these changes Jun 21, 2024

View reviewed changes

HowardBraham merged commit 3552a39 into develop Jun 21, 2024
74 checks passed

HowardBraham deleted the flaky-ens-redirect branch June 21, 2024 21:00

github-actions bot locked and limited conversation to collaborators Jun 21, 2024

metamaskbot added the release-12.1.0 Issue or pull request that will be included in release 12.1.0 label Jun 21, 2024

seaona self-assigned this Jul 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: flaky test `Settings Redirects to ENS domains when user inputs ENS into address bar` #24898

fix: flaky test `Settings Redirects to ENS domains when user inputs ENS into address bar` #24898

seaona commented May 30, 2024 •

edited

Loading

github-actions bot commented May 30, 2024

seaona May 30, 2024

seaona May 30, 2024

codecov bot commented May 30, 2024 •

edited

Loading

hjetpoluru commented May 30, 2024 •

edited

Loading

metamaskbot commented May 30, 2024

seaona commented May 31, 2024 •

edited

Loading

metamaskbot commented May 31, 2024

hjetpoluru commented May 31, 2024

seaona commented Jun 3, 2024

metamaskbot commented Jun 20, 2024

seaona commented Jun 21, 2024

metamaskbot commented Jun 21, 2024

fix: flaky test Settings Redirects to ENS domains when user inputs ENS into address bar #24898

fix: flaky test Settings Redirects to ENS domains when user inputs ENS into address bar #24898

Conversation

seaona commented May 30, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented May 30, 2024

seaona May 30, 2024

Choose a reason for hiding this comment

seaona May 30, 2024

Choose a reason for hiding this comment

codecov bot commented May 30, 2024 • edited Loading

Codecov Report

hjetpoluru commented May 30, 2024 • edited Loading

metamaskbot commented May 30, 2024

seaona commented May 31, 2024 • edited Loading

metamaskbot commented May 31, 2024

hjetpoluru commented May 31, 2024

seaona commented Jun 3, 2024

metamaskbot commented Jun 20, 2024

seaona commented Jun 21, 2024

metamaskbot commented Jun 21, 2024

fix: flaky test `Settings Redirects to ENS domains when user inputs ENS into address bar` #24898

fix: flaky test `Settings Redirects to ENS domains when user inputs ENS into address bar` #24898

seaona commented May 30, 2024 •

edited

Loading

codecov bot commented May 30, 2024 •

edited

Loading

hjetpoluru commented May 30, 2024 •

edited

Loading

seaona commented May 31, 2024 •

edited

Loading