Skip to content

Conversation

@racitores
Copy link
Contributor

@racitores racitores commented Mar 25, 2025

Description

Add tests based on PR #30896

  1. Add ETH to WETH swap
  2. Add send swap ETH to WETH (removed the previous one)

Open in GitHub Codespaces

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

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
Copy link
Contributor

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.

@metamaskbot metamaskbot added the team-qa QA team label Mar 25, 2025
@hjetpoluru hjetpoluru requested a review from Copilot March 25, 2025 14:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds an ETH to WETH swap test based on an earlier swap test PR, enhancing test coverage for token swap scenarios.

  • Updated the mockSwapQuotes callback to handle ETH to WETH swaps.
  • Added multiple test cases to cover both WETH-to-ETH and ETH-to-WETH swap scenarios.
  • Introduced a new dismissManualTokenWarning method in the swap page for handling potential warnings.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/e2e/tests/swaps/swap-erc20-with-loaded-state.spec.ts Updated swap tests to include both ETH to WETH and WETH to ETH scenarios with additional token balance checks
test/e2e/page-objects/pages/swap/swap-page.ts Added new dismissManualTokenWarning function as part of the ETH to WETH test flow
Comments suppressed due to low confidence (2)

test/e2e/tests/swaps/swap-erc20-with-loaded-state.spec.ts:282

  • [nitpick] Consider revising the comment 'Remove ETH linea token...' for clarity. Verify if 'linea' is a typo or if a more descriptive comment is needed.
          // Remove ETH linea token to avoid potential flakiness

test/e2e/tests/swaps/swap-erc20-with-loaded-state.spec.ts:214

  • [nitpick] Token names in the test cases should be consistent. Consider standardizing the naming (e.g., using 'ETH' or 'Ether') across all test cases.
      sourceToken: 'Ethereum',

@metamaskbot
Copy link
Collaborator

Builds ready [554169e]
Page Load Metrics (2392 ± 273 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint51530232044498239
domContentLoaded152333811981438210
load164536662392568273
domInteractive268644178
backgroundConnect124990425208100
firstReactRender442581135124
getState4554521512259
initialActions01000
loadScripts112224481400301145
setupStore253411569244
uiStartup226814609510628281358
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [572b82e]
Page Load Metrics (4217 ± 2296 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint168318212403347682289
domContentLoaded158017766381447012257
load169218317421747812296
domInteractive2866173931428686
backgroundConnect1031040407265127
firstReactRender233141016833
getState2357220316579
initialActions00000
loadScripts114817265320545852202
setupStore954411713464
uiStartup200122538666157472760
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@metamaskbot
Copy link
Collaborator

Builds ready [c205921]
Page Load Metrics (2544 ± 186 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint48328182117593285
domContentLoaded162925782072257123
load172533082544388186
domInteractive2611044189
backgroundConnect109981479219105
firstReactRender421691103617
getState2859923213766
initialActions01000
loadScripts11951761142215474
setupStore273231448139
uiStartup229810608542921061011
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@racitores racitores force-pushed the add-swap-test-eth-to-weth branch from c205921 to 2e617a7 Compare March 25, 2025 17:02
@metamaskbot
Copy link
Collaborator

Builds ready [3461e6f]
Page Load Metrics (4760 ± 2671 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint53918384453654922637
domContentLoaded152318175436552792535
load152818773476055632671
domInteractive263335264716344
backgroundConnect111587399420202
firstReactRender16184755124
getState6828198240115
initialActions01000
loadScripts111217647367750662433
setupStore850911314369
uiStartup175922290673266493193
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@racitores racitores changed the title test: add test eth to weth MMQA-224: add test eth to weth Mar 27, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [a02015b]
Page Load Metrics (2602 ± 348 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint50145452220803386
domContentLoaded168040772131512246
load171050772602724348
domInteractive28151543115
backgroundConnect731073470293140
firstReactRender282011134421
getState1862022314670
initialActions01000
loadScripts120828541502347167
setupStore132571237637
uiStartup202212621569030211451
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@racitores racitores changed the title MMQA-224: add test eth to weth test: MMQA-224 add test eth to weth Mar 27, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [084d75d]
UI Startup Metrics (1212 ± 62 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1212111014546212361329
load10659811302581131990
domContentLoaded10609731298591136989
domInteractive17137981529
firstPaint740881194419221988
backgroundConnect97273910
firstReactRender18152421923
getState11331869
initialActions001001
loadScripts805716103858829921
setupStore7412279
WebpackHomeuiStartup981808112360973996
load83662598175885935
domContentLoaded83061597375876923
domInteractive16135581439
firstPaint45555952347844916
backgroundConnect16123551629
firstReactRender15133531426
getState7426378
initialActions001000
loadScripts82860597176874922
setupStore8515289
FirefoxBrowserifyHomeuiStartup13601169188514414071750
load12221050172613912541606
domContentLoaded12211049172613912531606
domInteractive10540227308097
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect251688102549
firstReactRender23195152432
getState6414278
initialActions001001
loadScripts11981030169913812321582
setupStore6445467
WebpackHomeuiStartup9808331586169889964
load8577321379151822966
domContentLoaded8577321379151822966
domInteractive114322333023396
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect201396112331
firstReactRender19163631927
getState104611179
initialActions001001
loadScripts8407181352147813968
setupStore8549579
Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: 0 Bytes (0%)
  • common: 0 Bytes (0%)

gambinish and others added 4 commits March 31, 2025 10:16
## **Description**

We recently decided that we wanted to sort by token name, rather than
symbol. The problem is that native tokens for several chains display
`Ethereum` when in actuality the name we get from the token API is
something like `Linea`. This would cause native tokens on some EVM
chains to be miscategorized in sort.

This PR overrides the token name for this scenario. It also consolidates
the `name` and `title` fields to support both EVM and nonEVM chains.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31302?quickstart=1)

## **Related issues**

Fixes: #31235

## **Manual testing steps**

1. Please test on both flask and non flask builds.
2. EVM and Solana tokens should now be sorted by their display name (not
symbol)
3. Check on both firefox and chrome

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

Before scenarios in bug ticket linked.

### **After**

Chrome:

<img width="360" alt="Screenshot 2025-03-25 at 12 58 45 PM"
src="https://github.com/user-attachments/assets/b5262950-c649-47da-b18c-1b1daa9c5c01"
/>
<img width="1087" alt="Screenshot 2025-03-25 at 1 01 00 PM"
src="https://github.com/user-attachments/assets/a818d72a-22fc-438a-981b-318dc8faedaa"
/>

Firefox:

<img width="539" alt="Screenshot 2025-03-25 at 1 07 11 PM"
src="https://github.com/user-attachments/assets/f6a2143d-c4a3-4d83-b0ca-bc943b6ba77f"
/>


## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
-->

Updates the color of values from text-alternative to text-default. This
allows users to focus on the information and improves consistency across
pages.

## **Related issues**

None

## **Manual testing steps**

1. Open MetaMask
2. Click on NFT Details

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<img width="355" alt="Screenshot 2025-03-11 at 12 17 22 AM"
src="https://github.com/user-attachments/assets/79db2057-7700-4fdc-8f4a-63b1ee621536"
/>


### **After**

<img width="353" alt="Screenshot 2025-03-11 at 12 19 28 AM"
src="https://github.com/user-attachments/assets/05a3def1-53b9-42d5-9c45-1f622d94ede1"
/>


## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.

---------

Co-authored-by: Nick Gambino <35090461+gambinish@users.noreply.github.com>
)

## **Description**

Prevent tab from opening upon every browser startup.

A [recent PR](#29826)
updated the logic for how we determine whether the extension was just
installed or not; we used to check state, but now we use the
`runtime.onInstalled` event instead to store a flag in session storage
indicating whether the extension was just installed.

Unfortunately this PR had a couple of bugs; the condition using this
flag was accidentally reversed, and the read from session storage was
never successful because of a race condition (the write had not finished
yet).

To address both problems, the logic for detecting first install was
refactored to use a global variable instead of session storage. Globals
can be accessed and written synchronously, preventing any possibility of
a race condition.

The solution was complicated by the fact that MV3 builds must add an
`onInstalled` listener in the root service worker module, otherwise the
listener is never called (we tested this experimentally). It was also
unclear whether the listener would always run before or after the
`background.js` script loading, and we need to trigger an action in
`background.js` on install. But we've accounted for both possibilities
in this solution.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31332?quickstart=1)

## **Related issues**

Fixes: #30924

## **Manual testing steps**

* Create a production-like build (`yarn dist` and `yarn dist:mv2`), or
download the builds from the `metamaskbot` comment
* Test that the window opens only on first install, not on later browser
restarts.

## **Screenshots/Recordings**

N/A

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.
…0970)

<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**
This PR fixes a rare issue when a user, usually a developer, manually
opens notification.html in a browser tab.

We avoid using `browser.windows.remove(id)` to prevent closing all tabs
in a window.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/30970?quickstart=1)

## **Related issues**

* Fixes: #29821

## **Manual testing steps**
Test that browser tab does not auto-close:
1. Open MM in a browser tab
2. Change url path from /home.html => /notification.html
3. Browser tab should stay open (previously the entire window was
closed)

Test that notification popup auto-closes:
4. Open notification popup by visiting a dapp and doing something (like
connecting) that requires permission
5. Cancel/Decline the request in the notification popup
6. Notification popup should auto-close.

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.
@racitores racitores requested review from a team as code owners March 31, 2025 08:27
@metamaskbot
Copy link
Collaborator

Builds ready [54ed212]
UI Startup Metrics (1169 ± 55 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1169106413305512121249
load1023919118453951986
domContentLoaded1017915118054943984
domInteractive16133341527
firstPaint759731184383265984
backgroundConnect96535910
firstReactRender19145051932
getState11436769
initialActions001000
loadScripts80570091952840884
setupStore7515279
WebpackHomeuiStartup935767122972945960
load79460997264828882
domContentLoaded78858796265823872
domInteractive15124171333
firstPaint46866926336822879
backgroundConnect16115391438
firstReactRender14122731322
getState7414278
initialActions001000
loadScripts78657795265821863
setupStore7515278
FirefoxBrowserifyHomeuiStartup13411165188014113651717
load12101040173813512441555
domContentLoaded12101040173713512441555
domInteractive9641158219198
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect22156362333
firstReactRender22195542428
getState6312278
initialActions001001
loadScripts11891023171313312251530
setupStore6425369
WebpackHomeuiStartup9758341498161889990
load8557341310141798907
domContentLoaded8557341310141798907
domInteractive118442933013689
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect19124062034
firstReactRender18162621823
getState1048412784
initialActions001001
loadScripts8387211282136785936
setupStore9558878
Bundle size diffs
  • background: 0 Bytes (0%)
  • ui: 0 Bytes (0%)
  • common: 0 Bytes (0%)

@metamaskbot
Copy link
Collaborator

Builds ready [2f2995d]
UI Startup Metrics (1182 ± 46 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1182106313094612141265
load1028934114645949987
domContentLoaded1023929114046943982
domInteractive16133851630
firstPaint7611181138387253977
backgroundConnect96323910
firstReactRender19145461936
getState10441768
initialActions001000
loadScripts80872291743830887
setupStore8514279
WebpackHomeuiStartup921764117478934949
load78258492163818877
domContentLoaded77756791063813865
domInteractive15126491335
firstPaint42355863331812837
backgroundConnect16115481438
firstReactRender14122831322
getState6411168
initialActions001001
loadScripts77455789962811855
setupStore7514288
FirefoxBrowserifyHomeuiStartup13621193188515013801771
load12241061174013712451605
domContentLoaded12241061174013712441604
domInteractive9840186278797
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect23168982432
firstReactRender22195442328
getState7422378
initialActions002001
loadScripts12021043170313312261557
setupStore6425467
WebpackHomeuiStartup9858241546161894980
load8637141368142811968
domContentLoaded8637141368142811967
domInteractive122332113015281
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect211392142141
firstReactRender18163721822
getState8466879
initialActions001001
loadScripts8477011340138796959
setupStore95661078
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 1.97 KiB (0.03%)
  • ui: 17 Bytes (0%)
  • common: 1 Bytes (0%)

@racitores racitores enabled auto-merge March 31, 2025 10:49
@racitores racitores added this pull request to the merge queue Mar 31, 2025
Merged via the queue into main with commit 453944e Mar 31, 2025
147 checks passed
@racitores racitores deleted the add-swap-test-eth-to-weth branch March 31, 2025 11:40
@github-actions github-actions bot locked and limited conversation to collaborators Mar 31, 2025
@metamaskbot metamaskbot added the release-12.17.0 Issue or pull request that will be included in release 12.17.0 label Mar 31, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-12.17.0 Issue or pull request that will be included in release 12.17.0 team-qa QA team

Projects

None yet

Development

Successfully merging this pull request may close these issues.