fix: Fixed flaky test 24645 #25786

hjetpoluru · 2024-07-11T20:21:13Z

Description

This PR addresses a flaky test related to "Custom network Popular Networks List delete the Arbitrum network."

I was unable to reproduce the issue locally. However, after analyzing the CI screenshots, I identified the reason for the failure. The screenshot shows the support page appearing after the dropdown menu is displayed.

To resolve this, I have updated the test to click on the Settings using the data-testid attribute.

Related issues

Fixes: #24645

Manual testing steps

Run locally or using codepsaces
yarn
yarn build:test:mmi
yarn test:e2e:single test/e2e/tests/network/add-custom-network.spec.js --browser=chrome --debug --leave-running

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-11T20:21:27Z

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-07-11T20:33:13Z

Why was the support page appearing?

hjetpoluru · 2024-07-11T21:21:30Z

@danjm After seeing your comment, I wanted to attach a screenshot of the menu but found an interesting observation in the video attached.

menu-item-change.mov

The Portfolio Dashboard is taking time to load, and as a result, it is clicking Support instead of Settings. I believe this locator strategy should fix the issue.

… fix-flaky-test-delete-network

metamaskbot · 2024-07-12T15:13:51Z

Builds ready [ebd025d]

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 (282 ± 290 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	78	172	116	27	13
		domContentLoaded	9	56	30	14	7
		load	48	2243	282	604	290
		domInteractive	9	56	30	14	7

Bundle size diffs

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

DDDDDanica · 2024-07-12T16:29:10Z

test/e2e/tests/network/add-custom-network.spec.js

+          await driver.waitForSelector('[data-testid="global-menu-settings"]');
+          await driver.clickElement('[data-testid="global-menu-settings"]');
          await driver.clickElement({ text: 'Networks', tag: 'div' });


Should we just add timeout here since clickElement has findElement inside?

const element = await this.findClickableElement(rawLocator); await element.click();

Thanks, @DDDDDanica. That's a valid point about adding the timeout. This issue is happening only for the mmi build, and the menu item is changing. So, I have written a conditional wait for the menu in the code below. Let me know your thoughts.

if (process.env.METAMASK_BUILD_TYPE === 'mmi') { await driver.wait(async () => { const element = await driver.findVisibleElement( '[data-testid="global-menu-mmi-portfolio"]', ); if (!element) { console.log('Element not visible'); } }, 1000); }

@DDDDDanica, Thank you for the suggestion.

However, the initial use of waitForSelector but on the Portfolio Dashboard, combined with a conditional check is working effectively

metamaskbot · 2024-07-12T16:36:09Z

Builds ready [67a3f7e]

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 (103 ± 32 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	66	390	135	74	36
		domContentLoaded	10	178	49	36	17
		load	46	296	103	67	32
		domInteractive	10	178	49	36	17

Bundle size diffs

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

codecov · 2024-07-12T16:37:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.96%. Comparing base (24c95db) to head (67a3f7e).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #25786   +/-   ##
========================================
  Coverage    69.96%   69.96%           
========================================
  Files         1390     1390           
  Lines        48919    48919           
  Branches     13460    13460           
========================================
  Hits         34226    34226           
  Misses       14693    14693

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

test/e2e/tests/network/add-custom-network.spec.js

seaona · 2024-07-15T10:33:39Z

test/e2e/tests/network/add-custom-network.spec.js

@@ -113,12 +113,27 @@ const selectors = {

 async function navigateToAddNetwork(driver) {
  await driver.clickElement(selectors.accountOptionsMenuButton);
+  if (process.env.METAMASK_BUILD_TYPE === 'mmi') {
+    await checkIfPortfolioDashboardIsVisible(driver);


The race-condition identified in this test can also affect other mmi tests: in all tests where we click the Settings icon '[data-testid="account-options-menu-button"]',.

For this reason, we could have a generic function for checking if settings is ready, before proceeding with the next action.

We don't need to do this in this PR, but we could do it in the Page Object implementation, and having this guard for mmi builds can mitigate all flakiness related to the settings re-render case

cc @chloeYue

Yes, I agree with you. We could do it in separate PR

@seaona, I have introduced an additional wait for the portfolio dashboard and removed the checkIfPortfolioDashboardIsVisible function.

However, I believe this spec file is critical and contains many test cases that could be implemented using the Page Object Model. Do let me know what you think?

Also, this is completely different discuss and will leave it to @chloeYue

yeah agree, POM will definitely help on this !

… fix-flaky-test-delete-network

chloeYue

one small nit, otherwise LGTM !

chloeYue · 2024-07-16T12:21:33Z

test/e2e/tests/network/add-custom-network.spec.js

+  if (process.env.MMI) {
+    await driver.waitForSelector('[data-testid="global-menu-mmi-portfolio"]');
+  }
+  await driver.waitForSelector(selectors.settingsOption);


This line is not necessary, because we wait for element to be visible in clickElement function.

seaona · 2024-07-16T14:44:43Z

test/e2e/tests/network/add-custom-network.spec.js

@@ -66,7 +66,7 @@ const MOCK_CHAINLIST_RESPONSE = [
 const selectors = {
  accountOptionsMenuButton: '[data-testid="account-options-menu-button"]',
  informationSymbol: '[data-testid="info-tooltip"]',
-  settingsOption: { text: 'Settings', tag: 'div' },
+  settingsOption: '[data-testid="global-menu-settings"]',


I think this change is not strictly needed for fixing the issue, but I guess it's fine

Actually, this locator strategy was sufficient, and with only this change, the CI has passed. However, I added an additional safety net to check for MMI and wait for the 'Portfolio Dashboard'.

chloeYue

LGTM ! Good job !

… fix-flaky-test-delete-network

sonarcloud · 2024-07-17T12:20: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

metamaskbot · 2024-07-17T13:53:33Z

Builds ready [4386823]

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, 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 (365 ± 312 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	77	224	121	33	16
		domContentLoaded	10	86	35	23	11
		load	55	2061	365	649	312
		domInteractive	10	86	35	23	11

Bundle size diffs

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

Fixed flaky test

7583961

hjetpoluru added team-extension-platform flaky tests labels Jul 11, 2024

hjetpoluru self-assigned this Jul 11, 2024

hjetpoluru requested a review from a team as a code owner July 11, 2024 20:21

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

hjetpoluru marked this pull request as draft July 11, 2024 22:54

hjetpoluru added 4 commits July 12, 2024 09:02

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

395c8bf

… fix-flaky-test-delete-network

Included additional wait

ebd025d

Included additional wait

1362f9a

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

67a3f7e

… fix-flaky-test-delete-network

hjetpoluru marked this pull request as ready for review July 12, 2024 16:15

hjetpoluru requested review from HowardBraham and seaona July 12, 2024 16:17

DDDDDanica reviewed Jul 12, 2024

View reviewed changes

hjetpoluru added 2 commits July 12, 2024 14:16

Added condition wait before clicking the Settings menu

ae090f9

Added checkIfPortfolioDashboardIsVisible as there were multiple checks

5ec2d76

seaona reviewed Jul 15, 2024

View reviewed changes

test/e2e/tests/network/add-custom-network.spec.js Outdated Show resolved Hide resolved

seaona reviewed Jul 15, 2024

View reviewed changes

hjetpoluru added 5 commits July 15, 2024 10:10

Added console log to check build type in CI

aee3884

Fixed lint

0df2693

Added mmi flag

911f7e9

Added wait for portfolio menu item

94e4221

Clean up

58e9ace

hjetpoluru added 3 commits July 15, 2024 23:27

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

ea5fc8c

… fix-flaky-test-delete-network

Removed console log

d5d0a51

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

3c123a0

… fix-flaky-test-delete-network

chloeYue reviewed Jul 16, 2024

View reviewed changes

Removed wait

92fe89d

seaona reviewed Jul 16, 2024

View reviewed changes

seaona approved these changes Jul 16, 2024

View reviewed changes

chloeYue approved these changes Jul 17, 2024

View reviewed changes

Merge branch 'develop' of github.com:MetaMask/metamask-extension into…

4386823

… fix-flaky-test-delete-network

hjetpoluru merged commit b31c31e into develop Jul 17, 2024
78 of 79 checks passed

hjetpoluru deleted the fix-flaky-test-delete-network branch July 17, 2024 13:53

github-actions bot locked and limited conversation to collaborators Jul 17, 2024

metamaskbot added the release-12.3.0 Issue or pull request that will be included in release 12.3.0 label Jul 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Fixed flaky test 24645 #25786

fix: Fixed flaky test 24645 #25786

hjetpoluru commented Jul 11, 2024 •

edited

Loading

github-actions bot commented Jul 11, 2024

danjm commented Jul 11, 2024

hjetpoluru commented Jul 11, 2024 •

edited

Loading

metamaskbot commented Jul 12, 2024

DDDDDanica Jul 12, 2024

hjetpoluru Jul 12, 2024

hjetpoluru Jul 16, 2024

metamaskbot commented Jul 12, 2024

codecov bot commented Jul 12, 2024

seaona Jul 15, 2024

hjetpoluru Jul 15, 2024

hjetpoluru Jul 16, 2024

chloeYue Jul 16, 2024

chloeYue left a comment

chloeYue Jul 16, 2024

seaona Jul 16, 2024 •

edited

Loading

hjetpoluru Jul 16, 2024

chloeYue left a comment

sonarcloud bot commented Jul 17, 2024

metamaskbot commented Jul 17, 2024

fix: Fixed flaky test 24645 #25786

fix: Fixed flaky test 24645 #25786

Conversation

hjetpoluru commented Jul 11, 2024 • edited Loading

Description

Related issues

Manual testing steps

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jul 11, 2024

danjm commented Jul 11, 2024

hjetpoluru commented Jul 11, 2024 • edited Loading

metamaskbot commented Jul 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metamaskbot commented Jul 12, 2024

codecov bot commented Jul 12, 2024

Codecov Report

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

chloeYue left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seaona Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chloeYue left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Jul 17, 2024

Quality Gate passed

metamaskbot commented Jul 17, 2024

hjetpoluru commented Jul 11, 2024 •

edited

Loading

hjetpoluru commented Jul 11, 2024 •

edited

Loading

seaona Jul 16, 2024 •

edited

Loading